From a3bb1e2e05f61904e084f61eba28c92c328dbbd7 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 4 Dec 2024 15:45:10 -0300 Subject: [PATCH 1/2] change(diagnostics): Updates error messages to include inner error types (#9066) * add: add consensus validation reason to error messages * add additional instances --- zebra-chain/src/history_tree.rs | 2 +- zebra-consensus/src/block.rs | 8 ++++---- zebra-consensus/src/checkpoint.rs | 4 ++-- zebra-consensus/src/error.rs | 24 ++++++++++++---------- zebra-network/src/peer/error.rs | 4 ++-- zebra-state/src/error.rs | 6 +++--- zebrad/src/components/mempool/downloads.rs | 6 +++--- zebrad/src/components/mempool/error.rs | 8 +++++--- zebrad/src/components/mempool/storage.rs | 2 +- 9 files changed, 34 insertions(+), 30 deletions(-) diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index 91fa3a17628..d84f92321af 100644 --- a/zebra-chain/src/history_tree.rs +++ b/zebra-chain/src/history_tree.rs @@ -30,7 +30,7 @@ pub enum HistoryTreeError { #[non_exhaustive] InnerError { inner: zcash_history::Error }, - #[error("I/O error")] + #[error("I/O error: {0}")] IOError(#[from] io::Error), } diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index 611aea2ceba..247079c401a 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -74,19 +74,19 @@ pub enum VerifyBlockError { #[error(transparent)] Time(zebra_chain::block::BlockTimeError), - #[error("unable to commit block after semantic verification")] + #[error("unable to commit block after semantic verification: {0}")] // TODO: make this into a concrete type, and add it to is_duplicate_request() (#2908) Commit(#[source] BoxError), #[cfg(feature = "getblocktemplate-rpcs")] - #[error("unable to validate block proposal: failed semantic verification (proof of work is not checked for proposals)")] + #[error("unable to validate block proposal: failed semantic verification (proof of work is not checked for proposals): {0}")] // TODO: make this into a concrete type (see #5732) ValidateProposal(#[source] BoxError), - #[error("invalid transaction")] + #[error("invalid transaction: {0}")] Transaction(#[from] TransactionError), - #[error("invalid block subsidy")] + #[error("invalid block subsidy: {0}")] Subsidy(#[from] SubsidyError), } diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index 039ea6e33e3..36b3a76d57f 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -992,9 +992,9 @@ pub enum VerifyCheckpointError { CheckpointList(BoxError), #[error(transparent)] VerifyBlock(VerifyBlockError), - #[error("invalid block subsidy")] + #[error("invalid block subsidy: {0}")] SubsidyError(#[from] SubsidyError), - #[error("invalid amount")] + #[error("invalid amount: {0}")] AmountError(#[from] amount::Error), #[error("too many queued blocks at this height")] QueuedLimit, diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index 8fe14c62d52..b0c867fc148 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -121,7 +121,7 @@ pub enum TransactionError { transaction_hash: zebra_chain::transaction::Hash, }, - #[error("coinbase transaction failed subsidy validation")] + #[error("coinbase transaction failed subsidy validation: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] Subsidy(#[from] SubsidyError), @@ -140,7 +140,7 @@ pub enum TransactionError { #[error("if there are no Spends or Outputs, the value balance MUST be 0.")] BadBalance, - #[error("could not verify a transparent script")] + #[error("could not verify a transparent script: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] Script(#[from] zebra_script::Error), @@ -149,29 +149,29 @@ pub enum TransactionError { // TODO: the underlying error is bellman::VerificationError, but it does not implement // Arbitrary as required here. - #[error("spend proof MUST be valid given a primary input formed from the other fields except spendAuthSig")] + #[error("spend proof MUST be valid given a primary input formed from the other fields except spendAuthSig: {0}")] Groth16(String), // TODO: the underlying error is io::Error, but it does not implement Clone as required here. - #[error("Groth16 proof is malformed")] + #[error("Groth16 proof is malformed: {0}")] MalformedGroth16(String), #[error( - "Sprout joinSplitSig MUST represent a valid signature under joinSplitPubKey of dataToBeSigned" + "Sprout joinSplitSig MUST represent a valid signature under joinSplitPubKey of dataToBeSigned: {0}" )] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] Ed25519(#[from] zebra_chain::primitives::ed25519::Error), - #[error("Sapling bindingSig MUST represent a valid signature under the transaction binding validating key bvk of SigHash")] + #[error("Sapling bindingSig MUST represent a valid signature under the transaction binding validating key bvk of SigHash: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] RedJubjub(zebra_chain::primitives::redjubjub::Error), - #[error("Orchard bindingSig MUST represent a valid signature under the transaction binding validating key bvk of SigHash")] + #[error("Orchard bindingSig MUST represent a valid signature under the transaction binding validating key bvk of SigHash: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] RedPallas(zebra_chain::primitives::reddsa::Error), // temporary error type until #1186 is fixed - #[error("Downcast from BoxError to redjubjub::Error failed")] + #[error("Downcast from BoxError to redjubjub::Error failed: {0}")] InternalDowncastError(String), #[error("either vpub_old or vpub_new must be zero")] @@ -201,12 +201,12 @@ pub enum TransactionError { #[error("could not find a mempool transaction input UTXO in the best chain")] TransparentInputNotFound, - #[error("could not validate nullifiers and anchors on best chain")] + #[error("could not validate nullifiers and anchors on best chain: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] // This error variant is at least 128 bytes ValidateContextError(Box), - #[error("could not validate mempool transaction lock time on best chain")] + #[error("could not validate mempool transaction lock time on best chain: {0}")] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] // TODO: turn this into a typed error ValidateMempoolLockTimeError(String), @@ -236,7 +236,9 @@ pub enum TransactionError { min_spend_height: block::Height, }, - #[error("failed to verify ZIP-317 transaction rules, transaction was not inserted to mempool")] + #[error( + "failed to verify ZIP-317 transaction rules, transaction was not inserted to mempool: {0}" + )] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] Zip317(#[from] zebra_chain::transaction::zip317::Error), } diff --git a/zebra-network/src/peer/error.rs b/zebra-network/src/peer/error.rs index c40c34b1d1d..d85e0e143ba 100644 --- a/zebra-network/src/peer/error.rs +++ b/zebra-network/src/peer/error.rs @@ -251,10 +251,10 @@ pub enum HandshakeError { #[error("Peer closed connection")] ConnectionClosed, /// An error occurred while performing an IO operation. - #[error("Underlying IO error")] + #[error("Underlying IO error: {0}")] Io(#[from] std::io::Error), /// A serialization error occurred while reading or writing a message. - #[error("Serialization error")] + #[error("Serialization error: {0}")] Serialization(#[from] SerializationError), /// The remote peer offered a version older than our minimum version. #[error("Peer offered obsolete version: {0:?}")] diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index cf495311efb..632591f4cb3 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -220,13 +220,13 @@ pub enum ValidateContextError { height: Option, }, - #[error("error updating a note commitment tree")] + #[error("error updating a note commitment tree: {0}")] NoteCommitmentTreeError(#[from] zebra_chain::parallel::tree::NoteCommitmentTreeError), - #[error("error building the history tree")] + #[error("error building the history tree: {0}")] HistoryTreeError(#[from] Arc), - #[error("block contains an invalid commitment")] + #[error("block contains an invalid commitment: {0}")] InvalidBlockCommitment(#[from] block::CommitmentError), #[error( diff --git a/zebrad/src/components/mempool/downloads.rs b/zebrad/src/components/mempool/downloads.rs index 6e634717503..68d29aadcaf 100644 --- a/zebrad/src/components/mempool/downloads.rs +++ b/zebrad/src/components/mempool/downloads.rs @@ -115,16 +115,16 @@ pub enum TransactionDownloadVerifyError { #[error("transaction is already in state")] InState, - #[error("error in state service")] + #[error("error in state service: {0}")] StateError(#[source] CloneError), - #[error("error downloading transaction")] + #[error("error downloading transaction: {0}")] DownloadFailed(#[source] CloneError), #[error("transaction download / verification was cancelled")] Cancelled, - #[error("transaction did not pass consensus validation")] + #[error("transaction did not pass consensus validation: {0}")] Invalid(#[from] zebra_consensus::error::TransactionError), } diff --git a/zebrad/src/components/mempool/error.rs b/zebrad/src/components/mempool/error.rs index c70ca56cbc6..d27c78b6da3 100644 --- a/zebrad/src/components/mempool/error.rs +++ b/zebrad/src/components/mempool/error.rs @@ -23,7 +23,9 @@ pub enum MempoolError { /// /// Note that the mempool caches this error. See [`super::storage::Storage`] /// for more details. - #[error("the transaction will be rejected from the mempool until the next chain tip block")] + #[error( + "the transaction will be rejected from the mempool until the next chain tip block: {0}" + )] StorageExactTip(#[from] ExactTipRejectionError), /// Transaction rejected based on its effects (spends, outputs, transaction @@ -33,7 +35,7 @@ pub enum MempoolError { /// /// Note that the mempool caches this error. See [`super::storage::Storage`] /// for more details. - #[error("any transaction with the same effects will be rejected from the mempool until the next chain tip block")] + #[error("any transaction with the same effects will be rejected from the mempool until the next chain tip block: {0}")] StorageEffectsTip(#[from] SameEffectsTipRejectionError), /// Transaction rejected based on its effects (spends, outputs, transaction @@ -44,7 +46,7 @@ pub enum MempoolError { /// /// Note that the mempool caches this error. See [`super::storage::Storage`] /// for more details. - #[error("any transaction with the same effects will be rejected from the mempool until a chain reset")] + #[error("any transaction with the same effects will be rejected from the mempool until a chain reset: {0}")] StorageEffectsChain(#[from] SameEffectsChainRejectionError), /// Transaction rejected because the mempool already contains another diff --git a/zebrad/src/components/mempool/storage.rs b/zebrad/src/components/mempool/storage.rs index ce6f09cf1d6..be7cbc9593f 100644 --- a/zebrad/src/components/mempool/storage.rs +++ b/zebrad/src/components/mempool/storage.rs @@ -55,7 +55,7 @@ pub(crate) const MAX_EVICTION_MEMORY_ENTRIES: usize = 40_000; #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(dead_code)] pub enum ExactTipRejectionError { - #[error("transaction did not pass consensus validation")] + #[error("transaction did not pass consensus validation: {0}")] FailedVerification(#[from] zebra_consensus::error::TransactionError), } From bd122b6f7cfd15fe377d3b1ad402389c04568ae7 Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 5 Dec 2024 16:06:17 +0100 Subject: [PATCH 2/2] add(consensus): Check consensus branch ids in tx verifier (#9063) * Add a consensus branch id check to tx verifier * Allow updating tx network upgrades * Fix unit tests for txs * Remove `println` * Move test-only tx methods out of the default impl * Add a test for checking consensus branch ids * Simplify some tests * Docs formatting * Update zebra-consensus/src/transaction/check.rs Co-authored-by: Conrado Gouvea * Add `effectiveVersion` to txs * Refactor the consensus branch ID check * Update zebra-consensus/src/error.rs Co-authored-by: Alfredo Garcia * Refactor the consensus branch ID check * Remove `effective_version` * Refactor tests for consensus branch ID check --------- Co-authored-by: Conrado Gouvea Co-authored-by: Alfredo Garcia --- zebra-chain/src/transaction.rs | 481 ++++++++++++----------- zebra-consensus/src/error.rs | 6 + zebra-consensus/src/transaction.rs | 1 + zebra-consensus/src/transaction/check.rs | 41 ++ zebra-consensus/src/transaction/tests.rs | 363 +++++++++++------ 5 files changed, 554 insertions(+), 338 deletions(-) diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index 96b2378e273..1c121130fcc 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -324,7 +324,17 @@ impl Transaction { } } - /// Return the version of this transaction. + /// Returns the version of this transaction. + /// + /// Note that the returned version is equal to `effectiveVersion`, described in [§ 7.1 + /// Transaction Encoding and Consensus]: + /// + /// > `effectiveVersion` [...] is equal to `min(2, version)` when `fOverwintered = 0` and to + /// > `version` otherwise. + /// + /// Zebra handles the `fOverwintered` flag via the [`Self::is_overwintered`] method. + /// + /// [§ 7.1 Transaction Encoding and Consensus]: pub fn version(&self) -> u32 { match self { Transaction::V1 { .. } => 1, @@ -429,32 +439,6 @@ impl Transaction { } } - /// Modify the expiry height of this transaction. - /// - /// # Panics - /// - /// - if called on a v1 or v2 transaction - #[cfg(any(test, feature = "proptest-impl"))] - pub fn expiry_height_mut(&mut self) -> &mut block::Height { - match self { - Transaction::V1 { .. } | Transaction::V2 { .. } => { - panic!("v1 and v2 transactions are not supported") - } - Transaction::V3 { - ref mut expiry_height, - .. - } - | Transaction::V4 { - ref mut expiry_height, - .. - } - | Transaction::V5 { - ref mut expiry_height, - .. - } => expiry_height, - } - } - /// Get this transaction's network upgrade field, if any. /// This field is serialized as `nConsensusBranchId` ([7.1]). /// @@ -484,18 +468,6 @@ impl Transaction { } } - /// Modify the transparent inputs of this transaction, regardless of version. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn inputs_mut(&mut self) -> &mut Vec { - match self { - Transaction::V1 { ref mut inputs, .. } => inputs, - Transaction::V2 { ref mut inputs, .. } => inputs, - Transaction::V3 { ref mut inputs, .. } => inputs, - Transaction::V4 { ref mut inputs, .. } => inputs, - Transaction::V5 { ref mut inputs, .. } => inputs, - } - } - /// Access the [`transparent::OutPoint`]s spent by this transaction's [`transparent::Input`]s. pub fn spent_outpoints(&self) -> impl Iterator + '_ { self.inputs() @@ -514,28 +486,6 @@ impl Transaction { } } - /// Modify the transparent outputs of this transaction, regardless of version. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn outputs_mut(&mut self) -> &mut Vec { - match self { - Transaction::V1 { - ref mut outputs, .. - } => outputs, - Transaction::V2 { - ref mut outputs, .. - } => outputs, - Transaction::V3 { - ref mut outputs, .. - } => outputs, - Transaction::V4 { - ref mut outputs, .. - } => outputs, - Transaction::V5 { - ref mut outputs, .. - } => outputs, - } - } - /// Returns `true` if this transaction has valid inputs for a coinbase /// transaction, that is, has a single input and it is a coinbase input /// (null prevout). @@ -943,27 +893,6 @@ impl Transaction { } } - /// Modify the [`orchard::ShieldedData`] in this transaction, - /// regardless of version. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn orchard_shielded_data_mut(&mut self) -> Option<&mut orchard::ShieldedData> { - match self { - Transaction::V5 { - orchard_shielded_data: Some(orchard_shielded_data), - .. - } => Some(orchard_shielded_data), - - Transaction::V1 { .. } - | Transaction::V2 { .. } - | Transaction::V3 { .. } - | Transaction::V4 { .. } - | Transaction::V5 { - orchard_shielded_data: None, - .. - } => None, - } - } - /// Iterate over the [`orchard::Action`]s in this transaction, if there are any, /// regardless of version. pub fn orchard_actions(&self) -> impl Iterator { @@ -1035,14 +964,6 @@ impl Transaction { .map_err(ValueBalanceError::Transparent) } - /// Modify the transparent output values of this transaction, regardless of version. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn output_values_mut(&mut self) -> impl Iterator> { - self.outputs_mut() - .iter_mut() - .map(|output| &mut output.value) - } - /// Returns the `vpub_old` fields from `JoinSplit`s in this transaction, /// regardless of version, in the order they appear in the transaction. /// @@ -1090,55 +1011,6 @@ impl Transaction { } } - /// Modify the `vpub_old` fields from `JoinSplit`s in this transaction, - /// regardless of version, in the order they appear in the transaction. - /// - /// See `output_values_to_sprout` for details. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn output_values_to_sprout_mut( - &mut self, - ) -> Box> + '_> { - match self { - // JoinSplits with Bctv14 Proofs - Transaction::V2 { - joinsplit_data: Some(joinsplit_data), - .. - } - | Transaction::V3 { - joinsplit_data: Some(joinsplit_data), - .. - } => Box::new( - joinsplit_data - .joinsplits_mut() - .map(|joinsplit| &mut joinsplit.vpub_old), - ), - // JoinSplits with Groth16 Proofs - Transaction::V4 { - joinsplit_data: Some(joinsplit_data), - .. - } => Box::new( - joinsplit_data - .joinsplits_mut() - .map(|joinsplit| &mut joinsplit.vpub_old), - ), - // No JoinSplits - Transaction::V1 { .. } - | Transaction::V2 { - joinsplit_data: None, - .. - } - | Transaction::V3 { - joinsplit_data: None, - .. - } - | Transaction::V4 { - joinsplit_data: None, - .. - } - | Transaction::V5 { .. } => Box::new(std::iter::empty()), - } - } - /// Returns the `vpub_new` fields from `JoinSplit`s in this transaction, /// regardless of version, in the order they appear in the transaction. /// @@ -1186,55 +1058,6 @@ impl Transaction { } } - /// Modify the `vpub_new` fields from `JoinSplit`s in this transaction, - /// regardless of version, in the order they appear in the transaction. - /// - /// See `input_values_from_sprout` for details. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn input_values_from_sprout_mut( - &mut self, - ) -> Box> + '_> { - match self { - // JoinSplits with Bctv14 Proofs - Transaction::V2 { - joinsplit_data: Some(joinsplit_data), - .. - } - | Transaction::V3 { - joinsplit_data: Some(joinsplit_data), - .. - } => Box::new( - joinsplit_data - .joinsplits_mut() - .map(|joinsplit| &mut joinsplit.vpub_new), - ), - // JoinSplits with Groth Proofs - Transaction::V4 { - joinsplit_data: Some(joinsplit_data), - .. - } => Box::new( - joinsplit_data - .joinsplits_mut() - .map(|joinsplit| &mut joinsplit.vpub_new), - ), - // No JoinSplits - Transaction::V1 { .. } - | Transaction::V2 { - joinsplit_data: None, - .. - } - | Transaction::V3 { - joinsplit_data: None, - .. - } - | Transaction::V4 { - joinsplit_data: None, - .. - } - | Transaction::V5 { .. } => Box::new(std::iter::empty()), - } - } - /// Return a list of sprout value balances, /// the changes in the transaction value pool due to each sprout `JoinSplit`. /// @@ -1331,35 +1154,6 @@ impl Transaction { ValueBalance::from_sapling_amount(sapling_value_balance) } - /// Modify the `value_balance` field from the `sapling::ShieldedData` in this transaction, - /// regardless of version. - /// - /// See `sapling_value_balance` for details. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn sapling_value_balance_mut(&mut self) -> Option<&mut Amount> { - match self { - Transaction::V4 { - sapling_shielded_data: Some(sapling_shielded_data), - .. - } => Some(&mut sapling_shielded_data.value_balance), - Transaction::V5 { - sapling_shielded_data: Some(sapling_shielded_data), - .. - } => Some(&mut sapling_shielded_data.value_balance), - Transaction::V1 { .. } - | Transaction::V2 { .. } - | Transaction::V3 { .. } - | Transaction::V4 { - sapling_shielded_data: None, - .. - } - | Transaction::V5 { - sapling_shielded_data: None, - .. - } => None, - } - } - /// Return the orchard value balance, the change in the transaction value /// pool due to [`orchard::Action`]s. /// @@ -1380,16 +1174,6 @@ impl Transaction { ValueBalance::from_orchard_amount(orchard_value_balance) } - /// Modify the `value_balance` field from the `orchard::ShieldedData` in this transaction, - /// regardless of version. - /// - /// See `orchard_value_balance` for details. - #[cfg(any(test, feature = "proptest-impl"))] - pub fn orchard_value_balance_mut(&mut self) -> Option<&mut Amount> { - self.orchard_shielded_data_mut() - .map(|shielded_data| &mut shielded_data.value_balance) - } - /// Returns the value balances for this transaction using the provided transparent outputs. pub(crate) fn value_balance_from_outputs( &self, @@ -1428,3 +1212,246 @@ impl Transaction { self.value_balance_from_outputs(&outputs_from_utxos(utxos.clone())) } } + +#[cfg(any(test, feature = "proptest-impl"))] +impl Transaction { + /// Updates the [`NetworkUpgrade`] for this transaction. + /// + /// ## Notes + /// + /// - Updating the network upgrade for V1, V2, V3 and V4 transactions is not possible. + pub fn update_network_upgrade(&mut self, nu: NetworkUpgrade) -> Result<(), &str> { + match self { + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { .. } => Err( + "Updating the network upgrade for V1, V2, V3 and V4 transactions is not possible.", + ), + Transaction::V5 { + ref mut network_upgrade, + .. + } => { + *network_upgrade = nu; + Ok(()) + } + } + } + + /// Modify the expiry height of this transaction. + /// + /// # Panics + /// + /// - if called on a v1 or v2 transaction + pub fn expiry_height_mut(&mut self) -> &mut block::Height { + match self { + Transaction::V1 { .. } | Transaction::V2 { .. } => { + panic!("v1 and v2 transactions are not supported") + } + Transaction::V3 { + ref mut expiry_height, + .. + } + | Transaction::V4 { + ref mut expiry_height, + .. + } + | Transaction::V5 { + ref mut expiry_height, + .. + } => expiry_height, + } + } + + /// Modify the transparent inputs of this transaction, regardless of version. + pub fn inputs_mut(&mut self) -> &mut Vec { + match self { + Transaction::V1 { ref mut inputs, .. } => inputs, + Transaction::V2 { ref mut inputs, .. } => inputs, + Transaction::V3 { ref mut inputs, .. } => inputs, + Transaction::V4 { ref mut inputs, .. } => inputs, + Transaction::V5 { ref mut inputs, .. } => inputs, + } + } + + /// Modify the `value_balance` field from the `orchard::ShieldedData` in this transaction, + /// regardless of version. + /// + /// See `orchard_value_balance` for details. + pub fn orchard_value_balance_mut(&mut self) -> Option<&mut Amount> { + self.orchard_shielded_data_mut() + .map(|shielded_data| &mut shielded_data.value_balance) + } + + /// Modify the `value_balance` field from the `sapling::ShieldedData` in this transaction, + /// regardless of version. + /// + /// See `sapling_value_balance` for details. + pub fn sapling_value_balance_mut(&mut self) -> Option<&mut Amount> { + match self { + Transaction::V4 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Some(&mut sapling_shielded_data.value_balance), + Transaction::V5 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Some(&mut sapling_shielded_data.value_balance), + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { + sapling_shielded_data: None, + .. + } + | Transaction::V5 { + sapling_shielded_data: None, + .. + } => None, + } + } + + /// Modify the `vpub_new` fields from `JoinSplit`s in this transaction, + /// regardless of version, in the order they appear in the transaction. + /// + /// See `input_values_from_sprout` for details. + pub fn input_values_from_sprout_mut( + &mut self, + ) -> Box> + '_> { + match self { + // JoinSplits with Bctv14 Proofs + Transaction::V2 { + joinsplit_data: Some(joinsplit_data), + .. + } + | Transaction::V3 { + joinsplit_data: Some(joinsplit_data), + .. + } => Box::new( + joinsplit_data + .joinsplits_mut() + .map(|joinsplit| &mut joinsplit.vpub_new), + ), + // JoinSplits with Groth Proofs + Transaction::V4 { + joinsplit_data: Some(joinsplit_data), + .. + } => Box::new( + joinsplit_data + .joinsplits_mut() + .map(|joinsplit| &mut joinsplit.vpub_new), + ), + // No JoinSplits + Transaction::V1 { .. } + | Transaction::V2 { + joinsplit_data: None, + .. + } + | Transaction::V3 { + joinsplit_data: None, + .. + } + | Transaction::V4 { + joinsplit_data: None, + .. + } + | Transaction::V5 { .. } => Box::new(std::iter::empty()), + } + } + + /// Modify the `vpub_old` fields from `JoinSplit`s in this transaction, + /// regardless of version, in the order they appear in the transaction. + /// + /// See `output_values_to_sprout` for details. + pub fn output_values_to_sprout_mut( + &mut self, + ) -> Box> + '_> { + match self { + // JoinSplits with Bctv14 Proofs + Transaction::V2 { + joinsplit_data: Some(joinsplit_data), + .. + } + | Transaction::V3 { + joinsplit_data: Some(joinsplit_data), + .. + } => Box::new( + joinsplit_data + .joinsplits_mut() + .map(|joinsplit| &mut joinsplit.vpub_old), + ), + // JoinSplits with Groth16 Proofs + Transaction::V4 { + joinsplit_data: Some(joinsplit_data), + .. + } => Box::new( + joinsplit_data + .joinsplits_mut() + .map(|joinsplit| &mut joinsplit.vpub_old), + ), + // No JoinSplits + Transaction::V1 { .. } + | Transaction::V2 { + joinsplit_data: None, + .. + } + | Transaction::V3 { + joinsplit_data: None, + .. + } + | Transaction::V4 { + joinsplit_data: None, + .. + } + | Transaction::V5 { .. } => Box::new(std::iter::empty()), + } + } + + /// Modify the transparent output values of this transaction, regardless of version. + pub fn output_values_mut(&mut self) -> impl Iterator> { + self.outputs_mut() + .iter_mut() + .map(|output| &mut output.value) + } + + /// Modify the [`orchard::ShieldedData`] in this transaction, + /// regardless of version. + pub fn orchard_shielded_data_mut(&mut self) -> Option<&mut orchard::ShieldedData> { + match self { + Transaction::V5 { + orchard_shielded_data: Some(orchard_shielded_data), + .. + } => Some(orchard_shielded_data), + + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { .. } + | Transaction::V5 { + orchard_shielded_data: None, + .. + } => None, + } + } + + /// Modify the transparent outputs of this transaction, regardless of version. + pub fn outputs_mut(&mut self) -> &mut Vec { + match self { + Transaction::V1 { + ref mut outputs, .. + } => outputs, + Transaction::V2 { + ref mut outputs, .. + } => outputs, + Transaction::V3 { + ref mut outputs, .. + } => outputs, + Transaction::V4 { + ref mut outputs, .. + } => outputs, + Transaction::V5 { + ref mut outputs, .. + } => outputs, + } + } +} diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index b0c867fc148..ac7e339eb55 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -241,6 +241,12 @@ pub enum TransactionError { )] #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] Zip317(#[from] zebra_chain::transaction::zip317::Error), + + #[error("transaction uses an incorrect consensus branch id")] + WrongConsensusBranchId, + + #[error("wrong tx format: tx version is ≥ 5, but `nConsensusBranchId` is missing")] + MissingConsensusBranchId, } impl From for TransactionError { diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index a3729b0a280..ef20881bbbf 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -380,6 +380,7 @@ where // Do quick checks first check::has_inputs_and_outputs(&tx)?; check::has_enough_orchard_flags(&tx)?; + check::consensus_branch_id(&tx, req.height(), &network)?; // Validate the coinbase input consensus rules if req.is_mempool() && tx.is_coinbase() { diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index 66e3d0be595..d3ddc460264 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -495,3 +495,44 @@ pub fn tx_transparent_coinbase_spends_maturity( Ok(()) } + +/// Checks the `nConsensusBranchId` field. +/// +/// # Consensus +/// +/// ## [7.1.2 Transaction Consensus Rules] +/// +/// > [**NU5** onward] If `effectiveVersion` ≥ 5, the `nConsensusBranchId` field **MUST** match the +/// > consensus branch ID used for SIGHASH transaction hashes, as specified in [ZIP-244]. +/// +/// ### Notes +/// +/// - When deserializing transactions, Zebra converts the `nConsensusBranchId` into +/// [`NetworkUpgrade`]. +/// +/// - The values returned by [`Transaction::version`] match `effectiveVersion` so we use them in +/// place of `effectiveVersion`. More details in [`Transaction::version`]. +/// +/// [ZIP-244]: +/// [7.1.2 Transaction Consensus Rules]: +pub fn consensus_branch_id( + tx: &Transaction, + height: Height, + network: &Network, +) -> Result<(), TransactionError> { + let current_nu = NetworkUpgrade::current(network, height); + + if current_nu < NetworkUpgrade::Nu5 || tx.version() < 5 { + return Ok(()); + } + + let Some(tx_nu) = tx.network_upgrade() else { + return Err(TransactionError::MissingConsensusBranchId); + }; + + if tx_nu != current_nu { + return Err(TransactionError::WrongConsensusBranchId); + } + + Ok(()) +} diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index d42bbb8594c..8627a578c62 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -6,6 +6,7 @@ use std::{collections::HashMap, sync::Arc}; use chrono::{DateTime, TimeZone, Utc}; use color_eyre::eyre::Report; +use futures::{FutureExt, TryFutureExt}; use halo2::pasta::{group::ff::PrimeField, pallas}; use tower::{buffer::Buffer, service_fn, ServiceExt}; @@ -1002,58 +1003,47 @@ async fn v5_transaction_is_rejected_before_nu5_activation() { } #[test] -fn v5_transaction_is_accepted_after_nu5_activation_mainnet() { - v5_transaction_is_accepted_after_nu5_activation_for_network(Network::Mainnet) -} - -#[test] -fn v5_transaction_is_accepted_after_nu5_activation_testnet() { - v5_transaction_is_accepted_after_nu5_activation_for_network(Network::new_default_testnet()) -} - -fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Network) { +fn v5_transaction_is_accepted_after_nu5_activation() { let _init_guard = zebra_test::init(); - zebra_test::MULTI_THREADED_RUNTIME.block_on(async { - let nu5 = NetworkUpgrade::Nu5; - let nu5_activation_height = nu5 - .activation_height(&network) - .expect("NU5 activation height is specified"); - let blocks = network.block_iter(); - let state_service = service_fn(|_| async { unreachable!("Service should not be called") }); - let verifier = Verifier::new_for_tests(&network, state_service); + for network in Network::iter() { + zebra_test::MULTI_THREADED_RUNTIME.block_on(async { + let nu5_activation_height = NetworkUpgrade::Nu5 + .activation_height(&network) + .expect("NU5 activation height is specified"); - let mut transaction = fake_v5_transactions_for_network(&network, blocks) - .next_back() - .expect("At least one fake V5 transaction in the test vectors"); - if transaction - .expiry_height() - .expect("V5 must have expiry_height") - < nu5_activation_height - { - let expiry_height = transaction.expiry_height_mut(); - *expiry_height = nu5_activation_height; - } + let state = service_fn(|_| async { unreachable!("Service should not be called") }); - let expected_hash = transaction.unmined_id(); - let expiry_height = transaction - .expiry_height() - .expect("V5 must have expiry_height"); + let mut tx = fake_v5_transactions_for_network(&network, network.block_iter()) + .next_back() + .expect("At least one fake V5 transaction in the test vectors"); - let result = verifier - .oneshot(Request::Block { - transaction: Arc::new(transaction), - known_utxos: Arc::new(HashMap::new()), - height: expiry_height, - time: DateTime::::MAX_UTC, - }) - .await; + if tx.expiry_height().expect("V5 must have expiry_height") < nu5_activation_height { + *tx.expiry_height_mut() = nu5_activation_height; + tx.update_network_upgrade(NetworkUpgrade::Nu5) + .expect("updating the network upgrade for a V5 tx should succeed"); + } - assert_eq!( - result.expect("unexpected error response").tx_id(), - expected_hash - ); - }) + let expected_hash = tx.unmined_id(); + let expiry_height = tx.expiry_height().expect("V5 must have expiry_height"); + + let verification_result = Verifier::new_for_tests(&network, state) + .oneshot(Request::Block { + transaction: Arc::new(tx), + known_utxos: Arc::new(HashMap::new()), + height: expiry_height, + time: DateTime::::MAX_UTC, + }) + .await; + + assert_eq!( + verification_result + .expect("successful verification") + .tx_id(), + expected_hash + ); + }); + } } /// Test if V4 transaction with transparent funds is accepted. @@ -1872,7 +1862,13 @@ async fn v5_coinbase_transaction_expiry_height() { *new_transaction.expiry_height_mut() = new_expiry_height; - let result = verifier + // Setting the new expiry height as the block height will activate NU6, so we need to set NU6 + // for the tx as well. + new_transaction + .update_network_upgrade(NetworkUpgrade::Nu6) + .expect("updating the network upgrade for a V5 tx should succeed"); + + let verification_result = verifier .clone() .oneshot(Request::Block { transaction: Arc::new(new_transaction.clone()), @@ -1883,7 +1879,9 @@ async fn v5_coinbase_transaction_expiry_height() { .await; assert_eq!( - result.expect("unexpected error response").tx_id(), + verification_result + .expect("successful verification") + .tx_id(), new_transaction.unmined_id() ); } @@ -1941,22 +1939,18 @@ async fn v5_transaction_with_too_low_expiry_height() { ); } -/// Tests if a non-coinbase V5 transaction with an expiry height exceeding the -/// maximum is rejected. +/// Tests if a non-coinbase V5 transaction with an expiry height exceeding the maximum is rejected. #[tokio::test] async fn v5_transaction_with_exceeding_expiry_height() { - let state_service = - service_fn(|_| async { unreachable!("State service should not be called") }); - let verifier = Verifier::new_for_tests(&Network::Mainnet, state_service); + let state = service_fn(|_| async { unreachable!("State service should not be called") }); - let block_height = block::Height::MAX; + let height_max = block::Height::MAX; - let fund_height = (block_height - 1).expect("fake source fund block height is too small"); let (input, output, known_utxos) = mock_transparent_transfer( - fund_height, + height_max.previous().expect("valid height"), true, 0, - Amount::try_from(1).expect("invalid value"), + Amount::try_from(1).expect("valid amount"), ); // This expiry height exceeds the maximum defined by the specification. @@ -1970,25 +1964,27 @@ async fn v5_transaction_with_exceeding_expiry_height() { expiry_height, sapling_shielded_data: None, orchard_shielded_data: None, - network_upgrade: NetworkUpgrade::Nu5, + network_upgrade: NetworkUpgrade::Nu6, }; - let result = verifier + let transaction_hash = transaction.hash(); + + let verification_result = Verifier::new_for_tests(&Network::Mainnet, state) .oneshot(Request::Block { - transaction: Arc::new(transaction.clone()), + transaction: Arc::new(transaction), known_utxos: Arc::new(known_utxos), - height: block_height, + height: height_max, time: DateTime::::MAX_UTC, }) .await; assert_eq!( - result, + verification_result, Err(TransactionError::MaximumExpiryHeight { expiry_height, is_coinbase: false, - block_height, - transaction_hash: transaction.hash(), + block_height: height_max, + transaction_hash, }) ); } @@ -2105,59 +2101,49 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() { /// Test if V5 transaction with an internal double spend of transparent funds is rejected. #[tokio::test] async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() { - let network = Network::Mainnet; - let network_upgrade = NetworkUpgrade::Nu5; - - let canopy_activation_height = NetworkUpgrade::Canopy - .activation_height(&network) - .expect("Canopy activation height is specified"); - - let transaction_block_height = - (canopy_activation_height + 10).expect("transaction block height is too large"); - - let fake_source_fund_height = - (transaction_block_height - 1).expect("fake source fund block height is too small"); + for network in Network::iter() { + let canopy_activation_height = NetworkUpgrade::Canopy + .activation_height(&network) + .expect("Canopy activation height is specified"); - // Create a fake transparent transfer that should succeed - let (input, output, known_utxos) = mock_transparent_transfer( - fake_source_fund_height, - true, - 0, - Amount::try_from(1).expect("invalid value"), - ); + let height = (canopy_activation_height + 10).expect("valid height"); - // Create a V4 transaction - let transaction = Transaction::V5 { - inputs: vec![input.clone(), input.clone()], - outputs: vec![output], - lock_time: LockTime::Height(block::Height(0)), - expiry_height: (transaction_block_height + 1).expect("expiry height is too large"), - sapling_shielded_data: None, - orchard_shielded_data: None, - network_upgrade, - }; + // Create a fake transparent transfer that should succeed + let (input, output, known_utxos) = mock_transparent_transfer( + height.previous().expect("valid height"), + true, + 0, + Amount::try_from(1).expect("valid amount"), + ); - let state_service = - service_fn(|_| async { unreachable!("State service should not be called") }); - let verifier = Verifier::new_for_tests(&network, state_service); + let transaction = Transaction::V5 { + inputs: vec![input.clone(), input.clone()], + outputs: vec![output], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: height.next().expect("valid height"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Canopy, + }; - let result = verifier - .oneshot(Request::Block { - transaction: Arc::new(transaction), - known_utxos: Arc::new(known_utxos), - height: transaction_block_height, - time: DateTime::::MAX_UTC, - }) - .await; + let state = service_fn(|_| async { unreachable!("State service should not be called") }); - let expected_outpoint = input.outpoint().expect("Input should have an outpoint"); + let verification_result = Verifier::new_for_tests(&network, state) + .oneshot(Request::Block { + transaction: Arc::new(transaction), + known_utxos: Arc::new(known_utxos), + height, + time: DateTime::::MAX_UTC, + }) + .await; - assert_eq!( - result, - Err(TransactionError::DuplicateTransparentSpend( - expected_outpoint - )) - ); + assert_eq!( + verification_result, + Err(TransactionError::DuplicateTransparentSpend( + input.outpoint().expect("Input should have an outpoint") + )) + ); + } } /// Test if signed V4 transaction with a dummy [`sprout::JoinSplit`] is accepted. @@ -2577,6 +2563,161 @@ fn v5_with_duplicate_orchard_action() { }); } +/// Checks that the tx verifier handles consensus branch ids in V5 txs correctly. +#[tokio::test] +async fn v5_consensus_branch_ids() { + let mut state = MockService::build().for_unit_tests(); + + let (input, output, known_utxos) = mock_transparent_transfer( + Height(1), + true, + 0, + Amount::try_from(10001).expect("valid amount"), + ); + + let known_utxos = Arc::new(known_utxos); + + // NU5 is the first network upgrade that supports V5 txs. + let mut network_upgrade = NetworkUpgrade::Nu5; + + let mut tx = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: Height::MAX_EXPIRY_HEIGHT, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }; + + let outpoint = match tx.inputs()[0] { + transparent::Input::PrevOut { outpoint, .. } => outpoint, + transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"), + }; + + for network in Network::iter() { + let verifier = Buffer::new(Verifier::new_for_tests(&network, state.clone()), 10); + + while let Some(next_nu) = network_upgrade.next_upgrade() { + // Check an outdated network upgrade. + let height = next_nu.activation_height(&network).expect("height"); + + let block_req = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(tx.clone()), + known_utxos: known_utxos.clone(), + // The consensus branch ID of the tx is outdated for this height. + height, + time: DateTime::::MAX_UTC, + }) + .map_err(|err| *err.downcast().expect("`TransactionError` type")); + + let mempool_req = verifier + .clone() + .oneshot(Request::Mempool { + transaction: tx.clone().into(), + // The consensus branch ID of the tx is outdated for this height. + height, + }) + .map_err(|err| *err.downcast().expect("`TransactionError` type")); + + let (block_rsp, mempool_rsp) = futures::join!(block_req, mempool_req); + + assert_eq!(block_rsp, Err(TransactionError::WrongConsensusBranchId)); + assert_eq!(mempool_rsp, Err(TransactionError::WrongConsensusBranchId)); + + // Check the currently supported network upgrade. + let height = network_upgrade.activation_height(&network).expect("height"); + + let block_req = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(tx.clone()), + known_utxos: known_utxos.clone(), + // The consensus branch ID of the tx is supported by this height. + height, + time: DateTime::::MAX_UTC, + }) + .map_ok(|rsp| rsp.tx_id()) + .map_err(|e| format!("{e}")); + + let mempool_req = verifier + .clone() + .oneshot(Request::Mempool { + transaction: tx.clone().into(), + // The consensus branch ID of the tx is supported by this height. + height, + }) + .map_ok(|rsp| rsp.tx_id()) + .map_err(|e| format!("{e}")); + + let state_req = async { + state + .expect_request(zebra_state::Request::UnspentBestChainUtxo(outpoint)) + .map(|r| { + r.respond(zebra_state::Response::UnspentBestChainUtxo( + known_utxos.get(&outpoint).map(|utxo| utxo.utxo.clone()), + )) + }) + .await; + + state + .expect_request_that(|req| { + matches!( + req, + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) + ) + }) + .map(|r| { + r.respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors) + }) + .await; + }; + + let (block_rsp, mempool_rsp, _) = futures::join!(block_req, mempool_req, state_req); + let txid = tx.unmined_id(); + + assert_eq!(block_rsp, Ok(txid)); + assert_eq!(mempool_rsp, Ok(txid)); + + // Check a network upgrade that Zebra doesn't support yet. + tx.update_network_upgrade(next_nu) + .expect("V5 txs support updating NUs"); + + let height = network_upgrade.activation_height(&network).expect("height"); + + let block_req = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(tx.clone()), + known_utxos: known_utxos.clone(), + // The consensus branch ID of the tx is not supported by this height. + height, + time: DateTime::::MAX_UTC, + }) + .map_err(|err| *err.downcast().expect("`TransactionError` type")); + + let mempool_req = verifier + .clone() + .oneshot(Request::Mempool { + transaction: tx.clone().into(), + // The consensus branch ID of the tx is not supported by this height. + height, + }) + .map_err(|err| *err.downcast().expect("`TransactionError` type")); + + let (block_rsp, mempool_rsp) = futures::join!(block_req, mempool_req); + + assert_eq!(block_rsp, Err(TransactionError::WrongConsensusBranchId)); + assert_eq!(mempool_rsp, Err(TransactionError::WrongConsensusBranchId)); + + // Shift the network upgrade for the next loop iteration. + network_upgrade = next_nu; + } + } +} + // Utility functions /// Create a mock transparent transfer to be included in a transaction.