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

test(contracts): cleanup contracts tests #1721

Merged
merged 1 commit into from
Aug 1, 2024
Merged

test(contracts): cleanup contracts tests #1721

merged 1 commit into from
Aug 1, 2024

Conversation

ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented Aug 1, 2024

Description

Cleanup contract tests by removing unnecessary code.

Confirmation

@ctrlc03 ctrlc03 self-assigned this Aug 1, 2024
@ctrlc03 ctrlc03 added the testing Issue/PR related to tests label Aug 1, 2024
Copy link

vercel bot commented Aug 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
maci-website ✅ Ready (Inspect) Visit Preview Aug 1, 2024 5:26pm

Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

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

@ctrlc03 thanks!

@0xmad 0xmad merged commit f1f0b4f into dev Aug 1, 2024
22 checks passed
@0xmad 0xmad deleted the test/contracts branch August 1, 2024 17:36
Copy link
Collaborator

@kittybest kittybest left a comment

Choose a reason for hiding this comment

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

thanks!

@@ -34,7 +34,6 @@ describe("MACI", function test() {
let signer: Signer;

const maciState = new MaciState(STATE_TREE_DEPTH);
const signUpTxOpts = { gasLimit: 400000 };
Copy link
Collaborator

@kittybest kittybest Aug 1, 2024

Choose a reason for hiding this comment

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

why we need this before? (same as other gasLimit)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was there to check whether gas consumption wansn't going over 400k gas - though it seemed arbitrary, gas consumption for signup is also dependent on position of the leaf and size of tree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Issue/PR related to tests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants