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.

Sign up for GitHub

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

Jump to bottom

unit tests do not account for NOTHING_UP_MY_SLEEVE #1153

Open
ctrlc03 opened this issue Feb 5, 2024 · 6 comments · May be fixed by #1972
Open

unit tests do not account for NOTHING_UP_MY_SLEEVE #1153

ctrlc03 opened this issue Feb 5, 2024 · 6 comments · May be fixed by #1972
Assignees
@wildanvin
Labels
good first issue Good for newcomers testing Issue/PR related to tests

Comments

@ctrlc03
Copy link
Collaborator

ctrlc03 commented Feb 5, 2024

The unit tests for maci core, do not account that the NOTHING_UP_MY_SLEEVE message is to be inserted at position 0. While not a problem for unit tests, these should be amended.
@ctrlc03 ctrlc03 added good first issue Good for newcomers testing Issue/PR related to tests labels Jul 15, 2024
@wildanvin
Copy link
Contributor

Hello,
I would like to work on this issue please

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Sep 11, 2024

Hello, I would like to work on this issue please

Awesome, assigned you! thank you and lmk if you need any help/clarification

@ctrlc03 ctrlc03 added this to MACI Sep 11, 2024
@ctrlc03 ctrlc03 moved this to In Progress in MACI Sep 11, 2024
@wildanvin
Copy link
Contributor

Hello! Sorry for the delay, I got lost in the path of life...

I'll be submitting the PR later this week for sure... I will ask some questions maybe tomorrow in order to make sure I understand everything correctly

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Sep 25, 2024

Hello! Sorry for the delay, I got lost in the path of life...

I'll be submitting the PR later this week for sure... I will ask some questions maybe tomorrow in order to make sure I understand everything correctly

@wildanvin no problem at all, thanks again for tackling this.

@wildanvin
Copy link
Contributor

So, while reviewing the test files, I noticed that in e2e.test.ts (line 384), there is a check for the correct initialization of the NOTHING_UP_MY_SLEEVE value. It does so by creating a new AccQueue and asserting its zero value with the one of the current poll.

I was planning on doing something similar in the Poll.test.ts file. Would that be OK?

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Dec 2, 2024

So, while reviewing the test files, I noticed that in e2e.test.ts (line 384), there is a check for the correct initialization of the NOTHING_UP_MY_SLEEVE value. It does so by creating a new AccQueue and asserting its zero value with the one of the current poll.

I was planning on doing something similar in the Poll.test.ts file. Would that be OK?

Hey @wildanvin really sorry I missed this reply. actually the idea is to add it directly into the local poll message tree as shown here: https://github.com/privacy-scaling-explorations/maci/blob/dev/packages/contracts/tests/Poll.test.ts#L82-L91

@wildanvin wildanvin linked a pull request Dec 16, 2024 that will close this issue
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wildanvin wildanvin

Labels
good first issue Good for newcomers testing Issue/PR related to tests
Projects
MACI
Status: In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(contracts): check the NOTHING_UP_MY_SLEEVE value in Poll test suite wildanvin/maci
2 participants
@wildanvin @ctrlc03