From d70e6026ffc3c597d019b6cc2313af1368314a71 Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 7 Aug 2024 17:34:57 +0200 Subject: [PATCH] change: Refactor error handling for block subsidies (#8735) * Addresses clippy lints * checks network magic and returns early from `is_regtest()` * Moves `subsidy.rs` to `zebra-chain`, refactors funding streams into structs, splits them into pre/post NU6 funding streams, and adds them as a field on `testnet::Parameters` * Replaces Vec with HashMap, adds `ConfiguredFundingStreams` type and conversion logic with constraints. Minor refactors * Empties recipients list * Adds a comment on num_addresses calculation being invalid for configured Testnets, but that being okay since configured testnet parameters are checked when they're being built * Documentation fixes, minor cleanup, renames a test, adds TODOs, and fixes test logic * Removes unnecessary `ParameterSubsidy` impl for &Network, adds docs and TODOs * Adds a "deferred" FundingStreamReceiver, adds a post-NU6 funding streams, updates the `miner_fees_are_valid()` and `subsidy_is_valid()` functions to check that the deferred pool contribution is valid and that there are no unclaimed block subsidies after NU6 activation, and adds some TODOs * adds `lockbox_input_value()` fn * Adds TODOs for linking to relevant ZIPs and updating height ranges * Adds `nu6_lockbox_funding_stream` acceptance test * updates funding stream values test to check post-NU6 funding streams too, adds Mainnet/Testnet NU6 activation heights, fixes lints/compilation issue * Reverts Mainnet/Testnet NU6 activation height definitions, updates `test_funding_stream_values()` to use a configured testnet with the post-NU6 Mainnet funding streams height range * reverts unnecessary refactor * appease clippy * Adds a test for `lockbox_input_value()` * Applies suggestions from code review * Fixes potential panic * Fixes bad merge * Update zebra-chain/src/parameters/network_upgrade.rs * Updates acceptance test to check that invalid blocks are rejected * Checks that the original valid block template at height 2 is accepted as a block submission * Reverts changes for coinbase should balance exactly ZIP * Add `Deferred` to `ValueBalance` * Update snapshots * Unrelated: Revise docs * Add TODOs * Stop recalculating the block subsidy * Track deferred balances * Support heights below slow start shift in halvings * Fix `CheckpointVerifiedBlock` conversion in tests * Allow deserialization of legacy `ValueBalance`s * Simplify docs * Fix warnings raised by Clippy * Fix warnings raised by `cargo fmt` * Update zebra-chain/src/block.rs Co-authored-by: Arya * Refactor docs around chain value pool changes * updates test name * Updates deferred pool funding stream name to "Lockbox", moves post-NU6 height ranges to constants, updates TODO * Updates `get_block_subsidy()` RPC method to exclude lockbox funding stream from `fundingstreams` field * Adds a TODO for updating `FundingStreamReceiver::name()` method docs * Updates `FundingStreamRecipient::new()` to accept an iterator of items 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. * Update zebra-consensus/src/checkpoint.rs Co-authored-by: Arya * Update docs for value balances * Cleanup: Simplify getting info for FS receivers * Avoid a panic when deserializing value balances * Uses FPF Testnet address for post-NU6 testnet funding streams * Updates the NU6 consensus branch id * Update zebra-consensus/src/checkpoint.rs * Bump the major database format version * Add a database upgrade mark * Fix tests after merge * Improve docs * Consolidate error handling for block subsidies --------- Co-authored-by: Arya Co-authored-by: Pili Guerra --- zebra-chain/src/amount.rs | 2 +- zebra-chain/src/value_balance.rs | 2 +- zebra-consensus/src/block.rs | 3 +- zebra-consensus/src/block/subsidy/general.rs | 97 ++++++++++---------- zebra-consensus/src/checkpoint.rs | 8 +- zebra-consensus/src/error.rs | 16 +++- 6 files changed, 75 insertions(+), 53 deletions(-) diff --git a/zebra-chain/src/amount.rs b/zebra-chain/src/amount.rs index 7251e0e243c..f4a81c14893 100644 --- a/zebra-chain/src/amount.rs +++ b/zebra-chain/src/amount.rs @@ -431,7 +431,7 @@ where #[allow(missing_docs)] #[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] -/// Errors that can be returned when validating `Amount`s +/// Errors that can be returned when validating [`Amount`]s. pub enum Error { /// input {value} is outside of valid range for zatoshi Amount, valid_range={range:?} Constraint { diff --git a/zebra-chain/src/value_balance.rs b/zebra-chain/src/value_balance.rs index 7e93d349b3b..b2b33e878a7 100644 --- a/zebra-chain/src/value_balance.rs +++ b/zebra-chain/src/value_balance.rs @@ -342,7 +342,7 @@ impl ValueBalance { pub fn from_bytes(bytes: &[u8]) -> Result, ValueBalanceError> { let bytes_length = bytes.len(); - // Return an error early if bytes don't have the right lenght instead of panicking later. + // Return an error early if bytes don't have the right length instead of panicking later. match bytes_length { 32 | 40 => {} _ => return Err(Unparsable), diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index a428a67f91e..b7fd6740ecc 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -50,6 +50,7 @@ pub struct SemanticBlockVerifier { transaction_verifier: V, } +/// Block verification errors. // TODO: dedupe with crate::error::BlockError #[non_exhaustive] #[allow(missing_docs)] @@ -86,7 +87,7 @@ pub enum VerifyBlockError { Transaction(#[from] TransactionError), #[error("invalid block subsidy")] - Subsidy(#[from] zebra_chain::amount::Error), + Subsidy(#[from] SubsidyError), } impl VerifyBlockError { diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index 83257420bf8..233f7935c1c 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -11,7 +11,7 @@ use zebra_chain::{ transaction::Transaction, }; -use crate::funding_stream_values; +use crate::{block::SubsidyError, funding_stream_values}; /// The divisor used for halvings. /// @@ -25,9 +25,7 @@ pub fn halving_divisor(height: Height, network: &Network) -> Option { .activation_height(network) .expect("blossom activation height should be available"); - if height < network.slow_start_shift() { - None - } else if height < blossom_height { + if height < blossom_height { let pre_blossom_height = height - network.slow_start_shift(); let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL; @@ -62,7 +60,10 @@ pub fn halving_divisor(height: Height, network: &Network) -> Option { /// `BlockSubsidy(height)` as described in [protocol specification ยง7.8][7.8] /// /// [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies -pub fn block_subsidy(height: Height, network: &Network) -> Result, Error> { +pub fn block_subsidy( + height: Height, + network: &Network, +) -> Result, SubsidyError> { let blossom_height = Blossom .activation_height(network) .expect("blossom activation height should be available"); @@ -75,23 +76,20 @@ pub fn block_subsidy(height: Height, network: &Network) -> Result - if height < network.slow_start_interval() && !network.disable_pow() { - unreachable!( - "unsupported block height {height:?}: callers should handle blocks below {:?}", - network.slow_start_interval() - ) + // Zebra doesn't need to calculate block subsidies for blocks with heights in the slow start + // interval because it handles those blocks through checkpointing. + if height < network.slow_start_interval() { + Err(SubsidyError::UnsupportedHeight) } else if height < blossom_height { // this calculation is exact, because the halving divisor is 1 here - Amount::try_from(MAX_BLOCK_SUBSIDY / halving_div) + Ok(Amount::try_from(MAX_BLOCK_SUBSIDY / halving_div)?) } else { let scaled_max_block_subsidy = MAX_BLOCK_SUBSIDY / u64::from(BLOSSOM_POW_TARGET_SPACING_RATIO); // in future halvings, this calculation might not be exact // Amount division is implemented using integer division, // which truncates (rounds down) the result, as specified - Amount::try_from(scaled_max_block_subsidy / halving_div) + Ok(Amount::try_from(scaled_max_block_subsidy / halving_div)?) } } @@ -307,127 +305,132 @@ mod test { // After slow-start mining and before Blossom the block subsidy is 12.5 ZEC // https://z.cash/support/faq/#what-is-slow-start-mining assert_eq!( - Amount::try_from(1_250_000_000), - block_subsidy((network.slow_start_interval() + 1).unwrap(), network) + Amount::::try_from(1_250_000_000)?, + block_subsidy((network.slow_start_interval() + 1).unwrap(), network)? ); assert_eq!( - Amount::try_from(1_250_000_000), - block_subsidy((blossom_height - 1).unwrap(), network) + Amount::::try_from(1_250_000_000)?, + block_subsidy((blossom_height - 1).unwrap(), network)? ); // After Blossom the block subsidy is reduced to 6.25 ZEC without halving // https://z.cash/upgrade/blossom/ assert_eq!( - Amount::try_from(625_000_000), - block_subsidy(blossom_height, network) + Amount::::try_from(625_000_000)?, + block_subsidy(blossom_height, network)? ); // After the 1st halving, the block subsidy is reduced to 3.125 ZEC // https://z.cash/upgrade/canopy/ assert_eq!( - Amount::try_from(312_500_000), - block_subsidy(first_halving_height, network) + Amount::::try_from(312_500_000)?, + block_subsidy(first_halving_height, network)? ); // After the 2nd halving, the block subsidy is reduced to 1.5625 ZEC // See "7.8 Calculation of Block Subsidy and Founders' Reward" assert_eq!( - Amount::try_from(156_250_000), + Amount::::try_from(156_250_000)?, block_subsidy( (first_halving_height + POST_BLOSSOM_HALVING_INTERVAL).unwrap(), network - ) + )? ); // After the 7th halving, the block subsidy is reduced to 0.04882812 ZEC // Check that the block subsidy rounds down correctly, and there are no errors assert_eq!( - Amount::try_from(4_882_812), + Amount::::try_from(4_882_812)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 6)).unwrap(), network - ) + )? ); // After the 29th halving, the block subsidy is 1 zatoshi // Check that the block subsidy is calculated correctly at the limit assert_eq!( - Amount::try_from(1), + Amount::::try_from(1)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 28)).unwrap(), network - ) + )? ); // After the 30th halving, there is no block subsidy // Check that there are no errors assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 29)).unwrap(), network - ) + )? ); assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 39)).unwrap(), network - ) + )? ); assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 49)).unwrap(), network - ) + )? ); assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 59)).unwrap(), network - ) + )? ); // The largest possible integer divisor assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 62)).unwrap(), network - ) + )? ); // Other large divisors which should also result in zero assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 63)).unwrap(), network - ) + )? ); assert_eq!( - Amount::try_from(0), + Amount::::try_from(0)?, block_subsidy( (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 64)).unwrap(), network - ) + )? ); assert_eq!( - Amount::try_from(0), - block_subsidy(Height(Height::MAX_AS_U32 / 4), network) + Amount::::try_from(0)?, + block_subsidy(Height(Height::MAX_AS_U32 / 4), network)? ); + + assert_eq!( + Amount::::try_from(0)?, + block_subsidy(Height(Height::MAX_AS_U32 / 2), network)? + ); + assert_eq!( - Amount::try_from(0), - block_subsidy(Height(Height::MAX_AS_U32 / 2), network) + Amount::::try_from(0)?, + block_subsidy(Height::MAX, network)? ); - assert_eq!(Amount::try_from(0), block_subsidy(Height::MAX, network)); Ok(()) } diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index 1138d2dbe2b..dfdf915b5be 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -42,7 +42,7 @@ use crate::{ Progress::{self, *}, TargetHeight::{self, *}, }, - error::BlockError, + error::{BlockError, SubsidyError}, funding_stream_values, BoxError, ParameterCheckpoint as _, }; @@ -608,6 +608,8 @@ where crate::block::check::equihash_solution_is_valid(&block.header)?; } + // We can't get the block subsidy for blocks with heights in the slow start interval, so we + // omit the calculation of the expected deferred amount. let expected_deferred_amount = if height > self.network.slow_start_interval() { // TODO: Add link to lockbox stream ZIP funding_stream_values(height, &self.network, block_subsidy(height, &self.network)?)? @@ -991,7 +993,9 @@ pub enum VerifyCheckpointError { #[error(transparent)] VerifyBlock(VerifyBlockError), #[error("invalid block subsidy")] - SubsidyError(#[from] amount::Error), + SubsidyError(#[from] SubsidyError), + #[error("invalid amount")] + AmountError(#[from] amount::Error), #[error("too many queued blocks at this height")] QueuedLimit, #[error("the block hash does not match the chained checkpoint hash, expected {expected:?} found {found:?}")] diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index e5852368a98..91dfbf0ce3a 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -22,7 +22,8 @@ use proptest_derive::Arbitrary; /// Workaround for format string identifier rules. const MAX_EXPIRY_HEIGHT: block::Height = block::Height::MAX_EXPIRY_HEIGHT; -#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +/// Block subsidy errors. +#[derive(Error, Clone, Debug, PartialEq, Eq)] #[allow(missing_docs)] pub enum SubsidyError { #[error("no coinbase transaction in block")] @@ -36,8 +37,21 @@ pub enum SubsidyError { #[error("a sum of amounts overflowed")] SumOverflow, + + #[error("unsupported height")] + UnsupportedHeight, + + #[error("invalid amount")] + InvalidAmount(amount::Error), +} + +impl From for SubsidyError { + fn from(amount: amount::Error) -> Self { + Self::InvalidAmount(amount) + } } +/// Errors for semantic transaction validation. #[derive(Error, Clone, Debug, PartialEq, Eq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(missing_docs)]