Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasm, core, react: tidy unnecessary fns, build artifacts #106

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

sehyunc
Copy link
Contributor

@sehyunc sehyunc commented Oct 24, 2024

Purpose

This PR removes old Rust functions that are no longer used. It also removes from git tracking build artifacts of wasm-pack.

Testing

Tested locally on mainnet stack, all functions work as expected. wasm-pack build artifacts are included in bundle as seen in NPM registry.

@sehyunc sehyunc force-pushed the sehyun/tidy-wasm-1 branch 2 times, most recently from 7a57959 to d1e28cb Compare October 28, 2024 17:01
@sehyunc sehyunc force-pushed the sehyun/tidy-wasm-1 branch from d1e28cb to 77fcd0f Compare October 28, 2024 17:04
@sehyunc sehyunc requested a review from akirillo October 28, 2024 17:11
Copy link
Contributor

@akirillo akirillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, but if we're going to remove WASM build artifacts, we should:

  1. Remove Cargo.lock from the wasm crate's .gitignore
  2. Add a rust-toolchain file to the wasm crate specifying an exact Rust compiler version
    This gets us (mostly) reproducible WASM builds for a given commit. I say mostly because the WASM build script invokes wasm-pack, so the built artifacts depend on the version of wasm-pack on the build machine, but it's still good to lock down the Rust compilation artifacts.

@sehyunc sehyunc requested a review from akirillo October 28, 2024 17:59
Copy link
Contributor

@akirillo akirillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, it probably won't really matter but I would err on the side of matching the relayer's rust-toolchain version: nightly-2024-03-14

approving to unblock

@sehyunc sehyunc force-pushed the sehyun/tidy-wasm-1 branch from 0d14627 to 85c5539 Compare October 28, 2024 18:09
@sehyunc
Copy link
Contributor Author

sehyunc commented Oct 28, 2024

nightly-2024-03-14

☑️

@sehyunc sehyunc merged commit 2956b51 into main Oct 28, 2024
1 check passed
@sehyunc sehyunc deleted the sehyun/tidy-wasm-1 branch October 28, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants