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

change(consensus): Add lockbox funding stream #8694

Merged
merged 34 commits into from
Aug 1, 2024
Merged

change(consensus): Add lockbox funding stream #8694

merged 34 commits into from
Aug 1, 2024

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Jul 17, 2024

Motivation

We likely want to add a lockbox funding stream.

Will close #8686, and close #8687 once parameters/addresses are updated.

Part of #8695.

This PR will need to be updated once there's more information about which dev fund proposal we should implement, and, if it's alternative 2 in this draft ZIP, also what funding stream addresses to use for ZCG.

Specifications & References

Alternatives 2 and 3 in https://zips.z.cash/draft-nuttycom-funding-allocation
https://zips.z.cash/draft-nuttycom-lockbox-streams

Solution

  • Adds a new funding stream receiver for the "deferred pool"
  • Checks funding streams after the next halving
  • Generates coinbase transactions with new funding streams after NU6 is active
  • Checks that block subsidies are fully claimed after NU6 is active
  • Checks that coinbase transactions spend less than:
    block_subsidy + miner_fees - expected_lockbox_funding_stream_amount
  • Adds a lockbox_input_value() function

Related changes:

  • Factors Halving(height) calculation into its own num_halvings() function
  • In block::check::subsidy_is_valid():
    • Removes unnecessary check that halving_div.count_ones() == 1
    • Removes redundant check for height > slow_start_shift
    • Checks num_halvings() instead of halving_divisor()
    • Minor simplifications

Tests

  • Adds an acceptance test that relies on configurable funding streams on Testnet
  • Updates test to check NU6 funding stream values
  • Adds a check_lockbox_input_value() test for the lockbox_input_value() fn

Follow Up Work

Blocked on ZIP-253 status being changed to 'Proposed':

This PR also adds some TODOs for documentation updates that should be addressed promptly.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added A-consensus Area: Consensus rule updates NU-6 Network Upgrade: NU6 specific tasks P-Medium ⚡ labels Jul 17, 2024
@arya2 arya2 self-assigned this Jul 17, 2024
@arya2 arya2 changed the base branch from main to add-nu6-variant July 17, 2024 04:17
Base automatically changed from add-nu6-variant to main July 18, 2024 17:49
zebra-consensus/src/block/subsidy/general.rs Outdated Show resolved Hide resolved
zebra-consensus/src/block/subsidy/general.rs Show resolved Hide resolved
zebra-consensus/src/block/check.rs Outdated Show resolved Hide resolved
zebrad/tests/acceptance.rs Outdated Show resolved Hide resolved
zebrad/tests/acceptance.rs Outdated Show resolved Hide resolved
zebrad/tests/acceptance.rs Outdated Show resolved Hide resolved
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.

Left some comments and questions.

zebra-chain/src/parameters/network/subsidy.rs Outdated Show resolved Hide resolved
zebra-chain/src/parameters/network/subsidy.rs Outdated Show resolved Hide resolved
zebra-consensus/src/block/subsidy/general.rs Outdated Show resolved Hide resolved
zebrad/tests/acceptance.rs Show resolved Hide resolved
Copy link

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

added some minor comments

zebra-chain/src/parameters/network/subsidy.rs Outdated Show resolved Hide resolved
zebra-chain/src/parameters/network_upgrade.rs Show resolved Hide resolved
zebra-consensus/src/block/check.rs Show resolved Hide resolved
…s instead of an option of an iterator, updates a comment quoting the coinbase transaction balance consensus rule to note that the current code is inconsistent with the protocol spec, adds a TODO for updating the quote there once the protocol spec has been updated.
@arya2 arya2 requested a review from oxarbitrage August 1, 2024 14:12
Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

Looks good, I unresolved one conversation since I think you might have missing one change that you wanted to do, but I don't think it's required, so feel free to address it or mark as resolved

@mergify mergify bot merged commit e56ee4c into main Aug 1, 2024
136 checks passed
@mergify mergify bot deleted the deferred-pool branch August 1, 2024 23:22
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-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG NU-6 Network Upgrade: NU6 specific tasks P-Medium ⚡
Projects
None yet
7 participants