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

Use valuable::Enumerable to iterate through network upgrades #1974

Closed
8 tasks
teor2345 opened this issue Apr 1, 2021 · 2 comments
Closed
8 tasks

Use valuable::Enumerable to iterate through network upgrades #1974

teor2345 opened this issue Apr 1, 2021 · 2 comments
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement S-blocked Status: Blocked on other tasks

Comments

@teor2345
Copy link
Contributor

teor2345 commented Apr 1, 2021

Is your feature request related to a problem? Please describe.

Some of Zebra's tests iterate through the entire set of network upgrades. Others want to know the latest network upgrade, even if it doesn't have an activation height.

If we use the valuable crate's Enumerable derive, we won't need to change these tests every time we add a new network upgrade.

https://github.com/tokio-rs/valuable

Describe the solution you'd like

A list of network upgrades:

  • Change the version_consistent test to use Enumerable's EnumDef
  • Check if any zebra_chain::parameters tests could also use EnumDef

The latest network upgrade (even if it doesn't have an activation height):

  • Use EnumDef to find future network upgrades in Arbitrary for LedgerState
  • Replace hard-coded instances of Canopy, NU5, and future network upgrades with a "latest network upgrade" function, as needed
    • Replace the NU5 constant in zebra-chain/src/transaction/tests/vectors.rs with the latest network upgrade
    • Replace the list of network upgrades in the Transaction::branch_id() strategy with EnumDef, a "has branch id" filter, then a random choice

Describe alternatives you've considered

We could use strum, but we're going to have the valuable dependency when tracing hits 1.0 anyway
https://docs.rs/strum/0.20.0/strum/

We could do nothing.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-cleanup Category: This is a cleanup S-needs-triage Status: A bug report needs triage P-Optional labels Apr 1, 2021
@dconnolly
Copy link
Contributor

ooo this is handy

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Apr 13, 2021
@teor2345 teor2345 changed the title Use strum to iterate through network upgrades in tests Use valuable to iterate through network upgrades in tests May 28, 2021
@teor2345 teor2345 changed the title Use valuable to iterate through network upgrades in tests Use valuable to iterate through network upgrades May 28, 2021
@teor2345 teor2345 changed the title Use valuable to iterate through network upgrades Use valuable::Enumerable to iterate through network upgrades May 31, 2021
@mpguerra mpguerra added the S-blocked Status: Blocked on other tasks label Oct 13, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Mar 1, 2022

This is not needed, the current code works.

@teor2345 teor2345 closed this as completed Mar 1, 2022
@mpguerra mpguerra closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2023
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 S-blocked Status: Blocked on other tasks
Projects
None yet
Development

No branches or pull requests

3 participants