From dc3d1b02fcd542ce04a060556c04d2bdc3561db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ad=C3=A1n=20SDPC?= Date: Wed, 16 Oct 2024 16:07:17 +0200 Subject: [PATCH] feat(validations): fix versioned hashing for most transaction types --- data_structures/src/proto/versioning.rs | 70 +++++++++++++++++++++--- validations/src/tests/mod.rs | 51 ++++++++++++++++-- validations/src/validations.rs | 71 ++++++++++++++++++------- 3 files changed, 162 insertions(+), 30 deletions(-) diff --git a/data_structures/src/proto/versioning.rs b/data_structures/src/proto/versioning.rs index 541aa6d75..3cc34d128 100644 --- a/data_structures/src/proto/versioning.rs +++ b/data_structures/src/proto/versioning.rs @@ -278,14 +278,29 @@ impl Versioned for crate::transaction::DRTransactionBody { type LegacyType = ::ProtoStruct; } impl Versioned for crate::transaction::CommitTransactionBody { + // FIXME: implement proper versioning here for commit transactions type LegacyType = ::ProtoStruct; } impl Versioned for crate::transaction::RevealTransaction { + // FIXME: implement proper versioning here for reveal transactions type LegacyType = ::ProtoStruct; } impl Versioned for crate::transaction::RevealTransactionBody { type LegacyType = ::ProtoStruct; } +impl Versioned for crate::transaction::TallyTransaction { + // FIXME: implement proper versioning here for tally transactions + type LegacyType = ::ProtoStruct; +} +impl Versioned for crate::transaction::MintTransaction { + type LegacyType = ::ProtoStruct; +} +impl Versioned for crate::transaction::StakeTransactionBody { + type LegacyType = ::ProtoStruct; +} +impl Versioned for crate::transaction::UnstakeTransactionBody { + type LegacyType = ::ProtoStruct; +} pub trait AutoVersioned: ProtobufConvert {} @@ -294,8 +309,11 @@ impl AutoVersioned for crate::chain::SuperBlock {} impl AutoVersioned for crate::transaction::VTTransactionBody {} impl AutoVersioned for crate::transaction::DRTransactionBody {} impl AutoVersioned for crate::transaction::CommitTransactionBody {} -impl AutoVersioned for crate::transaction::RevealTransaction {} impl AutoVersioned for crate::transaction::RevealTransactionBody {} +impl AutoVersioned for crate::transaction::TallyTransaction {} +impl AutoVersioned for crate::transaction::MintTransaction {} +impl AutoVersioned for crate::transaction::StakeTransactionBody {} +impl AutoVersioned for crate::transaction::UnstakeTransactionBody {} pub trait VersionedHashable { fn versioned_hash(&self, version: ProtocolVersion) -> Hash; @@ -316,11 +334,17 @@ where impl VersionedHashable for crate::transaction::Transaction { fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + use crate::transaction::Transaction::*; + match self { - crate::transaction::Transaction::ValueTransfer(tx) => tx.versioned_hash(version), - _ => { - todo!(); - } + ValueTransfer(tx) => tx.versioned_hash(version), + DataRequest(tx) => tx.versioned_hash(version), + Commit(tx) => tx.versioned_hash(version), + Reveal(tx) => tx.versioned_hash(version), + Tally(tx) => tx.versioned_hash(version), + Mint(tx) => tx.versioned_hash(version), + Stake(tx) => tx.versioned_hash(version), + Unstake(tx) => tx.versioned_hash(version), } } } @@ -332,8 +356,42 @@ impl VersionedHashable for crate::transaction::VTTransaction { } } -impl VersionedHashable for crate::chain::Block { +impl VersionedHashable for crate::transaction::DRTransaction { + #[inline] + fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + self.body.versioned_hash(version) + } +} + +impl VersionedHashable for crate::transaction::CommitTransaction { + #[inline] + fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + self.body.versioned_hash(version) + } +} + +impl VersionedHashable for crate::transaction::RevealTransaction { #[inline] + fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + self.body.versioned_hash(version) + } +} + +impl VersionedHashable for crate::transaction::StakeTransaction { + #[inline] + fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + self.body.versioned_hash(version) + } +} + +impl VersionedHashable for crate::transaction::UnstakeTransaction { + #[inline] + fn versioned_hash(&self, version: ProtocolVersion) -> Hash { + self.body.versioned_hash(version) + } +} + +impl VersionedHashable for crate::chain::Block { fn versioned_hash(&self, version: ProtocolVersion) -> Hash { self.block_header.versioned_hash(version) } diff --git a/validations/src/tests/mod.rs b/validations/src/tests/mod.rs index 4713ea1e6..03179d531 100644 --- a/validations/src/tests/mod.rs +++ b/validations/src/tests/mod.rs @@ -23,7 +23,7 @@ use witnet_data_structures::{ calculate_tally_change, calculate_witness_reward, create_tally, DataRequestPool, }, error::{BlockError, DataRequestError, Secp256k1ConversionError, TransactionError}, - proto::versioning::ProtocolVersion, + proto::versioning::{ProtocolVersion, VersionedHashable}, radon_error::RadonError, radon_report::{RadonReport, ReportContext, TypeLike}, staking::stakes::StakesTracker, @@ -1497,6 +1497,7 @@ fn data_request_no_inputs() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1534,6 +1535,7 @@ fn data_request_no_inputs_but_one_signature() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1580,6 +1582,7 @@ fn data_request_one_input_but_no_signature() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1627,6 +1630,7 @@ fn data_request_one_input_signatures() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, )?; verify_signatures_test(signatures_to_verify)?; @@ -1670,6 +1674,7 @@ fn data_request_input_double_spend() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1710,6 +1715,7 @@ fn data_request_input_not_in_utxo() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1755,6 +1761,7 @@ fn data_request_input_not_enough_value() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1824,6 +1831,7 @@ fn data_request_output_value_overflow() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -1861,6 +1869,7 @@ fn test_drtx(dr_output: DataRequestOutput) -> Result<(), failure::Error> { u32::MAX, REQUIRED_REWARD_COLLATERAL_RATIO, &all_wips_active(), + None, ) .map(|_| ()) } @@ -2263,6 +2272,7 @@ fn data_request_http_post_before_wip_activation() { u32::MAX, REQUIRED_REWARD_COLLATERAL_RATIO, &active_wips, + None, ) .map(|_| ()) }; @@ -2331,6 +2341,7 @@ fn data_request_http_get_with_headers_before_wip_activation() { u32::MAX, REQUIRED_REWARD_COLLATERAL_RATIO, &active_wips, + None, ) .map(|_| ()) }; @@ -2389,6 +2400,7 @@ fn data_request_parse_xml_before_wip_activation() { u32::MAX, REQUIRED_REWARD_COLLATERAL_RATIO, &active_wips, + None, ) .map(|_| ()) }; @@ -2445,6 +2457,7 @@ fn data_request_parse_xml_after_wip_activation() { u32::MAX, REQUIRED_REWARD_COLLATERAL_RATIO, &active_wips, + None, ) .map(|_| ()) }; @@ -2478,6 +2491,7 @@ fn dr_validation_weight_limit_exceeded() { 1625 - 1, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( @@ -2566,6 +2580,7 @@ fn data_request_miner_fee() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ) .map(|(_, _, fee)| fee) .unwrap(); @@ -2617,6 +2632,7 @@ fn data_request_miner_fee_with_change() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ) .map(|(_, _, fee)| fee) .unwrap(); @@ -2668,6 +2684,7 @@ fn data_request_change_to_different_pkh() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( @@ -2729,6 +2746,7 @@ fn data_request_two_change_outputs() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( @@ -2782,6 +2800,7 @@ fn data_request_miner_fee_with_too_much_change() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -2830,6 +2849,7 @@ fn data_request_zero_value_output() { MAX_DR_WEIGHT, REQUIRED_REWARD_COLLATERAL_RATIO, ¤t_active_wips(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -2881,6 +2901,7 @@ fn data_request_reward_collateral_ratio_wip() { MAX_DR_WEIGHT, PSEUDO_CONSENSUS_CONSTANTS_WIP0022_REWARD_COLLATERAL_RATIO, &active_wips, + None, ); x.unwrap(); @@ -2897,6 +2918,7 @@ fn data_request_reward_collateral_ratio_wip() { MAX_DR_WEIGHT, PSEUDO_CONSENSUS_CONSTANTS_WIP0022_REWARD_COLLATERAL_RATIO, &active_wips, + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -2952,6 +2974,7 @@ fn data_request_reward_collateral_ratio_limit() { MAX_DR_WEIGHT, PSEUDO_CONSENSUS_CONSTANTS_WIP0022_REWARD_COLLATERAL_RATIO, &active_wips, + None, ); x.unwrap(); @@ -2979,6 +3002,7 @@ fn data_request_reward_collateral_ratio_limit() { MAX_DR_WEIGHT, PSEUDO_CONSENSUS_CONSTANTS_WIP0022_REWARD_COLLATERAL_RATIO, &active_wips, + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -10002,6 +10026,7 @@ fn test_blocks(txns: Vec<(BlockTransactions, u64)>) -> Result<(), failure::Error MAX_VT_WEIGHT, MAX_DR_WEIGHT, GENESIS_BLOCK_HASH.parse().unwrap(), + None, ) } @@ -10010,6 +10035,7 @@ fn test_blocks_with_limits( max_vt_weight: u32, max_dr_weight: u32, genesis_block_hash: Hash, + protocol_version: Option, ) -> Result<(), failure::Error> { if txns.len() > 1 { // FIXME(#685): add sequence validations @@ -10022,7 +10048,7 @@ fn test_blocks_with_limits( let mut utxo_set = UnspentOutputsPool::default(); let block_number = 0; let stakes = StakesTracker::default(); - let protocol_version = ProtocolVersion::default(); + let protocol_version = protocol_version.unwrap_or_default(); let consensus_constants = ConsensusConstants { checkpoint_zero_timestamp: 0, @@ -11305,6 +11331,7 @@ fn validate_vt_weight_overflow() { 2 * 493 - 1, 0, GENESIS_BLOCK_HASH.parse().unwrap(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -11343,7 +11370,13 @@ fn validate_vt_weight_valid() { DEFAULT_INPUT_VALUE - 2 * 10, ) }; - let x = test_blocks_with_limits(vec![t0], 2 * 493, 0, GENESIS_BLOCK_HASH.parse().unwrap()); + let x = test_blocks_with_limits( + vec![t0], + 2 * 493, + 0, + GENESIS_BLOCK_HASH.parse().unwrap(), + None, + ); x.unwrap(); } @@ -11370,7 +11403,7 @@ fn validate_vt_weight_genesis_valid() { 1_000_000 - 10, ) }; - let x = test_blocks_with_limits(vec![t0], 0, 0, new_genesis.parse().unwrap()); + let x = test_blocks_with_limits(vec![t0], 0, 0, new_genesis.parse().unwrap(), None); x.unwrap(); } @@ -11406,6 +11439,7 @@ fn validate_dr_weight_overflow() { 0, 2 * 1589 - 1, GENESIS_BLOCK_HASH.parse().unwrap(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -11445,6 +11479,7 @@ fn validate_dr_weight_overflow_126_witnesses() { 0, MAX_DR_WEIGHT, GENESIS_BLOCK_HASH.parse().unwrap(), + None, ); assert_eq!( x.unwrap_err().downcast::().unwrap(), @@ -11483,7 +11518,13 @@ fn validate_dr_weight_valid() { DEFAULT_INPUT_VALUE - 2 * dr_value, ) }; - let x = test_blocks_with_limits(vec![t0], 0, 2 * 1605, GENESIS_BLOCK_HASH.parse().unwrap()); + let x = test_blocks_with_limits( + vec![t0], + 0, + 2 * 1605, + GENESIS_BLOCK_HASH.parse().unwrap(), + None, + ); x.unwrap(); } diff --git a/validations/src/validations.rs b/validations/src/validations.rs index d1106d627..e27f0482c 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -515,6 +515,7 @@ pub fn validate_dr_transaction<'a>( max_dr_weight: u32, required_reward_collateral_ratio: u64, active_wips: &ActiveWips, + protocol_version: Option, ) -> Result<(Vec<&'a Input>, Vec<&'a ValueTransferOutput>, u64), failure::Error> { if dr_tx.weight() > max_dr_weight { return Err(TransactionError::DataRequestWeightLimitExceeded { @@ -524,11 +525,12 @@ pub fn validate_dr_transaction<'a>( } .into()); } + let protocol_version = protocol_version.unwrap_or_default(); validate_transaction_signature( &dr_tx.signatures, &dr_tx.body.inputs, - dr_tx.hash(), + dr_tx.versioned_hash(protocol_version), utxo_diff, signatures_to_verify, )?; @@ -1771,10 +1773,15 @@ pub fn validate_block_transactions( } vt_weight = acc_weight; - update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash()); + update_utxo_diff( + &mut utxo_diff, + inputs, + outputs, + transaction.versioned_hash(protocol_version), + ); // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; vt_mt.push(Sha256(sha)); @@ -1830,10 +1837,15 @@ pub fn validate_block_transactions( transaction.body.collateral.iter(), transaction.body.outputs.iter(), ); - update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash()); + update_utxo_diff( + &mut utxo_diff, + inputs, + outputs, + transaction.versioned_hash(protocol_version), + ); // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; co_mt.push(Sha256(sha)); } @@ -1866,7 +1878,7 @@ pub fn validate_block_transactions( total_fee += fee; // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; re_mt.push(Sha256(sha)); } @@ -1874,7 +1886,7 @@ pub fn validate_block_transactions( // Make sure that the block does not try to include data requests asking for too many witnesses for transaction in &block.txns.data_request_txns { - let dr_tx_hash = transaction.hash(); + let dr_tx_hash = transaction.versioned_hash(protocol_version); if !dr_pool.data_request_pool.contains_key(&dr_tx_hash) && data_request_has_too_many_witnesses( &transaction.body.dr_output, @@ -1884,9 +1896,13 @@ pub fn validate_block_transactions( { log::debug!( "Temporarily adding data request {} to data request pool for validation purposes", - transaction.hash() + transaction.versioned_hash(protocol_version) ); - if let Err(e) = dr_pool.process_data_request(transaction, epoch, &block.hash()) { + if let Err(e) = dr_pool.process_data_request( + transaction, + epoch, + &block.versioned_hash(protocol_version), + ) { log::error!("Error adding data request to the data request pool: {}", e); } if let Some(dr_state) = dr_pool.data_request_state_mutable(&dr_tx_hash) { @@ -1932,10 +1948,15 @@ pub fn validate_block_transactions( total_fee += fee; - update_utxo_diff(&mut utxo_diff, vec![], outputs, transaction.hash()); + update_utxo_diff( + &mut utxo_diff, + vec![], + outputs, + transaction.versioned_hash(protocol_version), + ); // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; ta_mt.push(Sha256(sha)); } @@ -1946,7 +1967,7 @@ pub fn validate_block_transactions( if !expected_tally_ready_drs.is_empty() { return Err(BlockError::MissingExpectedTallies { count: expected_tally_ready_drs.len(), - block_hash: block.hash(), + block_hash: block.versioned_hash(protocol_version), } .into()); } @@ -1986,13 +2007,19 @@ pub fn validate_block_transactions( consensus_constants.max_dr_weight, required_reward_collateral_ratio, active_wips, + None, )?; total_fee += fee; - update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash()); + update_utxo_diff( + &mut utxo_diff, + inputs, + outputs, + transaction.versioned_hash(protocol_version), + ); // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; dr_mt.push(Sha256(sha)); @@ -2061,10 +2088,15 @@ pub fn validate_block_transactions( st_weight = acc_weight; let outputs = change.iter().collect_vec(); - update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash()); + update_utxo_diff( + &mut utxo_diff, + inputs, + outputs, + transaction.versioned_hash(protocol_version), + ); // Add new hash to merkle tree - st_mt.push(transaction.hash().into()); + st_mt.push(transaction.versioned_hash(protocol_version).into()); // TODO: Move validations to a visitor // // Execute visitor @@ -2102,7 +2134,7 @@ pub fn validate_block_transactions( ut_weight = acc_weight; // Add new hash to merkle tree - let txn_hash = transaction.hash(); + let txn_hash = transaction.versioned_hash(protocol_version); let Hash::SHA256(sha) = txn_hash; ut_mt.push(Sha256(sha)); @@ -2135,13 +2167,13 @@ pub fn validate_block_transactions( &mut utxo_diff, vec![], block.txns.mint.outputs.iter(), - block.txns.mint.hash(), + block.txns.mint.versioned_hash(protocol_version), ); } // Validate Merkle Root let merkle_roots = BlockMerkleRoots { - mint_hash: block.txns.mint.hash(), + mint_hash: block.txns.mint.versioned_hash(protocol_version), vt_hash_merkle_root: Hash::from(vt_hash_merkle_root), dr_hash_merkle_root: Hash::from(dr_hash_merkle_root), commit_hash_merkle_root: Hash::from(co_hash_merkle_root), @@ -2319,6 +2351,7 @@ pub fn validate_new_transaction( max_dr_weight, required_reward_collateral_ratio, active_wips, + None, ) .map(|(_, _, fee)| fee), Transaction::Commit(tx) => validate_commit_transaction(