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.

Sign up for GitHub

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

Jump to bottom

Validate coinbase miner subsidy and miner fees #1162

Closed
yaahc opened this issue Oct 14, 2020 · 5 comments · Fixed by #3067
Closed

Validate coinbase miner subsidy and miner fees #1162

yaahc opened this issue Oct 14, 2020 · 5 comments · Fixed by #3067
Assignees
@oxarbitrage
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-4 Canopy Network Upgrade: Canopy specific tasks
Milestone
2021 Sprint 23

Comments

@yaahc
Copy link
Contributor

yaahc commented Oct 14, 2020 •
edited by teor2345
Loading

Motivation

We should check Coinbase transactions, to make sure they satisfy the miner subsidy and miner fee consensus rules.

Specifications

The total value in zatoshi of transparent outputs from a coinbase transaction, minus vbalanceSapling, minus vbalanceOrchard, MUST NOT be greater than the value in zatoshi of miner subsidy plus the transaction fees paid by transactions in this block .

https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus

Related Consensus Rules

Block Subsidy and Funding Streams

Implemented in #338:

Like Bitcoin, Zcash creates currency when blocks are mined. The value created on mining a block is called the block subsidy.

[Canopy onward] The block subsidy is composed of a miner subsidy and a series of funding streams.

As in Bitcoin, the miner of a block also receives transaction fees.

https://zips.z.cash/protocol/protocol.pdf#subsidyconcepts

Transaction value balances

Implemented in #2381:

Transparent inputs to a transaction insert value into a transparent transaction value pool associated with the transaction, and transparent outputs remove value from this pool. As in Bitcoin, the remaining value in the pool is available to miners as a fee.

Consensus rule: The remaining value in the transparent transaction value pool MUST be nonnegative.

Shielded value balances also add or remove funds from the transparent value pool. For the full set of shielded value balance rules, see ZIP-209: Validate transaction value balances #2381.
@yaahc yaahc added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Oct 14, 2020
@teor2345 teor2345 added this to the Transaction Validation milestone Oct 14, 2020
@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code labels Oct 14, 2020
@teor2345
Copy link
Contributor

I updated the modules in this ticket based on @hdevalence's comments.

@oxarbitrage can you add a subsidy::coinbase_script module to the block subsidy design RFC?

The subsidy::coinbase_script module should use the zcash_script crate to verify the consensus rules for transparent founders reward and funding stream outputs.

@teor2345 teor2345 mentioned this issue Oct 14, 2020
Closed
9 tasks
@teor2345 teor2345 changed the title Add support for Coinbase inputs to zcash_consensus::script::Verifier Add support for Coinbase inputs to the CoinbaseVerifier Oct 16, 2020
@teor2345
Copy link
Contributor

Updated the modules again based on today's Zebra meeting.

@teor2345 teor2345 added the S-blocked Status: Blocked on other tasks label Oct 16, 2020
@teor2345
Copy link
Contributor

teor2345 commented Oct 16, 2020 •
edited
Loading

Soft-blocked by #1166.

We could do this work before splitting out the CoinbaseVerifier, but it would be better to do #1166 first.

@teor2345 teor2345 changed the title Add support for Coinbase inputs to the CoinbaseVerifier Add support for Coinbase inputs to the BlockVerifier Nov 24, 2020
@teor2345 teor2345 removed the S-blocked Status: Blocked on other tasks label Nov 24, 2020
@teor2345
Copy link
Contributor

We're no longer creating a CoinbaseVerifier in #1166, so this ticket isn't blocked any more.

@mpguerra mpguerra removed this from the Transaction Validation milestone Jan 5, 2021
@mpguerra mpguerra added the NU-4 Canopy Network Upgrade: Canopy specific tasks label Jan 15, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 2, 2021
@mpguerra mpguerra added this to the 2021 Sprint 13 milestone May 13, 2021
@teor2345 teor2345 removed this from the 2021 Sprint 13 milestone Jun 17, 2021
@teor2345
Copy link
Contributor

This ticket validates coinbase inputs, so it is out of scope for the moment.

@teor2345 teor2345 changed the title Add support for Coinbase inputs to the BlockVerifier Fully validate Coinbase transactions in the Transaction verifier Jun 24, 2021
@teor2345 teor2345 changed the title Fully validate Coinbase transactions in the Transaction verifier Validate coinbase miner subsidy and miner fees Jun 24, 2021
@teor2345 teor2345 added this to the 2021 Sprint 21 milestone Jul 22, 2021
@oxarbitrage oxarbitrage mentioned this issue Nov 9, 2021
Merged
3 tasks
@oxarbitrage oxarbitrage mentioned this issue Nov 17, 2021
Merged
3 tasks
@teor2345 teor2345 mentioned this issue Nov 23, 2021
Merged
3 tasks
@mpguerra mpguerra mentioned this issue Dec 16, 2021
Closed
@arya2 arya2 mentioned this issue Mar 31, 2023
Merged
6 tasks
@mpguerra mpguerra mentioned this issue Apr 11, 2023
Closed
4 tasks
mpguerra added a commit that referenced this issue May 19, 2023
@mpguerra
#338 was fixed by #3040 and #1162 was fixed by #3067
c141645
mergify bot pushed a commit that referenced this issue May 23, 2023
@mpguerra @teor2345
Delete outdated TODOs refering to closed issues (#6732)
ec2e9ca 
* ZIPs were updated to remove ambiguity, this was tracked in #1267.

* #2105 was fixed by #3039 and #2379 was closed by #3069

* #2230 was a duplicate of #2231 which was closed by #2511

* #3235 was obsoleted by #2156 which was fixed by #3505

* #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969

* We migrated to Rust 2021 edition in Jan 2022 with #3332

* #1631 was closed as not needed

* #338 was fixed by #3040 and #1162 was fixed by #3067

* #2079 was fixed by #2445

* #4794 was fixed by #6122

* #1678 stopped being an issue

* #3151 was fixed by #3934

* #3204 was closed as not needed

* #1213 was fixed by #4586

* #1774 was closed as not needed

* #4633 was closed as not needed

* Clarify behaviour of difficulty spacing

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour when retrying block downloads

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify when we might want to fix

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify what we might want to change in future

Co-authored-by: teor <teor@riseup.net>

* Clarify benefits of how we do block verification

Co-authored-by: teor <teor@riseup.net>

* Fix rustfmt errors

---------

Co-authored-by: teor <teor@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oxarbitrage oxarbitrage

Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-4 Canopy Network Upgrade: Canopy specific tasks
Projects
None yet
Milestone
2021 Sprint 23
Development

Successfully merging a pull request may close this issue.

Validate miner transaction fees oxarbitrage/zebra
4 participants
@mpguerra @yaahc @teor2345 @oxarbitrage