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

Rename the NU5 block commitment variant based on what it commits to #1988

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Apr 7, 2021

Motivation

The NU5 block commitment variant has a generic name. But we want to name it based on the data it commits to.

This change helps avoid confusion with block commitment variants in future network upgrades, which may add more hashes to the commitment list.

Solution

  • Rename BlockCommitments to ChainHistoryBlockTxAuthCommitment
  • Do some minor comment tidy ups

Review

Anyone can review, this small change is not urgent.

Related Issues

Cleanup after #1957

Follow Up Work

Actually implement the hash

This change helps avoid confusion with block commitment variants in
future network upgrades, which may add more hashes to the commitment
list.
@teor2345 teor2345 added A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Apr 7, 2021
@teor2345 teor2345 added this to the 2021 Sprint 7 milestone Apr 7, 2021
@teor2345 teor2345 requested a review from a team April 7, 2021 04:07
@teor2345 teor2345 self-assigned this Apr 7, 2021
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

These names are a mishmosh but it does what it says on the tin!

@dconnolly dconnolly merged commit 7cb7b61 into ZcashFoundation:main Apr 7, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Apr 7, 2021

These names are a mishmosh but it does what it says on the tin!

Yeah I wasn't sure what to call it, but BlockCommitment is too vague.

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 NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants