-
Notifications
You must be signed in to change notification settings - Fork 471
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
Adding in support for mainnet #897
Conversation
// This enables multi-threading | ||
"--config", `build.rustflags=["-C", "target-feature=+atomics,+bulk-memory,+mutable-globals", "-C", "link-arg=--max-memory=4294967296"]`, | ||
"--no-default-features", | ||
"--features", `browser,${network}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the consequences on this on node.js support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be any consequences, everything should work the same in Node as it does right now.
This is just adding in a new testnet
/ mainnet
feature flag, the testnet
flag is identical to the current behavior.
#[cfg(feature = "testnet")] | ||
mod testnet; | ||
|
||
#[cfg(feature = "testnet")] | ||
pub use testnet::*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good way to approach it.
2faf1ec
to
4b65978
Compare
I added in some end-to-end tests under the Right now I just copied the Node template, so it's just doing a basic "hello world". But over time we can add more e2e tests and flesh them out. |
Also as discussed in Slack, I got rid of Jest and replaced it with Mocha + Chai + Sinon. The unit tests work so much better now. |
The PR is ready for review now, but it can't be merged until after mainnet launches, because it relies upon mainnet URLs that just don't exist right now. After mainnet launches, we will re-run all of the tests until they're all green! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One final comment from my end is to update all SnarkVM dependencies to rev 3d42aa0
(i.e. the latest stable rev). This ensures the correct Network
trait constants are present in the SDK!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR allows users to use either testnet or mainnet (or both).
The user imports whichever network they want to use: