diff --git a/.changelog/unreleased/improvements/1867-fix-replay-protection.md b/.changelog/unreleased/improvements/1867-fix-replay-protection.md new file mode 100644 index 00000000000..ad22c70c551 --- /dev/null +++ b/.changelog/unreleased/improvements/1867-fix-replay-protection.md @@ -0,0 +1,2 @@ +- Reworked the signature of inner transactions to improve safety and fix replay + protection. ([\#1867](https://github.com/anoma/namada/pull/1867)) \ No newline at end of file diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 53252065f11..fa329ae6818 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -213,9 +213,7 @@ where .pop() .expect("Missing wrapper tx in queue") .tx - .clone() - .update_header(TxType::Raw) - .header_hash(); + .raw_header_hash(); let tx_hash_key = replay_protection::get_replay_protection_key(&tx_hash); self.wl_storage @@ -276,7 +274,7 @@ where continue; } - let (mut tx_event, tx_unsigned_hash, mut tx_gas_meter, wrapper) = + let (mut tx_event, tx_header_hash, mut tx_gas_meter, wrapper) = match &tx_header.tx_type { TxType::Wrapper(wrapper) => { stats.increment_wrapper_txs(); @@ -286,7 +284,7 @@ where } TxType::Decrypted(inner) => { // We remove the corresponding wrapper tx from the queue - let mut tx_in_queue = self + let tx_in_queue = self .wl_storage .storage .tx_queue @@ -323,12 +321,7 @@ where ( event, - Some( - tx_in_queue - .tx - .update_header(TxType::Raw) - .header_hash(), - ), + Some(tx_in_queue.tx.raw_header_hash()), TxGasMeter::new_from_sub_limit(tx_in_queue.gas), None, ) @@ -511,7 +504,7 @@ where // If transaction type is Decrypted and failed because of // out of gas, remove its hash from storage to allow // rewrapping it - if let Some(hash) = tx_unsigned_hash { + if let Some(hash) = tx_header_hash { if let Error::TxApply(protocol::Error::GasError(_)) = msg { @@ -2272,11 +2265,8 @@ mod test_finalize_block { let wrapper_hash_key = replay_protection::get_replay_protection_key( &wrapper_tx.header_hash(), ); - let mut decrypted_tx = wrapper_tx; - - decrypted_tx.update_header(TxType::Raw); let decrypted_hash_key = replay_protection::get_replay_protection_key( - &decrypted_tx.header_hash(), + &wrapper_tx.raw_header_hash(), ); // merkle tree root before finalize_block @@ -2362,7 +2352,7 @@ mod test_finalize_block { // Write inner hash in storage let inner_hash_key = replay_protection::get_replay_protection_key( - &wrapper_tx.clone().update_header(TxType::Raw).header_hash(), + &wrapper_tx.raw_header_hash(), ); shell .wl_storage @@ -2439,7 +2429,7 @@ mod test_finalize_block { &wrapper.header_hash(), ); let inner_hash_key = replay_protection::get_replay_protection_key( - &wrapper.clone().update_header(TxType::Raw).header_hash(), + &wrapper.raw_header_hash(), ); let processed_tx = ProcessedTx { diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index a1c17fe4508..bfaf5135302 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -146,22 +146,19 @@ impl From for TxResult { #[derive(Debug, Copy, Clone, FromPrimitive, ToPrimitive, PartialEq, Eq)] pub enum ErrorCodes { Ok = 0, - InvalidDecryptedChainId = 1, - ExpiredDecryptedTx = 2, - DecryptedTxGasLimit = 3, - WasmRuntimeError = 4, - InvalidTx = 5, - InvalidSig = 6, - InvalidOrder = 7, - ExtraTxs = 8, - Undecryptable = 9, - AllocationError = 10, - ReplayTx = 11, - InvalidChainId = 12, - ExpiredTx = 13, - TxGasLimit = 14, - FeeError = 15, - InvalidVoteExtension = 16, + WasmRuntimeError = 1, + InvalidTx = 2, + InvalidSig = 3, + InvalidOrder = 4, + ExtraTxs = 5, + Undecryptable = 6, + AllocationError = 7, + ReplayTx = 8, + InvalidChainId = 9, + ExpiredTx = 10, + TxGasLimit = 11, + FeeError = 12, + InvalidVoteExtension = 13, } impl ErrorCodes { @@ -172,11 +169,7 @@ impl ErrorCodes { // NOTE: pattern match on all `ErrorCodes` variants, in order // to catch potential bugs when adding new codes match self { - Ok - | InvalidDecryptedChainId - | ExpiredDecryptedTx - | WasmRuntimeError - | DecryptedTxGasLimit => true, + Ok | WasmRuntimeError => true, InvalidTx | InvalidSig | InvalidOrder | ExtraTxs | Undecryptable | AllocationError | ReplayTx | InvalidChainId | ExpiredTx | TxGasLimit | FeeError | InvalidVoteExtension => false, @@ -929,11 +922,9 @@ where pub fn replay_protection_checks( &self, wrapper: &Tx, - tx_bytes: &[u8], temp_wl_storage: &mut TempWlStorage, ) -> Result<()> { - let inner_tx_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); + let inner_tx_hash = wrapper.raw_header_hash(); let inner_hash_key = replay_protection::get_replay_protection_key(&inner_tx_hash); if temp_wl_storage @@ -952,9 +943,7 @@ where .write(&inner_hash_key, vec![]) .expect("Couldn't write inner transaction hash to write log"); - let tx = - Tx::try_from(tx_bytes).expect("Deserialization shouldn't fail"); - let wrapper_hash = tx.header_hash(); + let wrapper_hash = wrapper.header_hash(); let wrapper_hash_key = replay_protection::get_replay_protection_key(&wrapper_hash); if temp_wl_storage @@ -1089,22 +1078,19 @@ where } }; - let tx_chain_id = tx.header.chain_id.clone(); - let tx_expiration = tx.header.expiration; - // Tx chain id - if tx_chain_id != self.chain_id { + if tx.header.chain_id != self.chain_id { response.code = ErrorCodes::InvalidChainId.into(); response.log = format!( "{INVALID_MSG}: Tx carries a wrong chain id: expected {}, \ found {}", - self.chain_id, tx_chain_id + self.chain_id, tx.header.chain_id ); return response; } // Tx expiration - if let Some(exp) = tx_expiration { + if let Some(exp) = tx.header.expiration { let last_block_timestamp = self.get_block_timestamp(None); if last_block_timestamp > exp { @@ -1263,11 +1249,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(); let inner_hash_key = - replay_protection::get_replay_protection_key(inner_tx_hash); + replay_protection::get_replay_protection_key( + &inner_tx_hash, + ); if self .wl_storage .storage @@ -2502,8 +2488,7 @@ mod tests { ) ); - let inner_tx_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); + let inner_tx_hash = wrapper.raw_header_hash(); // Write inner hash in storage let inner_hash_key = replay_protection::get_replay_protection_key(&inner_tx_hash); diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 3687a6d39b4..986bf32bbe9 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -246,7 +246,7 @@ where tx_gas_meter.add_tx_size_gas(tx_bytes).map_err(|_| ())?; // Check replay protection - self.replay_protection_checks(&tx, tx_bytes, temp_wl_storage) + self.replay_protection_checks(&tx, temp_wl_storage) .map_err(|_| ())?; // Check fees @@ -1279,8 +1279,7 @@ mod test_prepare_proposal { [(0, keypair)].into_iter().collect(), None, ))); - let inner_unsigned_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); + let inner_unsigned_hash = wrapper.raw_header_hash(); // Write inner hash to storage let hash_key = diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index ab544de3f83..3b75de9948d 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -721,14 +721,7 @@ where metadata.has_decrypted_txs = true; match tx_queue_iter.next() { Some(wrapper) => { - let mut inner_tx = tx.clone(); - inner_tx.update_header(TxType::Raw); - if wrapper - .tx - .clone() - .update_header(TxType::Raw) - .header_hash() - != inner_tx.header_hash() + if wrapper.tx.raw_header_hash() != tx.raw_header_hash() { TxResult { code: ErrorCodes::InvalidOrder.into(), @@ -742,35 +735,6 @@ where wrapper.tx.clone(), privkey, ) { - // Tx chain id - if wrapper.tx.header.chain_id != self.chain_id { - return TxResult { - code: ErrorCodes::InvalidDecryptedChainId - .into(), - info: format!( - "Decrypted tx carries a wrong chain \ - id: expected {}, found {}", - self.chain_id, - wrapper.tx.header.chain_id - ), - }; - } - - // Tx expiration - if let Some(exp) = wrapper.tx.header.expiration { - if block_time > exp { - return TxResult { - code: ErrorCodes::ExpiredDecryptedTx - .into(), - info: format!( - "Decrypted tx expired at {:#?}, \ - block time: {:#?}", - exp, block_time - ), - }; - } - } - TxResult { code: ErrorCodes::Ok.into(), info: "Process Proposal accepted this \ @@ -878,11 +842,9 @@ where } } else { // Replay protection checks - if let Err(e) = self.replay_protection_checks( - &tx, - tx_bytes, - temp_wl_storage, - ) { + if let Err(e) = + self.replay_protection_checks(&tx, temp_wl_storage) + { return TxResult { code: ErrorCodes::ReplayTx.into(), info: e.to_string(), @@ -2226,10 +2188,7 @@ mod test_process_proposal { format!( "Transaction replay attempt: Inner transaction hash \ {} already in storage", - wrapper - .clone() - .update_header(TxType::Raw) - .header_hash(), + wrapper.raw_header_hash() ) ); } @@ -2263,10 +2222,9 @@ mod test_process_proposal { [(0, keypair)].into_iter().collect(), None, ))); - let inner_unsigned_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); // Write inner hash to storage + let inner_unsigned_hash = wrapper.raw_header_hash(); let hash_key = replay_protection::get_replay_protection_key(&inner_unsigned_hash); shell @@ -2327,8 +2285,7 @@ mod test_process_proposal { [(0, keypair)].into_iter().collect(), None, ))); - let inner_unsigned_hash = - wrapper.clone().update_header(TxType::Raw).header_hash(); + let inner_unsigned_hash = wrapper.raw_header_hash(); new_wrapper.update_header(TxType::Wrapper(Box::new(WrapperTx::new( Fee { @@ -2432,70 +2389,6 @@ mod test_process_proposal { } } - /// Test that a decrypted transaction with a mismatching chain id gets - /// rejected without rejecting the entire block - #[test] - fn test_decrypted_wrong_chain_id() { - let (mut shell, _recv, _, _) = test_utils::setup(); - let keypair = crate::wallet::defaults::daewon_keypair(); - - let wrong_chain_id = ChainId("Wrong chain id".to_string()); - let mut wrapper = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: token::Amount::zero(), - token: shell.wl_storage.storage.native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - wrapper.header.chain_id = wrong_chain_id.clone(); - wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned())); - wrapper - .set_data(Data::new("new transaction data".as_bytes().to_owned())); - let mut decrypted = wrapper.clone(); - - decrypted.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - decrypted.add_section(Section::Signature(Signature::new( - decrypted.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); - let gas_limit = Gas::from(wrapper.header.wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(wrapper.to_bytes().len() as u64)) - .unwrap(); - let wrapper_in_queue = TxInQueue { - tx: wrapper, - gas: gas_limit, - }; - shell.wl_storage.storage.tx_queue.push(wrapper_in_queue); - - // Run validation - let request = ProcessProposal { - txs: vec![decrypted.to_bytes()], - }; - - match shell.process_proposal(request) { - Ok(response) => { - assert_eq!( - response[0].result.code, - u32::from(ErrorCodes::InvalidDecryptedChainId) - ); - assert_eq!( - response[0].result.info, - format!( - "Decrypted tx carries a wrong chain id: expected {}, \ - found {}", - shell.chain_id, wrong_chain_id - ) - ) - } - Err(_) => panic!("Test failed"), - } - } - /// Test that an expired wrapper transaction causes a block rejection #[test] fn test_expired_wrapper() { @@ -2538,62 +2431,6 @@ mod test_process_proposal { } } - /// Test that an expired decrypted transaction is correctly marked as so - /// without rejecting the entire block - #[test] - fn test_expired_decrypted() { - let (mut shell, _recv, _, _) = test_utils::setup(); - let keypair = crate::wallet::defaults::daewon_keypair(); - - let mut wrapper = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: token::Amount::zero(), - token: shell.wl_storage.storage.native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - wrapper.header.chain_id = shell.chain_id.clone(); - wrapper.header.expiration = Some(DateTimeUtc::default()); - wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned())); - wrapper - .set_data(Data::new("new transaction data".as_bytes().to_owned())); - let mut decrypted = wrapper.clone(); - - decrypted.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - decrypted.add_section(Section::Signature(Signature::new( - decrypted.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); - let gas_limit = Gas::from(wrapper.header.wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(wrapper.to_bytes().len() as u64)) - .unwrap(); - let wrapper_in_queue = TxInQueue { - tx: wrapper, - gas: gas_limit, - }; - shell.wl_storage.storage.tx_queue.push(wrapper_in_queue); - - // Run validation - let request = ProcessProposal { - txs: vec![decrypted.to_bytes()], - }; - match shell.process_proposal(request) { - Ok(response) => { - assert_eq!(response.len(), 1); - assert_eq!( - response[0].result.code, - u32::from(ErrorCodes::ExpiredDecryptedTx) - ); - } - Err(_) => panic!("Test failed"), - } - } - /// Check that a tx requiring more gas than the block limit causes a block /// rejection #[test] diff --git a/benches/lib.rs b/benches/lib.rs index 47645abdf40..91e65ec6ff0 100644 --- a/benches/lib.rs +++ b/benches/lib.rs @@ -452,7 +452,7 @@ pub fn generate_tx( if let Some(signer) = signer { tx.add_section(Section::Signature(Signature::new( - tx.sechashes(), + vec![tx.raw_header_hash()], [(0, signer.clone())].into_iter().collect(), None, ))); @@ -495,7 +495,7 @@ pub fn generate_foreign_key_tx(signer: &SecretKey) -> Tx { .unwrap(), )); tx.add_section(Section::Signature(Signature::new( - tx.sechashes(), + vec![tx.raw_header_hash()], [(0, signer.clone())].into_iter().collect(), None, ))); diff --git a/core/src/proto/types.rs b/core/src/proto/types.rs index a6082fbbabb..75bf5ad5bfb 100644 --- a/core/src/proto/types.rs +++ b/core/src/proto/types.rs @@ -628,6 +628,9 @@ impl CompressedSignature { if idx == 0 { // The "zeroth" section is the header targets.push(tx.header_hash()); + } else if idx == 255 { + // The 255th section is the raw header + targets.push(tx.raw_header_hash()); } else { targets.push(tx.sections[idx as usize - 1].get_hash()); } @@ -1250,6 +1253,14 @@ impl Tx { Section::Header(self.header.clone()).get_hash() } + /// Gets the hash of the decrypted transaction's header + pub fn raw_header_hash(&self) -> crate::types::hash::Hash { + let mut raw_header = self.header(); + raw_header.tx_type = TxType::Raw; + + Section::Header(raw_header).get_hash() + } + /// Get hashes of all the sections in this transaction pub fn sechashes(&self) -> Vec { let mut hashes = vec![self.header_hash()]; @@ -1272,6 +1283,10 @@ impl Tx { ) -> Option> { if self.header_hash() == *hash { return Some(Cow::Owned(Section::Header(self.header.clone()))); + } else if self.raw_header_hash() == *hash { + let mut header = self.header(); + header.tx_type = TxType::Raw; + return Some(Cow::Owned(Section::Header(header))); } for section in &self.sections { if section.get_hash() == *hash { @@ -1358,20 +1373,6 @@ impl Tx { bytes } - /// Get the inner section hashes - pub fn inner_section_targets(&self) -> Vec { - let mut sections_hashes = self - .sections - .iter() - .filter_map(|section| match section { - Section::Data(_) | Section::Code(_) => Some(section.get_hash()), - _ => None, - }) - .collect::>(); - sections_hashes.sort(); - sections_hashes - } - /// Verify that the section with the given hash has been signed by the given /// public key pub fn verify_signatures( @@ -1486,7 +1487,7 @@ impl Tx { public_keys_index_map: &AccountPublicKeysMap, signer: Option
, ) -> Vec { - let targets = self.inner_section_targets(); + let targets = vec![self.raw_header_hash()]; let mut signatures = Vec::new(); let section = Signature::new( targets, @@ -1764,8 +1765,10 @@ impl Tx { account_public_keys_map: AccountPublicKeysMap, signer: Option
, ) -> &mut Self { + // The inner tx signer signs the Decrypted version of the Header + let hashes = vec![self.raw_header_hash()]; self.protocol_filter(); - let hashes = self.inner_section_targets(); + self.add_section(Section::Signature(Signature::new( hashes, account_public_keys_map.index_secret_keys(keypairs), @@ -1781,7 +1784,7 @@ impl Tx { ) -> &mut Self { self.protocol_filter(); let mut pk_section = Signature { - targets: self.inner_section_targets(), + targets: vec![self.raw_header_hash()], signatures: BTreeMap::new(), signer: Signer::PubKeys(vec![]), }; @@ -1792,7 +1795,7 @@ impl Tx { // Add the signature under the given multisig address let section = sections.entry(addr.clone()).or_insert_with(|| Signature { - targets: self.inner_section_targets(), + targets: vec![self.raw_header_hash()], signatures: BTreeMap::new(), signer: Signer::Address(addr.clone()), }); diff --git a/core/src/types/transaction/mod.rs b/core/src/types/transaction/mod.rs index 8acb9e6c7ef..419611e01ec 100644 --- a/core/src/types/transaction/mod.rs +++ b/core/src/types/transaction/mod.rs @@ -234,7 +234,7 @@ mod test_process_tx { .set_data(Data::new("transaction data".as_bytes().to_owned())) .clone(); tx.add_section(Section::Signature(Signature::new( - vec![*tx.code_sechash(), *tx.data_sechash()], + vec![tx.raw_header_hash()], [(0, gen_keypair())].into_iter().collect(), None, ))); diff --git a/core/src/types/transaction/protocol.rs b/core/src/types/transaction/protocol.rs index 1a51434b296..7770df1fed1 100644 --- a/core/src/types/transaction/protocol.rs +++ b/core/src/types/transaction/protocol.rs @@ -335,11 +335,7 @@ mod protocol_txs { .expect("Serializing request should not fail"), )); outer_tx.add_section(Section::Signature(Signature::new( - vec![ - outer_tx.header_hash(), - *outer_tx.code_sechash(), - *outer_tx.data_sechash(), - ], + vec![outer_tx.header_hash()], [(0, signing_key.clone())].into_iter().collect(), None, ))); diff --git a/shared/src/ledger/native_vp/ibc/mod.rs b/shared/src/ledger/native_vp/ibc/mod.rs index 3b6521905b8..91b506fc0a1 100644 --- a/shared/src/ledger/native_vp/ibc/mod.rs +++ b/shared/src/ledger/native_vp/ibc/mod.rs @@ -724,7 +724,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1037,7 +1037,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1371,7 +1371,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1459,7 +1459,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1584,7 +1584,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1708,7 +1708,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); @@ -1817,7 +1817,7 @@ mod tests { outer_tx.set_code(Code::new(tx_code)); outer_tx.set_data(Data::new(tx_data)); outer_tx.add_section(Section::Signature(Signature::new( - vec![*outer_tx.code_sechash(), *outer_tx.data_sechash()], + vec![outer_tx.header_hash()], [(0, keypair_1())].into_iter().collect(), None, ))); diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index a23b026eeac..2f2af49cd78 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -231,7 +231,7 @@ where WLS: WriteLogAndStorage, { let mut changed_keys = BTreeSet::default(); - let mut tx: Tx = tx_bytes.try_into().unwrap(); + let tx: Tx = tx_bytes.try_into().unwrap(); // Writes wrapper tx hash to block write log (changes must be persisted even // in case of failure) @@ -258,7 +258,7 @@ where // If wrapper was succesful, write inner tx hash to storage let inner_hash_key = replay_protection::get_replay_protection_key( - &hash::Hash(tx.update_header(TxType::Raw).header_hash().0), + &hash::Hash(tx.raw_header_hash().0), ); shell_params .wl_storage diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index 7806d1abef4..338bc74a2f8 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -1809,7 +1809,7 @@ where let gas_meter = unsafe { env.ctx.gas_meter.get() }; vp_host_fns::add_gas(gas_meter, gas)?; - let hashes = <[Hash; 2]>::try_from_slice(&hash_list) + let hashes = <[Hash; 1]>::try_from_slice(&hash_list) .map_err(vp_host_fns::RuntimeError::EncodingError)?; let (public_keys_map, gas) = env diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 2f5bbe4ea71..92edce853ae 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -986,7 +986,7 @@ fn invalid_transactions() -> Result<()> { client.exp_string("Transaction accepted")?; client.exp_string("Transaction applied")?; client.exp_string("Transaction is invalid")?; - client.exp_string(r#""code": "5"#)?; + client.exp_string(r#""code": "2"#)?; client.assert_success(); let mut ledger = bg_ledger.foreground(); @@ -1042,7 +1042,7 @@ fn invalid_transactions() -> Result<()> { client.exp_string("Error trying to apply a transaction")?; - client.exp_string(r#""code": "4"#)?; + client.exp_string(r#""code": "1"#)?; client.assert_success(); Ok(()) diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index 68ebd76dff3..3a2ee2e7867 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -475,10 +475,7 @@ mod tests { assert!( signed_tx_data .verify_signatures( - &[ - *signed_tx_data.data_sechash(), - *signed_tx_data.code_sechash(), - ], + &[signed_tx_data.header_hash(),], pks_map, &None, 1, @@ -494,10 +491,7 @@ mod tests { assert!( signed_tx_data .verify_signatures( - &[ - *signed_tx_data.data_sechash(), - *signed_tx_data.code_sechash(), - ], + &[signed_tx_data.header_hash(),], AccountPublicKeysMap::from_iter([ other_keypair.ref_to() ]), diff --git a/vp_prelude/src/lib.rs b/vp_prelude/src/lib.rs index 09626283633..220dab1daf8 100644 --- a/vp_prelude/src/lib.rs +++ b/vp_prelude/src/lib.rs @@ -88,12 +88,10 @@ pub fn verify_signatures(ctx: &Ctx, tx: &Tx, owner: &Address) -> VpResult { let threshold = storage_api::account::threshold(&ctx.pre(), owner)?.unwrap_or(1); - let targets = [*tx.data_sechash(), *tx.code_sechash()]; - // Serialize parameters let max_signatures = max_signatures_per_transaction.try_to_vec().unwrap(); let public_keys_map = public_keys_index_map.try_to_vec().unwrap(); - let targets = targets.try_to_vec().unwrap(); + let targets = [tx.raw_header_hash()].try_to_vec().unwrap(); let signer = owner.try_to_vec().unwrap(); let valid = unsafe { diff --git a/wasm/checksums.json b/wasm/checksums.json index 614ab78e6bd..de46655847c 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,22 +1,22 @@ { - "tx_bond.wasm": "tx_bond.b322054eef9d45e299384b2a363049ce0b0160a0c4781ca357aa59970904726c.wasm", - "tx_bridge_pool.wasm": "tx_bridge_pool.6f6ad3b95e21072af9e854e374fa0d7f691f0743da8cf52a643ed1bdb0e16611.wasm", - "tx_change_validator_commission.wasm": "tx_change_validator_commission.9310e0a0b7c14fc7c2427040da8c91eb4067babfaaea9e3b646edbfdd09c8069.wasm", - "tx_ibc.wasm": "tx_ibc.54313469bcc9bcaabf661177f88cb90ac9008f542edbf686f286a02f8cdbfd41.wasm", - "tx_init_account.wasm": "tx_init_account.10ee01dac5325685360119ba8e4b597d776a018ea4c9ac3534dd876ec377789e.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.04cad5a3a71f833a5867bca3ced54b06d34ad07f3f21877599d38581d362ba10.wasm", - "tx_init_validator.wasm": "tx_init_validator.16d53a09e5df06400849aaa161c35e4e377284692f73a71dcbd4573656da7f64.wasm", - "tx_resign_steward.wasm": "tx_resign_steward.b5d92c1bd196be0d196ef16e2ceed9a9ced7ac61d7b177fdbad208c0e784e172.wasm", - "tx_reveal_pk.wasm": "tx_reveal_pk.32011ddc5316705ae005059d5916b071288a04fb4dee80854af16d61548b5c27.wasm", - "tx_transfer.wasm": "tx_transfer.963ec4c2705377423ddc46b4ff3de63f9b625351467d89290fa771a485710c41.wasm", - "tx_unbond.wasm": "tx_unbond.7f26336db8e8cfebc04d301dc4790138fdd9bc22878fe7542c3da525a09576be.wasm", - "tx_unjail_validator.wasm": "tx_unjail_validator.15a7a399d8fb79f8df959d0ddf4c193020886d1caab1e094cca10ea3aff44a72.wasm", - "tx_update_account.wasm": "tx_update_account.7b4e225a823449d3d8bffde197c439ad24f4f6c95cf754acf62b6373958c4486.wasm", - "tx_update_steward_commission.wasm": "tx_update_steward_commission.0001b21ef3ef4f9b33afb5a5ef75a6a5427fbe221a8350cfbd81781ac18ded6e.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.727e36112fcd0753f758370dff981cc93430fe7d6f95ceb570a02a37529a7531.wasm", - "tx_withdraw.wasm": "tx_withdraw.e70485a8b79c5bff17d3b6ea96a7546cb709137c8a64606bdd1e77637157de33.wasm", - "vp_implicit.wasm": "vp_implicit.e0958c2ec06863f7bd48cd9abb67cc7557f956ce9fa6c714deba885db721fa50.wasm", - "vp_masp.wasm": "vp_masp.037671b60b3e9f312c1c5fdc53d040ebfad21a646b9b1e2dac6b3e20fc0d01ec.wasm", - "vp_user.wasm": "vp_user.0203fddde57bc31ef411370b628963486928a7c4d34614980d1a52616e0f617b.wasm", - "vp_validator.wasm": "vp_validator.39c685bc1407ef484f963aff9f7576273d56bbf283dcbded9f01944cf7ff9bf0.wasm" + "tx_bond.wasm": "tx_bond.c9fde5719da6dd63a79e0da4a099717401fbb8b7a618b3cb0778015a0773ef23.wasm", + "tx_bridge_pool.wasm": "tx_bridge_pool.e07070f6127f18e47fd4e43ef60f78344c0724f400ed69e8f30af31ad1bfd3cb.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.34f0e93ee7e55410e75345c8077299f0cc4aa00aa807b09e0ff380f2709a51c8.wasm", + "tx_ibc.wasm": "tx_ibc.ab49f6c6164e4016b9662405ad8da2ec45d876c1e2d0b756a33a75a9e2f45d38.wasm", + "tx_init_account.wasm": "tx_init_account.926d20201221e325c687a39fe9d338f01fb770f6397efd3882b99493089c2ce4.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.6a95f3f1f6ceeeb57335e122dc2ce6a99cafdaef095f00f3439da63881d73e1a.wasm", + "tx_init_validator.wasm": "tx_init_validator.445646b22db97882b3bbdc82a1b276b2bc8f0e18e04c0ba46c096fd5c729d593.wasm", + "tx_resign_steward.wasm": "tx_resign_steward.7f6810fd9901093b044d1f759a3fec6faef26fac1501d1a0c22f7c36e5b90fb4.wasm", + "tx_reveal_pk.wasm": "tx_reveal_pk.cf7811df8c17d38faee925fa77996e8e58abf89b3d4e196972be8b99007b682a.wasm", + "tx_transfer.wasm": "tx_transfer.041e11a019e88466328a2508c1754ea49c16fe8d009ffa15fa9e4a8190e8e0d5.wasm", + "tx_unbond.wasm": "tx_unbond.0778900cdb631687121ee4fbf1cc95cead67b337f5e5e0a92ad6c22c14c3ebd9.wasm", + "tx_unjail_validator.wasm": "tx_unjail_validator.616b6874bbc91a7dc8751a34cd512050695622b5c4b2eeb82ea533b5184089d6.wasm", + "tx_update_account.wasm": "tx_update_account.c39ff535e6b67fa65e902352795a8573aeae77ea6158e93492cecfefa7f3c475.wasm", + "tx_update_steward_commission.wasm": "tx_update_steward_commission.d3ae5fca19609aa2cfa126bbf7926e6ba42b0e78570d8adb38e5f9ec786153e6.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.5d6493da13f1a815fe353f9d46b130b2ff779cd59bb7d89b34bb98cb4e271b3a.wasm", + "tx_withdraw.wasm": "tx_withdraw.df045a91abda536abbf7e68fee98cddea6d7faf0a0f0c8f8ecd6ec35b8dcead8.wasm", + "vp_implicit.wasm": "vp_implicit.fd0c536e007782a3b8d3672e9db119725872607a56611e748f185147ac4b3569.wasm", + "vp_masp.wasm": "vp_masp.856241eb315b01531ec3143eec72720b9608616a6f7bb4109c1f818f42c140dd.wasm", + "vp_user.wasm": "vp_user.131360a9656267034d87eaa391390be5d7007c1ffc3f88626e4e407b233c1d18.wasm", + "vp_validator.wasm": "vp_validator.952dcbb21bb2d0cd285e1d75b08174e29ab749ae927d78b130fb6b91bcdfe200.wasm" } \ No newline at end of file diff --git a/wasm/wasm_source/src/vp_implicit.rs b/wasm/wasm_source/src/vp_implicit.rs index 215dccf421d..93dfb82fef5 100644 --- a/wasm/wasm_source/src/vp_implicit.rs +++ b/wasm/wasm_source/src/vp_implicit.rs @@ -536,7 +536,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -672,7 +672,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -840,7 +840,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -933,7 +933,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -988,7 +988,7 @@ mod tests { tx.set_code(Code::new(vec![])); tx.set_data(Data::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); diff --git a/wasm/wasm_source/src/vp_testnet_faucet.rs b/wasm/wasm_source/src/vp_testnet_faucet.rs index 7298c0b1260..598b7a77efe 100644 --- a/wasm/wasm_source/src/vp_testnet_faucet.rs +++ b/wasm/wasm_source/src/vp_testnet_faucet.rs @@ -267,7 +267,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -277,10 +277,14 @@ mod tests { vp_env.all_touched_storage_keys(); let verifiers: BTreeSet
= BTreeSet::default(); vp_host_env::set(vp_env); - assert!( - validate_tx(&CTX, signed_tx, vp_owner, keys_changed, verifiers) - .unwrap() - ); + assert!(validate_tx( + &CTX, + signed_tx, + vp_owner, + keys_changed, + verifiers + ) + .unwrap()); } prop_compose! { @@ -400,7 +404,7 @@ mod tests { tx_data.set_data(Data::new(solution_bytes)); tx_data.set_code(Code::new(vec![])); tx_data.add_section(Section::Signature(Signature::new( - vec![*tx_data.data_sechash(), *tx_data.code_sechash()], + vec![tx_data.raw_header_hash()], [(0, target_key)].into_iter().collect(), None, ))); @@ -454,7 +458,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); diff --git a/wasm/wasm_source/src/vp_user.rs b/wasm/wasm_source/src/vp_user.rs index a334576b530..63d87aff101 100644 --- a/wasm/wasm_source/src/vp_user.rs +++ b/wasm/wasm_source/src/vp_user.rs @@ -393,7 +393,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -562,7 +562,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -687,56 +687,56 @@ mod tests { } proptest! { - /// Test that a signed tx that performs arbitrary storage writes or - /// deletes to the account is accepted. - #[test] - fn test_signed_arb_storage_write( - (vp_owner, storage_key) in arb_account_storage_subspace_key(), - // Generate bytes to write. If `None`, delete from the key instead - storage_value in any::>>(), - ) { - // Initialize a tx environment - let mut tx_env = TestTxEnv::default(); - - let keypair = key::testing::keypair_1(); - let public_key = keypair.ref_to(); - - // Spawn all the accounts in the storage key to be able to modify - // their storage - let storage_key_addresses = storage_key.find_addresses(); - tx_env.spawn_accounts(storage_key_addresses); - tx_env.init_account_storage(&vp_owner, vec![public_key.clone()], 1); - - // Initialize VP environment from a transaction - vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |_address| { - // Write or delete some data in the transaction - if let Some(value) = &storage_value { - tx::ctx().write(&storage_key, value).unwrap(); - } else { - tx::ctx().delete(&storage_key).unwrap(); - } - }); - - let pks_map = AccountPublicKeysMap::from_iter(vec![public_key]); - - let mut vp_env = vp_host_env::take(); - let mut tx = vp_env.tx.clone(); - tx.set_code(Code::new(vec![])); - tx.set_data(Data::new(vec![])); - tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], - pks_map.index_secret_keys(vec![keypair]), - None, - ))); - let signed_tx = tx.clone(); - vp_env.tx = signed_tx.clone(); - let keys_changed: BTreeSet = - vp_env.all_touched_storage_keys(); - let verifiers: BTreeSet
= BTreeSet::default(); - vp_host_env::set(vp_env); - assert!(validate_tx(&CTX, signed_tx, vp_owner, keys_changed, verifiers).unwrap()); + /// Test that a signed tx that performs arbitrary storage writes or + /// deletes to the account is accepted. + #[test] + fn test_signed_arb_storage_write( + (vp_owner, storage_key) in arb_account_storage_subspace_key(), + // Generate bytes to write. If `None`, delete from the key instead + storage_value in any::>>(), + ) { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + + // Spawn all the accounts in the storage key to be able to modify + // their storage + let storage_key_addresses = storage_key.find_addresses(); + tx_env.spawn_accounts(storage_key_addresses); + tx_env.init_account_storage(&vp_owner, vec![public_key.clone()], 1); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |_address| { + // Write or delete some data in the transaction + if let Some(value) = &storage_value { + tx::ctx().write(&storage_key, value).unwrap(); + } else { + tx::ctx().delete(&storage_key).unwrap(); + } + }); + + let pks_map = AccountPublicKeysMap::from_iter(vec![public_key]); + + let mut vp_env = vp_host_env::take(); + let mut tx = vp_env.tx.clone(); + tx.set_code(Code::new(vec![])); + tx.set_data(Data::new(vec![])); + tx.add_section(Section::Signature(Signature::new( + vec![ tx.raw_header_hash()], + pks_map.index_secret_keys(vec![keypair]), + None, + ))); + let signed_tx = tx.clone(); + vp_env.tx = signed_tx.clone(); + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!(validate_tx(&CTX, signed_tx, vp_owner, keys_changed, verifiers).unwrap()); + } } - } /// Test that a validity predicate update without a valid signature is /// rejected. @@ -811,7 +811,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -866,7 +866,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -922,7 +922,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -978,7 +978,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -1034,7 +1034,7 @@ mod tests { tx.set_code(Code::new(vec![])); tx.set_data(Data::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); diff --git a/wasm/wasm_source/src/vp_validator.rs b/wasm/wasm_source/src/vp_validator.rs index f929a8a0d1e..1c306099c11 100644 --- a/wasm/wasm_source/src/vp_validator.rs +++ b/wasm/wasm_source/src/vp_validator.rs @@ -400,7 +400,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -580,7 +580,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![secret_key]), None, ))); @@ -742,7 +742,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -828,7 +828,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -883,7 +883,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -939,7 +939,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -995,7 +995,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.set_code(Code::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, ))); @@ -1051,7 +1051,7 @@ mod tests { tx.set_code(Code::new(vec![])); tx.set_data(Data::new(vec![])); tx.add_section(Section::Signature(Signature::new( - vec![*tx.data_sechash(), *tx.code_sechash()], + vec![tx.raw_header_hash()], pks_map.index_secret_keys(vec![keypair]), None, )));