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

chore(circuits): remove assertMemberLength() on Tuple objects #2295

Closed
jeanmon opened this issue Sep 14, 2023 · 1 comment · Fixed by #2296
Closed

chore(circuits): remove assertMemberLength() on Tuple objects #2295

jeanmon opened this issue Sep 14, 2023 · 1 comment · Fixed by #2296
Assignees
@jeanmon
Labels
C-protocol-circuits Component: Protocol circuits (kernel & rollup)

Comments

@jeanmon
Copy link
Contributor

jeanmon commented Sep 14, 2023 •
edited
Loading

Before we introduced a Tuple type for fixed-size array, we used to enforce the correct size of object members of type array using some calls to assertMemberLength() in the constructor.
In between, a lot of such arrays were migrated into Tuples.
Therefore, passing an array or a Tuple of the wrong size would be detected at compile-time and therefore the use of assertMemberLength() is not needed anymore.

One might argue that maintaining assertMemberLength() use would detect abuse of type casting like 'array as Tuple<>' though.
@jeanmon jeanmon self-assigned this Sep 14, 2023
@jeanmon jeanmon added the C-protocol-circuits Component: Protocol circuits (kernel & rollup) label Sep 14, 2023
@jeanmon jeanmon mentioned this issue Sep 14, 2023
Merged
4 tasks
@jeanmon
Copy link
Contributor Author

jeanmon commented Sep 14, 2023

@ludamad @dbanks12 Do you agree about removing such assertMemberLength() calls on Tuple?

jeanmon added a commit that referenced this issue Sep 14, 2023
@jeanmon
chore(circuits): removing assertMemberLength on Tuple objects (#2296)
0247b85 
Resolves #2295 

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [x] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [x] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeanmon jeanmon

Labels
C-protocol-circuits Component: Protocol circuits (kernel & rollup)
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(circuits): removing assertMemberLength on Tuple objects AztecProtocol/aztec-packages
1 participant
@jeanmon