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

feat: test targets #3378

Open
wants to merge 2 commits into
base: mainnet-staging
Choose a base branch
from
Open

Conversation

joske
Copy link
Contributor

@joske joske commented Aug 9, 2024

Motivation

Add feature flag to be able to lower coinbase and proof targets for testing purposes. This feature flag will lower the proof and coinbase targets to 8 and 32 respectively.

The feature flag needed to be cascaded to all dependencies of snarkVM to be sure everything uses same constants.

To use, compile with --features test_targets
e.g.

cargo install --locked --features test_targets --path .

Test Plan

tested by using tx-cannon and adding println to verify correct targets.

CI link

Related PRs

Must first merge AleoNet/snarkVM#2529 and the snarkVM revision must be updated in snarkOS in 2 places before merging this.

vicsn
vicsn previously approved these changes Aug 12, 2024
Copy link
Contributor

@vicsn vicsn left a comment

Choose a reason for hiding this comment

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

LGTM if you update the Cargo.toml to the rev of AleoNet/snarkVM#2529 (which is just for illustrative purposes, as the maintainer will have to update it anyway)

@raychu86
Copy link
Contributor

Does this apply to compiled snarkOS binaries via cargo install --path .? What does that process look like? Additional documentation in the PR description/in the code would be helpful to know the implications.

@joske
Copy link
Contributor Author

joske commented Aug 12, 2024

@raychu86 I've updated the description. Let me know if still not clear.

@raychu86
Copy link
Contributor

raychu86 commented Aug 12, 2024

So if cargo install --locked --features test_targets --path . is used, the snarkOS binary will always have the test_targets version of the coinbase targets? We should have some failsafes where the node fails to boot-up in production mode if that is the case.

We need some sort of enforcement that the test_targets is being used only for devnets via --dev

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.

3 participants