diff --git a/.changelog/unreleased/improvements/1905-replay-protection-improvements.md b/.changelog/unreleased/improvements/1905-replay-protection-improvements.md new file mode 100644 index 0000000000..4e44cb732a --- /dev/null +++ b/.changelog/unreleased/improvements/1905-replay-protection-improvements.md @@ -0,0 +1,2 @@ +- Improved replay protection for invalid transactions. + ([\#1905](https://github.com/anoma/namada/pull/1905)) \ 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 2202f7a157..f4386cba16 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -226,7 +226,9 @@ where #[cfg(not(any(feature = "abciplus", feature = "abcipp")))] if let TxType::Wrapper(wrapper) = &tx_header.tx_type { // Charge fee if wrapper transaction went out of gas or - // failed because of fees + // failed because of fees (also write the wrapper hash to + // storage to prevent replays that would force the fee payer + // to pay more than once) let error_code = ErrorCodes::from_u32(processed_tx.result.code).unwrap(); if (error_code == ErrorCodes::TxGasLimit) @@ -238,7 +240,7 @@ where tx.get_section(hash) .map(|section| { if let Section::MaspTx(transaction) = - section + section.as_ref() { Some(transaction.to_owned()) } else { @@ -248,28 +250,30 @@ where .flatten() }) .flatten(); - if let Err(msg) = protocol::charge_fee( + if let Err(msg) = protocol::apply_wrapper_tx( wrapper, masp_transaction, + &processed_tx.tx, ShellParams::new( - TxGasMeter::new_from_sub_limit(u64::MAX), + &mut TxGasMeter::new_from_sub_limit( + u64::MAX.into(), + ), &mut self.wl_storage, &mut self.vp_wasm_cache, &mut self.tx_wasm_cache, ), Some(&native_block_proposer_address), - &mut BTreeSet::default(), ) { self.wl_storage.write_log.drop_tx(); tracing::error!( "Rejected wrapper tx {} could not pay fee: {}", - Hash::sha256( - tx::try_from(processed_tx.as_ref()) - .unwrap() - ), + hash::Hash::sha256(tx.header_hash()), msg ) } + self.wl_storage + .write_tx_hash(tx.header_hash()) + .expect("Error while writing tx hash to storage"); } } @@ -310,6 +314,9 @@ where "Tx with hash {} was un-decryptable", tx_in_queue.tx.header_hash() ); + // Remove inner tx hash from storage + self.allow_tx_replay(tx_in_queue.tx); + event["info"] = "Transaction is invalid.".into(); event["log"] = "Transaction could not be \ @@ -317,6 +324,7 @@ where .into(); event["code"] = ErrorCodes::Undecryptable.into(); + response.events.push(event); continue; } } @@ -487,6 +495,15 @@ where tx_event["hash"], result.vps_result.rejected_vps ); + + if let Some(wrapper) = embedding_wrapper { + if result.vps_result.invalid_sig { + // Invalid signature was found, remove the tx + // hash from storage to allow replay + self.allow_tx_replay(wrapper); + } + } + stats.increment_rejected_txs(); self.wl_storage.drop_tx(); tx_event["code"] = ErrorCodes::InvalidTx.into(); @@ -500,16 +517,18 @@ where tx_event["hash"], msg ); - stats.increment_errored_txs(); - self.wl_storage.drop_tx(); // If transaction type is Decrypted and failed because of - // out of gas, remove its hash from storage to allow - // rewrapping it + // out of gas or invalid section commtiment, remove its hash + // from storage to allow rewrapping it if let Some(wrapper) = embedding_wrapper { - if let Error::TxApply(protocol::Error::GasError(_)) = - msg - { + if matches!( + msg, + Error::TxApply(protocol::Error::GasError(_)) + | Error::TxApply( + protocol::Error::MissingSection(_) + ) + ) { self.allow_tx_replay(wrapper); } } else if let Some(wrapper) = wrapper { @@ -521,6 +540,9 @@ where .expect("Error while writing tx hash to storage"); } + stats.increment_errored_txs(); + self.wl_storage.drop_tx(); + tx_event["gas_used"] = tx_gas_meter.get_tx_consumed_gas().to_string(); tx_event["info"] = msg.to_string(); @@ -933,7 +955,7 @@ where 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"); + .expect("Error while writing tx hash to storage"); self.wl_storage .delete_tx_hash(wrapper_tx.raw_header_hash()) @@ -1025,6 +1047,7 @@ mod test_finalize_block { StorageProposalVote, VoteType, }; use namada::core::ledger::replay_protection; + use namada::core::types::storage::KeySeg; use namada::eth_bridge::storage::bridge_pool::{ self, get_key_from_hash, get_nonce_key, get_signed_root_key, }; @@ -1066,6 +1089,7 @@ mod test_finalize_block { use namada::types::uint::Uint; use namada::types::vote_extensions::ethereum_events; use namada_sdk::eth_bridge::MinimumConfirmations; + use namada_test_utils::tx_data::TxWriteData; use namada_test_utils::TestWasms; use test_log::test; @@ -1079,7 +1103,7 @@ mod test_finalize_block { FinalizeBlock, ProcessedTx, }; - const GAS_LIMIT_MULTIPLIER: u64 = 300_000; + const GAS_LIMIT_MULTIPLIER: u64 = 1_000_000; /// Make a wrapper tx and a processed tx from the wrapped tx that can be /// added to `FinalizeBlock` request. @@ -2287,7 +2311,10 @@ mod test_finalize_block { ); } - /// Test replay protection hash handling + /// Test that if a decrypted transaction fails because of out-of-gas, + /// undecryptable, invalid signature or wrong section commitment, its hash + /// is removed from storage. Also checks that a tx failing for other + /// reason has its hash persisted to storage. #[test] fn test_tx_hash_handling() { let (mut shell, _, _, _) = setup(); @@ -2295,83 +2322,221 @@ mod test_finalize_block { let mut batch = namada::core::ledger::storage::testing::TestStorage::batch(); - let (wrapper_tx, _) = mk_wrapper_tx(&shell, &keypair); - let (wrapper_tx_2, _) = mk_wrapper_tx(&shell, &keypair); - let mut invalid_wrapper_tx = + let (out_of_gas_wrapper, _) = mk_wrapper_tx(&shell, &keypair); + let (undecryptable_wrapper, _) = mk_wrapper_tx(&shell, &keypair); + let mut wasm_path = top_level_directory(); + // Write a key to trigger the vp to validate the signature + wasm_path.push("wasm_for_tests/tx_write.wasm"); + let tx_code = std::fs::read(wasm_path) + .expect("Expected a file at given code path"); + let mut unsigned_wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: 0.into(), + amount_per_gas_unit: Amount::zero(), token: shell.wl_storage.storage.native_token.clone(), }, keypair.ref_to(), Epoch(0), - 0.into(), + GAS_LIMIT_MULTIPLIER.into(), None, )))); - invalid_wrapper_tx.header.chain_id = shell.chain_id.clone(); - invalid_wrapper_tx - .set_code(Code::new("wasm_code".as_bytes().to_owned())); - invalid_wrapper_tx.set_data(Data::new( + unsigned_wrapper.header.chain_id = shell.chain_id.clone(); + let mut failing_wrapper = unsigned_wrapper.clone(); + unsigned_wrapper.set_code(Code::new(tx_code)); + let addr = Address::from(&keypair.to_public()); + let key = Key::from(addr.to_db_key()) + .join(&Key::from("test".to_string().to_db_key())); + unsigned_wrapper.set_data(Data::new( + borsh::to_vec(&TxWriteData { + key, + value: "test".as_bytes().to_owned(), + }) + .unwrap(), + )); + let mut wasm_path = top_level_directory(); + wasm_path.push("wasm_for_tests/tx_fail.wasm"); + let tx_code = std::fs::read(wasm_path) + .expect("Expected a file at given code path"); + failing_wrapper.set_code(Code::new(tx_code)); + failing_wrapper.set_data(Data::new( "Encrypted transaction data".as_bytes().to_owned(), )); - invalid_wrapper_tx.add_section(Section::Signature(Signature::new( - invalid_wrapper_tx.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); - - let wrapper_hash = wrapper_tx.header_hash(); - let wrapper_2_hash = wrapper_tx_2.header_hash(); - let invalid_wrapper_hash = invalid_wrapper_tx.header_hash(); - let mut decrypted_tx = wrapper_tx.clone(); - let mut decrypted_tx_2 = wrapper_tx_2.clone(); - - decrypted_tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - decrypted_tx_2.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - 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(); + let mut wrong_commitment_wrapper = failing_wrapper.clone(); + wrong_commitment_wrapper.set_code_sechash(Hash::default()); + + let mut out_of_gas_inner = out_of_gas_wrapper.clone(); + let mut undecryptable_inner = undecryptable_wrapper.clone(); + let mut unsigned_inner = unsigned_wrapper.clone(); + let mut wrong_commitment_inner = wrong_commitment_wrapper.clone(); + let mut failing_inner = failing_wrapper.clone(); + + undecryptable_inner + .update_header(TxType::Decrypted(DecryptedTx::Undecryptable)); + for inner in [ + &mut out_of_gas_inner, + &mut unsigned_inner, + &mut wrong_commitment_inner, + &mut failing_inner, + ] { + inner.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); + } // Write inner hashes in storage - for hash in [&decrypted_hash, &decrypted_2_hash] { + for inner in [ + &out_of_gas_inner, + &undecryptable_inner, + &unsigned_inner, + &wrong_commitment_inner, + &failing_inner, + ] { let hash_subkey = - replay_protection::get_replay_protection_last_subkey(hash); + replay_protection::get_replay_protection_last_subkey( + &inner.header_hash(), + ); shell .wl_storage .storage .write_replay_protection_entry(&mut batch, &hash_subkey) - .expect("Test failed"); + .unwrap(); + } + + let mut processed_txs: Vec = vec![]; + for inner in [ + &out_of_gas_inner, + &undecryptable_inner, + &unsigned_inner, + &wrong_commitment_inner, + &failing_inner, + ] { + processed_txs.push(ProcessedTx { + tx: inner.to_bytes(), + result: TxResult { + code: ErrorCodes::Ok.into(), + info: "".into(), + }, + }) } + shell.enqueue_tx(out_of_gas_wrapper.clone(), Gas::default()); + shell.enqueue_tx( + undecryptable_wrapper.clone(), + GAS_LIMIT_MULTIPLIER.into(), + ); + shell.enqueue_tx(unsigned_wrapper.clone(), u64::MAX.into()); // Prevent out of gas which would still make the test pass + shell.enqueue_tx( + wrong_commitment_wrapper.clone(), + GAS_LIMIT_MULTIPLIER.into(), + ); + shell.enqueue_tx(failing_wrapper.clone(), GAS_LIMIT_MULTIPLIER.into()); + // merkle tree root before finalize_block + let root_pre = shell.shell.wl_storage.storage.block.tree.root(); + + let event = &shell + .finalize_block(FinalizeBlock { + txs: processed_txs, + ..Default::default() + }) + .expect("Test failed"); + + // the merkle tree root should not change after finalize_block + let root_post = shell.shell.wl_storage.storage.block.tree.root(); + assert_eq!(root_pre.0, root_post.0); + + // Check inner tx hash has been removed from storage + assert_eq!(event[0].event_type.to_string(), String::from("applied")); + let code = event[0].attributes.get("code").unwrap().as_str(); + assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str()); + assert_eq!(event[1].event_type.to_string(), String::from("applied")); + let code = event[1].attributes.get("code").unwrap().as_str(); + assert_eq!(code, String::from(ErrorCodes::Undecryptable).as_str()); + assert_eq!(event[2].event_type.to_string(), String::from("applied")); + let code = event[2].attributes.get("code").unwrap().as_str(); + assert_eq!(code, String::from(ErrorCodes::InvalidTx).as_str()); + assert_eq!(event[3].event_type.to_string(), String::from("applied")); + let code = event[3].attributes.get("code").unwrap().as_str(); + assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str()); + assert_eq!(event[4].event_type.to_string(), String::from("applied")); + let code = event[4].attributes.get("code").unwrap().as_str(); + assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str()); + + for (invalid_inner, valid_wrapper) in [ + (out_of_gas_inner, out_of_gas_wrapper), + (undecryptable_inner, undecryptable_wrapper), + (unsigned_inner, unsigned_wrapper), + (wrong_commitment_inner, wrong_commitment_wrapper), + ] { + assert!( + !shell + .wl_storage + .write_log + .has_replay_protection_entry(&invalid_inner.header_hash()) + .unwrap_or_default() + ); + assert!( + shell + .wl_storage + .write_log + .has_replay_protection_entry(&valid_wrapper.header_hash()) + .unwrap_or_default() + ); + } + assert!( + shell + .wl_storage + .storage + .has_replay_protection_entry(&failing_inner.header_hash()) + .expect("test failed") + ); + assert!( + !shell + .wl_storage + .write_log + .has_replay_protection_entry(&failing_wrapper.header_hash()) + .unwrap_or_default() + ); + } + + #[test] + /// Test that the hash of the wrapper transaction is committed to storage + /// even if the wrapper tx fails. The inner transaction hash must instead be + /// removed + fn test_commits_hash_if_wrapper_failure() { + let (mut shell, _, _, _) = setup(); + let keypair = gen_keypair(); + + let mut wrapper = + Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( + Fee { + amount_per_gas_unit: 0.into(), + token: shell.wl_storage.storage.native_token.clone(), + }, + keypair.ref_to(), + Epoch(0), + 0.into(), + None, + )))); + wrapper.header.chain_id = shell.chain_id.clone(); + wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned())); + wrapper.set_data(Data::new( + "Encrypted transaction data".as_bytes().to_owned(), + )); + wrapper.add_section(Section::Signature(Signature::new( + wrapper.sechashes(), + [(0, keypair)].into_iter().collect(), + None, + ))); + + let wrapper_hash = wrapper.header_hash(); + // Invalid wrapper tx that should lead to a commitment of the wrapper // hash and no commitment of the inner hash - let mut processed_txs = vec![ProcessedTx { - tx: invalid_wrapper_tx.to_bytes(), + let processed_txs = vec![ProcessedTx { + tx: wrapper.to_bytes(), result: TxResult { code: ErrorCodes::Ok.into(), info: "".into(), }, }]; - // Out of gas error triggering inner hash removal and wrapper hash - // insert - processed_txs.push(ProcessedTx { - tx: decrypted_tx.to_bytes(), - result: TxResult { - code: ErrorCodes::Ok.into(), - info: "".into(), - }, - }); - // Wasm error that still leads to inner hash commitment and no wrapper - // hash insert - processed_txs.push(ProcessedTx { - tx: decrypted_tx_2.to_bytes(), - result: TxResult { - code: ErrorCodes::Ok.into(), - info: "".into(), - }, - }); - shell.enqueue_tx(wrapper_tx, Gas::default()); - shell.enqueue_tx(wrapper_tx_2, GAS_LIMIT_MULTIPLIER.into()); // merkle tree root before finalize_block let root_pre = shell.shell.wl_storage.storage.block.tree.root(); @@ -2386,10 +2551,6 @@ mod test_finalize_block { let root_post = shell.shell.wl_storage.storage.block.tree.root(); assert_eq!(root_pre.0, root_post.0); - // Check first inner tx hash has been removed from storage but - // corresponding wrapper hash is still there Check second inner - // tx is still there and corresponding wrapper hash has been removed - // since useless assert_eq!(event[0].event_type.to_string(), String::from("accepted")); let code = event[0] .attributes @@ -2397,42 +2558,7 @@ mod test_finalize_block { .expect("Test failed") .as_str(); assert_eq!(code, String::from(ErrorCodes::InvalidTx).as_str()); - assert_eq!(event[1].event_type.to_string(), String::from("applied")); - let code = event[1] - .attributes - .get("code") - .expect("Test failed") - .as_str(); - assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str()); - assert_eq!(event[2].event_type.to_string(), String::from("applied")); - let code = event[2] - .attributes - .get("code") - .expect("Test failed") - .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 @@ -2440,18 +2566,11 @@ mod test_finalize_block { .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) + .has_replay_protection_entry(&wrapper.raw_header_hash()) .unwrap_or_default() ); } diff --git a/core/src/ledger/tx_env.rs b/core/src/ledger/tx_env.rs index ad8b23e60c..4b87d97f7c 100644 --- a/core/src/ledger/tx_env.rs +++ b/core/src/ledger/tx_env.rs @@ -64,4 +64,7 @@ pub trait TxEnv: StorageRead + StorageWrite { &self, event_type: impl AsRef, ) -> Result, storage_api::Error>; + + /// Set the sentinel for an invalid section commitment + fn set_commitment_sentinel(&mut self); } diff --git a/core/src/proto/types.rs b/core/src/proto/types.rs index b1d2b40cb5..c650162339 100644 --- a/core/src/proto/types.rs +++ b/core/src/proto/types.rs @@ -26,7 +26,9 @@ use sha2::{Digest, Sha256}; use thiserror::Error; use super::generated::types; -use crate::ledger::gas::{GasMetering, VpGasMeter, VERIFY_TX_SIG_GAS_COST}; +use crate::ledger::gas::{ + self, GasMetering, VpGasMeter, VERIFY_TX_SIG_GAS_COST, +}; use crate::ledger::storage::{KeccakHasher, Sha256Hasher, StorageHasher}; #[cfg(any(feature = "tendermint", feature = "tendermint-abcipp"))] use crate::tendermint_proto::abci::ResponseDeliverTx; @@ -74,8 +76,8 @@ pub enum Error { InvalidJSONDeserialization(String), #[error("The wrapper signature is invalid.")] InvalidWrapperSignature, - #[error("Signature verification went out of gas")] - OutOfGas, + #[error("Signature verification went out of gas: {0}")] + OutOfGas(gas::Error), } pub type Result = std::result::Result; @@ -593,7 +595,7 @@ impl Signature { if let Some(meter) = gas_meter { meter .consume(VERIFY_TX_SIG_GAS_COST) - .map_err(|_| VerifySigError::OutOfGas)?; + .map_err(VerifySigError::OutOfGas)?; } common::SigScheme::verify_signature( &pk, @@ -618,7 +620,7 @@ impl Signature { if let Some(meter) = gas_meter { meter .consume(VERIFY_TX_SIG_GAS_COST) - .map_err(|_| VerifySigError::OutOfGas)?; + .map_err(VerifySigError::OutOfGas)?; } common::SigScheme::verify_signature( pk, @@ -1448,8 +1450,8 @@ impl Tx { gas_meter, ) .map_err(|e| { - if let VerifySigError::OutOfGas = e { - Error::OutOfGas + if let VerifySigError::OutOfGas(inner) = e { + Error::OutOfGas(inner) } else { Error::InvalidSectionSignature( "found invalid signature.".to_string(), diff --git a/core/src/types/key/mod.rs b/core/src/types/key/mod.rs index 52af46d5dc..d05030ea5b 100644 --- a/core/src/types/key/mod.rs +++ b/core/src/types/key/mod.rs @@ -124,8 +124,8 @@ pub enum VerifySigError { MissingData, #[error("Signature belongs to a different scheme from the public key.")] MismatchedScheme, - #[error("Signature verification went out of gas")] - OutOfGas, + #[error("Signature verification went out of gas: {0}")] + OutOfGas(crate::ledger::gas::Error), } #[allow(missing_docs)] diff --git a/core/src/types/transaction/mod.rs b/core/src/types/transaction/mod.rs index 3b50e55e05..b6cdb0cd07 100644 --- a/core/src/types/transaction/mod.rs +++ b/core/src/types/transaction/mod.rs @@ -80,6 +80,8 @@ pub struct VpsResult { pub gas_used: VpsGas, /// Errors occurred in any of the VPs, if any pub errors: Vec<(Address, String)>, + /// Sentinel to signal an invalid transaction signature + pub invalid_sig: bool, } impl fmt::Display for TxResult { @@ -166,6 +168,31 @@ impl TxType { } } +/// Sentinel used in transactions to signal events that require special +/// replay protection handling back to the protocol. +#[derive(Debug, Default)] +pub enum TxSentinel { + /// No action required + #[default] + None, + /// Exceeded gas limit + OutOfGas, + /// Found invalid commtiment to one of the transaction's sections + InvalidCommitment, +} + +impl TxSentinel { + /// Set the sentinel for an out of gas error + pub fn set_out_of_gas(&mut self) { + *self = Self::OutOfGas + } + + /// Set the sentinel for an invalid section commitment error + pub fn set_invalid_commitment(&mut self) { + *self = Self::InvalidCommitment + } +} + #[cfg(test)] mod test_process_tx { use super::*; diff --git a/core/src/types/validity_predicate.rs b/core/src/types/validity_predicate.rs index a40fc1f99d..60f73af912 100644 --- a/core/src/types/validity_predicate.rs +++ b/core/src/types/validity_predicate.rs @@ -17,3 +17,38 @@ pub struct EvalVp { /// The input for the `eval`ed VP pub input: Tx, } + +/// Sentinel used in validity predicates to signal events that require special +/// replay protection handling back to the protocol. +#[derive(Debug, Default)] +pub enum VpSentinel { + /// No action required + #[default] + None, + /// Exceeded gas limit + OutOfGas, + /// Found invalid transaction signature + InvalidSignature, +} + +impl VpSentinel { + /// Check if the Vp ran out of gas + pub fn is_out_of_gas(&self) -> bool { + matches!(self, Self::OutOfGas) + } + + /// Check if the Vp found an invalid signature + pub fn is_invalid_signature(&self) -> bool { + matches!(self, Self::InvalidSignature) + } + + /// Set the sentinel for an out of gas error + pub fn set_out_of_gas(&mut self) { + *self = Self::OutOfGas + } + + /// Set the sentinel for an invalid signature error + pub fn set_invalid_signature(&mut self) { + *self = Self::InvalidSignature + } +} diff --git a/shared/src/ledger/native_vp/mod.rs b/shared/src/ledger/native_vp/mod.rs index 39a6b65bb5..60c30daeb3 100644 --- a/shared/src/ledger/native_vp/mod.rs +++ b/shared/src/ledger/native_vp/mod.rs @@ -12,6 +12,7 @@ use std::collections::BTreeSet; use borsh::BorshDeserialize; use eyre::WrapErr; pub use namada_core::ledger::vp_env::VpEnv; +use namada_core::types::validity_predicate::VpSentinel; use super::storage_api::{self, ResultExt, StorageRead}; use super::vp_host_fns; @@ -66,6 +67,8 @@ where pub iterators: RefCell>, /// VP gas meter. pub gas_meter: RefCell, + /// Errors sentinel + pub sentinel: RefCell, /// Read-only access to the storage. pub storage: &'a Storage, /// Read-only access to the write log. @@ -136,6 +139,7 @@ where address, iterators: RefCell::new(PrefixIterators::default()), gas_meter: RefCell::new(gas_meter), + sentinel: RefCell::new(VpSentinel::default()), storage, write_log, tx, @@ -182,6 +186,7 @@ where self.ctx.storage, self.ctx.write_log, key, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -195,6 +200,7 @@ where self.ctx.storage, self.ctx.write_log, key, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -208,6 +214,7 @@ where self.ctx.write_log, self.ctx.storage, prefix, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -219,8 +226,12 @@ where &'iter self, iter: &mut Self::PrefixIter<'iter>, ) -> Result)>, storage_api::Error> { - vp_host_fns::iter_next::(&mut self.ctx.gas_meter.borrow_mut(), iter) - .into_storage_result() + vp_host_fns::iter_next::( + &mut self.ctx.gas_meter.borrow_mut(), + iter, + &mut self.ctx.sentinel.borrow_mut(), + ) + .into_storage_result() } fn get_chain_id(&self) -> Result { @@ -273,6 +284,7 @@ where self.ctx.storage, self.ctx.write_log, key, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -286,6 +298,7 @@ where self.ctx.storage, self.ctx.write_log, key, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -299,6 +312,7 @@ where self.ctx.write_log, self.ctx.storage, prefix, + &mut self.ctx.sentinel.borrow_mut(), ) .into_storage_result() } @@ -310,8 +324,12 @@ where &'iter self, iter: &mut Self::PrefixIter<'iter>, ) -> Result)>, storage_api::Error> { - vp_host_fns::iter_next::(&mut self.ctx.gas_meter.borrow_mut(), iter) - .into_storage_result() + vp_host_fns::iter_next::( + &mut self.ctx.gas_meter.borrow_mut(), + iter, + &mut self.ctx.sentinel.borrow_mut(), + ) + .into_storage_result() } fn get_chain_id(&self) -> Result { @@ -372,6 +390,7 @@ where &mut self.gas_meter.borrow_mut(), self.write_log, key, + &mut self.sentinel.borrow_mut(), ) .map(|data| data.and_then(|t| T::try_from_slice(&t[..]).ok())) .into_storage_result() @@ -385,6 +404,7 @@ where &mut self.gas_meter.borrow_mut(), self.write_log, key, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -393,6 +413,7 @@ where vp_host_fns::get_chain_id( &mut self.gas_meter.borrow_mut(), self.storage, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -401,6 +422,7 @@ where vp_host_fns::get_block_height( &mut self.gas_meter.borrow_mut(), self.storage, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -413,6 +435,7 @@ where &mut self.gas_meter.borrow_mut(), self.storage, height, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -421,6 +444,7 @@ where vp_host_fns::get_block_hash( &mut self.gas_meter.borrow_mut(), self.storage, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -429,6 +453,7 @@ where vp_host_fns::get_block_epoch( &mut self.gas_meter.borrow_mut(), self.storage, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -437,6 +462,7 @@ where vp_host_fns::get_tx_index( &mut self.gas_meter.borrow_mut(), self.tx_index, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -445,6 +471,7 @@ where vp_host_fns::get_native_token( &mut self.gas_meter.borrow_mut(), self.storage, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -470,6 +497,7 @@ where self.write_log, self.storage, prefix, + &mut self.sentinel.borrow_mut(), ) .into_storage_result() } @@ -501,6 +529,7 @@ where self.storage, self.write_log, &mut self.gas_meter.borrow_mut(), + &mut self.sentinel.borrow_mut(), self.tx, self.tx_index, &mut iterators, @@ -543,8 +572,12 @@ where } fn get_tx_code_hash(&self) -> Result, storage_api::Error> { - vp_host_fns::get_tx_code_hash(&mut self.gas_meter.borrow_mut(), self.tx) - .into_storage_result() + vp_host_fns::get_tx_code_hash( + &mut self.gas_meter.borrow_mut(), + self.tx, + &mut self.sentinel.borrow_mut(), + ) + .into_storage_result() } fn read_pre( diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index b8a9902ac5..e2292bd1a7 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -16,7 +16,7 @@ use namada_core::types::transaction::WrapperTx; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use thiserror::Error; -use crate::ledger::gas::{self, GasMetering, VpGasMeter}; +use crate::ledger::gas::{GasMetering, VpGasMeter}; use crate::ledger::governance::GovernanceVp; use crate::ledger::native_vp::ethereum_bridge::bridge_pool_vp::BridgePoolVp; use crate::ledger::native_vp::ethereum_bridge::nut::NonUsableTokens; @@ -42,8 +42,8 @@ use crate::vm::{self, wasm, WasmCacheAccess}; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { - #[error("Missing wasm code error")] - MissingCode, + #[error("Missing tx section: {0}")] + MissingSection(String), #[error("Storage error: {0}")] StorageError(crate::ledger::storage::Error), #[error("Error decoding a transaction from bytes: {0}")] @@ -56,10 +56,12 @@ pub enum Error { TxTypeError, #[error("Fee ushielding error: {0}")] FeeUnshieldingError(crate::types::transaction::WrapperTxErr), - #[error("{0}")] - GasError(#[from] gas::Error), + #[error("Gas error: {0}")] + GasError(String), #[error("Error while processing transaction's fees: {0}")] FeeError(String), + #[error("Invalid transaction signature")] + InvalidTxSignature, #[error("Error executing VP for addresses: {0:?}")] VpRunnerError(vm::wasm::run::Error), #[error("The address {0} doesn't exist")] @@ -242,7 +244,10 @@ where )?; // Account for gas - shell_params.tx_gas_meter.add_tx_size_gas(tx_bytes)?; + shell_params + .tx_gas_meter + .add_tx_size_gas(tx_bytes) + .map_err(|err| Error::GasError(err.to_string()))?; // If wrapper was succesful, write inner tx hash to storage shell_params @@ -280,7 +285,7 @@ pub fn get_fee_unshielding_transaction( /// - Fee amount overflows /// - Not enough funds are available to pay the entire amount of the fee /// - The accumulated fee amount to be credited to the block proposer overflows -pub fn charge_fee<'a, D, H, CA, WLS>( +fn charge_fee<'a, D, H, CA, WLS>( wrapper: &WrapperTx, masp_transaction: Option, shell_params: &mut ShellParams<'a, CA, WLS>, @@ -708,12 +713,10 @@ where vp_wasm_cache, tx_wasm_cache, ) - .map_err(|e| { - if let wasm::run::Error::GasError(gas_error) = e { - Error::GasError(gas_error) - } else { - Error::TxRunnerError(e) - } + .map_err(|err| match err { + wasm::run::Error::GasError(msg) => Error::GasError(msg), + wasm::run::Error::MissingSection(msg) => Error::MissingSection(msg), + _ => Error::TxRunnerError(err), }) } @@ -765,7 +768,9 @@ where )?; tracing::debug!("Total VPs gas cost {:?}", vps_result.gas_used); - tx_gas_meter.add_vps_gas(&vps_result.gas_used)?; + tx_gas_meter + .add_vps_gas(&vps_result.gas_used) + .map_err(|err| Error::GasError(err.to_string()))?; Ok(vps_result) } @@ -787,7 +792,7 @@ where H: 'static + StorageHasher + Sync, CA: 'static + WasmCacheAccess + Sync, { - verifiers + let vps_result = verifiers .par_iter() .try_fold(VpsResult::default, |mut result, addr| { let mut gas_meter = VpGasMeter::new_from_tx_meter(tx_gas_meter); @@ -796,14 +801,17 @@ where let (vp_hash, gas) = storage .validity_predicate(addr) .map_err(Error::StorageError)?; - gas_meter.consume(gas).map_err(Error::GasError)?; + gas_meter + .consume(gas) + .map_err(|err| Error::GasError(err.to_string()))?; let Some(vp_code_hash) = vp_hash else { return Err(Error::MissingAddress(addr.clone())); }; - // NOTE: because of the whitelisted gas and the gas metering - // for the exposed vm env functions, - // the first signature verification (if any) is accounted + // NOTE: because of the whitelisted gas and the gas + // metering for the exposed vm + // env functions, the first + // signature verification (if any) is accounted // twice wasm::run::vp( vp_code_hash, @@ -817,7 +825,13 @@ where &verifiers, vp_wasm_cache.clone(), ) - .map_err(Error::VpRunnerError) + .map_err(|err| match err { + wasm::run::Error::GasError(msg) => Error::GasError(msg), + wasm::run::Error::InvalidTxSignature => { + Error::InvalidTxSignature + } + _ => Error::VpRunnerError(err), + }) } Address::Internal(internal_addr) => { let ctx = native_vp::Ctx::new( @@ -832,139 +846,227 @@ where vp_wasm_cache.clone(), ); - let accepted: Result = match internal_addr { - InternalAddress::PoS => { - let pos = PosVP { ctx }; - let verifiers_addr_ref = &verifiers; - let pos_ref = &pos; - // TODO this is temporarily ran in a new thread to - // avoid crashing the ledger (required `UnwindSafe` - // and `RefUnwindSafe` in - // shared/src/ledger/pos/vp.rs) - let keys_changed_ref = &keys_changed; - let result = match panic::catch_unwind(move || { - pos_ref - .validate_tx( - tx, - keys_changed_ref, - verifiers_addr_ref, - ) - .map_err(Error::PosNativeVpError) - }) { - Ok(result) => result, - Err(err) => { - tracing::error!( - "PoS native VP failed with {:#?}", - err - ); - Err(Error::PosNativeVpRuntime) - } - }; - // Take the gas meter back out of the context - gas_meter = pos.ctx.gas_meter.into_inner(); - result - } - InternalAddress::Ibc => { - let ibc = Ibc { ctx }; - let result = ibc - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::IbcNativeVpError); - // Take the gas meter back out of the context - gas_meter = ibc.ctx.gas_meter.into_inner(); - result - } - InternalAddress::Parameters => { - let parameters = ParametersVp { ctx }; - let result = parameters - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::ParametersNativeVpError); - // Take the gas meter back out of the context - gas_meter = parameters.ctx.gas_meter.into_inner(); - result - } - InternalAddress::PosSlashPool => { - // Take the gas meter back out of the context - gas_meter = ctx.gas_meter.into_inner(); - Err(Error::AccessForbidden( - (*internal_addr).clone(), - )) - } - InternalAddress::Governance => { - let governance = GovernanceVp { ctx }; - let result = governance - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::GovernanceNativeVpError); - gas_meter = governance.ctx.gas_meter.into_inner(); - result + let (accepted, sentinel): (Result, _) = + match internal_addr { + InternalAddress::PoS => { + let pos = PosVP { ctx }; + let verifiers_addr_ref = &verifiers; + let pos_ref = &pos; + // TODO this is temporarily ran in a new thread + // to + // avoid crashing the ledger (required + // `UnwindSafe` + // and `RefUnwindSafe` in + // shared/src/ledger/pos/vp.rs) + let keys_changed_ref = &keys_changed; + let result = + match panic::catch_unwind(move || { + pos_ref + .validate_tx( + tx, + keys_changed_ref, + verifiers_addr_ref, + ) + .map_err(Error::PosNativeVpError) + }) { + Ok(result) => result, + Err(err) => { + tracing::error!( + "PoS native VP failed with \ + {:#?}", + err + ); + Err(Error::PosNativeVpRuntime) + } + }; + // Take the gas meter and sentinel + // back + // out of the context + gas_meter = pos.ctx.gas_meter.into_inner(); + (result, pos.ctx.sentinel.into_inner()) + } + InternalAddress::Ibc => { + let ibc = Ibc { ctx }; + let result = ibc + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::IbcNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = ibc.ctx.gas_meter.into_inner(); + (result, ibc.ctx.sentinel.into_inner()) + } + InternalAddress::Parameters => { + let parameters = ParametersVp { ctx }; + let result = parameters + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::ParametersNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = + parameters.ctx.gas_meter.into_inner(); + (result, parameters.ctx.sentinel.into_inner()) + } + InternalAddress::PosSlashPool => { + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = ctx.gas_meter.into_inner(); + ( + Err(Error::AccessForbidden( + (*internal_addr).clone(), + )), + ctx.sentinel.into_inner(), + ) + } + InternalAddress::Governance => { + let governance = GovernanceVp { ctx }; + let result = governance + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::GovernanceNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = + governance.ctx.gas_meter.into_inner(); + (result, governance.ctx.sentinel.into_inner()) + } + InternalAddress::Multitoken => { + let multitoken = MultitokenVp { ctx }; + let result = multitoken + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::MultitokenNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = + multitoken.ctx.gas_meter.into_inner(); + (result, multitoken.ctx.sentinel.into_inner()) + } + InternalAddress::EthBridge => { + let bridge = EthBridge { ctx }; + let result = bridge + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::EthBridgeNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = bridge.ctx.gas_meter.into_inner(); + (result, bridge.ctx.sentinel.into_inner()) + } + InternalAddress::EthBridgePool => { + let bridge_pool = BridgePoolVp { ctx }; + let result = bridge_pool + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::BridgePoolNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = + bridge_pool.ctx.gas_meter.into_inner(); + (result, bridge_pool.ctx.sentinel.into_inner()) + } + InternalAddress::Pgf => { + let pgf_vp = PgfVp { ctx }; + let result = pgf_vp + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::PgfNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = pgf_vp.ctx.gas_meter.into_inner(); + (result, pgf_vp.ctx.sentinel.into_inner()) + } + InternalAddress::Nut(_) => { + let non_usable_tokens = NonUsableTokens { ctx }; + let result = non_usable_tokens + .validate_tx(tx, &keys_changed, &verifiers) + .map_err(Error::NutNativeVpError); + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = non_usable_tokens + .ctx + .gas_meter + .into_inner(); + ( + result, + non_usable_tokens.ctx.sentinel.into_inner(), + ) + } + InternalAddress::IbcToken(_) + | InternalAddress::Erc20(_) => { + // The address should be a part of a multitoken + // key + // Take the gas meter and the sentinel + // back + // out of the context + gas_meter = ctx.gas_meter.into_inner(); + ( + Ok(verifiers.contains(&Address::Internal( + InternalAddress::Multitoken, + ))), + ctx.sentinel.into_inner(), + ) + } + }; + + accepted.map_err(|err| { + // No need to check invalid sig because internal vps + // don't check the signature + if sentinel.is_out_of_gas() { + Error::GasError(err.to_string()) + } else { + err } - InternalAddress::Multitoken => { - let multitoken = MultitokenVp { ctx }; - let result = multitoken - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::MultitokenNativeVpError); - gas_meter = multitoken.ctx.gas_meter.into_inner(); - result - } - InternalAddress::EthBridge => { - let bridge = EthBridge { ctx }; - let result = bridge - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::EthBridgeNativeVpError); - gas_meter = bridge.ctx.gas_meter.into_inner(); - result - } - InternalAddress::EthBridgePool => { - let bridge_pool = BridgePoolVp { ctx }; - let result = bridge_pool - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::BridgePoolNativeVpError); - gas_meter = bridge_pool.ctx.gas_meter.into_inner(); - result - } - InternalAddress::Pgf => { - let pgf_vp = PgfVp { ctx }; - let result = pgf_vp - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::PgfNativeVpError); - gas_meter = pgf_vp.ctx.gas_meter.into_inner(); - result - } - InternalAddress::Nut(_) => { - let non_usable_tokens = NonUsableTokens { ctx }; - let result = non_usable_tokens - .validate_tx(tx, &keys_changed, &verifiers) - .map_err(Error::NutNativeVpError); - gas_meter = - non_usable_tokens.ctx.gas_meter.into_inner(); - result - } - InternalAddress::IbcToken(_) - | InternalAddress::Erc20(_) => { - // The address should be a part of a multitoken key - gas_meter = ctx.gas_meter.into_inner(); - Ok(verifiers.contains(&Address::Internal( - InternalAddress::Multitoken, - ))) - } - }; - - accepted + }) } }; - // Returning error from here will short-circuit the VP parallel - // execution. - result.gas_used.set(gas_meter).map_err(Error::GasError)?; - if accept? { - result.accepted_vps.insert(addr.clone()); - } else { - result.rejected_vps.insert(addr.clone()); + match accept { + Ok(accepted) => { + if accepted { + result.accepted_vps.insert(addr.clone()); + } else { + result.rejected_vps.insert(addr.clone()); + } + } + Err(err) => match err { + // Execution of VPs can (and must) be short-circuited + // only in case of a gas overflow to prevent the + // transaction from conuming resources that have not + // been acquired in the corresponding wrapper tx. For + // all the other errors we keep evaluating the vps. This + // allow to display a consistent VpsResult accross all + // nodes and find any invalid signatures + Error::GasError(_) => { + return Err(err); + } + Error::InvalidTxSignature => { + result.invalid_sig = true; + result.rejected_vps.insert(addr.clone()); + // Don't push the error since this is just a flag error + } + _ => { + result.rejected_vps.insert(addr.clone()); + result.errors.push((addr.clone(), err.to_string())); + } + }, } + + result + .gas_used + .set(gas_meter) + .map_err(|err| Error::GasError(err.to_string()))?; + Ok(result) }) .try_reduce(VpsResult::default, |a, b| { merge_vp_results(a, b, tx_gas_meter) - }) + })?; + + Ok(vps_result) } /// Merge VP results from parallel runs @@ -979,15 +1081,19 @@ fn merge_vp_results( rejected_vps.extend(b.rejected_vps); let mut errors = a.errors; errors.append(&mut b.errors); + let invalid_sig = a.invalid_sig || b.invalid_sig; let mut gas_used = a.gas_used; - gas_used.merge(b.gas_used, tx_gas_meter)?; + gas_used + .merge(b.gas_used, tx_gas_meter) + .map_err(|err| Error::GasError(err.to_string()))?; Ok(VpsResult { accepted_vps, rejected_vps, gas_used, errors, + invalid_sig, }) } diff --git a/shared/src/ledger/vp_host_fns.rs b/shared/src/ledger/vp_host_fns.rs index ef8831ae88..757b2d688b 100644 --- a/shared/src/ledger/vp_host_fns.rs +++ b/shared/src/ledger/vp_host_fns.rs @@ -7,6 +7,7 @@ use namada_core::types::hash::Hash; use namada_core::types::storage::{ BlockHash, BlockHeight, Epoch, Header, Key, TxIndex, }; +use namada_core::types::validity_predicate::VpSentinel; use thiserror::Error; use super::gas::STORAGE_ACCESS_GAS_PER_BYTE; @@ -45,12 +46,16 @@ pub enum RuntimeError { pub type EnvResult = std::result::Result; /// Add a gas cost incured in a validity predicate -pub fn add_gas(gas_meter: &mut VpGasMeter, used_gas: u64) -> EnvResult<()> { - let result = gas_meter.consume(used_gas).map_err(RuntimeError::OutOfGas); - if let Err(err) = &result { +pub fn add_gas( + gas_meter: &mut VpGasMeter, + used_gas: u64, + sentinel: &mut VpSentinel, +) -> EnvResult<()> { + gas_meter.consume(used_gas).map_err(|err| { + sentinel.set_out_of_gas(); tracing::info!("Stopping VP execution because of gas error: {}", err); - } - result + RuntimeError::OutOfGas(err) + }) } /// Storage read prior state (before tx execution). It will try to read from the @@ -60,13 +65,14 @@ pub fn read_pre( storage: &Storage, write_log: &WriteLog, key: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult>> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (log_val, gas) = write_log.read_pre(key); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; match log_val { Some(write_log::StorageModification::Write { ref value }) => { Ok(Some(value.clone())) @@ -88,7 +94,7 @@ where // When not found in write log, try to read from the storage let (value, gas) = storage.read(key).map_err(RuntimeError::StorageError)?; - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(value) } } @@ -101,6 +107,7 @@ pub fn read_post( storage: &Storage, write_log: &WriteLog, key: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult>> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, @@ -108,7 +115,7 @@ where { // Try to read from the write log first let (log_val, gas) = write_log.read(key); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; match log_val { Some(write_log::StorageModification::Write { ref value }) => { Ok(Some(value.clone())) @@ -130,7 +137,7 @@ where // When not found in write log, try to read from the storage let (value, gas) = storage.read(key).map_err(RuntimeError::StorageError)?; - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(value) } } @@ -142,10 +149,11 @@ pub fn read_temp( gas_meter: &mut VpGasMeter, write_log: &WriteLog, key: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult>> { // Try to read from the write log first let (log_val, gas) = write_log.read(key); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; match log_val { Some(write_log::StorageModification::Temp { ref value }) => { Ok(Some(value.clone())) @@ -162,6 +170,7 @@ pub fn has_key_pre( storage: &Storage, write_log: &WriteLog, key: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, @@ -169,7 +178,7 @@ where { // Try to read from the write log first let (log_val, gas) = write_log.read_pre(key); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; match log_val { Some(&write_log::StorageModification::Write { .. }) => Ok(true), Some(&write_log::StorageModification::Delete) => { @@ -182,7 +191,7 @@ where // When not found in write log, try to check the storage let (present, gas) = storage.has_key(key).map_err(RuntimeError::StorageError)?; - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(present) } } @@ -195,6 +204,7 @@ pub fn has_key_post( storage: &Storage, write_log: &WriteLog, key: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, @@ -202,7 +212,7 @@ where { // Try to read from the write log first let (log_val, gas) = write_log.read(key); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; match log_val { Some(&write_log::StorageModification::Write { .. }) => Ok(true), Some(&write_log::StorageModification::Delete) => { @@ -215,7 +225,7 @@ where // When not found in write log, try to check the storage let (present, gas) = storage.has_key(key).map_err(RuntimeError::StorageError)?; - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(present) } } @@ -225,13 +235,14 @@ where pub fn get_chain_id( gas_meter: &mut VpGasMeter, storage: &Storage, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (chain_id, gas) = storage.get_chain_id(); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(chain_id) } @@ -240,13 +251,14 @@ where pub fn get_block_height( gas_meter: &mut VpGasMeter, storage: &Storage, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (height, gas) = storage.get_block_height(); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(height) } @@ -255,6 +267,7 @@ pub fn get_block_header( gas_meter: &mut VpGasMeter, storage: &Storage, height: BlockHeight, + sentinel: &mut VpSentinel, ) -> EnvResult> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, @@ -263,7 +276,7 @@ where let (header, gas) = storage .get_block_header(Some(height)) .map_err(RuntimeError::StorageError)?; - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(header) } @@ -272,13 +285,14 @@ where pub fn get_block_hash( gas_meter: &mut VpGasMeter, storage: &Storage, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (hash, gas) = storage.get_block_hash(); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(hash) } @@ -287,12 +301,13 @@ where pub fn get_tx_code_hash( gas_meter: &mut VpGasMeter, tx: &Tx, + sentinel: &mut VpSentinel, ) -> EnvResult> { let hash = tx .get_section(tx.code_sechash()) .and_then(|x| Section::code_sec(x.as_ref())) .map(|x| x.code.hash()); - add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE)?; + add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE, sentinel)?; Ok(hash) } @@ -301,13 +316,14 @@ pub fn get_tx_code_hash( pub fn get_block_epoch( gas_meter: &mut VpGasMeter, storage: &Storage, + sentinel: &mut VpSentinel, ) -> EnvResult where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (epoch, gas) = storage.get_current_epoch(); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(epoch) } @@ -316,8 +332,9 @@ where pub fn get_tx_index( gas_meter: &mut VpGasMeter, tx_index: &TxIndex, + sentinel: &mut VpSentinel, ) -> EnvResult { - add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE)?; + add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE, sentinel)?; Ok(*tx_index) } @@ -325,12 +342,13 @@ pub fn get_tx_index( pub fn get_native_token( gas_meter: &mut VpGasMeter, storage: &Storage, + sentinel: &mut VpSentinel, ) -> EnvResult
where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { - add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE)?; + add_gas(gas_meter, STORAGE_ACCESS_GAS_PER_BYTE, sentinel)?; Ok(storage.native_token.clone()) } @@ -355,13 +373,14 @@ pub fn iter_prefix_pre<'a, DB, H>( write_log: &'a WriteLog, storage: &'a Storage, prefix: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (iter, gas) = storage::iter_prefix_pre(write_log, storage, prefix); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(iter) } @@ -372,13 +391,14 @@ pub fn iter_prefix_post<'a, DB, H>( write_log: &'a WriteLog, storage: &'a Storage, prefix: &Key, + sentinel: &mut VpSentinel, ) -> EnvResult> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, H: StorageHasher, { let (iter, gas) = storage::iter_prefix_post(write_log, storage, prefix); - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; Ok(iter) } @@ -386,12 +406,13 @@ where pub fn iter_next( gas_meter: &mut VpGasMeter, iter: &mut storage::PrefixIter, + sentinel: &mut VpSentinel, ) -> EnvResult)>> where DB: storage::DB + for<'iter> storage::DBIter<'iter>, { if let Some((key, val, gas)) = iter.next() { - add_gas(gas_meter, gas)?; + add_gas(gas_meter, gas, sentinel)?; return Ok(Some((key, val))); } Ok(None) diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index 5f701b7e1f..a7540bcf62 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -9,6 +9,8 @@ use borsh_ext::BorshSerializeExt; use masp_primitives::transaction::Transaction; use namada_core::ledger::gas::{GasMetering, TxGasMeter}; use namada_core::types::internal::KeyVal; +use namada_core::types::transaction::TxSentinel; +use namada_core::types::validity_predicate::VpSentinel; use thiserror::Error; #[cfg(feature = "wasm-runtime")] @@ -100,6 +102,8 @@ where pub iterators: MutHostRef<'a, &'a PrefixIterators<'a, DB>>, /// Transaction gas meter. pub gas_meter: MutHostRef<'a, &'a TxGasMeter>, + /// Transaction sentinel + pub sentinel: MutHostRef<'a, &'a TxSentinel>, /// The transaction code is used for signature verification pub tx: HostRef<'a, &'a Tx>, /// The transaction index is used to identify a shielded transaction's @@ -142,6 +146,7 @@ where write_log: &mut WriteLog, iterators: &mut PrefixIterators<'a, DB>, gas_meter: &mut TxGasMeter, + sentinel: &mut TxSentinel, tx: &Tx, tx_index: &TxIndex, verifiers: &mut BTreeSet
, @@ -153,6 +158,7 @@ where let write_log = unsafe { MutHostRef::new(write_log) }; let iterators = unsafe { MutHostRef::new(iterators) }; let gas_meter = unsafe { MutHostRef::new(gas_meter) }; + let sentinel = unsafe { MutHostRef::new(sentinel) }; let tx = unsafe { HostRef::new(tx) }; let tx_index = unsafe { HostRef::new(tx_index) }; let verifiers = unsafe { MutHostRef::new(verifiers) }; @@ -166,6 +172,7 @@ where write_log, iterators, gas_meter, + sentinel, tx, tx_index, verifiers, @@ -209,6 +216,7 @@ where write_log: self.write_log.clone(), iterators: self.iterators.clone(), gas_meter: self.gas_meter.clone(), + sentinel: self.sentinel.clone(), tx: self.tx.clone(), tx_index: self.tx_index.clone(), verifiers: self.verifiers.clone(), @@ -256,6 +264,8 @@ where pub iterators: MutHostRef<'a, &'a PrefixIterators<'a, DB>>, /// VP gas meter. pub gas_meter: MutHostRef<'a, &'a VpGasMeter>, + /// Errors sentinel + pub sentinel: MutHostRef<'a, &'a VpSentinel>, /// The transaction code is used for signature verification pub tx: HostRef<'a, &'a Tx>, /// The transaction index is used to identify a shielded transaction's @@ -324,6 +334,7 @@ where storage: &Storage, write_log: &WriteLog, gas_meter: &mut VpGasMeter, + sentinel: &mut VpSentinel, tx: &Tx, tx_index: &TxIndex, iterators: &mut PrefixIterators<'a, DB>, @@ -338,6 +349,7 @@ where storage, write_log, gas_meter, + sentinel, tx, tx_index, iterators, @@ -389,6 +401,7 @@ where storage: &Storage, write_log: &WriteLog, gas_meter: &mut VpGasMeter, + sentinel: &mut VpSentinel, tx: &Tx, tx_index: &TxIndex, iterators: &mut PrefixIterators<'a, DB>, @@ -405,6 +418,7 @@ where let tx_index = unsafe { HostRef::new(tx_index) }; let iterators = unsafe { MutHostRef::new(iterators) }; let gas_meter = unsafe { MutHostRef::new(gas_meter) }; + let sentinel = unsafe { MutHostRef::new(sentinel) }; let verifiers = unsafe { HostRef::new(verifiers) }; let result_buffer = unsafe { MutHostRef::new(result_buffer) }; let keys_changed = unsafe { HostRef::new(keys_changed) }; @@ -417,6 +431,7 @@ where write_log, iterators, gas_meter, + sentinel, tx, tx_index, eval_runner, @@ -445,6 +460,7 @@ where write_log: self.write_log.clone(), iterators: self.iterators.clone(), gas_meter: self.gas_meter.clone(), + sentinel: self.sentinel.clone(), tx: self.tx.clone(), tx_index: self.tx_index.clone(), eval_runner: self.eval_runner.clone(), @@ -472,16 +488,16 @@ where { let gas_meter = unsafe { env.ctx.gas_meter.get() }; // if we run out of gas, we need to stop the execution - let result = gas_meter - .consume(used_gas) - .map_err(TxRuntimeError::OutOfGas); - if let Err(err) = &result { + gas_meter.consume(used_gas).map_err(|err| { + let sentinel = unsafe { env.ctx.sentinel.get() }; + sentinel.set_out_of_gas(); tracing::info!( "Stopping transaction execution because of gas error: {}", err ); - } - result + + TxRuntimeError::OutOfGas(err) + }) } /// Called from VP wasm to request to use the given gas amount @@ -497,7 +513,8 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, used_gas) + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, used_gas, sentinel) } /// Storage `has_key` function exposed to the wasm VM Tx environment. It will @@ -1030,14 +1047,16 @@ where .read_string(key_ptr, key_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; // try to read from the storage let key = Key::parse(key).map_err(vp_host_fns::RuntimeError::StorageDataError)?; let storage = unsafe { env.ctx.storage.get() }; let write_log = unsafe { env.ctx.write_log.get() }; - let value = vp_host_fns::read_pre(gas_meter, storage, write_log, &key)?; + let value = + vp_host_fns::read_pre(gas_meter, storage, write_log, &key, sentinel)?; tracing::debug!( "vp_read_pre addr {}, key {}, value {:?}", unsafe { env.ctx.address.get() }, @@ -1081,7 +1100,8 @@ where .read_string(key_ptr, key_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_read_post {}, key {}", key, key_ptr,); @@ -1090,7 +1110,8 @@ where Key::parse(key).map_err(vp_host_fns::RuntimeError::StorageDataError)?; let storage = unsafe { env.ctx.storage.get() }; let write_log = unsafe { env.ctx.write_log.get() }; - let value = vp_host_fns::read_post(gas_meter, storage, write_log, &key)?; + let value = + vp_host_fns::read_post(gas_meter, storage, write_log, &key, sentinel)?; Ok(match value { Some(value) => { let len: i64 = value @@ -1127,7 +1148,8 @@ where .read_string(key_ptr, key_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_read_temp {}, key {}", key, key_ptr); @@ -1135,7 +1157,7 @@ where let key = Key::parse(key).map_err(vp_host_fns::RuntimeError::StorageDataError)?; let write_log = unsafe { env.ctx.write_log.get() }; - let value = vp_host_fns::read_temp(gas_meter, write_log, &key)?; + let value = vp_host_fns::read_temp(gas_meter, write_log, &key, sentinel)?; Ok(match value { Some(value) => { let len: i64 = value @@ -1176,7 +1198,8 @@ where .write_bytes(result_ptr, value) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas) + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel) } /// Storage `has_key` in prior state (before tx execution) function exposed to @@ -1198,7 +1221,8 @@ where .read_string(key_ptr, key_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_has_key_pre {}, key {}", key, key_ptr,); @@ -1206,8 +1230,9 @@ where Key::parse(key).map_err(vp_host_fns::RuntimeError::StorageDataError)?; let storage = unsafe { env.ctx.storage.get() }; let write_log = unsafe { env.ctx.write_log.get() }; - let present = - vp_host_fns::has_key_pre(gas_meter, storage, write_log, &key)?; + let present = vp_host_fns::has_key_pre( + gas_meter, storage, write_log, &key, sentinel, + )?; Ok(HostEnvResult::from(present).to_i64()) } @@ -1231,7 +1256,8 @@ where .read_string(key_ptr, key_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_has_key_post {}, key {}", key, key_ptr,); @@ -1239,8 +1265,9 @@ where Key::parse(key).map_err(vp_host_fns::RuntimeError::StorageDataError)?; let storage = unsafe { env.ctx.storage.get() }; let write_log = unsafe { env.ctx.write_log.get() }; - let present = - vp_host_fns::has_key_post(gas_meter, storage, write_log, &key)?; + let present = vp_host_fns::has_key_post( + gas_meter, storage, write_log, &key, sentinel, + )?; Ok(HostEnvResult::from(present).to_i64()) } @@ -1265,7 +1292,8 @@ where .read_string(prefix_ptr, prefix_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_iter_prefix_pre {}", prefix); @@ -1274,8 +1302,9 @@ where let write_log = unsafe { env.ctx.write_log.get() }; let storage = unsafe { env.ctx.storage.get() }; - let iter = - vp_host_fns::iter_prefix_pre(gas_meter, write_log, storage, &prefix)?; + let iter = vp_host_fns::iter_prefix_pre( + gas_meter, write_log, storage, &prefix, sentinel, + )?; let iterators = unsafe { env.ctx.iterators.get() }; Ok(iterators.insert(iter).id()) @@ -1302,7 +1331,8 @@ where .read_string(prefix_ptr, prefix_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; tracing::debug!("vp_iter_prefix_post {}", prefix); @@ -1311,8 +1341,9 @@ where let write_log = unsafe { env.ctx.write_log.get() }; let storage = unsafe { env.ctx.storage.get() }; - let iter = - vp_host_fns::iter_prefix_post(gas_meter, write_log, storage, &prefix)?; + let iter = vp_host_fns::iter_prefix_post( + gas_meter, write_log, storage, &prefix, sentinel, + )?; let iterators = unsafe { env.ctx.iterators.get() }; Ok(iterators.insert(iter).id()) @@ -1340,7 +1371,10 @@ where let iter_id = PrefixIteratorId::new(iter_id); if let Some(iter) = iterators.get_mut(iter_id) { let gas_meter = unsafe { env.ctx.gas_meter.get() }; - if let Some((key, val)) = vp_host_fns::iter_next(gas_meter, iter)? { + let sentinel = unsafe { env.ctx.sentinel.get() }; + if let Some((key, val)) = + vp_host_fns::iter_next(gas_meter, iter, sentinel)? + { let key_val = borsh::to_vec(&KeyVal { key, val }) .map_err(vp_host_fns::RuntimeError::EncodingError)?; let len: i64 = key_val @@ -1530,8 +1564,9 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let tx_index = unsafe { env.ctx.tx_index.get() }; - let tx_idx = vp_host_fns::get_tx_index(gas_meter, tx_index)?; + let tx_idx = vp_host_fns::get_tx_index(gas_meter, tx_index, sentinel)?; Ok(tx_idx.0) } @@ -1641,13 +1676,14 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; - let chain_id = vp_host_fns::get_chain_id(gas_meter, storage)?; + let chain_id = vp_host_fns::get_chain_id(gas_meter, storage, sentinel)?; let gas = env .memory .write_string(result_ptr, chain_id) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas) + vp_host_fns::add_gas(gas_meter, gas, sentinel) } /// Getting the block height function exposed to the wasm VM VP @@ -1664,8 +1700,9 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; - let height = vp_host_fns::get_block_height(gas_meter, storage)?; + let height = vp_host_fns::get_block_height(gas_meter, storage, sentinel)?; Ok(height.0) } @@ -1682,11 +1719,12 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; let (header, gas) = storage .get_block_header(Some(BlockHeight(height))) .map_err(vp_host_fns::RuntimeError::StorageError)?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; Ok(match header { Some(h) => { let value = h.serialize_to_vec(); @@ -1716,13 +1754,14 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; - let hash = vp_host_fns::get_block_hash(gas_meter, storage)?; + let hash = vp_host_fns::get_block_hash(gas_meter, storage, sentinel)?; let gas = env .memory .write_bytes(result_ptr, hash.0) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas) + vp_host_fns::add_gas(gas_meter, gas, sentinel) } /// Getting the transaction hash function exposed to the wasm VM VP environment. @@ -1738,8 +1777,9 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let tx = unsafe { env.ctx.tx.get() }; - let hash = vp_host_fns::get_tx_code_hash(gas_meter, tx)?; + let hash = vp_host_fns::get_tx_code_hash(gas_meter, tx, sentinel)?; let mut result_bytes = vec![]; if let Some(hash) = hash { result_bytes.push(1); @@ -1751,7 +1791,7 @@ where .memory .write_bytes(result_ptr, result_bytes) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas) + vp_host_fns::add_gas(gas_meter, gas, sentinel) } /// Getting the block epoch function exposed to the wasm VM VP @@ -1768,8 +1808,9 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; - let epoch = vp_host_fns::get_block_epoch(gas_meter, storage)?; + let epoch = vp_host_fns::get_block_epoch(gas_meter, storage, sentinel)?; Ok(epoch.0) } @@ -1791,7 +1832,8 @@ where .read_string(event_type_ptr, event_type_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let write_log = unsafe { env.ctx.write_log.get() }; let events = vp_host_fns::get_ibc_events(gas_meter, write_log, event_type)?; @@ -1835,7 +1877,8 @@ where .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let hashes = <[Hash; 1]>::try_from_slice(&hash_list) .map_err(vp_host_fns::RuntimeError::EncodingError)?; @@ -1843,7 +1886,7 @@ where .memory .read_bytes(public_keys_map_ptr, public_keys_map_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let public_keys_map = namada_core::types::account::AccountPublicKeysMap::try_from_slice( &public_keys_map, @@ -1854,7 +1897,7 @@ where .memory .read_bytes(signer_ptr, signer_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let signer = Address::try_from_slice(&signer) .map_err(vp_host_fns::RuntimeError::EncodingError)?; @@ -1862,24 +1905,33 @@ where .memory .read_bytes(max_signatures_ptr, max_signatures_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let max_signatures = Option::::try_from_slice(&max_signatures) .map_err(vp_host_fns::RuntimeError::EncodingError)?; let tx = unsafe { env.ctx.tx.get() }; - Ok(HostEnvResult::from( - tx.verify_signatures( - &hashes, - public_keys_map, - &Some(signer), - threshold, - max_signatures, - &mut Some(gas_meter), - ) - .is_ok(), - ) - .to_i64()) + match tx.verify_signatures( + &hashes, + public_keys_map, + &Some(signer), + threshold, + max_signatures, + &mut Some(gas_meter), + ) { + Ok(_) => Ok(HostEnvResult::Success.to_i64()), + Err(err) => match err { + namada_core::proto::Error::OutOfGas(inner) => { + sentinel.set_out_of_gas(); + Err(vp_host_fns::RuntimeError::OutOfGas(inner)) + } + namada_core::proto::Error::InvalidSectionSignature(_) => { + sentinel.set_invalid_signature(); + Ok(HostEnvResult::Fail.to_i64()) + } + _ => Ok(HostEnvResult::Fail.to_i64()), + }, + } } /// Verify a ShieldedTransaction. @@ -1896,11 +1948,12 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let (tx_bytes, gas) = env .memory .read_bytes(tx_ptr, tx_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let shielded: Transaction = BorshDeserialize::try_from_slice(tx_bytes.as_slice()) @@ -1954,8 +2007,11 @@ where use namada_core::ledger::ibc::{IbcActions, TransferModule}; - let tx_data = unsafe { env.ctx.tx.get().data() } - .ok_or(TxRuntimeError::MissingTxData)?; + let tx_data = unsafe { env.ctx.tx.get().data() }.ok_or_else(|| { + let sentinel = unsafe { env.ctx.sentinel.get() }; + sentinel.set_invalid_commitment(); + TxRuntimeError::MissingTxData + })?; let ctx = Rc::new(RefCell::new(env.ctx.clone())); let mut actions = IbcActions::new(ctx.clone()); let module = TransferModule::new(ctx); @@ -1997,6 +2053,18 @@ where Ok(()) } +/// Set the sentinel for an invalid tx section commitment +pub fn tx_set_commitment_sentinel(env: &TxVmEnv) +where + MEM: VmMemory, + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + CA: WasmCacheAccess, +{ + let sentinel = unsafe { env.ctx.sentinel.get() }; + sentinel.set_invalid_commitment(); +} + /// Evaluate a validity predicate with the given input data. pub fn vp_eval( env: &VpVmEnv<'static, MEM, DB, H, EVAL, CA>, @@ -2017,13 +2085,14 @@ where .read_bytes(vp_code_hash_ptr, vp_code_hash_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = unsafe { env.ctx.gas_meter.get() }; - vp_host_fns::add_gas(gas_meter, gas)?; + let sentinel = unsafe { env.ctx.sentinel.get() }; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let (input_data, gas) = env .memory .read_bytes(input_data_ptr, input_data_len as _) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas)?; + vp_host_fns::add_gas(gas_meter, gas, sentinel)?; let input_data: Tx = BorshDeserialize::try_from_slice(&input_data) .map_err(vp_host_fns::RuntimeError::EncodingError)?; let vp_code_hash = Hash(vp_code_hash.try_into().map_err(|e| { @@ -2052,14 +2121,16 @@ where CA: WasmCacheAccess, { let gas_meter = unsafe { env.ctx.gas_meter.get() }; + let sentinel = unsafe { env.ctx.sentinel.get() }; let storage = unsafe { env.ctx.storage.get() }; - let native_token = vp_host_fns::get_native_token(gas_meter, storage)?; + let native_token = + vp_host_fns::get_native_token(gas_meter, storage, sentinel)?; let native_token_string = native_token.encode(); let gas = env .memory .write_string(result_ptr, native_token_string) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; - vp_host_fns::add_gas(gas_meter, gas) + vp_host_fns::add_gas(gas_meter, gas, sentinel) } /// Log a string from exposed to the wasm VM VP environment. The message will be @@ -2520,6 +2591,7 @@ pub mod testing { iterators: &mut PrefixIterators<'static, DB>, verifiers: &mut BTreeSet
, gas_meter: &mut TxGasMeter, + sentinel: &mut TxSentinel, tx: &Tx, tx_index: &TxIndex, result_buffer: &mut Option>, @@ -2537,6 +2609,7 @@ pub mod testing { write_log, iterators, gas_meter, + sentinel, tx, tx_index, verifiers, @@ -2556,6 +2629,7 @@ pub mod testing { write_log: &WriteLog, iterators: &mut PrefixIterators<'static, DB>, gas_meter: &mut VpGasMeter, + sentinel: &mut VpSentinel, tx: &Tx, tx_index: &TxIndex, verifiers: &BTreeSet
, @@ -2576,6 +2650,7 @@ pub mod testing { storage, write_log, gas_meter, + sentinel, tx, tx_index, iterators, diff --git a/shared/src/vm/wasm/host_env.rs b/shared/src/vm/wasm/host_env.rs index e9a63e631e..a7dc51cc6f 100644 --- a/shared/src/vm/wasm/host_env.rs +++ b/shared/src/vm/wasm/host_env.rs @@ -60,7 +60,7 @@ where // default namespace "env" => { "memory" => initial_memory, - // Wasm middleware gas injectiong hook + // Wasm middleware gas injection hook "gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_charge_gas), // Whitelisted gas exposed function, we need two different functions just because of colliding names in the vm_host_env macro to generate implementations "namada_tx_charge_gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_charge_gas), @@ -86,6 +86,7 @@ where "namada_tx_get_native_token" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_native_token), "namada_tx_log_string" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_log_string), "namada_tx_ibc_execute" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_ibc_execute), + "namada_tx_set_commitment_sentinel" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_set_commitment_sentinel) }, } } @@ -107,7 +108,7 @@ where // default namespace "env" => { "memory" => initial_memory, - // Wasm middleware gas injectiong hook + // Wasm middleware gas injection hook "gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_charge_gas), // Whitelisted gas exposed function, we need two different functions just because of colliding names in the vm_host_env macro to generate implementations "namada_vp_charge_gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_charge_gas), diff --git a/shared/src/vm/wasm/run.rs b/shared/src/vm/wasm/run.rs index 9740469b95..d4ce052124 100644 --- a/shared/src/vm/wasm/run.rs +++ b/shared/src/vm/wasm/run.rs @@ -5,9 +5,11 @@ use std::marker::PhantomData; use borsh::BorshDeserialize; use namada_core::ledger::gas::{ - self, GasMetering, TxGasMeter, WASM_MEMORY_PAGE_GAS_COST, + GasMetering, TxGasMeter, WASM_MEMORY_PAGE_GAS_COST, }; use namada_core::ledger::storage::write_log::StorageModification; +use namada_core::types::transaction::TxSentinel; +use namada_core::types::validity_predicate::VpSentinel; use parity_wasm::elements; use thiserror::Error; use wasmer::{BaseTunables, Module, Store}; @@ -38,8 +40,8 @@ const WASM_STACK_LIMIT: u32 = u16::MAX as u32; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { - #[error("Missing wasm code error")] - MissingCode, + #[error("Missing tx section: {0}")] + MissingSection(String), #[error("Memory error: {0}")] MemoryError(memory::Error), #[error("Unable to inject stack limiter")] @@ -77,10 +79,12 @@ pub enum Error { LoadWasmCode(String), #[error("Unable to find compiled wasm code")] NoCompiledWasmCode, - #[error("Error while accounting for gas: {0}")] - GasError(#[from] gas::Error), + #[error("Gas error: {0}")] + GasError(String), #[error("Failed type conversion: {0}")] ConversionError(String), + #[error("Invalid transaction signature")] + InvalidTxSignature, } /// Result for functions that may fail @@ -106,7 +110,7 @@ where let tx_code = tx .get_section(tx.code_sechash()) .and_then(|x| Section::code_sec(x.as_ref())) - .ok_or(Error::MissingCode)?; + .ok_or(Error::MissingSection(tx.code_sechash().to_string()))?; let (module, store) = fetch_or_compile( tx_wasm_cache, @@ -120,12 +124,14 @@ where let mut verifiers = BTreeSet::new(); let mut result_buffer: Option> = None; + let mut sentinel = TxSentinel::default(); let env = TxVmEnv::new( WasmMemory::default(), storage, write_log, &mut iterators, gas_meter, + &mut sentinel, tx, tx_index, &mut verifiers, @@ -162,16 +168,16 @@ where entrypoint: TX_ENTRYPOINT, error, })?; - match apply_tx - .call(tx_data_ptr, tx_data_len) - .map_err(Error::RuntimeError) - { - Err(Error::RuntimeError(err)) => { - tracing::debug!("Tx WASM failed with {}", err); - Err(Error::RuntimeError(err)) + apply_tx.call(tx_data_ptr, tx_data_len).map_err(|err| { + tracing::debug!("Tx WASM failed with {}", err); + match sentinel { + TxSentinel::None => Error::RuntimeError(err), + TxSentinel::OutOfGas => Error::GasError(err.to_string()), + TxSentinel::InvalidCommitment => { + Error::MissingSection(err.to_string()) + } } - _ => Ok(()), - }?; + })?; Ok(verifiers) } @@ -214,12 +220,14 @@ where cache_access: PhantomData, }; + let mut sentinel = VpSentinel::default(); let env = VpVmEnv::new( WasmMemory::default(), address, storage, write_log, gas_meter, + &mut sentinel, tx, tx_index, &mut iterators, @@ -234,7 +242,7 @@ where memory::prepare_vp_memory(&store).map_err(Error::MemoryError)?; let imports = vp_imports(&store, initial_memory, env); - run_vp( + match run_vp( module, imports, &vp_code_hash, @@ -243,7 +251,34 @@ where keys_changed, verifiers, gas_meter, - ) + ) { + Ok(accept) => { + if sentinel.is_invalid_signature() { + if accept { + // This is unexpected, if the signature is invalid the vp + // should have rejected the tx. Something must be wrong with + // the VP logic and we take the signature verification + // result as the reference. In this case we override the vp + // result and log the issue + tracing::warn!( + "VP of {address} accepted the transaction but \ + signaled that the signature was invalid. Overriding \ + the vp result to reject the transaction..." + ); + } + Err(Error::InvalidTxSignature) + } else { + Ok(accept) + } + } + Err(err) => { + if sentinel.is_out_of_gas() { + Err(Error::GasError(err.to_string())) + } else { + Err(err) + } + } + } } #[allow(clippy::too_many_arguments)] @@ -520,15 +555,21 @@ where } }; - gas_meter.add_wasm_load_from_storage_gas(tx_len)?; - gas_meter.add_compiling_gas(tx_len)?; + gas_meter + .add_wasm_load_from_storage_gas(tx_len) + .map_err(|e| Error::GasError(e.to_string()))?; + gas_meter + .add_compiling_gas(tx_len) + .map_err(|e| Error::GasError(e.to_string()))?; Ok((module, store)) } Commitment::Id(code) => { - gas_meter.add_compiling_gas( - u64::try_from(code.len()) - .map_err(|e| Error::ConversionError(e.to_string()))?, - )?; + gas_meter + .add_compiling_gas( + u64::try_from(code.len()) + .map_err(|e| Error::ConversionError(e.to_string()))?, + ) + .map_err(|e| Error::GasError(e.to_string()))?; validate_untrusted_wasm(code).map_err(Error::ValidationError)?; match wasm_cache.compile_or_fetch(code)? { Some((module, store)) => Ok((module, store)), @@ -540,6 +581,8 @@ where /// Get the gas rules used to meter wasm operations fn get_gas_rules() -> wasm_instrument::gas_metering::ConstantCostRules { + // NOTE: costs set to 0 don't actually trigger the injection of a call to + // the gas host function (no useless instructions are injected) let instruction_cost = 0; let memory_grow_cost = WASM_MEMORY_PAGE_GAS_COST; let call_per_local_cost = 0; diff --git a/tests/src/native_vp/mod.rs b/tests/src/native_vp/mod.rs index 00070380e6..77cf6b1458 100644 --- a/tests/src/native_vp/mod.rs +++ b/tests/src/native_vp/mod.rs @@ -54,6 +54,7 @@ impl TestNativeVpEnv { gas_meter: RefCell::new(VpGasMeter::new_from_tx_meter( &self.tx_env.gas_meter, )), + sentinel: Default::default(), storage: &self.tx_env.wl_storage.storage, write_log: &self.tx_env.wl_storage.write_log, tx: &self.tx_env.tx, diff --git a/tests/src/vm_host_env/tx.rs b/tests/src/vm_host_env/tx.rs index fdb5bd959e..96c1b67cda 100644 --- a/tests/src/vm_host_env/tx.rs +++ b/tests/src/vm_host_env/tx.rs @@ -19,6 +19,7 @@ use namada::vm::wasm::run::Error; use namada::vm::wasm::{self, TxCache, VpCache}; use namada::vm::{self, WasmCacheRwAccess}; use namada_tx_prelude::borsh_ext::BorshSerializeExt; +use namada_tx_prelude::transaction::TxSentinel; use namada_tx_prelude::{storage_api, Ctx}; use namada_vp_prelude::key::common; use tempfile::TempDir; @@ -51,6 +52,7 @@ pub struct TestTxEnv { pub iterators: PrefixIterators<'static, MockDB>, pub verifiers: BTreeSet
, pub gas_meter: TxGasMeter, + pub sentinel: TxSentinel, pub tx_index: TxIndex, pub result_buffer: Option>, pub vp_wasm_cache: VpCache, @@ -75,6 +77,7 @@ impl Default for TestTxEnv { wl_storage, iterators: PrefixIterators::default(), gas_meter: TxGasMeter::new_from_sub_limit(100_000_000.into()), + sentinel: TxSentinel::default(), tx_index: TxIndex::default(), verifiers: BTreeSet::default(), result_buffer: None, @@ -344,6 +347,7 @@ mod native_tx_host_env { iterators, verifiers, gas_meter, + sentinel, result_buffer, tx_index, vp_wasm_cache, @@ -359,6 +363,7 @@ mod native_tx_host_env { iterators, verifiers, gas_meter, + sentinel, tx, tx_index, result_buffer, @@ -385,6 +390,7 @@ mod native_tx_host_env { iterators, verifiers, gas_meter, + sentinel, result_buffer, vp_wasm_cache, vp_cache_dir: _, @@ -399,6 +405,7 @@ mod native_tx_host_env { iterators, verifiers, gas_meter, + sentinel, tx, tx_index, result_buffer, @@ -412,7 +419,48 @@ mod native_tx_host_env { }) } }); - } + }; + + // unit, non-result, return type + ( "non-result", $fn:ident ( $($arg:ident : $type:ty),* $(,)?) ) => { + concat_idents!(extern_fn_name = namada, _, $fn { + #[no_mangle] + extern "C" fn extern_fn_name( $($arg: $type),* ) { + with(|TestTxEnv { + wl_storage, + iterators, + verifiers, + gas_meter, + sentinel, + result_buffer, + tx_index, + vp_wasm_cache, + vp_cache_dir: _, + tx_wasm_cache, + tx_cache_dir: _, + tx, + }: &mut TestTxEnv| { + + let tx_env = vm::host_env::testing::tx_env( + &wl_storage.storage, + &mut wl_storage.write_log, + iterators, + verifiers, + gas_meter, + sentinel, + tx, + tx_index, + result_buffer, + vp_wasm_cache, + tx_wasm_cache, + ); + + // Call the `host_env` function + $fn( &tx_env, $($arg),* ) + }) + } + }); + }; } // Implement all the exported functions from @@ -458,4 +506,5 @@ mod native_tx_host_env { native_host_fn!(tx_get_native_token(result_ptr: u64)); native_host_fn!(tx_log_string(str_ptr: u64, str_len: u64)); native_host_fn!(tx_charge_gas(used_gas: u64)); + native_host_fn!("non-result", tx_set_commitment_sentinel()); } diff --git a/tests/src/vm_host_env/vp.rs b/tests/src/vm_host_env/vp.rs index 69d7af9bb1..22b2562ed3 100644 --- a/tests/src/vm_host_env/vp.rs +++ b/tests/src/vm_host_env/vp.rs @@ -13,6 +13,7 @@ use namada::vm::prefix_iter::PrefixIterators; use namada::vm::wasm::{self, VpCache}; use namada::vm::{self, WasmCacheRwAccess}; use namada_core::ledger::gas::TxGasMeter; +use namada_tx_prelude::validity_predicate::VpSentinel; use namada_vp_prelude::Ctx; use tempfile::TempDir; @@ -44,6 +45,7 @@ pub struct TestVpEnv { pub wl_storage: WlStorage, pub iterators: PrefixIterators<'static, MockDB>, pub gas_meter: VpGasMeter, + pub sentinel: VpSentinel, pub tx: Tx, pub tx_index: TxIndex, pub keys_changed: BTreeSet, @@ -77,6 +79,7 @@ impl Default for TestVpEnv { gas_meter: VpGasMeter::new_from_tx_meter( &TxGasMeter::new_from_sub_limit(10_000_000.into()), ), + sentinel: VpSentinel::default(), tx, tx_index: TxIndex::default(), keys_changed: BTreeSet::default(), @@ -258,6 +261,7 @@ mod native_vp_host_env { wl_storage, iterators, gas_meter, + sentinel, tx, tx_index, keys_changed, @@ -274,6 +278,7 @@ mod native_vp_host_env { &wl_storage.write_log, iterators, gas_meter, + sentinel, tx, tx_index, verifiers, @@ -301,6 +306,7 @@ mod native_vp_host_env { wl_storage, iterators, gas_meter, + sentinel, tx, tx_index, keys_changed, @@ -317,6 +323,7 @@ mod native_vp_host_env { &wl_storage.write_log, iterators, gas_meter, + sentinel, tx, tx_index, verifiers, @@ -362,16 +369,15 @@ mod native_vp_host_env { ) -> i64); native_host_fn!(vp_log_string(str_ptr: u64, str_len: u64)); native_host_fn!(vp_verify_tx_section_signature( - hash_list_ptr: u64, - hash_list_len: u64, - public_keys_map_ptr: u64, - public_keys_map_len: u64, - signer_ptr: u64, - signer_len: u64, - threshold: u8, - max_signatures_ptr: u64, - max_signatures_len: u64,) - -> i64 - ); + hash_list_ptr: u64, + hash_list_len: u64, + public_keys_map_ptr: u64, + public_keys_map_len: u64, + signer_ptr: u64, + signer_len: u64, + threshold: u8, + max_signatures_ptr: u64, + max_signatures_len: u64, + ) -> i64); native_host_fn!(vp_charge_gas(used_gas: u64)); } diff --git a/tx_prelude/src/lib.rs b/tx_prelude/src/lib.rs index d2a286d542..0bfefdec5e 100644 --- a/tx_prelude/src/lib.rs +++ b/tx_prelude/src/lib.rs @@ -350,6 +350,10 @@ impl TxEnv for Ctx { None => Ok(Vec::new()), } } + + fn set_commitment_sentinel(&mut self) { + unsafe { namada_tx_set_commitment_sentinel() } + } } /// Execute IBC tx. diff --git a/vm_env/src/lib.rs b/vm_env/src/lib.rs index baadd56182..c3497c5c27 100644 --- a/vm_env/src/lib.rs +++ b/vm_env/src/lib.rs @@ -114,6 +114,9 @@ pub mod tx { /// Execute IBC tx. // Temp. workaround for pub fn namada_tx_ibc_execute(); + + /// Set the sentinel for a wrong tx section commitment + pub fn namada_tx_set_commitment_sentinel(); } } diff --git a/wasm/wasm_source/src/tx_bond.rs b/wasm/wasm_source/src/tx_bond.rs index 6b7263ce9b..9f0e71d005 100644 --- a/wasm/wasm_source/src/tx_bond.rs +++ b/wasm/wasm_source/src/tx_bond.rs @@ -5,7 +5,10 @@ use namada_tx_prelude::*; #[transaction(gas = 160000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let bond = transaction::pos::Bond::try_from_slice(&data[..]) .wrap_err("failed to decode Bond") .unwrap(); diff --git a/wasm/wasm_source/src/tx_bridge_pool.rs b/wasm/wasm_source/src/tx_bridge_pool.rs index b287f84a6a..364e597550 100644 --- a/wasm/wasm_source/src/tx_bridge_pool.rs +++ b/wasm/wasm_source/src/tx_bridge_pool.rs @@ -8,7 +8,10 @@ use namada_tx_prelude::*; #[transaction(gas = 100000)] fn apply_tx(ctx: &mut Ctx, signed: Tx) -> TxResult { - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let transfer = PendingTransfer::try_from_slice(&data[..]) .map_err(|e| Error::wrap("Error deserializing PendingTransfer", e))?; log_string("Received transfer to add to pool."); diff --git a/wasm/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs index 0923797e36..dd0a1be0c1 100644 --- a/wasm/wasm_source/src/tx_change_validator_commission.rs +++ b/wasm/wasm_source/src/tx_change_validator_commission.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 220000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let CommissionChange { validator, new_rate, diff --git a/wasm/wasm_source/src/tx_ibc.rs b/wasm/wasm_source/src/tx_ibc.rs index 5a8bac9d4e..2a6d2fcae3 100644 --- a/wasm/wasm_source/src/tx_ibc.rs +++ b/wasm/wasm_source/src/tx_ibc.rs @@ -8,7 +8,10 @@ use namada_tx_prelude::*; #[transaction(gas = 1240000)] fn apply_tx(_ctx: &mut Ctx, _tx_data: Tx) -> TxResult { // let signed = tx_data; - // let data = signed.data().ok_or_err_msg("Missing data")?; + // let data = signed.data().ok_or_err_msg("Missing data").or_else(|err| { + // ctx.set_commitment_sentinel(); + // Err(err) + // })?; // ibc::ibc_actions(ctx).execute(&data).into_storage_result() diff --git a/wasm/wasm_source/src/tx_init_account.rs b/wasm/wasm_source/src/tx_init_account.rs index 78a8edb018..da8ddc21f1 100644 --- a/wasm/wasm_source/src/tx_init_account.rs +++ b/wasm/wasm_source/src/tx_init_account.rs @@ -6,16 +6,27 @@ use namada_tx_prelude::*; #[transaction(gas = 230000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let tx_data = transaction::account::InitAccount::try_from_slice(&data[..]) .wrap_err("failed to decode InitAccount")?; debug_log!("apply_tx called to init a new established account"); let vp_code = signed .get_section(&tx_data.vp_code_hash) - .ok_or_err_msg("vp code section not found")? + .ok_or_err_msg("vp code section not found") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data_sec() - .ok_or_err_msg("vp code section must be tagged as extra")? + .ok_or_err_msg("vp code section must be tagged as extra") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .code .hash(); diff --git a/wasm/wasm_source/src/tx_init_proposal.rs b/wasm/wasm_source/src/tx_init_proposal.rs index 870d53331b..8c860d9d22 100644 --- a/wasm/wasm_source/src/tx_init_proposal.rs +++ b/wasm/wasm_source/src/tx_init_proposal.rs @@ -4,7 +4,10 @@ use namada_tx_prelude::*; #[transaction(gas = 40000)] fn apply_tx(ctx: &mut Ctx, tx: Tx) -> TxResult { - let data = tx.data().ok_or_err_msg("Missing data")?; + let data = tx.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let tx_data = transaction::governance::InitProposalData::try_from_slice(&data[..]) .wrap_err("failed to decode InitProposalData")?; @@ -12,18 +15,34 @@ fn apply_tx(ctx: &mut Ctx, tx: Tx) -> TxResult { // Get the content from the referred to section let content = tx .get_section(&tx_data.content) - .ok_or_err_msg("Missing proposal content")? + .ok_or_err_msg("Missing proposal content") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data() - .ok_or_err_msg("Missing full proposal content")?; + .ok_or_err_msg("Missing full proposal content") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; // Get the code from the referred to section let code_hash = tx_data.get_section_code_hash(); let code = match code_hash { Some(hash) => Some( tx.get_section(&hash) - .ok_or_err_msg("Missing proposal code")? + .ok_or_err_msg("Missing proposal code") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data() - .ok_or_err_msg("Missing full proposal code")?, + .ok_or_err_msg("Missing full proposal code") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?, ), None => None, }; diff --git a/wasm/wasm_source/src/tx_init_validator.rs b/wasm/wasm_source/src/tx_init_validator.rs index 5605764ba2..bea7ed0a9f 100644 --- a/wasm/wasm_source/src/tx_init_validator.rs +++ b/wasm/wasm_source/src/tx_init_validator.rs @@ -7,7 +7,10 @@ use namada_tx_prelude::*; #[transaction(gas = 730000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let init_validator = InitValidator::try_from_slice(&data[..]) .wrap_err("failed to decode InitValidator")?; debug_log!("apply_tx called to init a new validator account"); @@ -15,9 +18,17 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { // Get the validator vp code from the extra section let validator_vp_code_hash = signed .get_section(&init_validator.validator_vp_code_hash) - .ok_or_err_msg("validator vp section not found")? + .ok_or_err_msg("validator vp section not found") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data_sec() - .ok_or_err_msg("validator vp section must be tagged as extra")? + .ok_or_err_msg("validator vp section must be tagged as extra") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .code .hash(); diff --git a/wasm/wasm_source/src/tx_redelegate.rs b/wasm/wasm_source/src/tx_redelegate.rs index adae605a81..84f45093aa 100644 --- a/wasm/wasm_source/src/tx_redelegate.rs +++ b/wasm/wasm_source/src/tx_redelegate.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 460000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let transaction::pos::Redelegation { src_validator, dest_validator, diff --git a/wasm/wasm_source/src/tx_resign_steward.rs b/wasm/wasm_source/src/tx_resign_steward.rs index f87aea4bd9..79ea99ab2f 100644 --- a/wasm/wasm_source/src/tx_resign_steward.rs +++ b/wasm/wasm_source/src/tx_resign_steward.rs @@ -5,7 +5,10 @@ use namada_tx_prelude::*; #[transaction(gas = 40000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let steward_address = Address::try_from_slice(&data[..]) .wrap_err("failed to decode an Address")?; diff --git a/wasm/wasm_source/src/tx_reveal_pk.rs b/wasm/wasm_source/src/tx_reveal_pk.rs index 189ef2f4b3..ba2078d806 100644 --- a/wasm/wasm_source/src/tx_reveal_pk.rs +++ b/wasm/wasm_source/src/tx_reveal_pk.rs @@ -9,7 +9,10 @@ use namada_tx_prelude::*; #[transaction(gas = 170000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let pk = common::PublicKey::try_from_slice(&data[..]) .wrap_err("failed to decode common::PublicKey from tx_data")?; debug_log!("tx_reveal_pk called with pk: {pk}"); diff --git a/wasm/wasm_source/src/tx_transfer.rs b/wasm/wasm_source/src/tx_transfer.rs index f36f52c74d..9164ff1656 100644 --- a/wasm/wasm_source/src/tx_transfer.rs +++ b/wasm/wasm_source/src/tx_transfer.rs @@ -7,7 +7,10 @@ use namada_tx_prelude::*; #[transaction(gas = 110000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let transfer = token::Transfer::try_from_slice(&data[..]) .wrap_err("failed to decode token::Transfer")?; debug_log!("apply_tx called with transfer: {:#?}", transfer); @@ -28,6 +31,10 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { .get_section(hash) .and_then(|x| x.as_ref().masp_tx()) .ok_or_err_msg("unable to find shielded section") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + }) }) .transpose()?; if let Some(shielded) = shielded { diff --git a/wasm/wasm_source/src/tx_unbond.rs b/wasm/wasm_source/src/tx_unbond.rs index 7f66b7a338..a440692d43 100644 --- a/wasm/wasm_source/src/tx_unbond.rs +++ b/wasm/wasm_source/src/tx_unbond.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 430000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let unbond = transaction::pos::Unbond::try_from_slice(&data[..]) .wrap_err("failed to decode Unbond")?; diff --git a/wasm/wasm_source/src/tx_unjail_validator.rs b/wasm/wasm_source/src/tx_unjail_validator.rs index b487c44f7b..9beeb8086a 100644 --- a/wasm/wasm_source/src/tx_unjail_validator.rs +++ b/wasm/wasm_source/src/tx_unjail_validator.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 340000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let validator = Address::try_from_slice(&data[..]) .wrap_err("failed to decode an Address")?; ctx.unjail_validator(&validator) diff --git a/wasm/wasm_source/src/tx_update_account.rs b/wasm/wasm_source/src/tx_update_account.rs index d6315f9ace..a4279ca6a2 100644 --- a/wasm/wasm_source/src/tx_update_account.rs +++ b/wasm/wasm_source/src/tx_update_account.rs @@ -8,7 +8,10 @@ use namada_tx_prelude::*; #[transaction(gas = 140000)] fn apply_tx(ctx: &mut Ctx, tx: Tx) -> TxResult { let signed = tx; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let tx_data = transaction::account::UpdateAccount::try_from_slice(&data[..]) .wrap_err("failed to decode UpdateAccount")?; @@ -19,9 +22,17 @@ fn apply_tx(ctx: &mut Ctx, tx: Tx) -> TxResult { if let Some(hash) = tx_data.vp_code_hash { let vp_code_hash = signed .get_section(&hash) - .ok_or_err_msg("vp code section not found")? + .ok_or_err_msg("vp code section not found") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data_sec() - .ok_or_err_msg("vp code section must be tagged as extra")? + .ok_or_err_msg("vp code section must be tagged as extra") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .code .hash(); diff --git a/wasm/wasm_source/src/tx_update_steward_commission.rs b/wasm/wasm_source/src/tx_update_steward_commission.rs index a389e3d36a..0d98bd1ba1 100644 --- a/wasm/wasm_source/src/tx_update_steward_commission.rs +++ b/wasm/wasm_source/src/tx_update_steward_commission.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 40000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let steward_commission = UpdateStewardCommission::try_from_slice(&data[..]) .wrap_err("failed to decode an UpdateStewardCommission")?; diff --git a/wasm/wasm_source/src/tx_update_vp.rs b/wasm/wasm_source/src/tx_update_vp.rs index 65c5a5b1da..7d372ee749 100644 --- a/wasm/wasm_source/src/tx_update_vp.rs +++ b/wasm/wasm_source/src/tx_update_vp.rs @@ -7,16 +7,27 @@ use namada_tx_prelude::*; #[transaction(gas = 40000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let update_vp = transaction::UpdateVp::try_from_slice(&data[..]) .wrap_err("failed to decode UpdateVp")?; debug_log!("update VP for: {:#?}", update_vp.addr); let vp_code_hash = signed .get_section(&update_vp.vp_code_hash) - .ok_or_err_msg("vp code section not found")? + .ok_or_err_msg("vp code section not found") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .extra_data_sec() - .ok_or_err_msg("vp code section must be tagged as extra")? + .ok_or_err_msg("vp code section must be tagged as extra") + .map_err(|err| { + ctx.set_commitment_sentinel(); + err + })? .code .hash(); ctx.update_validity_predicate(&update_vp.addr, vp_code_hash) diff --git a/wasm/wasm_source/src/tx_vote_proposal.rs b/wasm/wasm_source/src/tx_vote_proposal.rs index 9bfd4d891b..d23820ab8a 100644 --- a/wasm/wasm_source/src/tx_vote_proposal.rs +++ b/wasm/wasm_source/src/tx_vote_proposal.rs @@ -5,7 +5,10 @@ use namada_tx_prelude::*; #[transaction(gas = 120000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let tx_data = transaction::governance::VoteProposalData::try_from_slice(&data[..]) .wrap_err("failed to decode VoteProposalData")?; diff --git a/wasm/wasm_source/src/tx_withdraw.rs b/wasm/wasm_source/src/tx_withdraw.rs index 5bedf44c42..4ba555c9dc 100644 --- a/wasm/wasm_source/src/tx_withdraw.rs +++ b/wasm/wasm_source/src/tx_withdraw.rs @@ -6,7 +6,10 @@ use namada_tx_prelude::*; #[transaction(gas = 260000)] fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let signed = tx_data; - let data = signed.data().ok_or_err_msg("Missing data")?; + let data = signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; let withdraw = transaction::pos::Withdraw::try_from_slice(&data[..]) .wrap_err("failed to decode Withdraw")?; diff --git a/wasm_for_tests/tx_fail.wasm b/wasm_for_tests/tx_fail.wasm new file mode 100755 index 0000000000..dce18fc1be Binary files /dev/null and b/wasm_for_tests/tx_fail.wasm differ diff --git a/wasm_for_tests/wasm_source/Cargo.toml b/wasm_for_tests/wasm_source/Cargo.toml index 789571e27e..33eb8a5f60 100644 --- a/wasm_for_tests/wasm_source/Cargo.toml +++ b/wasm_for_tests/wasm_source/Cargo.toml @@ -14,6 +14,7 @@ crate-type = ["cdylib"] [features] tx_memory_limit = [] tx_no_op = [] +tx_fail = [] tx_read_storage_key = [] tx_write = [] vp_always_false = [] diff --git a/wasm_for_tests/wasm_source/Makefile b/wasm_for_tests/wasm_source/Makefile index f56ab0afb8..524d302f54 100644 --- a/wasm_for_tests/wasm_source/Makefile +++ b/wasm_for_tests/wasm_source/Makefile @@ -7,6 +7,7 @@ nightly := $(shell cat ../../rust-nightly-version) # Wasms can be added via the Cargo.toml `[features]` list. wasms := tx_memory_limit wasms += tx_no_op +wasms += tx_fail wasms += tx_read_storage_key wasms += tx_write wasms += vp_always_false diff --git a/wasm_for_tests/wasm_source/src/lib.rs b/wasm_for_tests/wasm_source/src/lib.rs index 35198a1a87..d1c1b6696c 100644 --- a/wasm_for_tests/wasm_source/src/lib.rs +++ b/wasm_for_tests/wasm_source/src/lib.rs @@ -9,6 +9,17 @@ pub mod main { } } +/// A tx that fails everytime. +#[cfg(feature = "tx_fail")] +pub mod main { + use namada_tx_prelude::*; + + #[transaction(gas = 1000)] + fn apply_tx(_ctx: &mut Ctx, _tx_data: Tx) -> TxResult { + Err(Error::SimpleMessage("failed tx")) + } +} + /// A tx that allocates a memory of size given from the `tx_data: usize`. #[cfg(feature = "tx_memory_limit")] pub mod main {