From 7e011228469c4c5b2ce1c715dfda0cd2b8c0ee01 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 23 Oct 2023 13:06:04 +0200 Subject: [PATCH] evil: updates calls to `raw_header_hash` --- .../lib/node/ledger/shell/finalize_block.rs | 281 ++++++++++-------- apps/src/lib/node/ledger/shell/mod.rs | 13 +- .../lib/node/ledger/shell/process_proposal.rs | 27 +- sdk/src/tx.rs | 8 +- shared/src/ledger/protocol/mod.rs | 6 +- 5 files changed, 182 insertions(+), 153 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 1f635133e6..4fef1fc7ef 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -930,13 +930,13 @@ where // Allow to replay a specific wasm transaction. Needs as argument the // corresponding wrapper transaction to avoid replay of that in the process - fn allow_tx_replay(&mut self, mut wrapper_tx: Tx) { + fn allow_tx_replay(&mut self, wrapper_tx: Tx) { self.wl_storage .write_tx_hash(wrapper_tx.header_hash()) .expect("Error while deleting tx hash from storage"); self.wl_storage - .delete_tx_hash(wrapper_tx.update_header(TxType::Raw).header_hash()) + .delete_tx_hash(wrapper_tx.raw_header_hash()) .expect("Error while deleting tx hash from storage"); } } @@ -2048,9 +2048,11 @@ mod test_finalize_block { // won't receive votes from TM since we receive votes at a 1-block // delay, so votes will be empty here next_block_for_inflation(&mut shell, pkh1.clone(), vec![], None); - assert!(rewards_accumulator_handle() - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + rewards_accumulator_handle() + .is_empty(&shell.wl_storage) + .unwrap() + ); // FINALIZE BLOCK 2. Tell Namada that val1 is the block proposer. // Include votes that correspond to block 1. Make val2 the next block's @@ -2060,9 +2062,11 @@ mod test_finalize_block { assert!(rewards_prod_2.is_empty(&shell.wl_storage).unwrap()); assert!(rewards_prod_3.is_empty(&shell.wl_storage).unwrap()); assert!(rewards_prod_4.is_empty(&shell.wl_storage).unwrap()); - assert!(!rewards_accumulator_handle() - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + !rewards_accumulator_handle() + .is_empty(&shell.wl_storage) + .unwrap() + ); // Val1 was the proposer, so its reward should be larger than all // others, which should themselves all be equal let acc_sum = get_rewards_sum(&shell.wl_storage); @@ -2176,9 +2180,11 @@ mod test_finalize_block { None, ); } - assert!(rewards_accumulator_handle() - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + rewards_accumulator_handle() + .is_empty(&shell.wl_storage) + .unwrap() + ); let rp1 = rewards_prod_1 .get(&shell.wl_storage, &Epoch::default()) .unwrap() @@ -2230,12 +2236,10 @@ mod test_finalize_block { let (wrapper_tx, processed_tx) = mk_wrapper_tx(&shell, &crate::wallet::defaults::albert_keypair()); - let mut decrypted_tx = wrapper_tx; - decrypted_tx.update_header(TxType::Raw); let decrypted_hash_key = replay_protection::get_replay_protection_last_key( - &decrypted_tx.header_hash(), + &wrapper_tx.raw_header_hash(), ); // merkle tree root before finalize_block @@ -2263,21 +2267,25 @@ mod test_finalize_block { assert_eq!(root_pre.0, root_post.0); // Check transaction's hash in storage - assert!(shell - .shell - .wl_storage - .write_log - .has_replay_protection_entry(&decrypted_tx.header_hash()) - .unwrap_or_default()); + assert!( + shell + .shell + .wl_storage + .write_log + .has_replay_protection_entry(&wrapper_tx.raw_header_hash()) + .unwrap_or_default() + ); // Check that the hash is present in the merkle tree - assert!(!shell - .shell - .wl_storage - .storage - .block - .tree - .has_key(&decrypted_hash_key) - .unwrap()); + assert!( + !shell + .shell + .wl_storage + .storage + .block + .tree + .has_key(&decrypted_hash_key) + .unwrap() + ); } /// Test replay protection hash handling @@ -2321,16 +2329,9 @@ mod test_finalize_block { decrypted_tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); decrypted_tx_2.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - let decrypted_hash = - wrapper_tx.clone().update_header(TxType::Raw).header_hash(); - let decrypted_2_hash = wrapper_tx_2 - .clone() - .update_header(TxType::Raw) - .header_hash(); - let decrypted_3_hash = invalid_wrapper_tx - .clone() - .update_header(TxType::Raw) - .header_hash(); + let decrypted_hash = wrapper_tx.raw_header_hash(); + let decrypted_2_hash = wrapper_tx_2.raw_header_hash(); + let decrypted_3_hash = invalid_wrapper_tx.raw_header_hash(); // Write inner hashes in storage for hash in [&decrypted_hash, &decrypted_2_hash] { @@ -2412,36 +2413,48 @@ mod test_finalize_block { .as_str(); assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str()); - assert!(shell - .wl_storage - .write_log - .has_replay_protection_entry(&invalid_wrapper_hash) - .unwrap_or_default()); - assert!(!shell - .wl_storage - .write_log - .has_replay_protection_entry(&decrypted_3_hash) - .unwrap_or_default()); - assert!(!shell - .wl_storage - .write_log - .has_replay_protection_entry(&decrypted_hash) - .unwrap_or_default()); - assert!(shell - .wl_storage - .write_log - .has_replay_protection_entry(&wrapper_hash) - .unwrap_or_default()); - assert!(shell - .wl_storage - .storage - .has_replay_protection_entry(&decrypted_2_hash) - .expect("test failed")); - assert!(!shell - .wl_storage - .write_log - .has_replay_protection_entry(&wrapper_2_hash) - .unwrap_or_default()); + assert!( + shell + .wl_storage + .write_log + .has_replay_protection_entry(&invalid_wrapper_hash) + .unwrap_or_default() + ); + assert!( + !shell + .wl_storage + .write_log + .has_replay_protection_entry(&decrypted_3_hash) + .unwrap_or_default() + ); + assert!( + !shell + .wl_storage + .write_log + .has_replay_protection_entry(&decrypted_hash) + .unwrap_or_default() + ); + assert!( + shell + .wl_storage + .write_log + .has_replay_protection_entry(&wrapper_hash) + .unwrap_or_default() + ); + assert!( + shell + .wl_storage + .storage + .has_replay_protection_entry(&decrypted_2_hash) + .expect("test failed") + ); + assert!( + !shell + .wl_storage + .write_log + .has_replay_protection_entry(&wrapper_2_hash) + .unwrap_or_default() + ); } // Test that if the fee payer doesn't have enough funds for fee payment the @@ -2729,9 +2742,11 @@ mod test_finalize_block { .unwrap(), Some(ValidatorState::Consensus) ); - assert!(enqueued_slashes_handle() - .at(&Epoch::default()) - .is_empty(&shell.wl_storage)?); + assert!( + enqueued_slashes_handle() + .at(&Epoch::default()) + .is_empty(&shell.wl_storage)? + ); assert_eq!( get_num_consensus_validators(&shell.wl_storage, Epoch::default()) .unwrap(), @@ -2750,17 +2765,21 @@ mod test_finalize_block { .unwrap(), Some(ValidatorState::Jailed) ); - assert!(enqueued_slashes_handle() - .at(&epoch) - .is_empty(&shell.wl_storage)?); + assert!( + enqueued_slashes_handle() + .at(&epoch) + .is_empty(&shell.wl_storage)? + ); assert_eq!( get_num_consensus_validators(&shell.wl_storage, epoch).unwrap(), 5_u64 ); } - assert!(!enqueued_slashes_handle() - .at(&processing_epoch) - .is_empty(&shell.wl_storage)?); + assert!( + !enqueued_slashes_handle() + .at(&processing_epoch) + .is_empty(&shell.wl_storage)? + ); // Advance to the processing epoch loop { @@ -2783,9 +2802,11 @@ mod test_finalize_block { // println!("Reached processing epoch"); break; } else { - assert!(enqueued_slashes_handle() - .at(&shell.wl_storage.storage.block.epoch) - .is_empty(&shell.wl_storage)?); + assert!( + enqueued_slashes_handle() + .at(&shell.wl_storage.storage.block.epoch) + .is_empty(&shell.wl_storage)? + ); let stake1 = read_validator_stake( &shell.wl_storage, ¶ms, @@ -3344,13 +3365,15 @@ mod test_finalize_block { ) .unwrap(); assert_eq!(last_slash, Some(Epoch(4))); - assert!(namada_proof_of_stake::is_validator_frozen( - &shell.wl_storage, - &val1.address, - current_epoch, - ¶ms - ) - .unwrap()); + assert!( + namada_proof_of_stake::is_validator_frozen( + &shell.wl_storage, + &val1.address, + current_epoch, + ¶ms + ) + .unwrap() + ); assert!( namada_proof_of_stake::validator_slashes_handle(&val1.address) .is_empty(&shell.wl_storage) @@ -3859,14 +3882,12 @@ mod test_finalize_block { assert!(!consensus_val_set.at(&ep).is_empty(storage).unwrap()); // assert!(!below_cap_val_set.at(&ep).is_empty(storage). // unwrap()); - assert!(!validator_positions - .at(&ep) - .is_empty(storage) - .unwrap()); - assert!(!all_validator_addresses - .at(&ep) - .is_empty(storage) - .unwrap()); + assert!( + !validator_positions.at(&ep).is_empty(storage).unwrap() + ); + assert!( + !all_validator_addresses.at(&ep).is_empty(storage).unwrap() + ); } }; @@ -3905,25 +3926,33 @@ mod test_finalize_block { Epoch(1), Epoch(params.pipeline_len + default_past_epochs + 1), ); - assert!(!consensus_val_set - .at(&Epoch(0)) - .is_empty(&shell.wl_storage) - .unwrap()); - assert!(validator_positions - .at(&Epoch(0)) - .is_empty(&shell.wl_storage) - .unwrap()); - assert!(all_validator_addresses - .at(&Epoch(0)) - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + !consensus_val_set + .at(&Epoch(0)) + .is_empty(&shell.wl_storage) + .unwrap() + ); + assert!( + validator_positions + .at(&Epoch(0)) + .is_empty(&shell.wl_storage) + .unwrap() + ); + assert!( + all_validator_addresses + .at(&Epoch(0)) + .is_empty(&shell.wl_storage) + .unwrap() + ); // Advance to the epoch `consensus_val_set_len` + 1 loop { - assert!(!consensus_val_set - .at(&Epoch(0)) - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + !consensus_val_set + .at(&Epoch(0)) + .is_empty(&shell.wl_storage) + .unwrap() + ); let votes = get_default_true_votes( &shell.wl_storage, shell.wl_storage.storage.block.epoch, @@ -3934,10 +3963,12 @@ mod test_finalize_block { } } - assert!(consensus_val_set - .at(&Epoch(0)) - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + consensus_val_set + .at(&Epoch(0)) + .is_empty(&shell.wl_storage) + .unwrap() + ); // Advance one more epoch let votes = get_default_true_votes( @@ -3946,19 +3977,23 @@ mod test_finalize_block { ); current_epoch = advance_epoch(&mut shell, &pkh1, &votes, None); for ep in Epoch::default().iter_range(2) { - assert!(consensus_val_set - .at(&ep) - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + consensus_val_set + .at(&ep) + .is_empty(&shell.wl_storage) + .unwrap() + ); } for ep in Epoch::iter_bounds_inclusive( Epoch(2), current_epoch + params.pipeline_len, ) { - assert!(!consensus_val_set - .at(&ep) - .is_empty(&shell.wl_storage) - .unwrap()); + assert!( + !consensus_val_set + .at(&ep) + .is_empty(&shell.wl_storage) + .unwrap() + ); } Ok(()) diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index ce49a7f211..ab8ebd5b05 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -942,13 +942,12 @@ where ))); } - // Write wrapper hash to tx WAL + // Write wrapper hash to WAL temp_wl_storage .write_tx_hash(wrapper_hash) .map_err(|e| Error::ReplayAttempt(e.to_string()))?; - let inner_tx_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); + let inner_tx_hash = wrapper.raw_header_hash(); if temp_wl_storage .has_replay_protection_entry(&inner_tx_hash) .expect("Error while checking inner tx hash key in storage") @@ -959,7 +958,7 @@ where ))); } - // Write inner hash to tx WAL + // Write inner hash to WAL temp_wl_storage .write_tx_hash(inner_tx_hash) .map_err(|e| Error::ReplayAttempt(e.to_string())) @@ -1249,13 +1248,11 @@ where } // Replay protection check - let mut inner_tx = tx; - inner_tx.update_header(TxType::Raw); - let inner_tx_hash = &inner_tx.header_hash(); + let inner_tx_hash = tx.raw_header_hash(); if self .wl_storage .storage - .has_replay_protection_entry(inner_tx_hash) + .has_replay_protection_entry(&tx.raw_header_hash()) .expect("Error while checking inner tx hash key in storage") { response.code = ErrorCodes::ReplayTx.into(); diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index e05ae21fd0..a83bc7f62e 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -1087,11 +1087,13 @@ mod test_process_proposal { shell.chain_id.clone(), ) .to_bytes(); - assert!(shell - .process_proposal(ProcessProposal { - txs: vec![tx.clone(), tx] - }) - .is_err()); + assert!( + shell + .process_proposal(ProcessProposal { + txs: vec![tx.clone(), tx] + }) + .is_err() + ); } #[cfg(feature = "abcipp")] @@ -1248,9 +1250,11 @@ mod test_process_proposal { sig, } .sign(shell.mode.get_protocol_key().expect("Test failed")); - let mut txs = vec![EthereumTxData::BridgePool(vote_ext.into()) - .sign(protocol_key, shell.chain_id.clone()) - .to_bytes()]; + let mut txs = vec![ + EthereumTxData::BridgePool(vote_ext.into()) + .sign(protocol_key, shell.chain_id.clone()) + .to_bytes(), + ]; let event = EthereumEvent::TransfersToNamada { nonce: 0u64.into(), @@ -2176,15 +2180,12 @@ mod test_process_proposal { response[1].result.code, u32::from(ErrorCodes::ReplayTx) ); - // The checks happens on the inner hash first, so the tx is - // rejected because of this hash, not the - // wrapper one assert_eq!( response[1].result.info, format!( "Transaction replay attempt: Wrapper transaction hash \ {} already in storage", - wrapper.raw_header_hash() + wrapper.header_hash() ) ); } @@ -2247,7 +2248,7 @@ mod test_process_proposal { format!( "Transaction replay attempt: Inner transaction hash \ {} already in storage", - inner_unsigned_hash + wrapper.raw_header_hash() ) ); } diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index f1a85ebca8..962c6964a3 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -45,7 +45,7 @@ use namada_core::types::transaction::governance::{ InitProposalData, VoteProposalData, }; use namada_core::types::transaction::pgf::UpdateStewardCommission; -use namada_core::types::transaction::{pos, TxType}; +use namada_core::types::transaction::pos; use namada_core::types::{storage, token}; use namada_proof_of_stake::parameters::PosParams; use namada_proof_of_stake::types::{CommissionPair, ValidatorState}; @@ -200,11 +200,7 @@ pub async fn process_tx<'a>( let wrapper_hash = tx.header_hash().to_string(); // We use this to determine when the decrypted inner tx makes it // on-chain - let decrypted_hash = tx - .clone() - .update_header(TxType::Raw) - .header_hash() - .to_string(); + let decrypted_hash = tx.raw_header_hash().to_string(); let to_broadcast = TxBroadcastData::Live { tx, wrapper_hash, diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index dcbcb9b5ba..b8a9902ac5 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -217,7 +217,7 @@ where /// Returns the set of changed storage keys. The caller should write the hash of /// the wrapper header to storage in case of failure. pub(crate) fn apply_wrapper_tx<'a, D, H, CA, WLS>( - mut tx: Tx, + tx: Tx, wrapper: &WrapperTx, fee_unshield_transaction: Option, tx_bytes: &[u8], @@ -247,10 +247,10 @@ where // If wrapper was succesful, write inner tx hash to storage shell_params .wl_storage - .write_tx_hash(tx.update_header(TxType::Raw).header_hash()) + .write_tx_hash(tx.raw_header_hash()) .expect("Error while writing tx hash to storage"); changed_keys.insert(replay_protection::get_replay_protection_last_key( - &tx.header_hash(), + &tx.raw_header_hash(), )); Ok(changed_keys)