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

tests: merkle, darkpool: enable feature flags in e2e tests #85

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

akirillo
Copy link
Contributor

@akirillo akirillo commented Sep 3, 2023

This PR updates the merkle and darkpool e2e test suites to properly enable the new feature flags.

Tests pass, as do ad-hoc tests asserting that the contracts behave as expected when the flags are disabled.

@akirillo akirillo added the enhancement New feature or request label Sep 3, 2023
@akirillo akirillo requested a review from joeykraut September 3, 2023 00:47
@akirillo akirillo self-assigned this Sep 3, 2023
@@ -277,7 +277,11 @@ pub async fn deploy_darkpool(

// Deploy darkpool
debug!("Deploying darkpool contract...");
let calldata = vec![account.address()];
let calldata = vec![
Copy link
Member

Choose a reason for hiding this comment

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

maybe we should have a constructor for the contract that sets the feature flags to their default values. Want to make absolutely sure we don't have bugs initializing our contract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can certainly make a separate function in the deploy scripts that does this. at the contract level, though, there can only be one constructor, and I want it to accept the feature flags as an argument so that we can later redeploy the darkpool with different feature flags successfully (it will be deployed to a different address since it has different constructor calldata). FWIW when we go to mainnet the feature flags will be removed completely.

@akirillo akirillo force-pushed the andrew/verifier-poseidon-feature-flags branch from d5024f4 to 1d5ba3c Compare September 4, 2023 21:50
Base automatically changed from andrew/verifier-poseidon-feature-flags to main September 4, 2023 21:51
@akirillo akirillo force-pushed the andrew/feature-flags-e2e-tests branch from 1f44fd9 to c389682 Compare September 4, 2023 23:14
@akirillo akirillo merged commit e8a057e into main Sep 4, 2023
1 check passed
@akirillo akirillo deleted the andrew/feature-flags-e2e-tests branch September 4, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants