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

Document chain value balances consensus rules with new format #3286

Merged

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Dec 23, 2021

Motivation

We want to change the doc of this rules to follow a new format standard for easier consensus rule searching in Zebra.

Close #3221
Close #3219
Close #3220

Additionally the consensus rule about the transparent value pool inside the transaction itself was also updated (this does not have an open issue as far as i know)

Close #3209

Solution

As we implement the chain value balances rules in the same function i decided to document all together in a single PR.

Review

Anyone can review, no code is changed.

@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #3286 (e8b3303) into main (534494f) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head e8b3303 differs from pull request most recent head e3f92b1. Consider uploading reports for the commit e3f92b1 to get more accurate results

@@            Coverage Diff             @@
##             main    #3286      +/-   ##
==========================================
- Coverage   77.13%   77.13%   -0.01%     
==========================================
  Files         265      265              
  Lines       31342    31342              
==========================================
- Hits        24177    24176       -1     
- Misses       7165     7166       +1     

@teor2345
Copy link
Contributor

Additionally the consensus rule about the transparent value pool inside the transaction itself was also updated (this does not have an open issue as far as i know)

It's #3209, I updated the PR description.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

We need to be careful that we don't confuse transaction fees (the remaining transaction value) and transparent outputs (the spendable transparent value).

zebra-chain/src/value_balance.rs Outdated Show resolved Hide resolved
zebra-chain/src/value_balance.rs Outdated Show resolved Hide resolved
zebra-chain/src/value_balance.rs Outdated Show resolved Hide resolved
zebra-chain/src/value_balance.rs Outdated Show resolved Hide resolved
@oxarbitrage oxarbitrage enabled auto-merge (squash) December 30, 2021 12:42
@oxarbitrage
Copy link
Contributor Author

I think this is ready but i don't have admin merge privileges so enabled auto merge for when someone approves.

@teor2345
Copy link
Contributor

I think this is ready but i don't have admin merge privileges so enabled auto merge for when someone approves.

Hopefully the build bot will fix this too.

@oxarbitrage oxarbitrage merged commit d2f71f0 into ZcashFoundation:main Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment