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

Limit length of zkapp URIs #12346

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Limit length of zkapp URIs #12346

merged 1 commit into from
Dec 12, 2022

Conversation

psteckler
Copy link
Member

@psteckler psteckler commented Dec 9, 2022 •
edited
Loading

Limit length of zkapp URIs to 255 characters, using the same mechanism used for symbol tokens. That is, introduce a length check for Bin_prot, sexp, and yojson deserializations, and a check when reading runtime configurations.

Version linter error is expected, and benign.

Closes #12296.

@psteckler psteckler requested a review from a team as a code owner December 9, 2022 00:53
@psteckler
Copy link
Member Author

!ci-build-me

bkase
bkase reviewed Dec 9, 2022
View reviewed changes
Copy link
Member

@bkase bkase left a comment

Choose a reason for hiding this comment

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

We also need the limit in the snark right? Or will this be handled in another PR?

@psteckler
Copy link
Member Author

psteckler commented Dec 9, 2022 •
edited
Loading

We also need the limit in the snark right? Or will this be handled in another PR?

We weren't doing so for token symbols, following the lead on that. Maybe @mrmr1993 can comment.

There is code in Account.Token_symbol for generating inputs, based on the length limit; I could add similar code for the zkapp URIs.

@mrmr1993
Copy link
Member

We also need the limit in the snark right?

@bkase the URI should be opaque to the circuit. It's not useful to expose it there, so we just use a hash instead of paying the additional cost.

@mrmr1993 mrmr1993 merged commit c6a5f94 into develop Dec 12, 2022
@mrmr1993 mrmr1993 deleted the feature/limit-zkapp-uri-length branch December 12, 2022 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkase bkase bkase left review comments

@mrmr1993 mrmr1993 mrmr1993 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

limit zkapp uri length
3 participants
@psteckler @mrmr1993 @bkase