Skip to content

Sum transaction miner fees in the block verifier #3093

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

Merged
merged 2 commits into from
Nov 23, 2021
Merged

Conversation

teor2345
Copy link
Contributor

Motivation

To validate the miner's spends in the coinbase transaction, we need to calculate the total miner fees for the block.

This is part of ticket #1162. It blocks PR #3067.

Specifications

This PR calculates the "the transaction fees paid by transactions in this block" part of this rule:

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

Solution

  • Sum the miner fees for each block into a total amount
  • Return an error if the calculation overflows
  • Skip the coinbase transaction, because it doesn't contribute to miner fees
  • Make sure the transaction verifier response is the correct type

Unrelated changes:

  • Improve docs in the block verifier
  • Move a slow check after all the quick checks
  • Add some consensus rule TODOs

Review

@oxarbitrage wanted this for PR #3067.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Tests will be part of PR #3067.

Follow Up Work

@teor2345 teor2345 added A-consensus Area: Consensus rule updates NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) C-enhancement Category: This is an improvement P-Medium labels Nov 23, 2021
@teor2345 teor2345 requested a review from oxarbitrage November 23, 2021 05:46
@teor2345 teor2345 self-assigned this Nov 23, 2021
@teor2345 teor2345 mentioned this pull request Nov 23, 2021
3 tasks
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks great, very appreciated.

@oxarbitrage oxarbitrage merged commit 8e49663 into main Nov 23, 2021
@oxarbitrage oxarbitrage deleted the total-block-fees branch November 23, 2021 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates C-enhancement Category: This is an improvement NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants