From 76591ceeede5d917ad6db54ae4fce12d67cddd2d Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 12 Aug 2021 22:38:16 +1000 Subject: [PATCH] Generate test chains with valid chain value pools (#2597) * Generate chains with valid chain value pool balances * Move MAX_PARTIAL_CHAIN_BLOCKS to zebra-chain * Fix generated value overflow based on the maximum number of values And split it into its own method. * Split fix_remaining_value into smaller methods * Remove unused methods Co-authored-by: Alfredo Garcia --- zebra-chain/src/block/arbitrary.rs | 37 +++- zebra-chain/src/transaction/arbitrary.rs | 266 ++++++++++++++++------- zebra-chain/src/transparent.rs | 9 + zebra-state/src/constants.rs | 9 +- zebra-state/src/service/arbitrary.rs | 18 +- 5 files changed, 233 insertions(+), 106 deletions(-) diff --git a/zebra-chain/src/block/arbitrary.rs b/zebra-chain/src/block/arbitrary.rs index da4a1e49b..f7aa470f3 100644 --- a/zebra-chain/src/block/arbitrary.rs +++ b/zebra-chain/src/block/arbitrary.rs @@ -8,6 +8,7 @@ use proptest::{ use std::{collections::HashMap, sync::Arc}; use crate::{ + amount::NonNegative, block, fmt::SummaryDebug, parameters::{ @@ -17,13 +18,16 @@ use crate::{ }, serialization, transaction::arbitrary::MAX_ARBITRARY_ITEMS, - transparent::{new_transaction_ordered_outputs, CoinbaseSpendRestriction}, + transparent::{ + new_transaction_ordered_outputs, CoinbaseSpendRestriction, + MIN_TRANSPARENT_COINBASE_MATURITY, + }, work::{difficulty::CompactDifficulty, equihash}, }; use super::*; -/// The chain length for zebra-chain proptests. +/// The chain length for most zebra-chain proptests. /// /// Most generated chains will contain transparent spends at or before this height. /// @@ -43,6 +47,17 @@ use super::*; /// To increase the proportion of test runs with proptest spends, increase `PREVOUTS_CHAIN_HEIGHT`. pub const PREVOUTS_CHAIN_HEIGHT: usize = 4; +/// The chain length for most zebra-state proptests. +/// +/// Most generated chains will contain transparent spends at or before this height. +/// +/// This height was chosen as a tradeoff between chains with no transparent spends, +/// and chains which spend outputs created by previous spends. +/// +/// See [`block::arbitrary::PREVOUTS_CHAIN_HEIGHT`] for details. +pub const MAX_PARTIAL_CHAIN_BLOCKS: usize = + MIN_TRANSPARENT_COINBASE_MATURITY as usize + PREVOUTS_CHAIN_HEIGHT; + #[derive(Debug, Clone, Copy)] #[non_exhaustive] /// The configuration data for proptest when generating arbitrary chains @@ -386,6 +401,7 @@ impl Block { vec.prop_map(move |mut vec| { let mut previous_block_hash = None; let mut utxos = HashMap::new(); + let mut chain_value_pools = ValueBalance::zero(); for (height, block) in vec.iter_mut() { // fixup the previous block hash @@ -399,6 +415,7 @@ impl Block { (*transaction).clone(), tx_index_in_block, *height, + &mut chain_value_pools, &mut utxos, check_transparent_coinbase_spend, ) { @@ -436,6 +453,7 @@ pub fn fix_generated_transaction( mut transaction: Transaction, tx_index_in_block: usize, height: Height, + chain_value_pools: &mut ValueBalance, utxos: &mut HashMap, check_transparent_coinbase_spend: F, ) -> Option @@ -455,6 +473,8 @@ where // fixup the transparent spends for mut input in transaction.inputs().to_vec().into_iter() { if input.outpoint().is_some() { + // the transparent chain value pool is the sum of unspent UTXOs, + // so we don't need to check it separately, because we only spend unspent UTXOs if let Some(selected_outpoint) = find_valid_utxo_for_spend( &mut transaction, &mut spend_restriction, @@ -480,16 +500,17 @@ where // delete invalid inputs *transaction.inputs_mut() = new_inputs; - transaction - .fix_remaining_value(&spent_outputs) - .expect("generated chain value fixes always succeed"); + let (_remaining_transaction_value, new_chain_value_pools) = transaction + .fix_chain_value_pools(*chain_value_pools, &spent_outputs) + .expect("value fixes produce valid chain value pools and remaining transaction values"); // TODO: if needed, check output count here as well if transaction.has_transparent_or_shielded_inputs() { - // skip genesis created UTXOs + // consensus rule: skip genesis created UTXOs + // Zebra implementation: also skip shielded chain value pool changes if height > Height(0) { - // non-coinbase outputs can be spent from the next transaction in this block onwards - // coinbase outputs have to wait 100 blocks, and be shielded + *chain_value_pools = new_chain_value_pools; + utxos.extend(new_transaction_ordered_outputs( &transaction, transaction.hash(), diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 235484d58..1761287cf 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -1,6 +1,7 @@ //! Arbitrary data generation for transaction proptests use std::{ + cmp::max, collections::HashMap, convert::{TryFrom, TryInto}, ops::Neg, @@ -12,7 +13,9 @@ use proptest::{arbitrary::any, array, collection::vec, option, prelude::*}; use crate::{ amount::{self, Amount, NegativeAllowed, NonNegative}, - at_least_one, block, orchard, + at_least_one, + block::{self, arbitrary::MAX_PARTIAL_CHAIN_BLOCKS}, + orchard, parameters::{Network, NetworkUpgrade}, primitives::{ redpallas::{Binding, Signature}, @@ -20,9 +23,8 @@ use crate::{ }, sapling::{self, AnchorVariant, PerSpendAnchor, SharedAnchor}, serialization::{ZcashDeserialize, ZcashDeserializeInto}, - sprout, - transparent::{self, outputs_from_utxos, utxos_from_ordered_utxos}, - value_balance::ValueBalanceError, + sprout, transparent, + value_balance::{ValueBalance, ValueBalanceError}, LedgerState, }; @@ -218,59 +220,168 @@ impl Transaction { } } - /// Fixup non-coinbase transparent values and shielded value balances, - /// so that this transaction passes the "remaining transaction value pool" check. + /// Fixup transparent values and shielded value balances, + /// so that transaction and chain value pools won't overflow MAX_MONEY. /// - /// Returns the remaining transaction value. + /// These fixes are applied to coinbase and non-coinbase transactions. + // + // TODO: do we want to allow overflow, based on an arbitrary bool? + pub fn fix_overflow(&mut self) { + fn scale_to_avoid_overflow(amount: &mut Amount) + where + Amount: Copy, + { + const POOL_COUNT: u64 = 4; + + let max_arbitrary_items: u64 = MAX_ARBITRARY_ITEMS.try_into().unwrap(); + let max_partial_chain_blocks: u64 = MAX_PARTIAL_CHAIN_BLOCKS.try_into().unwrap(); + + // inputs/joinsplits/spends|outputs/actions * pools * transactions + let transaction_pool_scaling_divisor = + max_arbitrary_items * POOL_COUNT * max_arbitrary_items; + // inputs/joinsplits/spends|outputs/actions * transactions * blocks + let chain_pool_scaling_divisor = + max_arbitrary_items * max_arbitrary_items * max_partial_chain_blocks; + let scaling_divisor = max(transaction_pool_scaling_divisor, chain_pool_scaling_divisor); + + *amount = (*amount / scaling_divisor).expect("divisor is not zero"); + } + + self.for_each_value_mut(scale_to_avoid_overflow); + self.for_each_value_balance_mut(scale_to_avoid_overflow); + } + + /// Fixup transparent values and shielded value balances, + /// so that this transaction passes the "non-negative chain value pool" checks. + /// (These checks use the sum of unspent outputs for each transparent and shielded pool.) /// - /// `outputs` must contain all the [`Output`]s spent in this block. + /// These fixes are applied to coinbase and non-coinbase transactions. /// - /// Currently, this code almost always leaves some remaining value in the - /// transaction value pool. + /// `chain_value_pools` contains the chain value pool balances, + /// as of the previous transaction in this block + /// (or the last transaction in the previous block). + /// + /// `outputs` must contain all the [`transparent::Output`]s spent in this transaction. + /// + /// Currently, these fixes almost always leave some remaining value in each transparent + /// and shielded chain value pool. + /// + /// Before fixing the chain value balances, this method calls `fix_overflow` + /// to make sure that transaction and chain value pools don't overflow MAX_MONEY. + /// + /// After fixing the chain value balances, this method calls `fix_remaining_value` + /// to fix the remaining value in the transaction value pool. + /// + /// Returns the remaining transaction value, and the updated chain value balances. /// /// # Panics /// /// If any spent [`Output`] is missing from `outpoints`. // - // TODO: split this method up, after we've implemented chain value balance adjustments - // - // TODO: take an extra arbitrary bool, which selects between zero and non-zero - // remaining value in the transaction value pool - pub fn fix_remaining_value( + // TODO: take some extra arbitrary flags, which select between zero and non-zero + // remaining value in each chain value pool + pub fn fix_chain_value_pools( &mut self, + chain_value_pools: ValueBalance, + outputs: &HashMap, + ) -> Result<(Amount, ValueBalance), ValueBalanceError> { + self.fix_overflow(); + + // a temporary value used to check that inputs don't break the chain value balance + // consensus rules + let mut input_chain_value_pools = chain_value_pools; + + for input in self.inputs() { + input_chain_value_pools = input_chain_value_pools + .update_with_transparent_input(input, outputs) + .expect("find_valid_utxo_for_spend only spends unspent transparent outputs"); + } + + // update the input chain value pools, + // zeroing any inputs that would exceed the input value + + // TODO: consensus rule: normalise sprout JoinSplit values + // so at least one of the values in each JoinSplit is zero + for input in self.input_values_from_sprout_mut() { + match input_chain_value_pools + .update_with_chain_value_pool_change(ValueBalance::from_sprout_amount(input.neg())) + { + Ok(new_chain_pools) => input_chain_value_pools = new_chain_pools, + // set the invalid input value to zero + Err(_) => *input = Amount::zero(), + } + } + + // positive value balances subtract from the chain value pool + + let sapling_input = self.sapling_value_balance().constrain::(); + if let Ok(sapling_input) = sapling_input { + if sapling_input != ValueBalance::zero() { + match input_chain_value_pools.update_with_chain_value_pool_change(-sapling_input) { + Ok(new_chain_pools) => input_chain_value_pools = new_chain_pools, + Err(_) => *self.sapling_value_balance_mut().unwrap() = Amount::zero(), + } + } + } + + let orchard_input = self.orchard_value_balance().constrain::(); + if let Ok(orchard_input) = orchard_input { + if orchard_input != ValueBalance::zero() { + match input_chain_value_pools.update_with_chain_value_pool_change(-orchard_input) { + Ok(new_chain_pools) => input_chain_value_pools = new_chain_pools, + Err(_) => *self.orchard_value_balance_mut().unwrap() = Amount::zero(), + } + } + } + + let remaining_transaction_value = self.fix_remaining_value(outputs)?; + + // check our calculations are correct + let transaction_chain_value_pool_change = + self + .value_balance_from_outputs(outputs) + .expect("chain value pool and remaining transaction value fixes produce valid transaction value balances") + .neg(); + + let chain_value_pools = chain_value_pools + .update_with_transaction(self, outputs) + .unwrap_or_else(|err| { + panic!( + "unexpected chain value pool error: {:?}, \n\ + original chain value pools: {:?}, \n\ + transaction chain value change: {:?}, \n\ + input-only transaction chain value pools: {:?}, \n\ + calculated remaining transaction value: {:?}", + err, + chain_value_pools, // old value + transaction_chain_value_pool_change, + input_chain_value_pools, + remaining_transaction_value, + ) + }); + + Ok((remaining_transaction_value, chain_value_pools)) + } + + /// Returns the total input value of this transaction's value pool. + /// + /// This is the sum of transparent inputs, sprout input values, + /// and if positive, the sapling and orchard value balances. + /// + /// `outputs` must contain all the [`transparent::Output`]s spent in this transaction. + fn input_value_pool( + &self, outputs: &HashMap, ) -> Result, ValueBalanceError> { - // Temporarily make amounts smaller, so the total never overflows MAX_MONEY - // in Zebra's ~100-block chain tests. (With up to 7 values per transaction, - // and 3 transactions per block.) - // TODO: replace this scaling with chain value balance adjustments - fn scale_to_avoid_overflow(amount: &mut Amount) - where - Amount: Copy, - { - *amount = (*amount / 10_000).expect("divisor is not zero"); - } - - self.for_each_value_mut(scale_to_avoid_overflow); - self.for_each_value_balance_mut(scale_to_avoid_overflow); - - if self.is_coinbase() { - // TODO: if needed, fixup coinbase: - // - miner subsidy - // - founders reward or funding streams (hopefully not?) - // - remaining transaction value - return Ok(Amount::zero()); - } - - // calculate the total input value - let transparent_inputs = self .inputs() .iter() .map(|input| input.value_from_outputs(outputs)) .sum::, amount::Error>>() .map_err(ValueBalanceError::Transparent)?; - // TODO: fix callers with invalid values, maybe due to cached outputs? + // TODO: fix callers which cause overflows, check for: + // cached `outputs` that don't go through `fix_overflow`, and + // values much larger than MAX_MONEY //.expect("chain is limited to MAX_MONEY"); let sprout_inputs = self @@ -291,10 +402,49 @@ impl Transaction { .constrain::() .unwrap_or_else(|_| Amount::zero()); - let mut remaining_input_value = + let transaction_input_value_pool = (transparent_inputs + sprout_inputs + sapling_input + orchard_input) .expect("chain is limited to MAX_MONEY"); + Ok(transaction_input_value_pool) + } + + /// Fixup non-coinbase transparent values and shielded value balances, + /// so that this transaction passes the "non-negative remaining transaction value" + /// check. (This check uses the sum of inputs minus outputs.) + /// + /// Returns the remaining transaction value. + /// + /// `outputs` must contain all the [`transparent::Output`]s spent in this transaction. + /// + /// Currently, these fixes almost always leave some remaining value in the + /// transaction value pool. + /// + /// # Panics + /// + /// If any spent [`Output`] is missing from `outpoints`. + // + // TODO: split this method up, after we've implemented chain value balance adjustments + // + // TODO: take an extra arbitrary bool, which selects between zero and non-zero + // remaining value in the transaction value pool + pub fn fix_remaining_value( + &mut self, + outputs: &HashMap, + ) -> Result, ValueBalanceError> { + if self.is_coinbase() { + // TODO: if needed, fixup coinbase: + // - miner subsidy + // - founders reward or funding streams (hopefully not?) + // - remaining transaction value + + // Act as if the generated test case spends all the miner subsidy, miner fees, and + // founders reward / funding stream correctly. + return Ok(Amount::zero()); + } + + let mut remaining_input_value = self.input_value_pool(outputs)?; + // assign remaining input value to outputs, // zeroing any outputs that would exceed the input value @@ -358,40 +508,6 @@ impl Transaction { Ok(remaining_transaction_value) } - - /// Fixup non-coinbase transparent values and shielded value balances. - /// See `fix_remaining_value` for details. - /// - /// `utxos` must contain all the [`Utxo`]s spent in this block. - /// - /// # Panics - /// - /// If any spent [`Utxo`] is missing from `utxos`. - #[allow(dead_code)] - pub fn fix_remaining_value_from_utxos( - &mut self, - utxos: &HashMap, - ) -> Result, ValueBalanceError> { - self.fix_remaining_value(&outputs_from_utxos(utxos.clone())) - } - - /// Fixup non-coinbase transparent values and shielded value balances. - /// See `fix_remaining_value` for details. - /// - /// `ordered_utxos` must contain all the [`OrderedUtxo`]s spent in this block. - /// - /// # Panics - /// - /// If any spent [`OrderedUtxo`] is missing from `ordered_utxos`. - #[allow(dead_code)] - pub fn fix_remaining_value_from_ordered_utxos( - &mut self, - ordered_utxos: &HashMap, - ) -> Result, ValueBalanceError> { - self.fix_remaining_value(&outputs_from_utxos(utxos_from_ordered_utxos( - ordered_utxos.clone(), - ))) - } } impl Arbitrary for Memo { diff --git a/zebra-chain/src/transparent.rs b/zebra-chain/src/transparent.rs index 5e93dada8..b16a74c3b 100644 --- a/zebra-chain/src/transparent.rs +++ b/zebra-chain/src/transparent.rs @@ -35,6 +35,15 @@ use crate::{ use std::{collections::HashMap, iter}; +/// The maturity threshold for transparent coinbase outputs. +/// +/// "A transaction MUST NOT spend a transparent output of a coinbase transaction +/// from a block less than 100 blocks prior to the spend. Note that transparent +/// outputs of coinbase transactions include Founders' Reward outputs and +/// transparent Funding Stream outputs." +/// [7.1](https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus) +pub const MIN_TRANSPARENT_COINBASE_MATURITY: u32 = 100; + /// Arbitrary data inserted by miners into a coinbase transaction. #[derive(Clone, Eq, PartialEq, Serialize, Deserialize)] pub struct CoinbaseData( diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index a26e58620..57a1ec823 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -1,13 +1,6 @@ //! Definitions of constants. -/// The maturity threshold for transparent coinbase outputs. -/// -/// "A transaction MUST NOT spend a transparent output of a coinbase transaction -/// from a block less than 100 blocks prior to the spend. Note that transparent -/// outputs of coinbase transactions include Founders' Reward outputs and -/// transparent Funding Stream outputs." -/// [7.1](https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus) -pub const MIN_TRANSPARENT_COINBASE_MATURITY: u32 = 100; +pub use zebra_chain::transparent::MIN_TRANSPARENT_COINBASE_MATURITY; /// The maximum chain reorganisation height. /// diff --git a/zebra-state/src/service/arbitrary.rs b/zebra-state/src/service/arbitrary.rs index b335c90fe..982b82831 100644 --- a/zebra-state/src/service/arbitrary.rs +++ b/zebra-state/src/service/arbitrary.rs @@ -8,27 +8,15 @@ use proptest::{ }; use zebra_chain::{ - block::{self, Block}, - fmt::SummaryDebug, - history_tree::HistoryTree, - parameters::NetworkUpgrade, + block::Block, fmt::SummaryDebug, history_tree::HistoryTree, parameters::NetworkUpgrade, LedgerState, }; -use crate::{arbitrary::Prepare, constants}; +use crate::arbitrary::Prepare; use super::*; -/// The chain length for state proptests. -/// -/// Most generated chains will contain transparent spends at or before this height. -/// -/// This height was chosen as a tradeoff between chains with no transparent spends, -/// and chains which spend outputs created by previous spends. -/// -/// See [`block::arbitrary::PREVOUTS_CHAIN_HEIGHT`] for details. -pub const MAX_PARTIAL_CHAIN_BLOCKS: usize = - constants::MIN_TRANSPARENT_COINBASE_MATURITY as usize + block::arbitrary::PREVOUTS_CHAIN_HEIGHT; +pub use zebra_chain::block::arbitrary::MAX_PARTIAL_CHAIN_BLOCKS; #[derive(Debug)] pub struct PreparedChainTree {