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

zebra-chain: Port array-wrapper types to const generics #2042

Closed
dconnolly opened this issue Apr 20, 2021 · 4 comments
Closed

zebra-chain: Port array-wrapper types to const generics #2042

dconnolly opened this issue Apr 20, 2021 · 4 comments
Assignees
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement E-help-wanted Call for participation: Help is requested to fix this issue. good first issue

Comments

@dconnolly
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We created several types that wrap arrays of bytes that are longer than 32, such as the Groth16Proof and Halo2Proof types. Since they are longer, Rust did not automatically derive several common traits for them, so we had to do that manually. Now that const generics have been stablized, we can clean up some of this code.

Describe the solution you'd like
Define the types in zebra-chain that wrap arrays of bytes longer than 32 as wrapping arrays with lengths defined by a const. This will allow us to #[derive()] common traits on the wrapper type that we could not before, and get rid of the directly implemented traits, like Copy, Clone, etc.

Describe alternatives you've considered
Leave the impls as they are.

Additional context
I think this should just work? But I haven't tested it.

@dconnolly dconnolly added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Low labels Apr 20, 2021
@dconnolly dconnolly self-assigned this Apr 20, 2021
@teor2345
Copy link
Contributor

PR #2021 already uses const generics for some of the AtLeastOne From impls. So this PR doesn't change the rust version requirement.

@mpguerra mpguerra added C-cleanup Category: This is a cleanup E-help-wanted Call for participation: Help is requested to fix this issue. good first issue labels Apr 21, 2021
@teor2345

This comment was marked as outdated.

@teor2345
Copy link
Contributor

Some of these macro-based impls were removed as part of #4004.

@teor2345
Copy link
Contributor

teor2345 commented Jun 2, 2022

Let's do this when we actually change those types.

@teor2345 teor2345 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement E-help-wanted Call for participation: Help is requested to fix this issue. good first issue
Projects
None yet
Development

No branches or pull requests

3 participants