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

feat: add support for encoding & decoding variable-length inputs #27

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

grjte
Copy link
Contributor

@grjte grjte commented Nov 7, 2024

Description

This PR adds support for encoding & decoding inputs whose size is not known at compile-time. For example, an app using this library might use BoundedVec inputs for base64-encoded usernames which can vary depending on user input.

Problem*

Resolves #26

Summary*

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@grjte
Copy link
Contributor Author

grjte commented Nov 7, 2024

@saleel I think the need for this will come up a lot & we'll want a standard pattern across noir libraries, so before completing this PR (DRYing up code, implementing/testing decode, benching, optimizing, etc) I'd like to agree on the signature & approach.

The goal is to avoid requiring any generics related to the actual size, since these can't always be known at compile-time. I think this leads to an output type of BoundedVec. This could also be implemented with 2 inputs, e.g. (input: [u8; MAX_LEN], input_len: u32). I think since the output type is already going to be a BoundedVec we may as well just use BoundedVec for the input as well

@saleel
Copy link
Contributor

saleel commented Nov 7, 2024

Yes, I think it make sense to use BoundedVec as input and output to the _var version and we can take the same approach in other libs. Its not used in sha256 and some other libs at the moment, but it make sense to use BoundedVec instead of two separate params whenever possible IMO.

@TomAFrench what are your thoughts here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enable encoding/decoding of variable-length input data (e.g. BoundedVec)
2 participants