diff --git a/.changelog/unreleased/bug-fixes/2819-allowlist-fix.md b/.changelog/unreleased/bug-fixes/2819-allowlist-fix.md new file mode 100644 index 0000000000..81bfabeb83 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/2819-allowlist-fix.md @@ -0,0 +1,2 @@ +- Adjusts the tx allowlist check to not prevent fee payment. + ([\#2819](https://github.com/anoma/namada/pull/2819)) \ No newline at end of file diff --git a/.changelog/unreleased/improvements/2627-remove-tx-queue.md b/.changelog/unreleased/improvements/2627-remove-tx-queue.md new file mode 100644 index 0000000000..989d7824e8 --- /dev/null +++ b/.changelog/unreleased/improvements/2627-remove-tx-queue.md @@ -0,0 +1,3 @@ +- Instead of having every user tx be executed across two blocks, the first executing a wrapper and the + second executing the main payload, this change makes it so that the entire tx is executed in a single + block (or rejected). ([\#2627](https://github.com/anoma/namada/pull/2627)) \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index efd89f312d..5e6aa7b3fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1632,6 +1632,12 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" +[[package]] +name = "drain_filter_polyfill" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "669a445ee724c5c69b1b06fe0b63e70a1c84bc9bb7d9696cd4f4e3ec45050408" + [[package]] name = "dtoa" version = "0.4.8" @@ -4221,6 +4227,7 @@ dependencies = [ "data-encoding", "derivative", "directories", + "drain_filter_polyfill", "ed25519-consensus 1.2.1", "ethabi", "ethbridge-bridge-events", diff --git a/Cargo.toml b/Cargo.toml index 1e02a8eda6..1457e7d640 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,7 @@ data-encoding = "2.3.2" derivation-path = "0.2.0" derivative = "2.2.0" directories = "4.0.1" +drain_filter_polyfill = "0.1.3" ed25519-consensus = "1.2.0" escargot = "0.5.7" ethabi = "18.0.0" diff --git a/crates/apps/Cargo.toml b/crates/apps/Cargo.toml index 72b1161e6a..32f1cae83c 100644 --- a/crates/apps/Cargo.toml +++ b/crates/apps/Cargo.toml @@ -84,6 +84,7 @@ config.workspace = true data-encoding.workspace = true derivative.workspace = true directories.workspace = true +drain_filter_polyfill.workspace = true ed25519-consensus = { workspace = true, features = ["std"] } ethabi.workspace = true ethbridge-bridge-events.workspace = true diff --git a/crates/apps/src/lib/bench_utils.rs b/crates/apps/src/lib/bench_utils.rs index 7b886eacdb..56f2ffd1ee 100644 --- a/crates/apps/src/lib/bench_utils.rs +++ b/crates/apps/src/lib/bench_utils.rs @@ -71,7 +71,7 @@ use namada::ledger::queries::{ }; use namada::state::StorageRead; use namada::tx::data::pos::Bond; -use namada::tx::data::{TxResult, VpsResult}; +use namada::tx::data::{Fee, TxResult, VpsResult}; use namada::tx::{Code, Data, Section, Signature, Tx}; use namada::vm::wasm::run; use namada::{proof_of_stake, tendermint}; @@ -289,9 +289,7 @@ impl BenchShell { extra_sections: Option>, signers: Vec<&SecretKey>, ) -> Tx { - let mut tx = Tx::from_type(namada::tx::data::TxType::Decrypted( - namada::tx::data::DecryptedTx::Decrypted, - )); + let mut tx = Tx::from_type(namada::tx::data::TxType::Raw); // NOTE: here we use the code hash to avoid including the cost for the // wasm validation. The wasm codes (both txs and vps) are always @@ -331,9 +329,7 @@ impl BenchShell { pub fn generate_ibc_tx(&self, wasm_code_path: &str, msg: impl Msg) -> Tx { // This function avoid serializaing the tx data with Borsh - let mut tx = Tx::from_type(namada::tx::data::TxType::Decrypted( - namada::tx::data::DecryptedTx::Decrypted, - )); + let mut tx = Tx::from_type(namada::tx::data::TxType::Raw); let code_hash = self .read_storage_key(&Key::wasm_hash(wasm_code_path)) .unwrap(); @@ -564,7 +560,18 @@ impl BenchShell { // Commit a masp transaction and cache the tx and the changed keys for // client queries - pub fn commit_masp_tx(&mut self, masp_tx: Tx) { + pub fn commit_masp_tx(&mut self, mut masp_tx: Tx) { + use namada::core::key::RefTo; + masp_tx.add_wrapper( + Fee { + amount_per_gas_unit: DenominatedAmount::native(0.into()), + token: self.state.in_mem().native_token.clone(), + }, + defaults::albert_keypair().ref_to(), + self.state.in_mem().last_epoch, + 0.into(), + None, + ); self.last_block_masp_txs .push((masp_tx, self.state.write_log().get_keys())); self.state.commit_tx(); @@ -575,9 +582,7 @@ pub fn generate_foreign_key_tx(signer: &SecretKey) -> Tx { let wasm_code = std::fs::read("../../wasm_for_tests/tx_write.wasm").unwrap(); - let mut tx = Tx::from_type(namada::tx::data::TxType::Decrypted( - namada::tx::data::DecryptedTx::Decrypted, - )); + let mut tx = Tx::from_type(namada::tx::data::TxType::Raw); tx.set_code(Code::new(wasm_code, None)); tx.set_data(Data::new( TxWriteData { @@ -857,6 +862,7 @@ impl Client for BenchShell { .map(|(idx, (_tx, changed_keys))| { let tx_result = TxResult { gas_used: 0.into(), + wrapper_changed_keys: Default::default(), changed_keys: changed_keys.to_owned(), vps_result: VpsResult::default(), initialized_accounts: vec![], diff --git a/crates/apps/src/lib/client/rpc.rs b/crates/apps/src/lib/client/rpc.rs index 52f2c56ba4..3ada4ceb84 100644 --- a/crates/apps/src/lib/client/rpc.rs +++ b/crates/apps/src/lib/client/rpc.rs @@ -58,7 +58,7 @@ use namada_sdk::rpc::{ self, enriched_bonds_and_unbonds, query_epoch, TxResponse, }; use namada_sdk::tendermint_rpc::endpoint::status; -use namada_sdk::tx::{display_inner_resp, display_wrapper_resp_and_get_result}; +use namada_sdk::tx::display_inner_resp; use namada_sdk::wallet::AddressVpType; use namada_sdk::{display, display_line, edisplay_line, error, prompt, Namada}; use tokio::time::Instant; @@ -201,8 +201,12 @@ pub async fn query_transfers( .map(|fvk| (ExtendedFullViewingKey::from(*fvk).fvk.vk, fvk)) .collect(); // Now display historical shielded and transparent transactions - for (IndexedTx { height, index: idx }, (epoch, tfer_delta, tx_delta)) in - transfers + for ( + IndexedTx { + height, index: idx, .. + }, + (epoch, tfer_delta, tx_delta), + ) in transfers { // Check if this transfer pertains to the supplied owner let mut relevant = match &query_owner { @@ -2769,23 +2773,10 @@ pub async fn query_result(context: &impl Namada, args: args::QueryResult) { Ok(resp) => { display_inner_resp(context, &resp); } - Err(err1) => { - // If this fails then instead look for an acceptance event. - let wrapper_resp = query_tx_response( - context.client(), - namada_sdk::rpc::TxEventQuery::Accepted(&args.tx_hash), - ) - .await; - match wrapper_resp { - Ok(resp) => { - display_wrapper_resp_and_get_result(context, &resp); - } - Err(err2) => { - // Print the errors that caused the lookups to fail - edisplay_line!(context.io(), "{}\n{}", err1, err2); - cli::safe_exit(1) - } - } + Err(err) => { + // Print the errors that caused the lookups to fail + edisplay_line!(context.io(), "{}", err); + cli::safe_exit(1) } } } diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc.rs index 09bb6d7847..625712cfe0 100644 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc.rs +++ b/crates/apps/src/lib/node/ledger/shell/block_alloc.rs @@ -16,22 +16,17 @@ //! In the current implementation, we allocate space for transactions //! in the following order of preference: //! -//! - First, we allot space for DKG encrypted txs. We allow DKG encrypted txs to -//! take up at most 1/3 of the total block space. -//! - Next, we allot space for DKG decrypted txs. Decrypted txs take up as much -//! space as needed. We will see, shortly, why in practice this is fine. -//! - Finally, we allot space for protocol txs. Protocol txs get half of the -//! remaining block space allotted to them. +//! - First, we allot space for protocol txs. We allow them to take up at most +//! 1/2 of the total block space unless there is extra room due to a lack of +//! user txs. +//! - Next, we allot space for user submitted txs until the block is filled. +//! - If we cannot fill the block with normal txs, we try to fill it with +//! protocol txs that were not allocated in the initial phase. //! -//! Since at some fixed height `H` decrypted txs only take up as -//! much space as the encrypted txs from height `H - 1`, and we -//! restrict the space of encrypted txs to at most 1/3 of the -//! total block space, we roughly divide the Tendermint block -//! space in 3, for each major type of tx. //! //! # How gas is allocated //! -//! Gas is only relevant to DKG encrypted txs. Every encrypted tx defines its +//! Gas is only relevant to non-protocol txs. Every such tx defines its //! gas limit. We take this entire gas limit as the amount of gas requested by //! the tx. @@ -48,11 +43,6 @@ pub mod states; // and alloc space for large tx right at the start. the problem with // this is that then we may not have enough space for decrypted txs -// TODO: panic if we don't have enough space reserved for a -// decrypted tx; in theory, we should always have enough space -// reserved for decrypted txs, given the invariants of the state -// machine - use std::marker::PhantomData; use namada::proof_of_stake::pos_queries::PosQueries; @@ -60,6 +50,7 @@ use namada::state::{self, WlState}; #[allow(unused_imports)] use crate::facade::tendermint_proto::abci::RequestPrepareProposal; +use crate::node::ledger::shell::block_alloc::states::WithNormalTxs; /// Block allocation failure status responses. #[derive(Debug, Copy, Clone, Eq, PartialEq)] @@ -121,11 +112,10 @@ impl Resource for BlockGas { /// /// We keep track of the current space utilized by: /// -/// - DKG encrypted transactions. -/// - DKG decrypted transactions. +/// - normal transactions. /// - Protocol transactions. /// -/// Gas usage of DKG encrypted txs is also tracked. +/// Gas usage of normal txs is also tracked. #[derive(Debug, Default)] pub struct BlockAllocator { /// The current state of the [`BlockAllocator`] state machine. @@ -135,14 +125,12 @@ pub struct BlockAllocator { block: TxBin, /// The current space utilized by protocol transactions. protocol_txs: TxBin, - /// The current space and gas utilized by DKG encrypted transactions. - encrypted_txs: EncryptedTxsBins, - /// The current space utilized by DKG decrypted transactions. - decrypted_txs: TxBin, + /// The current space and gas utilized by normal user transactions. + normal_txs: NormalTxsBins, } -impl From<&WlState> - for BlockAllocator> +impl From<&WlState> + for BlockAllocator> where D: 'static + state::DB + for<'iter> state::DBIter<'iter>, H: 'static + state::StorageHasher, @@ -156,7 +144,29 @@ where } } -impl BlockAllocator> { +impl BlockAllocator> { + /// Construct a new [`BlockAllocator`], with an upper bound + /// on the max size of all txs in a block defined by CometBFT and an upper + /// bound on the max gas in a block. + #[inline] + pub fn init( + cometbft_max_block_space_in_bytes: u64, + max_block_gas: u64, + ) -> Self { + let max = cometbft_max_block_space_in_bytes; + Self { + _state: PhantomData, + block: TxBin::init(max), + protocol_txs: { + let allotted_space_in_bytes = threshold::ONE_HALF.over(max); + TxBin::init(allotted_space_in_bytes) + }, + normal_txs: NormalTxsBins::new(max_block_gas), + } + } +} + +impl BlockAllocator { /// Construct a new [`BlockAllocator`], with an upper bound /// on the max size of all txs in a block defined by Tendermint and an upper /// bound on the max gas in a block. @@ -170,8 +180,10 @@ impl BlockAllocator> { _state: PhantomData, block: TxBin::init(max), protocol_txs: TxBin::default(), - encrypted_txs: EncryptedTxsBins::new(max, max_block_gas), - decrypted_txs: TxBin::default(), + normal_txs: NormalTxsBins { + space: TxBin::init(tendermint_max_block_space_in_bytes), + gas: TxBin::init(max_block_gas), + }, } } } @@ -184,10 +196,9 @@ impl BlockAllocator { /// block space for a given round and the sum of the allotted space /// to each [`TxBin`] instance in a [`BlockAllocator`]. #[inline] - fn uninitialized_space_in_bytes(&self) -> u64 { - let total_bin_space = self.protocol_txs.allotted - + self.encrypted_txs.space.allotted - + self.decrypted_txs.allotted; + fn unoccupied_space_in_bytes(&self) -> u64 { + let total_bin_space = + self.protocol_txs.occupied + self.normal_txs.space.occupied; self.block.allotted - total_bin_space } } @@ -256,16 +267,15 @@ impl TxBin { } #[derive(Debug, Default)] -pub struct EncryptedTxsBins { +pub struct NormalTxsBins { space: TxBin, gas: TxBin, } -impl EncryptedTxsBins { - pub fn new(max_bytes: u64, max_gas: u64) -> Self { - let allotted_space_in_bytes = threshold::ONE_THIRD.over(max_bytes); +impl NormalTxsBins { + pub fn new(max_gas: u64) -> Self { Self { - space: TxBin::init(allotted_space_in_bytes), + space: TxBin::default(), gas: TxBin::init(max_gas), } } @@ -273,10 +283,10 @@ impl EncryptedTxsBins { pub fn try_dump(&mut self, tx: &[u8], gas: u64) -> Result<(), String> { self.space.try_dump(tx).map_err(|e| match e { AllocFailure::Rejected { .. } => { - "No more space left in the block for wrapper txs".to_string() + "No more space left in the block for normal txs".to_string() } AllocFailure::OverflowsBin { .. } => "The given wrapper tx is \ - larger than 1/3 of the \ + larger than the remaining \ available block space" .to_string(), })?; @@ -316,33 +326,30 @@ pub mod threshold { } } - /// Divide free space in three. - pub const ONE_THIRD: Threshold = Threshold::new(1, 3); + /// Divide free space in half. + pub const ONE_HALF: Threshold = Threshold::new(1, 2); } #[cfg(test)] mod tests { - use std::cell::RefCell; use assert_matches::assert_matches; use proptest::prelude::*; use super::states::{ - BuildingEncryptedTxBatch, NextState, TryAlloc, WithEncryptedTxs, - WithoutEncryptedTxs, + BuildingNormalTxBatch, BuildingProtocolTxBatch, NextState, TryAlloc, }; use super::*; use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; - /// Convenience alias for a block space allocator at a state with encrypted + /// Convenience alias for a block space allocator at a state with protocol /// txs. - type BsaWrapperTxs = - BlockAllocator>; + type BsaInitialProtocolTxs = + BlockAllocator>; - /// Convenience alias for a block space allocator at a state without - /// encrypted txs. - type BsaNoWrapperTxs = - BlockAllocator>; + /// Convenience alias for a block allocator at a state with protocol + /// txs. + type BsaNormalTxs = BlockAllocator; /// Proptest generated txs. #[derive(Debug)] @@ -350,45 +357,46 @@ mod tests { tendermint_max_block_space_in_bytes: u64, max_block_gas: u64, protocol_txs: Vec, - encrypted_txs: Vec, - decrypted_txs: Vec, + normal_txs: Vec, } - /// Check that at most 1/3 of the block space is + /// Check that at most 1/2 of the block space is /// reserved for each kind of tx type, in the - /// allocator's common path. + /// allocator's common path. Further check that + /// if not enough normal txs are present, the rest + /// is filled with protocol txs #[test] - fn test_txs_are_evenly_split_across_block() { + fn test_filling_up_with_protocol() { const BLOCK_SIZE: u64 = 60; const BLOCK_GAS: u64 = 1_000; - // reserve block space for encrypted txs - let mut alloc = BsaWrapperTxs::init(BLOCK_SIZE, BLOCK_GAS); + // reserve block space for protocol txs + let mut alloc = BsaInitialProtocolTxs::init(BLOCK_SIZE, BLOCK_GAS); - // allocate ~1/3 of the block space to encrypted txs - assert!(alloc.try_alloc(BlockResources::new(&[0; 18], 0)).is_ok()); + // allocate ~1/2 of the block space to encrypted txs + assert!(alloc.try_alloc(&[0; 29]).is_ok()); - // reserve block space for decrypted txs + // reserve block space for normal txs let mut alloc = alloc.next_state(); - // the space we allotted to encrypted txs was shrunk to + // the space we allotted to protocol txs was shrunk to // the total space we actually used up - assert_eq!(alloc.encrypted_txs.space.allotted, 18); + assert_eq!(alloc.protocol_txs.allotted, 29); - // check that the allotted space for decrypted txs is correct - assert_eq!(alloc.decrypted_txs.allotted, BLOCK_SIZE - 18); + // check that the allotted space for normal txs is correct + assert_eq!(alloc.normal_txs.space.allotted, BLOCK_SIZE - 29); - // add about ~1/3 worth of decrypted txs - assert!(alloc.try_alloc(&[0; 17]).is_ok()); + // add about ~1/3 worth of normal txs + assert!(alloc.try_alloc(BlockResources::new(&[0; 17], 0)).is_ok()); - // reserve block space for protocol txs + // fill the rest of the block with protocol txs let mut alloc = alloc.next_state(); // check that space was shrunk - assert_eq!(alloc.protocol_txs.allotted, BLOCK_SIZE - (18 + 17)); + assert_eq!(alloc.protocol_txs.allotted, BLOCK_SIZE - (29 + 17)); // add protocol txs to the block space allocator - assert!(alloc.try_alloc(&[0; 25]).is_ok()); + assert!(alloc.try_alloc(&[0; 14]).is_ok()); // the block should be full at this point assert_matches!( @@ -397,15 +405,42 @@ mod tests { ); } - // Test that we cannot include encrypted txs in a block - // when the state invariants banish them from inclusion. + /// Test that if less than half of the block can be initially filled + /// with protocol txs, the rest if filled with normal txs. #[test] - fn test_encrypted_txs_are_rejected() { - let mut alloc = BsaNoWrapperTxs::init(1234, 1_000); + fn test_less_than_half_protocol() { + const BLOCK_SIZE: u64 = 60; + const BLOCK_GAS: u64 = 1_000; + + // reserve block space for protocol txs + let mut alloc = BsaInitialProtocolTxs::init(BLOCK_SIZE, BLOCK_GAS); + + // allocate ~1/3 of the block space to protocol txs + assert!(alloc.try_alloc(&[0; 18]).is_ok()); + + // reserve block space for normal txs + let mut alloc = alloc.next_state(); + + // the space we allotted to protocol txs was shrunk to + // the total space we actually used up + assert_eq!(alloc.protocol_txs.allotted, 18); + + // check that the allotted space for normal txs is correct + assert_eq!(alloc.normal_txs.space.allotted, BLOCK_SIZE - 18); + + // add about ~2/3 worth of normal txs + assert!(alloc.try_alloc(BlockResources::new(&[0; 42], 0)).is_ok()); + // the block should be full at this point assert_matches!( alloc.try_alloc(BlockResources::new(&[0; 1], 0)), Err(AllocFailure::Rejected { .. }) ); + + let mut alloc = alloc.next_state(); + assert_matches!( + alloc.try_alloc(&[0; 1]), + Err(AllocFailure::OverflowsBin { .. }) + ); } proptest! { @@ -436,21 +471,21 @@ mod tests { tendermint_max_block_space_in_bytes: u64, ) { let mut bins = - BsaWrapperTxs::init(tendermint_max_block_space_in_bytes, 1_000); + BsaNormalTxs::init(tendermint_max_block_space_in_bytes, 1_000); - // fill the entire bin of encrypted txs - bins.encrypted_txs.space.occupied = bins.encrypted_txs.space.allotted; + // fill the entire bin of protocol txs + bins.normal_txs.space.occupied = bins.normal_txs.space.allotted; - // make sure we can't dump any new encrypted txs in the bin + // make sure we can't dump any new protocol txs in the bin assert_matches!( bins.try_alloc(BlockResources::new(b"arbitrary tx bytes", 0)), Err(AllocFailure::Rejected { .. }) ); // Reset space bin - bins.encrypted_txs.space.occupied = 0; + bins.normal_txs.space.occupied = 0; // Fill the entire gas bin - bins.encrypted_txs.gas.occupied = bins.encrypted_txs.gas.allotted; + bins.normal_txs.gas.occupied = bins.normal_txs.gas.allotted; // Make sure we can't dump any new wncrypted txs in the bin assert_matches!( @@ -461,11 +496,16 @@ mod tests { /// Implementation of [`test_initial_bin_capacity`]. fn proptest_initial_bin_capacity(tendermint_max_block_space_in_bytes: u64) { - let bins = - BsaWrapperTxs::init(tendermint_max_block_space_in_bytes, 1_000); - let expected = tendermint_max_block_space_in_bytes - - threshold::ONE_THIRD.over(tendermint_max_block_space_in_bytes); - assert_eq!(expected, bins.uninitialized_space_in_bytes()); + let bins = BsaInitialProtocolTxs::init( + tendermint_max_block_space_in_bytes, + 1_000, + ); + let expected = tendermint_max_block_space_in_bytes; + assert_eq!( + bins.protocol_txs.allotted, + threshold::ONE_HALF.over(tendermint_max_block_space_in_bytes) + ); + assert_eq!(expected, bins.unoccupied_space_in_bytes()); } /// Implementation of [`test_tx_dump_doesnt_fill_up_bin`]. @@ -474,8 +514,7 @@ mod tests { tendermint_max_block_space_in_bytes, max_block_gas, protocol_txs, - encrypted_txs, - decrypted_txs, + normal_txs, } = args; // produce new txs until the moment we would have @@ -484,41 +523,57 @@ mod tests { // iterate over the produced txs to make sure we can keep // dumping new txs without filling up the bins - let bins = RefCell::new(BsaWrapperTxs::init( + let mut bins = BsaInitialProtocolTxs::init( tendermint_max_block_space_in_bytes, max_block_gas, - )); - let encrypted_txs = encrypted_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().encrypted_txs.space; - let new_size = bin.occupied + tx.len() as u64; - new_size < bin.allotted - }); - for tx in encrypted_txs { - assert!( - bins.borrow_mut() - .try_alloc(BlockResources::new(&tx, 0)) - .is_ok() - ); + ); + let mut protocol_tx_iter = protocol_txs.iter(); + let mut allocated_txs = vec![]; + let mut new_size = 0; + for tx in protocol_tx_iter.by_ref() { + let bin = bins.protocol_txs; + if new_size + tx.len() as u64 >= bin.allotted { + break; + } else { + new_size += tx.len() as u64; + allocated_txs.push(tx); + } + } + for tx in allocated_txs { + assert!(bins.try_alloc(tx).is_ok()); } - let bins = RefCell::new(bins.into_inner().next_state()); - let decrypted_txs = decrypted_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().decrypted_txs; - let new_size = bin.occupied + tx.len() as u64; - new_size < bin.allotted - }); + let mut bins = bins.next_state(); + let mut new_size = bins.normal_txs.space.allotted; + let mut decrypted_txs = vec![]; + for tx in normal_txs { + let bin = bins.normal_txs.space; + if (new_size + tx.len() as u64) < bin.allotted { + new_size += tx.len() as u64; + decrypted_txs.push(tx); + } else { + break; + } + } for tx in decrypted_txs { - assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); + assert!(bins.try_alloc(BlockResources::new(&tx, 0)).is_ok()); + } + + let mut bins = bins.next_state(); + let mut allocated_txs = vec![]; + let mut new_size = bins.protocol_txs.allotted; + for tx in protocol_tx_iter.by_ref() { + let bin = bins.protocol_txs; + if new_size + tx.len() as u64 >= bin.allotted { + break; + } else { + new_size += tx.len() as u64; + allocated_txs.push(tx); + } } - let bins = RefCell::new(bins.into_inner().next_state()); - let protocol_txs = protocol_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().protocol_txs; - let new_size = bin.occupied + tx.len() as u64; - new_size < bin.allotted - }); - for tx in protocol_txs { - assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); + for tx in allocated_txs { + assert!(bins.try_alloc(tx).is_ok()); } } @@ -527,7 +582,7 @@ mod tests { fn arb_transactions() // create base strategies ( - (tendermint_max_block_space_in_bytes, max_block_gas, protocol_tx_max_bin_size, encrypted_tx_max_bin_size, + (tendermint_max_block_space_in_bytes, max_block_gas, protocol_tx_max_bin_size, decrypted_tx_max_bin_size) in arb_max_bin_sizes(), ) // compose strategies @@ -535,36 +590,30 @@ mod tests { tendermint_max_block_space_in_bytes in Just(tendermint_max_block_space_in_bytes), max_block_gas in Just(max_block_gas), protocol_txs in arb_tx_list(protocol_tx_max_bin_size), - encrypted_txs in arb_tx_list(encrypted_tx_max_bin_size), - decrypted_txs in arb_tx_list(decrypted_tx_max_bin_size), + normal_txs in arb_tx_list(decrypted_tx_max_bin_size), ) -> PropTx { PropTx { tendermint_max_block_space_in_bytes, max_block_gas, protocol_txs: protocol_txs.into_iter().map(prost::bytes::Bytes::from).collect(), - encrypted_txs: encrypted_txs.into_iter().map(prost::bytes::Bytes::from).collect(), - decrypted_txs: decrypted_txs.into_iter().map(prost::bytes::Bytes::from).collect(), + normal_txs: normal_txs.into_iter().map(prost::bytes::Bytes::from).collect(), } } } /// Return random bin sizes for a [`BlockAllocator`]. - fn arb_max_bin_sizes() - -> impl Strategy { + fn arb_max_bin_sizes() -> impl Strategy { const MAX_BLOCK_SIZE_BYTES: u64 = 1000; (1..=MAX_BLOCK_SIZE_BYTES).prop_map( |tendermint_max_block_space_in_bytes| { ( tendermint_max_block_space_in_bytes, tendermint_max_block_space_in_bytes, - threshold::ONE_THIRD - .over(tendermint_max_block_space_in_bytes) - as usize, - threshold::ONE_THIRD + threshold::ONE_HALF .over(tendermint_max_block_space_in_bytes) as usize, - threshold::ONE_THIRD + threshold::ONE_HALF .over(tendermint_max_block_space_in_bytes) as usize, ) diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc/states.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc/states.rs index 7163cdf877..ed5d5e3004 100644 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc/states.rs +++ b/crates/apps/src/lib/node/ledger/shell/block_alloc/states.rs @@ -1,4 +1,4 @@ -//! All the states of the [`BlockAllocator`] state machine, +//! All the states of the `BlockAllocator` state machine, //! over the extent of a Tendermint consensus round //! block proposal. //! @@ -6,73 +6,52 @@ //! //! The state machine moves through the following state DAG: //! -//! 1. [`BuildingEncryptedTxBatch`] - the initial state. In this state, we -//! populate a block with DKG encrypted txs. This state supports two modes of -//! operation, which you can think of as two sub-states: -//! * [`WithoutEncryptedTxs`] - When this mode is active, no encrypted txs are -//! included in a block proposal. -//! * [`WithEncryptedTxs`] - When this mode is active, we are able to include -//! encrypted txs in a block proposal. -//! 2. [`BuildingDecryptedTxBatch`] - the second state. In this state, we -//! populate a block with DKG decrypted txs. -//! 3. [`BuildingProtocolTxBatch`] - the third state. In this state, we populate -//! a block with protocol txs. +//! 1. [`BuildingProtocolTxBatch`] - the initial state. In this state, we +//! populate a block with protocol txs. +//! 2. [`BuildingNormalTxBatch`] - the second state. In this state, we populate +//! a block with non-protocol txs. +//! 3. [`BuildingProtocolTxBatch`] - we return to this state to fill up any +//! remaining block space if possible. -mod decrypted_txs; -mod encrypted_txs; +mod normal_txs; mod protocol_txs; -use super::{AllocFailure, BlockAllocator}; - -/// Convenience wrapper for a [`BlockAllocator`] state that allocates -/// encrypted transactions. -#[allow(dead_code)] -pub enum EncryptedTxBatchAllocator { - WithEncryptedTxs( - BlockAllocator>, - ), - WithoutEncryptedTxs( - BlockAllocator>, - ), -} +use super::AllocFailure; /// The leader of the current Tendermint round is building -/// a new batch of DKG decrypted transactions. +/// a new batch of protocol txs. /// -/// For more info, read the module docs of -/// [`crate::node::ledger::shell::block_alloc::states`]. -pub enum BuildingDecryptedTxBatch {} - -/// The leader of the current Tendermint round is building -/// a new batch of Namada protocol transactions. +/// This happens twice, in the first stage, we fill up to 1/2 +/// of the block. At the end of allocating user txs, we fill +/// up any remaining space with un-allocated protocol txs. /// /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. -pub enum BuildingProtocolTxBatch {} +pub struct BuildingProtocolTxBatch { + /// One of [`WithNormalTxs`] and [`WithoutNormalTxs`]. + _mode: Mode, +} -/// The leader of the current Tendermint round is building -/// a new batch of DKG encrypted transactions. +/// Allow block proposals to include user submitted txs. /// /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. -pub struct BuildingEncryptedTxBatch { - /// One of [`WithEncryptedTxs`] and [`WithoutEncryptedTxs`]. - _mode: Mode, -} +pub enum WithNormalTxs {} /// Allow block proposals to include encrypted txs. /// /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. -pub enum WithEncryptedTxs {} +pub enum WithoutNormalTxs {} -/// Prohibit block proposals from including encrypted txs. +/// The leader of the current Tendermint round is building +/// a new batch of user submitted (non-protocol) transactions. /// /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. -pub enum WithoutEncryptedTxs {} +pub struct BuildingNormalTxBatch {} -/// Try to allocate a new transaction on a [`BlockAllocator`] state. +/// Try to allocate a new transaction on a `BlockAllocator` state. /// /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. @@ -86,7 +65,7 @@ pub trait TryAlloc { ) -> Result<(), AllocFailure>; } -/// Represents a state transition in the [`BlockAllocator`] state machine. +/// Represents a state transition in the `BlockAllocator` state machine. /// /// This trait should not be used directly. Instead, consider using /// [`NextState`]. @@ -94,10 +73,10 @@ pub trait TryAlloc { /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. pub trait NextStateImpl { - /// The next state in the [`BlockAllocator`] state machine. + /// The next state in the `BlockAllocator` state machine. type Next; - /// Transition to the next state in the [`BlockAllocator`] state + /// Transition to the next state in the `BlockAllocator`] state /// machine. fn next_state_impl(self) -> Self::Next; } @@ -108,7 +87,7 @@ pub trait NextStateImpl { /// For more info, read the module docs of /// [`crate::node::ledger::shell::block_alloc::states`]. pub trait NextState: NextStateImpl { - /// Transition to the next state in the [`BlockAllocator`] state, + /// Transition to the next state in the `BlockAllocator` state, /// using a null transiiton function. #[inline] fn next_state(self) -> Self::Next diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/decrypted_txs.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/decrypted_txs.rs deleted file mode 100644 index 7d7cc51d90..0000000000 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/decrypted_txs.rs +++ /dev/null @@ -1,48 +0,0 @@ -use std::marker::PhantomData; - -use super::super::{AllocFailure, BlockAllocator, TxBin}; -use super::{ - BuildingDecryptedTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, -}; - -impl TryAlloc for BlockAllocator { - type Resources<'tx> = &'tx [u8]; - - #[inline] - fn try_alloc( - &mut self, - tx: Self::Resources<'_>, - ) -> Result<(), AllocFailure> { - self.decrypted_txs.try_dump(tx) - } -} - -impl NextStateImpl for BlockAllocator { - type Next = BlockAllocator; - - #[inline] - fn next_state_impl(mut self) -> Self::Next { - self.decrypted_txs.shrink_to_fit(); - - // the remaining space is allocated to protocol txs - let remaining_free_space = self.uninitialized_space_in_bytes(); - self.protocol_txs = TxBin::init(remaining_free_space); - - // cast state - let Self { - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - .. - } = self; - - BlockAllocator { - _state: PhantomData, - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - } - } -} diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/encrypted_txs.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/encrypted_txs.rs deleted file mode 100644 index 05f74d1d56..0000000000 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/encrypted_txs.rs +++ /dev/null @@ -1,127 +0,0 @@ -use std::marker::PhantomData; - -use super::super::{AllocFailure, BlockAllocator, TxBin}; -use super::{ - BuildingDecryptedTxBatch, BuildingEncryptedTxBatch, - EncryptedTxBatchAllocator, NextStateImpl, TryAlloc, WithEncryptedTxs, - WithoutEncryptedTxs, -}; -use crate::node::ledger::shell::block_alloc::BlockResources; - -impl TryAlloc for BlockAllocator> { - type Resources<'tx> = BlockResources<'tx>; - - #[inline] - fn try_alloc( - &mut self, - resource_required: Self::Resources<'_>, - ) -> Result<(), AllocFailure> { - self.encrypted_txs.space.try_dump(resource_required.tx)?; - self.encrypted_txs.gas.try_dump(resource_required.gas) - } -} - -impl NextStateImpl - for BlockAllocator> -{ - type Next = BlockAllocator; - - #[inline] - fn next_state_impl(self) -> Self::Next { - next_state(self) - } -} - -impl TryAlloc - for BlockAllocator> -{ - type Resources<'tx> = BlockResources<'tx>; - - #[inline] - fn try_alloc( - &mut self, - _resource_required: Self::Resources<'_>, - ) -> Result<(), AllocFailure> { - Err(AllocFailure::Rejected { - bin_resource_left: 0, - }) - } -} - -impl NextStateImpl - for BlockAllocator> -{ - type Next = BlockAllocator; - - #[inline] - fn next_state_impl(self) -> Self::Next { - next_state(self) - } -} - -#[inline] -fn next_state( - mut alloc: BlockAllocator>, -) -> BlockAllocator { - alloc.encrypted_txs.space.shrink_to_fit(); - - // decrypted txs can use as much space as they need - which - // in practice will only be, at most, 1/3 of the block space - // used by encrypted txs at the prev height - let remaining_free_space = alloc.uninitialized_space_in_bytes(); - alloc.decrypted_txs = TxBin::init(remaining_free_space); - - // cast state - let BlockAllocator { - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - .. - } = alloc; - - BlockAllocator { - _state: PhantomData, - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - } -} - -impl TryAlloc for EncryptedTxBatchAllocator { - type Resources<'tx> = BlockResources<'tx>; - - #[inline] - fn try_alloc( - &mut self, - resource_required: Self::Resources<'_>, - ) -> Result<(), AllocFailure> { - match self { - EncryptedTxBatchAllocator::WithEncryptedTxs(state) => { - state.try_alloc(resource_required) - } - EncryptedTxBatchAllocator::WithoutEncryptedTxs(state) => { - // NOTE: this operation will cause the allocator to - // run out of memory immediately - state.try_alloc(resource_required) - } - } - } -} - -impl NextStateImpl for EncryptedTxBatchAllocator { - type Next = BlockAllocator; - - #[inline] - fn next_state_impl(self) -> Self::Next { - match self { - EncryptedTxBatchAllocator::WithEncryptedTxs(state) => { - state.next_state_impl() - } - EncryptedTxBatchAllocator::WithoutEncryptedTxs(state) => { - state.next_state_impl() - } - } - } -} diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/normal_txs.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/normal_txs.rs new file mode 100644 index 0000000000..e15333216f --- /dev/null +++ b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/normal_txs.rs @@ -0,0 +1,45 @@ +use std::marker::PhantomData; + +use super::super::{AllocFailure, BlockAllocator, TxBin}; +use super::{ + BuildingNormalTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, + WithoutNormalTxs, +}; +use crate::node::ledger::shell::block_alloc::BlockResources; + +impl TryAlloc for BlockAllocator { + type Resources<'tx> = BlockResources<'tx>; + + #[inline] + fn try_alloc( + &mut self, + resource_required: Self::Resources<'_>, + ) -> Result<(), AllocFailure> { + self.normal_txs.space.try_dump(resource_required.tx)?; + self.normal_txs.gas.try_dump(resource_required.gas) + } +} + +impl NextStateImpl for BlockAllocator { + type Next = BlockAllocator>; + + #[inline] + fn next_state_impl(mut self) -> Self::Next { + let remaining_free_space = self.unoccupied_space_in_bytes(); + self.protocol_txs = TxBin::init(remaining_free_space); + // cast state + let Self { + block, + protocol_txs, + normal_txs, + .. + } = self; + + BlockAllocator { + _state: PhantomData, + block, + protocol_txs, + normal_txs, + } + } +} diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/protocol_txs.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/protocol_txs.rs index aba289113e..302dc83824 100644 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc/states/protocol_txs.rs +++ b/crates/apps/src/lib/node/ledger/shell/block_alloc/states/protocol_txs.rs @@ -1,7 +1,13 @@ +use std::marker::PhantomData; + use super::super::{AllocFailure, BlockAllocator}; -use super::{BuildingProtocolTxBatch, TryAlloc}; +use super::{ + BuildingNormalTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, + WithNormalTxs, +}; +use crate::node::ledger::shell::block_alloc::TxBin; -impl TryAlloc for BlockAllocator { +impl TryAlloc for BlockAllocator> { type Resources<'tx> = &'tx [u8]; #[inline] @@ -12,3 +18,28 @@ impl TryAlloc for BlockAllocator { self.protocol_txs.try_dump(tx) } } + +impl NextStateImpl for BlockAllocator> { + type Next = BlockAllocator; + + #[inline] + fn next_state_impl(mut self) -> Self::Next { + self.protocol_txs.shrink_to_fit(); + let remaining_free_space = self.unoccupied_space_in_bytes(); + self.normal_txs.space = TxBin::init(remaining_free_space); + // cast state + let BlockAllocator { + block, + protocol_txs, + normal_txs, + .. + } = self; + + BlockAllocator { + _state: PhantomData, + block, + protocol_txs, + normal_txs, + } + } +} diff --git a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs index ec0e535c4d..0f2d6edb20 100644 --- a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -5,7 +5,7 @@ use masp_primitives::merkle_tree::CommitmentTree; use masp_primitives::sapling::Node; use namada::core::storage::{BlockHash, BlockResults, Epoch, Header}; use namada::governance::pgf::inflation as pgf_inflation; -use namada::ledger::events::EventType; +use namada::hash::Hash; use namada::ledger::gas::GasMetering; use namada::ledger::pos::namada_proof_of_stake; use namada::ledger::protocol::WrapperArgs; @@ -201,220 +201,156 @@ where format!("Tx rejected: {}", &processed_tx.result.info); tx_event["gas_used"] = "0".into(); response.events.push(tx_event); - // if the rejected tx was decrypted, remove it - // from the queue of txs to be processed - if let TxType::Decrypted(_) = &tx_header.tx_type { - self.state - .in_mem_mut() - .tx_queue - .pop() - .expect("Missing wrapper tx in queue"); - } - continue; } - let ( - mut tx_event, - embedding_wrapper, - tx_gas_meter, - wrapper, - mut wrapper_args, - ) = match &tx_header.tx_type { - TxType::Wrapper(wrapper) => { - stats.increment_wrapper_txs(); - let tx_event = new_tx_event(&tx, height.0); - let gas_meter = TxGasMeter::new(wrapper.gas_limit); - ( - tx_event, - None, - gas_meter, - Some(tx.clone()), - Some(WrapperArgs { - block_proposer: &native_block_proposer_address, - is_committed_fee_unshield: false, - }), - ) - } - TxType::Decrypted(inner) => { - // We remove the corresponding wrapper tx from the queue - let tx_in_queue = self - .state - .in_mem_mut() - .tx_queue - .pop() - .expect("Missing wrapper tx in queue"); - let mut event = new_tx_event(&tx, height.0); - - match inner { - DecryptedTx::Decrypted => { - if let Some(code_sec) = tx - .get_section(tx.code_sechash()) - .and_then(|x| Section::code_sec(x.as_ref())) - { - stats.increment_tx_type( - code_sec.code.hash().to_string(), - ); - } - } - DecryptedTx::Undecryptable => { - tracing::info!( - "Tx with hash {} was un-decryptable", - tx_in_queue.tx.header_hash() + let (mut tx_event, tx_gas_meter, mut wrapper_args) = + match &tx_header.tx_type { + TxType::Wrapper(wrapper) => { + stats.increment_wrapper_txs(); + let tx_event = new_tx_event(&tx, height.0); + let gas_meter = TxGasMeter::new(wrapper.gas_limit); + if let Some(code_sec) = tx + .get_section(tx.code_sechash()) + .and_then(|x| Section::code_sec(x.as_ref())) + { + stats.increment_tx_type( + code_sec.code.hash().to_string(), ); - event["info"] = "Transaction is invalid.".into(); - event["log"] = - "Transaction could not be decrypted.".into(); - event["code"] = ResultCode::Undecryptable.into(); - response.events.push(event); - continue; } + ( + tx_event, + gas_meter, + Some(WrapperArgs { + block_proposer: &native_block_proposer_address, + is_committed_fee_unshield: false, + }), + ) } - - ( - event, - Some(tx_in_queue.tx), - TxGasMeter::new_from_sub_limit(tx_in_queue.gas), - None, - None, - ) - } - TxType::Raw => { - tracing::error!( - "Internal logic error: FinalizeBlock received a \ - TxType::Raw transaction" - ); - continue; - } - TxType::Protocol(protocol_tx) => match protocol_tx.tx { - ProtocolTxType::BridgePoolVext - | ProtocolTxType::BridgePool - | ProtocolTxType::ValSetUpdateVext - | ProtocolTxType::ValidatorSetUpdate => ( - new_tx_event(&tx, height.0), - None, - TxGasMeter::new_from_sub_limit(0.into()), - None, - None, - ), - ProtocolTxType::EthEventsVext => { - let ext = + TxType::Raw => { + tracing::error!( + "Internal logic error: FinalizeBlock received a \ + TxType::Raw transaction" + ); + continue; + } + TxType::Protocol(protocol_tx) => match protocol_tx.tx { + ProtocolTxType::BridgePoolVext + | ProtocolTxType::BridgePool + | ProtocolTxType::ValSetUpdateVext + | ProtocolTxType::ValidatorSetUpdate => ( + new_tx_event(&tx, height.0), + TxGasMeter::new_from_sub_limit(0.into()), + None, + ), + ProtocolTxType::EthEventsVext => { + let ext = ethereum_tx_data_variants::EthEventsVext::try_from( &tx, ) .unwrap(); - if self - .mode - .get_validator_address() - .map(|validator| { - validator == &ext.data.validator_addr - }) - .unwrap_or(false) - { - for event in ext.data.ethereum_events.iter() { - self.mode.dequeue_eth_event(event); + if self + .mode + .get_validator_address() + .map(|validator| { + validator == &ext.data.validator_addr + }) + .unwrap_or(false) + { + for event in ext.data.ethereum_events.iter() { + self.mode.dequeue_eth_event(event); + } } + ( + new_tx_event(&tx, height.0), + TxGasMeter::new_from_sub_limit(0.into()), + None, + ) } - ( - new_tx_event(&tx, height.0), - None, - TxGasMeter::new_from_sub_limit(0.into()), - None, - None, - ) - } - ProtocolTxType::EthereumEvents => { - let digest = + ProtocolTxType::EthereumEvents => { + let digest = ethereum_tx_data_variants::EthereumEvents::try_from( &tx, ).unwrap(); - if let Some(address) = - self.mode.get_validator_address().cloned() - { - let this_signer = &( - address, - self.state.in_mem().get_last_block_height(), - ); - for MultiSignedEthEvent { event, signers } in - &digest.events + if let Some(address) = + self.mode.get_validator_address().cloned() { - if signers.contains(this_signer) { - self.mode.dequeue_eth_event(event); + let this_signer = &( + address, + self.state.in_mem().get_last_block_height(), + ); + for MultiSignedEthEvent { event, signers } in + &digest.events + { + if signers.contains(this_signer) { + self.mode.dequeue_eth_event(event); + } } } + ( + new_tx_event(&tx, height.0), + TxGasMeter::new_from_sub_limit(0.into()), + None, + ) } - ( - new_tx_event(&tx, height.0), - None, - TxGasMeter::new_from_sub_limit(0.into()), - None, - None, - ) - } - }, - }; + }, + }; + let replay_protection_hashes = + if matches!(tx_header.tx_type, TxType::Wrapper(_)) { + Some(ReplayProtectionHashes { + raw_header_hash: tx.raw_header_hash(), + header_hash: tx.header_hash(), + }) + } else { + None + }; let tx_gas_meter = RefCell::new(tx_gas_meter); - let tx_result = protocol::check_tx_allowed(&tx, &self.state) - .and_then(|()| { - protocol::dispatch_tx( - tx, - processed_tx.tx.as_ref(), - TxIndex( - tx_index - .try_into() - .expect("transaction index out of bounds"), - ), - &tx_gas_meter, - &mut self.state, - &mut self.vp_wasm_cache, - &mut self.tx_wasm_cache, - wrapper_args.as_mut(), - ) - }) - .map_err(Error::TxApply); + let tx_result = protocol::dispatch_tx( + tx, + processed_tx.tx.as_ref(), + TxIndex( + tx_index + .try_into() + .expect("transaction index out of bounds"), + ), + &tx_gas_meter, + &mut self.state, + &mut self.vp_wasm_cache, + &mut self.tx_wasm_cache, + wrapper_args.as_mut(), + ) + .map_err(Error::TxApply); let tx_gas_meter = tx_gas_meter.into_inner(); match tx_result { Ok(result) => { if result.is_accepted() { - if let EventType::Accepted = tx_event.event_type { - // Wrapper transaction - tracing::trace!( - "Wrapper transaction {} was accepted", - tx_event["hash"] - ); - if wrapper_args - .expect("Missing required wrapper arguments") - .is_committed_fee_unshield - { - tx_event["is_valid_masp_tx"] = - format!("{}", tx_index); - } - self.state.in_mem_mut().tx_queue.push(TxInQueue { - tx: wrapper.expect("Missing expected wrapper"), - gas: tx_gas_meter.get_available_gas(), - }); - } else { - tracing::trace!( - "all VPs accepted transaction {} storage \ - modification {:#?}", - tx_event["hash"], - result - ); - if result.vps_result.accepted_vps.contains( + if wrapper_args + .map(|args| args.is_committed_fee_unshield) + .unwrap_or_default() + || result.vps_result.accepted_vps.contains( &Address::Internal( address::InternalAddress::Masp, ), - ) { - tx_event["is_valid_masp_tx"] = - format!("{}", tx_index); - } - changed_keys - .extend(result.changed_keys.iter().cloned()); - stats.increment_successful_txs(); - if let Some(wrapper) = embedding_wrapper { - self.commit_inner_tx_hash(wrapper); - } + ) + { + tx_event["is_valid_masp_tx"] = + format!("{}", tx_index); } + tracing::trace!( + "all VPs accepted transaction {} storage \ + modification {:#?}", + tx_event["hash"], + result + ); + + changed_keys + .extend(result.changed_keys.iter().cloned()); + changed_keys.extend( + result.wrapper_changed_keys.iter().cloned(), + ); + stats.increment_successful_txs(); + self.commit_inner_tx_hash(replay_protection_hashes); + self.state.commit_tx(); if !tx_event.contains_key("code") { tx_event["code"] = ResultCode::Ok.into(); @@ -446,20 +382,28 @@ where ), ); } else { + // this branch can only be reached by inner txs tracing::trace!( "some VPs rejected transaction {} storage \ modification {:#?}", tx_event["hash"], result.vps_result.rejected_vps ); + // The fee unshield operation could still have been + // committed + if wrapper_args + .map(|args| args.is_committed_fee_unshield) + .unwrap_or_default() + { + tx_event["is_valid_masp_tx"] = + format!("{}", tx_index); + } - if let Some(wrapper) = embedding_wrapper { - // If decrypted tx failed for any reason but invalid - // signature, commit its hash to storage, otherwise - // allow for a replay - if !result.vps_result.invalid_sig { - self.commit_inner_tx_hash(wrapper); - } + // If an inner tx failed for any reason but invalid + // signature, commit its hash to storage, otherwise + // allow for a replay + if !result.vps_result.invalid_sig { + self.commit_inner_tx_hash(replay_protection_hashes); } stats.increment_rejected_txs(); @@ -470,6 +414,19 @@ where tx_event["info"] = "Check inner_tx for result.".to_string(); tx_event["inner_tx"] = result.to_string(); } + Err(Error::TxApply(protocol::Error::WrapperRunnerError( + msg, + ))) => { + tracing::info!( + "Wrapper transaction {} failed with: {}", + tx_event["hash"], + msg, + ); + tx_event["gas_used"] = + tx_gas_meter.get_tx_consumed_gas().to_string(); + tx_event["info"] = msg.to_string(); + tx_event["code"] = ResultCode::InvalidTx.into(); + } Err(msg) => { tracing::info!( "Transaction {} failed with: {}", @@ -477,10 +434,10 @@ where msg ); - // If transaction type is Decrypted and didn't fail + // If user transaction didn't fail // because of out of gas nor invalid // section commitment, commit its hash to prevent replays - if let Some(wrapper) = embedding_wrapper { + if matches!(tx_header.tx_type, TxType::Wrapper(_)) { if !matches!( msg, Error::TxApply(protocol::Error::GasError(_)) @@ -491,7 +448,7 @@ where protocol::Error::ReplayAttempt(_) ) ) { - self.commit_inner_tx_hash(wrapper); + self.commit_inner_tx_hash(replay_protection_hashes); } else if let Error::TxApply( protocol::Error::ReplayAttempt(_), ) = msg @@ -500,11 +457,10 @@ where // hash. A replay of the wrapper is impossible since // the inner tx hash is committed to storage and // we validate the wrapper against that hash too - self.state - .delete_tx_hash(wrapper.header_hash()) - .expect( - "Error while deleting tx hash from storage", - ); + let header_hash = replay_protection_hashes + .expect("This cannot fail") + .header_hash; + self.state.delete_tx_hash(header_hash); } } @@ -514,21 +470,18 @@ where tx_event["gas_used"] = tx_gas_meter.get_tx_consumed_gas().to_string(); tx_event["info"] = msg.to_string(); - if let EventType::Accepted = tx_event.event_type { - // If wrapper, invalid tx error code - tx_event["code"] = ResultCode::InvalidTx.into(); - // The fee unshield operation could still have been - // committed - if wrapper_args - .expect("Missing required wrapper arguments") - .is_committed_fee_unshield - { - tx_event["is_valid_masp_tx"] = - format!("{}", tx_index); - } - } else { - tx_event["code"] = ResultCode::WasmRuntimeError.into(); + + // If wrapper, invalid tx error code + tx_event["code"] = ResultCode::InvalidTx.into(); + // The fee unshield operation could still have been + // committed + if wrapper_args + .map(|args| args.is_committed_fee_unshield) + .unwrap_or_default() + { + tx_event["is_valid_masp_tx"] = format!("{}", tx_index); } + tx_event["code"] = ResultCode::WasmRuntimeError.into(); } } response.events.push(tx_event); @@ -661,17 +614,26 @@ where // hash since it's redundant (we check the inner tx hash too when validating // the wrapper). Requires the wrapper transaction as argument to recover // both the hashes. - fn commit_inner_tx_hash(&mut self, wrapper_tx: Tx) { - self.state - .write_tx_hash(wrapper_tx.raw_header_hash()) - .expect("Error while writing tx hash to storage"); + fn commit_inner_tx_hash(&mut self, hashes: Option) { + if let Some(ReplayProtectionHashes { + raw_header_hash, + header_hash, + }) = hashes + { + self.state + .write_tx_hash(raw_header_hash) + .expect("Error while writing tx hash to storage"); - self.state - .delete_tx_hash(wrapper_tx.header_hash()) - .expect("Error while deleting tx hash from storage"); + self.state.delete_tx_hash(header_hash) + } } } +struct ReplayProtectionHashes { + raw_header_hash: Hash, + header_hash: Hash, +} + /// Convert ABCI vote info to PoS vote info. Any info which fails the conversion /// will be skipped and errors logged. /// @@ -808,6 +770,7 @@ mod test_finalize_block { shell: &TestShell, keypair: &common::SecretKey, ) -> (Tx, ProcessedTx) { + let tx_code = TestWasms::TxNoOp.read_bytes(); let mut wrapper_tx = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { @@ -820,10 +783,10 @@ mod test_finalize_block { None, )))); wrapper_tx.header.chain_id = shell.chain_id.clone(); - wrapper_tx.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); wrapper_tx.set_data(Data::new( "Encrypted transaction data".as_bytes().to_owned(), )); + wrapper_tx.set_code(Code::new(tx_code, None)); wrapper_tx.add_section(Section::Signature(Signature::new( wrapper_tx.sechashes(), [(0, keypair.clone())].into_iter().collect(), @@ -842,44 +805,6 @@ mod test_finalize_block { ) } - /// Make a wrapper tx and a processed tx from the wrapped tx that can be - /// added to `FinalizeBlock` request. - fn mk_decrypted_tx( - shell: &mut TestShell, - keypair: &common::SecretKey, - ) -> ProcessedTx { - let tx_code = TestWasms::TxNoOp.read_bytes(); - let mut outer_tx = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - outer_tx.header.chain_id = shell.chain_id.clone(); - outer_tx.set_code(Code::new(tx_code, None)); - outer_tx.set_data(Data::new( - "Decrypted transaction data".as_bytes().to_owned(), - )); - let gas_limit = - Gas::from(outer_tx.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(outer_tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(outer_tx.clone(), gas_limit); - outer_tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - ProcessedTx { - tx: outer_tx.to_bytes().into(), - result: TxResult { - code: ResultCode::Ok.into(), - info: "".into(), - }, - } - } - /// Check that if a wrapper tx was rejected by [`process_proposal`], /// check that the correct event is returned. Check that it does /// not appear in the queue of txs to be decrypted @@ -888,9 +813,8 @@ mod test_finalize_block { let (mut shell, _, _, _) = setup(); let keypair = gen_keypair(); let mut processed_txs = vec![]; - let mut valid_wrappers = vec![]; - // Add unshielded balance for fee paymenty + // Add unshielded balance for fee payment let balance_key = token::storage_key::balance_key( &shell.state.in_mem().native_token, &Address::from(&keypair.ref_to()), @@ -901,30 +825,10 @@ mod test_finalize_block { .unwrap(); // create some wrapper txs - for i in 1u64..5 { - let (wrapper, mut processed_tx) = mk_wrapper_tx(&shell, &keypair); - if i > 1 { - processed_tx.result.code = - u32::try_from(i.rem_euclid(2)).unwrap(); - processed_txs.push(processed_tx); - } else { - let wrapper_info = - if let TxType::Wrapper(w) = wrapper.header().tx_type { - w - } else { - panic!("Unexpected tx type"); - }; - shell.enqueue_tx( - wrapper.clone(), - Gas::from(wrapper_info.gas_limit) - .checked_sub(Gas::from(wrapper.to_bytes().len() as u64)) - .unwrap(), - ); - } - - if i != 3 { - valid_wrappers.push(wrapper) - } + for i in 0u64..4 { + let (_, mut processed_tx) = mk_wrapper_tx(&shell, &keypair); + processed_tx.result.code = u32::try_from(i.rem_euclid(2)).unwrap(); + processed_txs.push(processed_tx); } // check that the correct events were created @@ -936,209 +840,11 @@ mod test_finalize_block { .expect("Test failed") .iter() .enumerate() - { - assert_eq!(event.event_type.to_string(), String::from("accepted")); - let code = event.attributes.get("code").expect("Test failed"); - assert_eq!(code, &index.rem_euclid(2).to_string()); - } - // verify that the queue of wrapper txs to be processed is correct - let mut valid_tx = valid_wrappers.iter(); - let mut counter = 0; - for wrapper in shell.iter_tx_queue() { - // we cannot easily implement the PartialEq trait for WrapperTx - // so we check the hashes of the inner txs for equality - let valid_tx = valid_tx.next().expect("Test failed"); - assert_eq!(wrapper.tx.header.code_hash, *valid_tx.code_sechash()); - assert_eq!(wrapper.tx.header.data_hash, *valid_tx.data_sechash()); - counter += 1; - } - assert_eq!(counter, 3); - } - - /// Check that if a decrypted tx was rejected by [`process_proposal`], - /// the correct event is returned. Check that it is still - /// removed from the queue of txs to be included in the next block - /// proposal - #[test] - fn test_process_proposal_rejected_decrypted_tx() { - let (mut shell, _, _, _) = setup(); - let keypair = gen_keypair(); - let mut outer_tx = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native( - Default::default(), - ), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - outer_tx.header.chain_id = shell.chain_id.clone(); - outer_tx.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - outer_tx.set_data(Data::new( - String::from("transaction data").as_bytes().to_owned(), - )); - let gas_limit = - Gas::from(outer_tx.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(outer_tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(outer_tx.clone(), gas_limit); - - outer_tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - let processed_tx = ProcessedTx { - tx: outer_tx.to_bytes().into(), - result: TxResult { - code: ResultCode::InvalidTx.into(), - info: "".into(), - }, - }; - - // check that the decrypted tx was not applied - for event in shell - .finalize_block(FinalizeBlock { - txs: vec![processed_tx], - ..Default::default() - }) - .expect("Test failed") { assert_eq!(event.event_type.to_string(), String::from("applied")); let code = event.attributes.get("code").expect("Test failed"); - assert_eq!(code, &String::from(ResultCode::InvalidTx)); - } - // check that the corresponding wrapper tx was removed from the queue - assert!(shell.state.in_mem().tx_queue.is_empty()); - } - - /// Test that if a tx is undecryptable, it is applied - /// but the tx result contains the appropriate error code. - #[test] - fn test_undecryptable_returns_error_code() { - let (mut shell, _, _, _) = setup(); - - let keypair = crate::wallet::defaults::daewon_keypair(); - // not valid tx bytes - let wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native(0.into()), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - let processed_tx = ProcessedTx { - tx: Tx::from_type(TxType::Decrypted(DecryptedTx::Undecryptable)) - .to_bytes() - .into(), - result: TxResult { - code: ResultCode::Ok.into(), - info: "".into(), - }, - }; - - let gas_limit = - Gas::from(wrapper.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(wrapper.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(wrapper, gas_limit); - - // check that correct error message is returned - for event in shell - .finalize_block(FinalizeBlock { - txs: vec![processed_tx], - ..Default::default() - }) - .expect("Test failed") - { - assert_eq!(event.event_type.to_string(), String::from("applied")); - let code = event.attributes.get("code").expect("Test failed"); - assert_eq!(code, &String::from(ResultCode::Undecryptable)); - let log = event.attributes.get("log").expect("Test failed"); - assert!(log.contains("Transaction could not be decrypted.")) - } - // check that the corresponding wrapper tx was removed from the queue - assert!(shell.state.in_mem().tx_queue.is_empty()); - } - - /// Test that the wrapper txs are queued in the order they - /// are received from the block. Tests that the previously - /// decrypted txs are de-queued. - #[test] - fn test_mixed_txs_queued_in_correct_order() { - let (mut shell, _, _, _) = setup(); - let keypair = gen_keypair(); - let mut processed_txs = vec![]; - let mut valid_txs = vec![]; - - // Add unshielded balance for fee payment - let balance_key = token::storage_key::balance_key( - &shell.state.in_mem().native_token, - &Address::from(&keypair.ref_to()), - ); - shell - .state - .write(&balance_key, Amount::native_whole(1000)) - .unwrap(); - - // create two decrypted txs - for _ in 0..2 { - processed_txs.push(mk_decrypted_tx(&mut shell, &keypair)); - } - // create two wrapper txs - for _ in 0..2 { - let (tx, processed_tx) = mk_wrapper_tx(&shell, &keypair); - valid_txs.push(tx.clone()); - processed_txs.push(processed_tx); - } - // Put the wrapper txs in front of the decrypted txs - processed_txs.rotate_left(2); - // check that the correct events were created - for (index, event) in shell - .finalize_block(FinalizeBlock { - txs: processed_txs, - ..Default::default() - }) - .expect("Test failed") - .iter() - .enumerate() - { - if index < 2 { - // these should be accepted wrapper txs - assert_eq!( - event.event_type.to_string(), - String::from("accepted") - ); - let code = - event.attributes.get("code").expect("Test failed").as_str(); - assert_eq!(code, String::from(ResultCode::Ok).as_str()); - } else { - // these should be accepted decrypted txs - assert_eq!( - event.event_type.to_string(), - String::from("applied") - ); - let code = - event.attributes.get("code").expect("Test failed").as_str(); - assert_eq!(code, String::from(ResultCode::Ok).as_str()); - } - } - - // check that the applied decrypted txs were dequeued and the - // accepted wrappers were enqueued in correct order - let mut txs = valid_txs.iter(); - - let mut counter = 0; - for wrapper in shell.iter_tx_queue() { - let next = txs.next().expect("Test failed"); - assert_eq!(wrapper.tx.header.code_hash, *next.code_sechash()); - assert_eq!(wrapper.tx.header.data_hash, *next.data_sechash()); - counter += 1; + assert_eq!(code, &index.rem_euclid(2).to_string()); } - assert_eq!(counter, 2); } /// Test if a rejected protocol tx is applied and emits @@ -1604,10 +1310,6 @@ mod test_finalize_block { for _ in 0..20 { // Add some txs let mut txs = vec![]; - // create two decrypted txs - for _ in 0..2 { - txs.push(mk_decrypted_tx(&mut shell, &txs_key)); - } // create two wrapper txs for _ in 0..2 { let (_tx, processed_tx) = mk_wrapper_tx(&shell, &txs_key); @@ -2703,15 +2405,8 @@ mod test_finalize_block { ..Default::default() }) .expect("Test failed")[0]; - assert_eq!(event.event_type.to_string(), String::from("accepted")); - let code = event - .attributes - .get("code") - .expect( - "Test - failed", - ) - .as_str(); + assert_eq!(event.event_type.to_string(), String::from("applied")); + let code = event.attributes.get("code").expect("Test failed").as_str(); assert_eq!(code, String::from(ResultCode::Ok).as_str()); // the merkle tree root should not change after finalize_block @@ -2724,7 +2419,7 @@ mod test_finalize_block { .shell .state .write_log() - .has_replay_protection_entry(&wrapper_tx.header_hash()) + .has_replay_protection_entry(&wrapper_tx.raw_header_hash()) .unwrap_or_default() ); // Check that the hash is present in the merkle tree @@ -2740,14 +2435,13 @@ mod test_finalize_block { ); } - /// Test that a decrypted tx that has already been applied in the same block + /// Test that a tx that has already been applied in the same block /// doesn't get reapplied #[test] - fn test_duplicated_decrypted_tx_same_block() { + fn test_duplicated_tx_same_block() { let (mut shell, _, _, _) = setup(); - let keypair = gen_keypair(); - let keypair_2 = gen_keypair(); - let mut batch = namada::state::testing::TestState::batch(); + let keypair = crate::wallet::defaults::albert_keypair(); + let keypair_2 = crate::wallet::defaults::bertha_keypair(); let tx_code = TestWasms::TxNoOp.read_bytes(); let mut wrapper = @@ -2763,9 +2457,7 @@ mod test_finalize_block { )))); wrapper.header.chain_id = shell.chain_id.clone(); wrapper.set_code(Code::new(tx_code, None)); - wrapper.set_data(Data::new( - "Decrypted transaction data".as_bytes().to_owned(), - )); + wrapper.set_data(Data::new("transaction data".as_bytes().to_owned())); let mut new_wrapper = wrapper.clone(); new_wrapper.update_header(TxType::Wrapper(Box::new(WrapperTx::new( @@ -2789,26 +2481,10 @@ mod test_finalize_block { None, ))); - let mut inner = wrapper.clone(); - let mut new_inner = new_wrapper.clone(); - - for inner in [&mut inner, &mut new_inner] { - inner.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - } - - // Write wrapper hashes in storage - for tx in [&wrapper, &new_wrapper] { - let hash_subkey = replay_protection::last_key(&tx.header_hash()); - shell - .state - .write_replay_protection_entry(&mut batch, &hash_subkey) - .expect("Test failed"); - } - let mut processed_txs: Vec = vec![]; - for inner in [&inner, &new_inner] { + for tx in [&wrapper, &new_wrapper] { processed_txs.push(ProcessedTx { - tx: inner.to_bytes().into(), + tx: tx.to_bytes().into(), result: TxResult { code: ResultCode::Ok.into(), info: "".into(), @@ -2816,8 +2492,6 @@ mod test_finalize_block { }) } - shell.enqueue_tx(wrapper.clone(), GAS_LIMIT_MULTIPLIER.into()); - shell.enqueue_tx(new_wrapper.clone(), GAS_LIMIT_MULTIPLIER.into()); // merkle tree root before finalize_block let root_pre = shell.shell.state.in_mem().block.tree.root(); @@ -2839,12 +2513,12 @@ mod test_finalize_block { let code = event[1].attributes.get("code").unwrap().as_str(); assert_eq!(code, String::from(ResultCode::WasmRuntimeError).as_str()); - for (inner, wrapper) in [(inner, wrapper), (new_inner, new_wrapper)] { + for wrapper in [&wrapper, &new_wrapper] { assert!( shell .state .write_log() - .has_replay_protection_entry(&inner.raw_header_hash()) + .has_replay_protection_entry(&wrapper.raw_header_hash()) .unwrap_or_default() ); assert!( @@ -2857,23 +2531,48 @@ mod test_finalize_block { } } - /// Test that if a decrypted transaction fails because of out-of-gas, - /// undecryptable, invalid signature or wrong section commitment, its hash + /// Test that if a transaction fails because of out-of-gas, + /// invalid signature or wrong section commitment, its hash /// is not committed to storage. Also checks that a tx failing for other /// reason has its hash written to storage. #[test] fn test_tx_hash_handling() { let (mut shell, _, _, _) = setup(); - let keypair = gen_keypair(); - let mut batch = namada::state::testing::TestState::batch(); + let keypair = crate::wallet::defaults::bertha_keypair(); + let mut out_of_gas_wrapper = { + let tx_code = TestWasms::TxNoOp.read_bytes(); + let mut wrapper_tx = + Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( + Fee { + amount_per_gas_unit: DenominatedAmount::native( + 1.into(), + ), + token: shell.state.in_mem().native_token.clone(), + }, + keypair.ref_to(), + Epoch(0), + 0.into(), + None, + )))); + wrapper_tx.header.chain_id = shell.chain_id.clone(); + wrapper_tx.set_data(Data::new( + "Encrypted transaction data".as_bytes().to_owned(), + )); + wrapper_tx.set_code(Code::new(tx_code, None)); + wrapper_tx.add_section(Section::Signature(Signature::new( + wrapper_tx.sechashes(), + [(0, keypair.clone())].into_iter().collect(), + None, + ))); + 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 { @@ -2888,7 +2587,9 @@ mod test_finalize_block { None, )))); unsigned_wrapper.header.chain_id = shell.chain_id.clone(); + let mut failing_wrapper = unsigned_wrapper.clone(); + unsigned_wrapper.set_code(Code::new(tx_code, None)); let addr = Address::from(&keypair.to_public()); let key = Key::from(addr.to_db_key()) @@ -2900,6 +2601,7 @@ mod test_finalize_block { }) .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) @@ -2908,55 +2610,38 @@ mod test_finalize_block { failing_wrapper.set_data(Data::new( "Encrypted transaction data".as_bytes().to_owned(), )); - 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 = failing_wrapper.clone(); + let mut wrong_commitment_wrapper = failing_wrapper.clone(); + let tx_code = TestWasms::TxInvalidData.read_bytes(); + wrong_commitment_wrapper.set_code(Code::new(tx_code, None)); + wrong_commitment_wrapper + .sections + .retain(|sec| !matches!(sec, Section::Data(_))); // Add some extra data to avoid having the same Tx hash as the // `failing_wrapper` - wrong_commitment_inner.add_memo(&[0_u8]); - 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)); - } + wrong_commitment_wrapper.add_memo(&[0_u8]); - // Write wrapper hashes in storage - for wrapper in [ - &out_of_gas_wrapper, - &undecryptable_wrapper, - &unsigned_wrapper, - &wrong_commitment_wrapper, - &failing_wrapper, + let mut processed_txs: Vec = vec![]; + for tx in [ + &mut out_of_gas_wrapper, + &mut wrong_commitment_wrapper, + &mut failing_wrapper, ] { - let hash_subkey = - replay_protection::last_key(&wrapper.header_hash()); - shell - .state - .write_replay_protection_entry(&mut batch, &hash_subkey) - .unwrap(); + tx.sign_raw( + vec![keypair.clone()], + vec![keypair.ref_to()].into_iter().collect(), + None, + ); } - - let mut processed_txs: Vec = vec![]; - for inner in [ - &out_of_gas_inner, - &undecryptable_inner, - &unsigned_inner, - &wrong_commitment_inner, - &failing_inner, + for tx in [ + &mut out_of_gas_wrapper, + &mut unsigned_wrapper, + &mut wrong_commitment_wrapper, + &mut failing_wrapper, ] { + tx.sign_wrapper(keypair.clone()); processed_txs.push(ProcessedTx { - tx: inner.to_bytes().into(), + tx: tx.to_bytes().into(), result: TxResult { code: ResultCode::Ok.into(), info: "".into(), @@ -2964,17 +2649,6 @@ mod test_finalize_block { }) } - 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.state.in_mem().block.tree.root(); @@ -2991,38 +2665,35 @@ mod test_finalize_block { 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(ResultCode::WasmRuntimeError).as_str()); + assert_eq!(code, String::from(ResultCode::InvalidTx).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(ResultCode::Undecryptable).as_str()); + assert_eq!(code, String::from(ResultCode::InvalidTx).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(ResultCode::InvalidTx).as_str()); + assert_eq!(code, String::from(ResultCode::WasmRuntimeError).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(ResultCode::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(ResultCode::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), + for valid_wrapper in [ + out_of_gas_wrapper, + unsigned_wrapper, + wrong_commitment_wrapper, ] { assert!( !shell .state .write_log() .has_replay_protection_entry( - &invalid_inner.raw_header_hash() + &valid_wrapper.raw_header_hash() ) .unwrap_or_default() ); assert!( shell .state + .write_log() .has_replay_protection_entry(&valid_wrapper.header_hash()) .unwrap_or_default() ); @@ -3031,7 +2702,7 @@ mod test_finalize_block { shell .state .write_log() - .has_replay_protection_entry(&failing_inner.raw_header_hash()) + .has_replay_protection_entry(&failing_wrapper.raw_header_hash()) .expect("test failed") ); assert!( @@ -3098,7 +2769,7 @@ mod test_finalize_block { let root_post = shell.shell.state.in_mem().block.tree.root(); assert_eq!(root_pre.0, root_post.0); - assert_eq!(event[0].event_type.to_string(), String::from("accepted")); + assert_eq!(event[0].event_type.to_string(), String::from("applied")); let code = event[0] .attributes .get("code") @@ -3168,8 +2839,8 @@ mod test_finalize_block { .expect("Test failed")[0]; // Check balance of fee payer is 0 - assert_eq!(event.event_type.to_string(), String::from("accepted")); - let code = event.attributes.get("code").expect("Testfailed").as_str(); + assert_eq!(event.event_type.to_string(), String::from("applied")); + let code = event.attributes.get("code").expect("Test failed").as_str(); assert_eq!(code, String::from(ResultCode::InvalidTx).as_str()); let balance_key = token::storage_key::balance_key( &shell.state.in_mem().native_token, @@ -3226,9 +2897,7 @@ mod test_finalize_block { )))); wrapper.header.chain_id = shell.chain_id.clone(); wrapper.set_code(Code::new(tx_code, None)); - wrapper.set_data(Data::new( - "Enxrypted transaction data".as_bytes().to_owned(), - )); + wrapper.set_data(Data::new("Transaction data".as_bytes().to_owned())); wrapper.add_section(Section::Signature(Signature::new( wrapper.sechashes(), [(0, crate::wallet::defaults::albert_keypair())] @@ -3269,7 +2938,7 @@ mod test_finalize_block { .expect("Test failed")[0]; // Check fee payment - assert_eq!(event.event_type.to_string(), String::from("accepted")); + assert_eq!(event.event_type.to_string(), String::from("applied")); let code = event.attributes.get("code").expect("Test failed").as_str(); assert_eq!(code, String::from(ResultCode::Ok).as_str()); diff --git a/crates/apps/src/lib/node/ledger/shell/governance.rs b/crates/apps/src/lib/node/ledger/shell/governance.rs index e568f5e212..5a3a619295 100644 --- a/crates/apps/src/lib/node/ledger/shell/governance.rs +++ b/crates/apps/src/lib/node/ledger/shell/governance.rs @@ -299,7 +299,7 @@ where let pending_execution_key = gov_storage::get_proposal_execution_key(id); shell.state.write(&pending_execution_key, ())?; - let mut tx = Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); + let mut tx = Tx::from_type(TxType::Raw); tx.header.chain_id = shell.chain_id.clone(); tx.set_data(Data::new(encode(&id))); tx.set_code(Code::new(code, None)); diff --git a/crates/apps/src/lib/node/ledger/shell/mod.rs b/crates/apps/src/lib/node/ledger/shell/mod.rs index e144794e2c..c725948068 100644 --- a/crates/apps/src/lib/node/ledger/shell/mod.rs +++ b/crates/apps/src/lib/node/ledger/shell/mod.rs @@ -10,6 +10,7 @@ mod finalize_block; mod governance; mod init_chain; pub use init_chain::InitChainValidation; +use namada::vm::wasm::run::check_tx_allowed; use namada_sdk::state::StateRead; use namada_sdk::tx::data::GasLimit; pub mod prepare_proposal; @@ -55,14 +56,14 @@ use namada::ledger::protocol::{ use namada::ledger::{parameters, protocol}; use namada::parameters::validate_tx_bytes; use namada::proof_of_stake::storage::read_pos_params; -use namada::state::tx_queue::{ExpiredTx, TxInQueue}; +use namada::state::tx_queue::ExpiredTx; use namada::state::{ DBIter, FullAccessState, Sha256Hasher, StorageHasher, StorageRead, TempWlState, WlState, DB, EPOCH_SWITCH_BLOCKS_DELAY, }; use namada::token; pub use namada::tx::data::ResultCode; -use namada::tx::data::{DecryptedTx, TxType, WrapperTx, WrapperTxErr}; +use namada::tx::data::{TxType, WrapperTx, WrapperTxErr}; use namada::tx::{Section, Tx}; use namada::vm::wasm::{TxCache, VpCache}; use namada::vm::{WasmCacheAccess, WasmCacheRwAccess}; @@ -542,12 +543,6 @@ where &mut self.event_log } - /// Iterate over the wrapper txs in order - #[allow(dead_code)] - fn iter_tx_queue(&mut self) -> impl Iterator { - self.state.in_mem().tx_queue.iter() - } - /// Load the Merkle root hash and the height of the last committed block, if /// any. This is returned when ABCI sends an `info` request. pub fn last_state(&mut self) -> response::Info { @@ -1042,11 +1037,22 @@ where } }, TxType::Wrapper(wrapper) => { + // Tx allowlist + if let Err(err) = check_tx_allowed(&tx, &self.state) { + response.code = ResultCode::TxNotAllowlisted.into(); + response.log = format!( + "{INVALID_MSG}: Wrapper transaction code didn't pass \ + the allowlist checks {}", + err + ); + return response; + } + // Tx gas limit let mut gas_meter = TxGasMeter::new(wrapper.gas_limit); if gas_meter.add_wrapper_gas(tx_bytes).is_err() { response.code = ResultCode::TxGasLimit.into(); - response.log = "{INVALID_MSG}: Wrapper transactions \ + response.log = "{INVALID_MSG}: Wrapper transaction \ exceeds its gas limit" .to_string(); return response; @@ -1115,12 +1121,6 @@ where the mempool" ); } - TxType::Decrypted(_) => { - response.code = ResultCode::InvalidTx.into(); - response.log = format!( - "{INVALID_MSG}: Decrypted txs cannot be sent by clients" - ); - } } if response.code == ResultCode::Ok.into() { @@ -1407,16 +1407,11 @@ mod test_utils { use namada::core::keccak::KeccakHash; use namada::core::key::*; use namada::core::storage::{BlockHash, Epoch, Header}; - use namada::core::time::DurationSecs; - use namada::ledger::parameters::{EpochDuration, Parameters}; use namada::proof_of_stake::parameters::PosParams; use namada::proof_of_stake::storage::validator_consensus_key_handle; use namada::state::mockdb::MockDB; use namada::state::{LastBlock, StorageWrite}; use namada::tendermint::abci::types::VoteInfo; - use namada::token::conversion::update_allowed_conversions; - use namada::tx::data::Fee; - use namada::tx::{Code, Data}; use tempfile::tempdir; use tokio::sync::mpsc::{Sender, UnboundedReceiver}; @@ -1427,12 +1422,10 @@ mod test_utils { use crate::facade::tendermint_proto::v0_37::abci::{ RequestPrepareProposal, RequestProcessProposal, }; - use crate::node::ledger::shell::token::DenominatedAmount; use crate::node::ledger::shims::abcipp_shim_types; use crate::node::ledger::shims::abcipp_shim_types::shim::request::{ FinalizeBlock, ProcessedTx, }; - use crate::node::ledger::storage::{PersistentDB, PersistentStorageHasher}; #[derive(Error, Debug)] pub enum TestError { @@ -1700,17 +1693,6 @@ mod test_utils { self.shell.prepare_proposal(req) } - /// Add a wrapper tx to the queue of txs to be decrypted - /// in the current block proposal. Takes the length of the encoded - /// wrapper as parameter. - #[cfg(test)] - pub fn enqueue_tx(&mut self, tx: Tx, inner_tx_gas: Gas) { - self.shell.state.in_mem_mut().tx_queue.push(TxInQueue { - tx, - gas: inner_tx_gas, - }); - } - /// Start a counter for the next epoch in `num_blocks`. pub fn start_new_epoch_in(&mut self, num_blocks: u64) { self.state.in_mem_mut().next_epoch_min_start_height = @@ -1898,130 +1880,6 @@ mod test_utils { .expect("Test failed"); } - /// We test that on shell shutdown, the tx queue gets persisted in a DB, and - /// on startup it is read successfully - #[test] - fn test_tx_queue_persistence() { - let base_dir = tempdir().unwrap().as_ref().canonicalize().unwrap(); - // we have to use RocksDB for this test - let (sender, _) = tokio::sync::mpsc::unbounded_channel(); - let (_, eth_receiver) = - tokio::sync::mpsc::channel(ORACLE_CHANNEL_BUFFER_SIZE); - let (control_sender, _) = oracle::control::channel(); - let (_, last_processed_block_receiver) = - last_processed_block::channel(); - let eth_oracle = EthereumOracleChannels::new( - eth_receiver, - control_sender, - last_processed_block_receiver, - ); - let vp_wasm_compilation_cache = 50 * 1024 * 1024; // 50 kiB - let tx_wasm_compilation_cache = 50 * 1024 * 1024; // 50 kiB - let native_token = address::testing::nam(); - let mut shell = Shell::::new( - config::Ledger::new( - base_dir.clone(), - Default::default(), - TendermintMode::Validator, - ), - top_level_directory().join("wasm"), - sender.clone(), - Some(eth_oracle), - None, - vp_wasm_compilation_cache, - tx_wasm_compilation_cache, - ); - shell - .state - .in_mem_mut() - .begin_block(BlockHash::default(), BlockHeight(1)) - .expect("begin_block failed"); - let keypair = gen_keypair(); - // enqueue a wrapper tx - let mut wrapper = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native(0.into()), - token: native_token, - }, - keypair.ref_to(), - Epoch(0), - 300_000.into(), - None, - )))); - wrapper.header.chain_id = shell.chain_id.clone(); - wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - wrapper.set_data(Data::new("transaction data".as_bytes().to_owned())); - - shell.state.in_mem_mut().tx_queue.push(TxInQueue { - tx: wrapper, - gas: u64::MAX.into(), - }); - // Artificially increase the block height so that chain - // will read the new block when restarted - shell - .state - .in_mem_mut() - .block - .pred_epochs - .new_epoch(BlockHeight(1)); - // initialize parameter storage - let params = Parameters { - max_tx_bytes: 1024 * 1024, - epoch_duration: EpochDuration { - min_num_of_blocks: 1, - min_duration: DurationSecs(3600), - }, - max_expected_time_per_block: DurationSecs(3600), - max_proposal_bytes: Default::default(), - max_block_gas: 100, - vp_allowlist: vec![], - tx_allowlist: vec![], - implicit_vp_code_hash: Default::default(), - epochs_per_year: 365, - max_signatures_per_transaction: 10, - staked_ratio: Default::default(), - pos_inflation_amount: Default::default(), - fee_unshielding_gas_limit: 0, - fee_unshielding_descriptions_limit: 0, - minimum_gas_price: Default::default(), - }; - parameters::init_storage(¶ms, &mut shell.state) - .expect("Test failed"); - // make state to update conversion for a new epoch - update_allowed_conversions(&mut shell.state) - .expect("update conversions failed"); - shell.state.commit_block().expect("commit failed"); - - // Drop the shell - std::mem::drop(shell); - let (_, eth_receiver) = - tokio::sync::mpsc::channel(ORACLE_CHANNEL_BUFFER_SIZE); - let (control_sender, _) = oracle::control::channel(); - let (_, last_processed_block_receiver) = - last_processed_block::channel(); - let eth_oracle = EthereumOracleChannels::new( - eth_receiver, - control_sender, - last_processed_block_receiver, - ); - // Reboot the shell and check that the queue was restored from DB - let shell = Shell::::new( - config::Ledger::new( - base_dir, - Default::default(), - TendermintMode::Validator, - ), - top_level_directory().join("wasm"), - sender, - Some(eth_oracle), - None, - vp_wasm_compilation_cache, - tx_wasm_compilation_cache, - ); - assert!(!shell.state.in_mem().tx_queue.is_empty()); - } - pub(super) fn get_pkh_from_address( storage: &S, params: &PosParams, diff --git a/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs index b08c2bec59..d4a28740fd 100644 --- a/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -2,22 +2,20 @@ use masp_primitives::transaction::Transaction; use namada::core::address::Address; -use namada::core::hints; use namada::core::key::tm_raw_hash_to_string; use namada::gas::TxGasMeter; use namada::ledger::protocol; -use namada::ledger::storage::tx_queue::TxInQueue; use namada::proof_of_stake::storage::find_validator_by_raw_hash; use namada::state::{DBIter, StorageHasher, TempWlState, DB}; -use namada::tx::data::{DecryptedTx, TxType, WrapperTx}; +use namada::tx::data::{TxType, WrapperTx}; use namada::tx::Tx; use namada::vm::wasm::{TxCache, VpCache}; use namada::vm::WasmCacheAccess; use super::super::*; use super::block_alloc::states::{ - BuildingDecryptedTxBatch, BuildingProtocolTxBatch, - EncryptedTxBatchAllocator, NextState, TryAlloc, + BuildingNormalTxBatch, BuildingProtocolTxBatch, NextState, TryAlloc, + WithNormalTxs, WithoutNormalTxs, }; use super::block_alloc::{AllocFailure, BlockAllocator, BlockResources}; use crate::config::ValidatorLocalConfig; @@ -38,17 +36,21 @@ where /// /// INVARIANT: Any changes applied in this method must be reverted if /// the proposal is rejected (unless we can simply overwrite - /// them in the next block). + /// them in the next block). Furthermore, protocol transactions cannot + /// affect the ability of a tx to pay its wrapper fees. pub fn prepare_proposal( &self, - req: RequestPrepareProposal, + mut req: RequestPrepareProposal, ) -> response::PrepareProposal { let txs = if let ShellMode::Validator { ref local_config, .. } = self.mode { // start counting allotted space for txs - let alloc = self.get_encrypted_txs_allocator(); + let alloc = self.get_protocol_txs_allocator(); + // add initial protocol txs + let (alloc, mut txs) = + self.build_protocol_tx_with_normal_txs(alloc, &mut req.txs); // add encrypted txs let tm_raw_hash_string = @@ -60,22 +62,17 @@ where "Unable to find native validator address of block \ proposer from tendermint raw hash", ); - let (encrypted_txs, alloc) = self.build_encrypted_txs( + let (mut normal_txs, alloc) = self.build_normal_txs( alloc, &req.txs, req.time, &block_proposer, local_config.as_ref(), ); - let mut txs = encrypted_txs; - // decrypt the wrapper txs included in the previous block - let (mut decrypted_txs, alloc) = self.build_decrypted_txs(alloc); - txs.append(&mut decrypted_txs); - - // add vote extension protocol txs - let mut protocol_txs = self.build_protocol_txs(alloc, &req.txs); - txs.append(&mut protocol_txs); - + txs.append(&mut normal_txs); + let mut remaining_txs = + self.build_protocol_tx_without_normal_txs(alloc, &mut req.txs); + txs.append(&mut remaining_txs); txs } else { vec![] @@ -90,47 +87,28 @@ where response::PrepareProposal { txs } } - /// Depending on the current block height offset within the epoch, - /// transition state accordingly, return a block space allocator - /// with or without encrypted txs. - /// - /// # How to determine which path to take in the states DAG - /// - /// If we are at the second or third block height offset within an - /// epoch, we do not allow encrypted transactions to be included in - /// a block, therefore we return an allocator wrapped in an - /// [`EncryptedTxBatchAllocator::WithoutEncryptedTxs`] value. - /// Otherwise, we return an allocator wrapped in an - /// [`EncryptedTxBatchAllocator::WithEncryptedTxs`] value. + /// Get the first state of the block allocator. This is for protocol + /// transactions. #[inline] - fn get_encrypted_txs_allocator(&self) -> EncryptedTxBatchAllocator { - let is_2nd_height_off = self.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = self.is_deciding_offset_within_epoch(2); - - if hints::unlikely(is_2nd_height_off || is_3rd_height_off) { - tracing::warn!( - proposal_height = - ?self.state.in_mem().block.height, - "No mempool txs are being included in the current proposal" - ); - EncryptedTxBatchAllocator::WithoutEncryptedTxs( - (&*self.state).into(), - ) - } else { - EncryptedTxBatchAllocator::WithEncryptedTxs((&*self.state).into()) - } + fn get_protocol_txs_allocator( + &self, + ) -> BlockAllocator> { + self.state.read_only().into() } /// Builds a batch of encrypted transactions, retrieved from - /// Tendermint's mempool. - fn build_encrypted_txs( + /// CometBFT's mempool. + fn build_normal_txs( &self, - mut alloc: EncryptedTxBatchAllocator, + mut alloc: BlockAllocator, txs: &[TxBytes], block_time: Option, block_proposer: &Address, proposer_local_config: Option<&ValidatorLocalConfig>, - ) -> (Vec, BlockAllocator) { + ) -> ( + Vec, + BlockAllocator>, + ) { let block_time = block_time.and_then(|block_time| { // If error in conversion, default to last block datetime, it's // valid because of mempool check @@ -191,85 +169,46 @@ where (txs, alloc) } - /// Builds a batch of DKG decrypted transactions. - // NOTE: we won't have frontrunning protection until V2 of the - // Anoma protocol; Namada runs V1, therefore this method is - // essentially a NOOP - // - // sources: - // - https://specs.namada.net/main/releases/v2.html - // - https://github.com/anoma/ferveo - fn build_decrypted_txs( + /// Allocate an initial set of protocol txs and advance to the + /// next allocation state. + fn build_protocol_tx_with_normal_txs( &self, - mut alloc: BlockAllocator, - ) -> (Vec, BlockAllocator) { - let txs = self - .state - .in_mem() - .tx_queue - .iter() - .map( - |TxInQueue { - tx, - gas: _, - }| { - let mut tx = tx.clone(); - tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - tx.to_bytes().into() - }, - ) - // TODO: make sure all decrypted txs are accepted - .take_while(|tx_bytes: &TxBytes| { - alloc.try_alloc(&tx_bytes[..]).map_or_else( - |status| match status { - AllocFailure::Rejected { bin_resource_left: bin_space_left } => { - tracing::warn!( - ?tx_bytes, - bin_space_left, - proposal_height = - ?self.get_current_decision_height(), - "Dropping decrypted tx from the current proposal", - ); - false - } - AllocFailure::OverflowsBin { bin_resource: bin_size } => { - tracing::warn!( - ?tx_bytes, - bin_size, - proposal_height = - ?self.get_current_decision_height(), - "Dropping large decrypted tx from the current proposal", - ); - true - } - }, - |()| true, - ) - }) - .collect(); - let alloc = alloc.next_state(); + alloc: BlockAllocator>, + txs: &mut Vec, + ) -> (BlockAllocator, Vec) { + let (alloc, txs) = self.build_protocol_txs(alloc, txs); + (alloc.next_state(), txs) + } - (txs, alloc) + /// Allocate protocol txs into any remaining space. After this, no + /// more allocation will take place. + fn build_protocol_tx_without_normal_txs( + &self, + alloc: BlockAllocator>, + txs: &mut Vec, + ) -> Vec { + let (_, txs) = self.build_protocol_txs(alloc, txs); + txs } /// Builds a batch of protocol transactions. - fn build_protocol_txs( + fn build_protocol_txs( &self, - mut alloc: BlockAllocator, - txs: &[TxBytes], - ) -> Vec { + mut alloc: BlockAllocator>, + txs: &mut Vec, + ) -> (BlockAllocator>, Vec) { if self.state.in_mem().last_block.is_none() { // genesis should not contain vote extensions. // // this is because we have not decided any block through // consensus yet (hence height 0), which in turn means we // have not committed any vote extensions to a block either. - return vec![]; + return (alloc, vec![]); } - let deserialized_iter = self.deserialize_vote_extensions(txs); + let mut deserialized_iter = self.deserialize_vote_extensions(txs); - deserialized_iter.take_while(|tx_bytes| + let taken = deserialized_iter.by_ref().take_while(|tx_bytes| alloc.try_alloc(&tx_bytes[..]) .map_or_else( |status| match status { @@ -307,7 +246,10 @@ where |()| true, ) ) - .collect() + .collect(); + // avoid dropping the txs that couldn't be included in the block + deserialized_iter.keep_rest(); + (alloc, taken) } } @@ -424,12 +366,10 @@ where mod test_prepare_proposal { use std::collections::BTreeSet; - use borsh_ext::BorshSerializeExt; use namada::core::address; use namada::core::ethereum_events::EthereumEvent; use namada::core::key::RefTo; use namada::core::storage::{BlockHeight, InnerEthEventsQueue}; - use namada::ledger::gas::Gas; use namada::ledger::pos::PosQueries; use namada::proof_of_stake::storage::{ consensus_validator_set_handle, @@ -440,7 +380,7 @@ mod test_prepare_proposal { use namada::state::collections::lazy_map::{NestedSubKey, SubKey}; use namada::token::{read_denom, Amount, DenominatedAmount}; use namada::tx::data::Fee; - use namada::tx::{Code, Data, Header, Section, Signature, Signed}; + use namada::tx::{Code, Data, Section, Signature, Signed}; use namada::vote_ext::{ethereum_events, ethereum_tx_data_variants}; use namada::{replay_protection, token}; use namada_sdk::storage::StorageWrite; @@ -476,7 +416,7 @@ mod test_prepare_proposal { #[test] fn test_prepare_proposal_rejects_non_wrapper_tx() { let (shell, _recv, _, _) = test_utils::setup(); - let mut tx = Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); + let mut tx = Tx::from_type(TxType::Raw); tx.header.chain_id = shell.chain_id.clone(); let req = RequestPrepareProposal { txs: vec![tx.to_bytes().into()], @@ -764,100 +704,6 @@ mod test_prepare_proposal { assert_eq!(signed_eth_ev_vote_extension, rsp_ext.0); } - /// Test that the decrypted txs are included - /// in the proposal in the same order as their - /// corresponding wrappers - #[test] - fn test_decrypted_txs_in_correct_order() { - let (mut shell, _recv, _, _) = test_utils::setup(); - let keypair = gen_keypair(); - let mut expected_wrapper = vec![]; - let mut expected_decrypted = vec![]; - - // Load some tokens to tx signer to pay fees - let balance_key = token::storage_key::balance_key( - &shell.state.in_mem().native_token, - &Address::from(&keypair.ref_to()), - ); - shell - .state - .db_write( - &balance_key, - Amount::native_whole(1_000).serialize_to_vec(), - ) - .unwrap(); - - let mut req = RequestPrepareProposal { - txs: vec![], - ..Default::default() - }; - // create a request with two new wrappers from mempool and - // two wrappers from the previous block to be decrypted - for i in 0..2 { - let mut tx = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native( - 1.into(), - ), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - tx.header.chain_id = shell.chain_id.clone(); - tx.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - tx.set_data(Data::new( - format!("transaction data: {}", i).as_bytes().to_owned(), - )); - tx.add_section(Section::Signature(Signature::new( - tx.sechashes(), - [(0, keypair.clone())].into_iter().collect(), - None, - ))); - - let gas = Gas::from( - tx.header().wrapper().expect("Wrong tx type").gas_limit, - ) - .checked_sub(Gas::from(tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(tx.clone(), gas); - expected_wrapper.push(tx.clone()); - req.txs.push(tx.to_bytes().into()); - tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - expected_decrypted.push(tx.clone()); - } - // we extract the inner data from the txs for testing - // equality since otherwise changes in timestamps would - // fail the test - let expected_txs: Vec
= expected_wrapper - .into_iter() - .chain(expected_decrypted) - .map(|tx| tx.header) - .collect(); - let received: Vec
= shell - .prepare_proposal(req) - .txs - .into_iter() - .map(|tx_bytes| { - Tx::try_from(tx_bytes.as_ref()).expect("Test failed").header - }) - .collect(); - // check that the order of the txs is correct - assert_eq!( - received - .iter() - .map(|x| x.serialize_to_vec()) - .collect::>(), - expected_txs - .iter() - .map(|x| x.serialize_to_vec()) - .collect::>(), - ); - } - /// Test that if the unsigned wrapper tx hash is known (replay attack), the /// transaction is not included in the block #[test] diff --git a/crates/apps/src/lib/node/ledger/shell/process_proposal.rs b/crates/apps/src/lib/node/ledger/shell/process_proposal.rs index 73648c7207..5b25856ee8 100644 --- a/crates/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/crates/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -7,7 +7,7 @@ use namada::proof_of_stake::storage::find_validator_by_raw_hash; use namada::tx::data::protocol::ProtocolTxType; use namada::vote_ext::ethereum_tx_data_variants; -use super::block_alloc::{BlockSpace, EncryptedTxsBins}; +use super::block_alloc::{BlockGas, BlockSpace}; use super::*; use crate::facade::tendermint_proto::v0_37::abci::RequestProcessProposal; use crate::node::ledger::shell::block_alloc::{AllocFailure, TxBin}; @@ -18,19 +18,10 @@ use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; /// transaction numbers, in a block proposal. #[derive(Default)] pub struct ValidationMeta { - /// Space and gas utilized by encrypted txs. - pub encrypted_txs_bins: EncryptedTxsBins, - /// Vote extension digest counters. + /// Gas emitted by users. + pub user_gas: TxBin, /// Space utilized by all txs. pub txs_bin: TxBin, - /// Check if the decrypted tx queue has any elements - /// left. - /// - /// This field will only evaluate to true if a block - /// proposer didn't include all decrypted txs in a block. - pub decrypted_queue_has_remaining_txs: bool, - /// Check if a block has decrypted txs. - pub has_decrypted_txs: bool, } impl From<&WlState> for ValidationMeta @@ -43,15 +34,10 @@ where state.pos_queries().get_max_proposal_bytes().get(); let max_block_gas = namada::parameters::get_max_block_gas(state).unwrap(); - let encrypted_txs_bin = - EncryptedTxsBins::new(max_proposal_bytes, max_block_gas); + + let user_gas = TxBin::init(max_block_gas); let txs_bin = TxBin::init(max_proposal_bytes); - Self { - decrypted_queue_has_remaining_txs: false, - has_decrypted_txs: false, - encrypted_txs_bins: encrypted_txs_bin, - txs_bin, - } + Self { user_gas, txs_bin } } } @@ -94,7 +80,7 @@ where ) }; - let (tx_results, meta) = self.process_txs( + let tx_results = self.process_txs( &req.txs, req.time .expect("Missing timestamp in proposed block") @@ -104,9 +90,8 @@ where ); // Erroneous transactions were detected when processing - // the leader's proposal. We allow txs that do not - // deserialize properly, that have invalid signatures - // and that have invalid wasm code to reach FinalizeBlock. + // the leader's proposal. We allow txs that are invalid at runtime + // (wasm) to reach FinalizeBlock. let invalid_txs = tx_results.iter().any(|res| { let error = ResultCode::from_u32(res.code).expect( "All error codes returned from process_single_tx are valid", @@ -121,22 +106,8 @@ where "Found invalid transactions, proposed block will be rejected" ); } - - let has_remaining_decrypted_txs = - meta.decrypted_queue_has_remaining_txs; - if has_remaining_decrypted_txs { - tracing::warn!( - proposer = ?HEXUPPER.encode(&req.proposer_address), - height = req.height, - hash = ?HEXUPPER.encode(&req.hash), - "Not all decrypted txs from the previous height were included in - the proposal, the block will be rejected" - ); - } - - let will_reject_proposal = invalid_txs || has_remaining_decrypted_txs; ( - if will_reject_proposal { + if invalid_txs { ProcessProposal::Reject } else { ProcessProposal::Accept @@ -157,8 +128,7 @@ where txs: &[TxBytes], block_time: DateTimeUtc, block_proposer: &Address, - ) -> (Vec, ValidationMeta) { - let mut tx_queue_iter = self.state.in_mem().tx_queue.iter(); + ) -> Vec { let mut temp_state = self.state.with_temp_write_log(); let mut metadata = ValidationMeta::from(self.state.read_only()); let mut vp_wasm_cache = self.vp_wasm_cache.clone(); @@ -169,7 +139,6 @@ where .map(|tx_bytes| { let result = self.check_proposal_tx( tx_bytes, - &mut tx_queue_iter, &mut metadata, &mut temp_state, block_time, @@ -192,10 +161,7 @@ where result }) .collect(); - metadata.decrypted_queue_has_remaining_txs = - !self.state.in_mem().tx_queue.is_empty() - && tx_queue_iter.next().is_some(); - (tx_results, metadata) + tx_results } /// Checks if the Tx can be deserialized from bytes. Checks the fees and @@ -221,10 +187,9 @@ where /// proposal is rejected (unless we can simply overwrite them in the /// next block). #[allow(clippy::too_many_arguments)] - pub fn check_proposal_tx<'a, CA>( + pub fn check_proposal_tx( &self, tx_bytes: &[u8], - tx_queue_iter: &mut impl Iterator, metadata: &mut ValidationMeta, temp_state: &mut TempWlState, block_time: DateTimeUtc, @@ -437,75 +402,14 @@ where }, } } - TxType::Decrypted(tx_header) => { - metadata.has_decrypted_txs = true; - match tx_queue_iter.next() { - Some(wrapper) => { - if wrapper.tx.raw_header_hash() != tx.raw_header_hash() - { - TxResult { - code: ResultCode::InvalidOrder.into(), - info: "Process proposal rejected a decrypted \ - transaction that violated the tx order \ - determined in the previous block" - .into(), - } - } else if matches!( - tx_header, - DecryptedTx::Undecryptable - ) { - // DKG is disabled, txs are not actually encrypted - TxResult { - code: ResultCode::InvalidTx.into(), - info: "The encrypted payload of tx was \ - incorrectly marked as un-decryptable" - .into(), - } - } else { - match tx.header().expiration { - Some(tx_expiration) - if block_time > tx_expiration => - { - TxResult { - code: ResultCode::ExpiredDecryptedTx - .into(), - info: format!( - "Tx expired at {:#?}, block time: \ - {:#?}", - tx_expiration, block_time - ), - } - } - _ => TxResult { - code: ResultCode::Ok.into(), - info: "Process Proposal accepted this \ - transaction" - .into(), - }, - } - } - } - None => TxResult { - code: ResultCode::ExtraTxs.into(), - info: "Received more decrypted txs than expected" - .into(), - }, - } - } TxType::Wrapper(wrapper) => { - // Account for gas and space. This is done even if the - // transaction is later deemed invalid, to - // incentivize the proposer to include only - // valid transaction and avoid wasting block - // resources (ABCI only) + // Account for the tx's resources + let allocated_gas = + metadata.user_gas.try_dump(u64::from(wrapper.gas_limit)); let mut tx_gas_meter = TxGasMeter::new(wrapper.gas_limit); - if tx_gas_meter.add_wrapper_gas(tx_bytes).is_err() { - // Account for the tx's resources even in case of an error. - // Ignore any allocation error - let _ = metadata - .encrypted_txs_bins - .try_dump(tx_bytes, u64::from(wrapper.gas_limit)); - + if tx_gas_meter.add_wrapper_gas(tx_bytes).is_err() + || allocated_gas.is_err() + { return TxResult { code: ResultCode::TxGasLimit.into(), info: "Wrapper transactions exceeds its gas limit" @@ -513,31 +417,14 @@ where }; } - // try to allocate space and gas for this encrypted tx - if let Err(e) = metadata - .encrypted_txs_bins - .try_dump(tx_bytes, u64::from(wrapper.gas_limit)) - { - return TxResult { - code: ResultCode::AllocationError.into(), - info: e, - }; - } - // decrypted txs shouldn't show up before wrapper txs - if metadata.has_decrypted_txs { - return TxResult { - code: ResultCode::InvalidTx.into(), - info: "Decrypted txs should not be proposed before \ - wrapper txs" - .into(), - }; - } - if hints::unlikely(self.encrypted_txs_not_allowed()) { + // Tx allowlist + if let Err(err) = check_tx_allowed(&tx, &self.state) { return TxResult { - code: ResultCode::AllocationError.into(), - info: "Wrapper txs not allowed at the current block \ - height" - .into(), + code: ResultCode::TxNotAllowlisted.into(), + info: format!( + "Tx code didn't pass the allowlist check: {}", + err + ), }; } @@ -604,14 +491,6 @@ where ) -> shim::response::RevertProposal { Default::default() } - - /// Checks if it is not possible to include encrypted txs at the current - /// block height. - pub(super) fn encrypted_txs_not_allowed(&self) -> bool { - let is_2nd_height_off = self.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = self.is_deciding_offset_within_epoch(2); - is_2nd_height_off || is_3rd_height_off - } } fn process_proposal_fee_check( @@ -1134,190 +1013,6 @@ mod test_process_proposal { ); } - /// Test that if the expected order of decrypted txs is - /// validated, [`process_proposal`] rejects it - #[test] - fn test_decrypted_txs_out_of_order() { - let (mut shell, _recv, _, _) = test_utils::setup_at_height(3u64); - let keypair = gen_keypair(); - let mut txs = vec![]; - for i in 0..3 { - let mut outer_tx = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native( - Amount::native_whole(i as u64), - ), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - outer_tx.header.chain_id = shell.chain_id.clone(); - outer_tx - .set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - outer_tx.set_data(Data::new( - format!("transaction data: {}", i).as_bytes().to_owned(), - )); - let gas_limit = - Gas::from(outer_tx.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(outer_tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(outer_tx.clone(), gas_limit); - - outer_tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - txs.push(outer_tx); - } - let response = { - let request = ProcessProposal { - txs: vec![ - txs[0].to_bytes(), - txs[2].to_bytes(), - txs[1].to_bytes(), - ], - }; - if let Err(TestError::RejectProposal(mut resp)) = - shell.process_proposal(request) - { - assert_eq!(resp.len(), 3); - resp.remove(1) - } else { - panic!("Test failed") - } - }; - assert_eq!(response.result.code, u32::from(ResultCode::InvalidOrder)); - assert_eq!( - response.result.info, - String::from( - "Process proposal rejected a decrypted transaction that \ - violated the tx order determined in the previous block" - ), - ); - } - - /// Test that a block containing a tx incorrectly labelled as undecryptable - /// is rejected by [`process_proposal`] - #[test] - fn test_incorrectly_labelled_as_undecryptable() { - let (mut shell, _recv, _, _) = test_utils::setup_at_height(3u64); - let keypair = gen_keypair(); - - let mut tx = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native( - Default::default(), - ), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - tx.header.chain_id = shell.chain_id.clone(); - tx.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - tx.set_data(Data::new("transaction data".as_bytes().to_owned())); - let gas_limit = Gas::from(tx.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(tx.clone(), gas_limit); - - tx.header.tx_type = TxType::Decrypted(DecryptedTx::Undecryptable); - - let response = { - let request = ProcessProposal { - txs: vec![tx.to_bytes()], - }; - if let Err(TestError::RejectProposal(resp)) = - shell.process_proposal(request) - { - if let [resp] = resp.as_slice() { - resp.clone() - } else { - panic!("Test failed") - } - } else { - panic!("Test failed") - } - }; - assert_eq!(response.result.code, u32::from(ResultCode::InvalidTx)); - assert_eq!( - response.result.info, - String::from( - "The encrypted payload of tx was incorrectly marked as \ - un-decryptable" - ), - ) - } - - /// Test that if a wrapper tx contains marked undecryptable the proposal is - /// rejected - #[test] - fn test_undecryptable() { - let (mut shell, _recv, _, _) = test_utils::setup_at_height(3u64); - let keypair = crate::wallet::defaults::daewon_keypair(); - // not valid tx bytes - let wrapper = WrapperTx { - fee: Fee { - amount_per_gas_unit: DenominatedAmount::native( - Default::default(), - ), - token: shell.state.in_mem().native_token.clone(), - }, - pk: keypair.ref_to(), - epoch: Epoch(0), - gas_limit: GAS_LIMIT_MULTIPLIER.into(), - unshield_section_hash: None, - }; - - let tx = Tx::from_type(TxType::Wrapper(Box::new(wrapper))); - let mut decrypted = tx.clone(); - decrypted.update_header(TxType::Decrypted(DecryptedTx::Undecryptable)); - - let gas_limit = Gas::from(tx.header().wrapper().unwrap().gas_limit) - .checked_sub(Gas::from(tx.to_bytes().len() as u64)) - .unwrap(); - shell.enqueue_tx(tx, gas_limit); - - let request = ProcessProposal { - txs: vec![decrypted.to_bytes()], - }; - shell.process_proposal(request).expect_err("Test failed"); - } - - /// Test that if more decrypted txs are submitted to - /// [`process_proposal`] than expected, they are rejected - #[test] - fn test_too_many_decrypted_txs() { - let (shell, _recv, _, _) = test_utils::setup_at_height(3u64); - let mut tx = Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); - tx.header.chain_id = shell.chain_id.clone(); - tx.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - tx.set_data(Data::new("transaction data".as_bytes().to_owned())); - - let request = ProcessProposal { - txs: vec![tx.to_bytes()], - }; - let response = if let Err(TestError::RejectProposal(resp)) = - shell.process_proposal(request) - { - if let [resp] = resp.as_slice() { - resp.clone() - } else { - panic!("Test failed") - } - } else { - panic!("Test failed") - }; - assert_eq!(response.result.code, u32::from(ResultCode::ExtraTxs)); - assert_eq!( - response.result.info, - String::from("Received more decrypted txs than expected"), - ); - } - /// Process Proposal should reject a block containing a RawTx, but not panic #[test] fn test_raw_tx_rejected() { @@ -1704,55 +1399,6 @@ mod test_process_proposal { } } - /// Test that an expired decrypted transaction is marked as rejected but - /// still allows the block to be accepted - #[test] - fn test_expired_decrypted() { - let (mut shell, _recv, _, _) = test_utils::setup(); - let keypair = crate::wallet::defaults::daewon_keypair(); - - let mut wrapper = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - wrapper.header.chain_id = shell.chain_id.clone(); - wrapper.header.expiration = Some(DateTimeUtc::default()); - wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - wrapper.set_data(Data::new("transaction data".as_bytes().to_owned())); - wrapper.add_section(Section::Signature(Signature::new( - wrapper.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); - - shell.enqueue_tx(wrapper.clone(), GAS_LIMIT_MULTIPLIER.into()); - - let decrypted = - wrapper.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - - // Run validation - let request = ProcessProposal { - txs: vec![decrypted.to_bytes()], - }; - match shell.process_proposal(request) { - Ok(txs) => { - assert_eq!(txs.len(), 1); - assert_eq!( - txs[0].result.code, - u32::from(ResultCode::ExpiredDecryptedTx) - ); - } - Err(_) => panic!("Test failed"), - } - } - /// Check that a tx requiring more gas than the block limit causes a block /// rejection #[test] @@ -1792,7 +1438,7 @@ mod test_process_proposal { Err(TestError::RejectProposal(response)) => { assert_eq!( response[0].result.code, - u32::from(ResultCode::AllocationError) + u32::from(ResultCode::TxGasLimit) ); } } @@ -2023,65 +1669,6 @@ mod test_process_proposal { } } - /// Test if we reject wrapper txs when they shouldn't be included in blocks. - /// - /// Currently, the conditions to reject wrapper - /// txs are simply to check if we are at the 2nd - /// or 3rd height offset within an epoch. - #[test] - fn test_include_only_protocol_txs() { - let (mut shell, _recv, _, _) = test_utils::setup_at_height(1u64); - let keypair = gen_keypair(); - let mut wrapper = - Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( - Fee { - amount_per_gas_unit: DenominatedAmount::native(0.into()), - token: shell.state.in_mem().native_token.clone(), - }, - keypair.ref_to(), - Epoch(0), - GAS_LIMIT_MULTIPLIER.into(), - None, - )))); - wrapper.header.chain_id = shell.chain_id.clone(); - wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None)); - wrapper.set_data(Data::new("transaction data".as_bytes().to_owned())); - wrapper.add_section(Section::Signature(Signature::new( - wrapper.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); - let wrapper = wrapper.to_bytes(); - for height in [1u64, 2] { - if let Some(b) = shell.state.in_mem_mut().last_block.as_mut() { - b.height = height.into(); - } - let response = { - let request = ProcessProposal { - txs: vec![wrapper.clone()], - }; - if let Err(TestError::RejectProposal(mut resp)) = - shell.process_proposal(request) - { - assert_eq!(resp.len(), 1); - resp.remove(0) - } else { - panic!("Test failed") - } - }; - assert_eq!( - response.result.code, - u32::from(ResultCode::AllocationError) - ); - assert_eq!( - response.result.info, - String::from( - "Wrapper txs not allowed at the current block height" - ), - ); - } - } - /// Test max tx bytes parameter in ProcessProposal #[test] fn test_max_tx_bytes_process_proposal() { diff --git a/crates/apps/src/lib/node/ledger/shell/testing/node.rs b/crates/apps/src/lib/node/ledger/shell/testing/node.rs index f133082f8e..8c22cac956 100644 --- a/crates/apps/src/lib/node/ledger/shell/testing/node.rs +++ b/crates/apps/src/lib/node/ledger/shell/testing/node.rs @@ -501,26 +501,10 @@ impl MockNode { ); } - /// Advance to a block height that allows - /// txs - fn advance_to_allowed_block(&self) { - loop { - let not_allowed = - { self.shell.lock().unwrap().encrypted_txs_not_allowed() }; - if not_allowed { - self.finalize_and_commit(); - } else { - break; - } - } - } - /// Send a tx through Process Proposal and Finalize Block /// and register the results. pub fn submit_txs(&self, txs: Vec>) { - // The block space allocator disallows encrypted txs in certain blocks. - // Advance to block height that allows txs. - self.advance_to_allowed_block(); + self.finalize_and_commit(); let (proposer_address, votes) = self.prepare_request(); let time = DateTimeUtc::now(); @@ -779,21 +763,6 @@ impl<'a> Client for &'a MockNode { } else { self.clear_results(); } - let (proposer_address, _) = self.prepare_request(); - let req = RequestPrepareProposal { - proposer_address: proposer_address.into(), - ..Default::default() - }; - let txs: Vec> = { - let locked = self.shell.lock().unwrap(); - locked.prepare_proposal(req).txs - } - .into_iter() - .map(|tx| tx.into()) - .collect(); - if !txs.is_empty() { - self.submit_txs(txs); - } Ok(resp) } @@ -972,7 +941,7 @@ fn parse_tm_query( query: namada::tendermint_rpc::query::Query, ) -> dumb_queries::QueryMatcher { const QUERY_PARSING_REGEX_STR: &str = - r"^tm\.event='NewBlock' AND (accepted|applied)\.hash='([^']+)'$"; + r"^tm\.event='NewBlock' AND applied\.hash='([^']+)'$"; lazy_static! { /// Compiled regular expression used to parse Tendermint queries. @@ -983,13 +952,10 @@ fn parse_tm_query( let captures = QUERY_PARSING_REGEX.captures(&query).unwrap(); match captures.get(0).unwrap().as_str() { - "accepted" => dumb_queries::QueryMatcher::accepted( - captures.get(1).unwrap().as_str().try_into().unwrap(), - ), "applied" => dumb_queries::QueryMatcher::applied( captures.get(1).unwrap().as_str().try_into().unwrap(), ), - _ => unreachable!("We only query accepted or applied txs"), + _ => unreachable!("We only query applied txs"), } } diff --git a/crates/apps/src/lib/node/ledger/shell/vote_extensions.rs b/crates/apps/src/lib/node/ledger/shell/vote_extensions.rs index 9bf9f9bd1b..fa6fdbfcf9 100644 --- a/crates/apps/src/lib/node/ledger/shell/vote_extensions.rs +++ b/crates/apps/src/lib/node/ledger/shell/vote_extensions.rs @@ -4,6 +4,7 @@ pub mod bridge_pool_vext; pub mod eth_events; pub mod val_set_update; +use drain_filter_polyfill::DrainFilter; use namada::ethereum_bridge::protocol::transactions::bridge_pool_roots::sign_bridge_pool_root; use namada::ethereum_bridge::protocol::transactions::ethereum_events::sign_ethereum_events; use namada::ethereum_bridge::protocol::transactions::validator_set_update::sign_validator_set_update; @@ -115,9 +116,10 @@ where /// ones we could deserialize to vote extension protocol txs. pub fn deserialize_vote_extensions<'shell>( &'shell self, - txs: &'shell [TxBytes], - ) -> impl Iterator + 'shell { - txs.iter().filter_map(move |tx_bytes| { + txs: &'shell mut Vec, + ) -> DrainFilter<'shell, TxBytes, impl FnMut(&mut TxBytes) -> bool + 'shell> + { + drain_filter_polyfill::VecExt::drain_filter(txs, move |tx_bytes| { let tx = match Tx::try_from(tx_bytes.as_ref()) { Ok(tx) => tx, Err(err) => { @@ -126,25 +128,21 @@ where "Failed to deserialize tx in \ deserialize_vote_extensions" ); - return None; + return false; } }; - match (&tx).try_into().ok()? { - EthereumTxData::BridgePoolVext(_) => Some(tx_bytes.clone()), - EthereumTxData::EthEventsVext(ext) => { + match (&tx).try_into().ok() { + Some(EthereumTxData::BridgePoolVext(_)) => true, + Some(EthereumTxData::EthEventsVext(ext)) => { // NB: only propose events with at least // one valid nonce - ext.data - .ethereum_events - .iter() - .any(|event| { - self.state - .ethbridge_queries() - .validate_eth_event_nonce(event) - }) - .then(|| tx_bytes.clone()) + ext.data.ethereum_events.iter().any(|event| { + self.state + .ethbridge_queries() + .validate_eth_event_nonce(event) + }) } - EthereumTxData::ValSetUpdateVext(ext) => { + Some(EthereumTxData::ValSetUpdateVext(ext)) => { // only include non-stale validator set updates // in block proposals. it might be sitting long // enough in the mempool for it to no longer be @@ -154,13 +152,13 @@ where // to remove it from the mempool this way, but it // will eventually be evicted, getting replaced // by newer txs. - (!self + let is_seen = self .state .ethbridge_queries() - .valset_upd_seen(ext.data.signing_epoch.next())) - .then(|| tx_bytes.clone()) + .valset_upd_seen(ext.data.signing_epoch.next()); + !is_seen } - _ => None, + _ => false, } }) } diff --git a/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs index 11788fe9d1..eaf929d4da 100644 --- a/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs +++ b/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs @@ -143,7 +143,7 @@ impl AbcippShim { proposer from tendermint raw hash", ); - let (processing_results, _) = self.service.process_txs( + let processing_results = self.service.process_txs( &self.delivered_txs, block_time, &block_proposer, diff --git a/crates/apps/src/lib/node/ledger/storage/rocksdb.rs b/crates/apps/src/lib/node/ledger/storage/rocksdb.rs index 18e2fa13f6..1bd88e2072 100644 --- a/crates/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/crates/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -7,14 +7,12 @@ //! - `eth_events_queue`: a queue of confirmed ethereum events to be processed //! in order //! - `height`: the last committed block height -//! - `tx_queue`: txs to be decrypted in the next block //! - `next_epoch_min_start_height`: minimum block height from which the next //! epoch can start //! - `next_epoch_min_start_time`: minimum block time from which the next //! epoch can start //! - `replay_protection`: hashes of the processed transactions //! - `pred`: predecessor values of the top-level keys of the same name -//! - `tx_queue` //! - `next_epoch_min_start_height` //! - `next_epoch_min_start_time` //! - `conversion_state`: MASP conversion state @@ -56,7 +54,6 @@ use namada::core::time::DateTimeUtc; use namada::core::{decode, encode, ethereum_events, ethereum_structs}; use namada::eth_bridge::storage::proof::BridgePoolRootProof; use namada::ledger::eth_bridge::storage::bridge_pool; -use namada::ledger::storage::tx_queue::TxQueue; use namada::replay_protection; use namada::state::merkle_tree::{base_tree_key_prefix, subtree_key_prefix}; use namada::state::{ @@ -512,11 +509,9 @@ impl RocksDB { // restarting the chain tracing::info!("Reverting non-height-prepended metadata keys"); batch.put_cf(state_cf, "height", encode(&previous_height)); - for metadata_key in [ - "next_epoch_min_start_height", - "next_epoch_min_start_time", - "tx_queue", - ] { + for metadata_key in + ["next_epoch_min_start_height", "next_epoch_min_start_time"] + { let previous_key = format!("pred/{}", metadata_key); let previous_value = self .0 @@ -731,17 +726,6 @@ impl DB for RocksDB { return Ok(None); } }; - let tx_queue: TxQueue = match self - .0 - .get_cf(state_cf, "tx_queue") - .map_err(|e| Error::DBError(e.into_string()))? - { - Some(bytes) => decode(bytes).map_err(Error::CodingError)?, - None => { - tracing::error!("Couldn't load tx queue from the DB"); - return Ok(None); - } - }; let ethereum_height: Option = match self .0 @@ -890,7 +874,6 @@ impl DB for RocksDB { next_epoch_min_start_time, update_epoch_blocks_delay, address_gen, - tx_queue, ethereum_height, eth_events_queue, })), @@ -921,7 +904,6 @@ impl DB for RocksDB { address_gen, results, conversion_state, - tx_queue, ethereum_height, eth_events_queue, }: BlockStateWrite = state; @@ -1002,16 +984,6 @@ impl DB for RocksDB { ); } - // Tx queue - if let Some(pred_tx_queue) = self - .0 - .get_cf(state_cf, "tx_queue") - .map_err(|e| Error::DBError(e.into_string()))? - { - // Write the predecessor value for rollback - batch.0.put_cf(state_cf, "pred/tx_queue", pred_tx_queue); - } - batch.0.put_cf(state_cf, "tx_queue", encode(&tx_queue)); batch .0 .put_cf(state_cf, "ethereum_height", encode(ðereum_height)); @@ -2258,7 +2230,6 @@ mod test { let next_epoch_min_start_time = DateTimeUtc::now(); let update_epoch_blocks_delay = None; let address_gen = EstablishedAddressGen::new("whatever"); - let tx_queue = TxQueue::default(); let results = BlockResults::default(); let eth_events_queue = EthEventsQueue::default(); let block = BlockStateWrite { @@ -2275,7 +2246,6 @@ mod test { next_epoch_min_start_time, update_epoch_blocks_delay, address_gen: &address_gen, - tx_queue: &tx_queue, ethereum_height: None, eth_events_queue: ð_events_queue, }; diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index e9c9708b50..0f724c4f56 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -966,10 +966,7 @@ fn parameters(c: &mut Criterion) { shell.state.write(&proposal_key, 0).unwrap(); // Return a dummy tx for validation - let mut tx = - Tx::from_type(namada::tx::data::TxType::Decrypted( - namada::tx::data::DecryptedTx::Decrypted, - )); + let mut tx = Tx::from_type(namada::tx::data::TxType::Raw); tx.set_data(namada::tx::Data::new(borsh::to_vec(&0).unwrap())); tx } @@ -1041,10 +1038,7 @@ fn pos(c: &mut Criterion) { shell.state.write(&proposal_key, 0).unwrap(); // Return a dummy tx for validation - let mut tx = - Tx::from_type(namada::tx::data::TxType::Decrypted( - namada::tx::data::DecryptedTx::Decrypted, - )); + let mut tx = Tx::from_type(namada::tx::data::TxType::Raw); tx.set_data(namada::tx::Data::new(borsh::to_vec(&0).unwrap())); tx } diff --git a/crates/benches/process_wrapper.rs b/crates/benches/process_wrapper.rs index da26796e78..fa2b7fa7fe 100644 --- a/crates/benches/process_wrapper.rs +++ b/crates/benches/process_wrapper.rs @@ -59,7 +59,6 @@ fn process_tx(c: &mut Criterion) { b.iter_batched( || { ( - shell.state.in_mem().tx_queue.clone(), // Prevent block out of gas and replay protection shell.state.with_temp_write_log(), ValidationMeta::from(shell.state.read_only()), @@ -69,7 +68,6 @@ fn process_tx(c: &mut Criterion) { ) }, |( - tx_queue, mut temp_state, mut validation_meta, mut vp_wasm_cache, @@ -81,7 +79,6 @@ fn process_tx(c: &mut Criterion) { shell .check_proposal_tx( &wrapper, - &mut tx_queue.iter(), &mut validation_meta, &mut temp_state, datetime, diff --git a/crates/core/src/event.rs b/crates/core/src/event.rs index d82121de50..ab0aebbf6b 100644 --- a/crates/core/src/event.rs +++ b/crates/core/src/event.rs @@ -49,8 +49,6 @@ pub struct Event { /// The two types of custom events we currently use #[derive(Clone, Debug, Eq, PartialEq, BorshSerialize, BorshDeserialize)] pub enum EventType { - /// The transaction was accepted to be included in a block - Accepted, /// The transaction was applied during block finalization Applied, /// The IBC transaction was applied during block finalization @@ -66,7 +64,6 @@ pub enum EventType { impl Display for EventType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - EventType::Accepted => write!(f, "accepted"), EventType::Applied => write!(f, "applied"), EventType::Ibc(t) => write!(f, "{}", t), EventType::Proposal => write!(f, "proposal"), @@ -82,7 +79,6 @@ impl FromStr for EventType { fn from_str(s: &str) -> Result { match s { - "accepted" => Ok(EventType::Accepted), "applied" => Ok(EventType::Applied), "proposal" => Ok(EventType::Proposal), "pgf_payments" => Ok(EventType::PgfPayment), diff --git a/crates/core/src/storage.rs b/crates/core/src/storage.rs index dddb367f7c..58bfbc9124 100644 --- a/crates/core/src/storage.rs +++ b/crates/core/src/storage.rs @@ -1468,6 +1468,9 @@ pub struct IndexedTx { pub height: BlockHeight, /// The index in the block of the tx pub index: TxIndex, + /// A transcation can have up to two shielded transfers. + /// This indicates if the wrapper contained a shielded transfer. + pub is_wrapper: bool, } #[cfg(test)] diff --git a/crates/namada/src/ledger/mod.rs b/crates/namada/src/ledger/mod.rs index ed59846e93..041a1af407 100644 --- a/crates/namada/src/ledger/mod.rs +++ b/crates/namada/src/ledger/mod.rs @@ -43,14 +43,14 @@ mod dry_run_tx { { use borsh_ext::BorshSerializeExt; use namada_gas::{Gas, GasMetering, TxGasMeter}; - use namada_tx::data::{DecryptedTx, TxType}; + use namada_tx::data::TxType; use namada_tx::Tx; use crate::ledger::protocol::ShellParams; use crate::storage::TxIndex; let mut temp_state = ctx.state.with_temp_write_log(); - let mut tx = Tx::try_from(&request.data[..]).into_storage_result()?; + let tx = Tx::try_from(&request.data[..]).into_storage_result()?; tx.validate_tx().into_storage_result()?; let mut cumulated_gas = Gas::default(); @@ -77,12 +77,10 @@ mod dry_run_tx { temp_state.write_log_mut().commit_tx(); cumulated_gas = tx_gas_meter.borrow_mut().get_tx_consumed_gas(); - - tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); let available_gas = tx_gas_meter.borrow().get_available_gas(); TxGasMeter::new_from_sub_limit(available_gas) } - TxType::Protocol(_) | TxType::Decrypted(_) => { + TxType::Protocol(_) => { // If dry run only the inner tx, use the max block gas as // the gas limit TxGasMeter::new(GasLimit::from( @@ -91,7 +89,6 @@ mod dry_run_tx { } TxType::Raw => { // Cast tx to a decrypted for execution - tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); // If dry run only the inner tx, use the max block gas as // the gas limit @@ -145,7 +142,6 @@ mod test { use namada_state::testing::TestState; use namada_state::StorageWrite; use namada_test_utils::TestWasms; - use namada_tx::data::decrypted::DecryptedTx; use namada_tx::data::TxType; use namada_tx::{Code, Data, Tx}; use tempfile::TempDir; @@ -286,8 +282,7 @@ mod test { assert_eq!(current_epoch, read_epoch); // Request dry run tx - let mut outer_tx = - Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); + let mut outer_tx = Tx::from_type(TxType::Raw); outer_tx.header.chain_id = client.state.in_mem().chain_id.clone(); outer_tx.set_code(Code::from_hash(tx_hash, None)); outer_tx.set_data(Data::new(vec![])); diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 874bb6a464..2eb6f7e117 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -267,7 +267,7 @@ where .ctx .read_post::(pin_keys.first().unwrap())? { - Some(IndexedTx { height, index }) + Some(IndexedTx { height, index, .. }) if height == self.ctx.get_block_height()? && index == self.ctx.get_tx_index()? => {} Some(_) => { diff --git a/crates/namada/src/ledger/protocol/mod.rs b/crates/namada/src/ledger/protocol/mod.rs index 6a7991ef65..12ee722cef 100644 --- a/crates/namada/src/ledger/protocol/mod.rs +++ b/crates/namada/src/ledger/protocol/mod.rs @@ -13,9 +13,7 @@ use namada_gas::TxGasMeter; use namada_sdk::tx::TX_TRANSFER_WASM; use namada_state::StorageWrite; use namada_tx::data::protocol::ProtocolTxType; -use namada_tx::data::{ - DecryptedTx, GasLimit, TxResult, TxType, VpsResult, WrapperTx, -}; +use namada_tx::data::{GasLimit, TxResult, TxType, VpsResult, WrapperTx}; use namada_tx::{Section, Tx}; use namada_vote_ext::EthereumTxData; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; @@ -50,6 +48,8 @@ pub enum Error { StateError(namada_state::Error), #[error("Storage error: {0}")] StorageError(namada_state::StorageError), + #[error("Wrapper tx runner error: {0}")] + WrapperRunnerError(String), #[error("Transaction runner error: {0}")] TxRunnerError(vm::wasm::run::Error), #[error("{0:?}")] @@ -96,8 +96,6 @@ pub enum Error { MaspNativeVpError(native_vp::masp::Error), #[error("Access to an internal address {0:?} is forbidden")] AccessForbidden(InternalAddress), - #[error("Tx is not allowed in allowlist parameter.")] - DisallowedTx, } /// Shell parameters for running wasm transactions. @@ -174,8 +172,8 @@ where CA: 'static + WasmCacheAccess + Sync, { match tx.header().tx_type { - TxType::Raw => Err(Error::TxTypeError), - TxType::Decrypted(DecryptedTx::Decrypted) => apply_wasm_tx( + // Raw trasaction type is allowed only for governance proposals + TxType::Raw => apply_wasm_tx( tx, &tx_index, ShellParams { @@ -192,7 +190,7 @@ where let fee_unshielding_transaction = get_fee_unshielding_transaction(&tx, wrapper); let changed_keys = apply_wrapper_tx( - tx, + tx.clone(), wrapper, fee_unshielding_transaction, tx_bytes, @@ -203,18 +201,21 @@ where tx_wasm_cache, }, wrapper_args, + ) + .map_err(|e| Error::WrapperRunnerError(e.to_string()))?; + let mut inner_res = apply_wasm_tx( + tx, + &tx_index, + ShellParams { + tx_gas_meter, + state, + vp_wasm_cache, + tx_wasm_cache, + }, )?; - Ok(TxResult { - gas_used: tx_gas_meter.borrow().get_tx_consumed_gas(), - changed_keys, - vps_result: VpsResult::default(), - initialized_accounts: vec![], - ibc_events: BTreeSet::default(), - eth_bridge_events: BTreeSet::default(), - }) - } - TxType::Decrypted(DecryptedTx::Undecryptable) => { - Ok(TxResult::default()) + + inner_res.wrapper_changed_keys = changed_keys; + Ok(inner_res) } } } @@ -618,6 +619,7 @@ where Ok(TxResult { gas_used, + wrapper_changed_keys: Default::default(), changed_keys, vps_result, initialized_accounts, @@ -626,29 +628,6 @@ where }) } -/// Returns [`Error::DisallowedTx`] when the given tx is inner (decrypted) tx -/// and its code `Hash` is not included in the `tx_allowlist` parameter. -pub fn check_tx_allowed(tx: &Tx, state: &WlState) -> Result<()> -where - D: 'static + DB + for<'iter> DBIter<'iter> + Sync, - H: 'static + StorageHasher + Sync, -{ - if let TxType::Decrypted(DecryptedTx::Decrypted) = tx.header().tx_type { - if let Some(code_sec) = tx - .get_section(tx.code_sechash()) - .and_then(|x| Section::code_sec(&x)) - { - if crate::parameters::is_tx_allowed(state, &code_sec.code.hash()) - .map_err(Error::StorageError)? - { - return Ok(()); - } - } - return Err(Error::DisallowedTx); - } - Ok(()) -} - /// Apply a derived transaction to storage based on some protocol transaction. /// The logic here must be completely deterministic and will be executed by all /// full nodes every time a protocol transaction is included in a block. Storage @@ -1036,7 +1015,6 @@ mod tests { use borsh::BorshDeserialize; use eyre::Result; - use namada_core::chain::ChainId; use namada_core::ethereum_events::testing::DAI_ERC20_ETH_ADDRESS; use namada_core::ethereum_events::{EthereumEvent, TransferToNamada}; use namada_core::keccak::keccak_hash; @@ -1187,44 +1165,4 @@ mod tests { Ok(()) } - - #[test] - fn test_apply_wasm_tx_allowlist() { - let (mut state, _validators) = test_utils::setup_default_storage(); - - let mut tx = Tx::new(ChainId::default(), None); - tx.update_header(TxType::Decrypted(DecryptedTx::Decrypted)); - // pseudo-random code hash - let code = vec![1_u8, 2, 3]; - let tx_hash = Hash::sha256(&code); - tx.set_code(namada_tx::Code::new(code, None)); - - // Check that using a disallowed tx leads to an error - { - let allowlist = vec![format!("{}-bad", tx_hash)]; - crate::parameters::update_tx_allowlist_parameter( - &mut state, allowlist, - ) - .unwrap(); - state.commit_tx(); - - let result = check_tx_allowed(&tx, &state); - assert_matches!(result.unwrap_err(), Error::DisallowedTx); - } - - // Check that using an allowed tx doesn't lead to `Error::DisallowedTx` - { - let allowlist = vec![tx_hash.to_string()]; - crate::parameters::update_tx_allowlist_parameter( - &mut state, allowlist, - ) - .unwrap(); - state.commit_tx(); - - let result = check_tx_allowed(&tx, &state); - if let Err(result) = result { - assert!(!matches!(result, Error::DisallowedTx)); - } - } - } } diff --git a/crates/namada/src/vm/wasm/run.rs b/crates/namada/src/vm/wasm/run.rs index 817e5c6432..7e3ff8be15 100644 --- a/crates/namada/src/vm/wasm/run.rs +++ b/crates/namada/src/vm/wasm/run.rs @@ -9,8 +9,8 @@ use borsh::BorshDeserialize; use namada_core::validity_predicate::VpSentinel; use namada_gas::{GasMetering, TxGasMeter, WASM_MEMORY_PAGE_GAS}; use namada_state::write_log::StorageModification; -use namada_state::{DBIter, State, StateRead, StorageHasher, DB}; -use namada_tx::data::TxSentinel; +use namada_state::{DBIter, State, StateRead, StorageHasher, StorageRead, DB}; +use namada_tx::data::{TxSentinel, TxType}; use namada_tx::{Commitment, Section, Tx}; use parity_wasm::elements; use thiserror::Error; @@ -84,11 +84,37 @@ pub enum Error { ConversionError(String), #[error("Invalid transaction signature")] InvalidTxSignature, + #[error("Storage error: {0}")] + StorageError(String), + #[error("Tx is not allowed in allowlist parameter")] + DisallowedTx, } /// Result for functions that may fail pub type Result = std::result::Result; +/// Returns [`Error::DisallowedTx`] when the given tx is a user tx and its code +/// `Hash` is not included in the `tx_allowlist` parameter. +pub fn check_tx_allowed(tx: &Tx, storage: &S) -> Result<()> +where + S: StorageRead, +{ + if let TxType::Wrapper(_) = tx.header().tx_type { + if let Some(code_sec) = tx + .get_section(tx.code_sechash()) + .and_then(|x| Section::code_sec(&x)) + { + if crate::parameters::is_tx_allowed(storage, &code_sec.code.hash()) + .map_err(|e| Error::StorageError(e.to_string()))? + { + return Ok(()); + } + } + return Err(Error::DisallowedTx); + } + Ok(()) +} + /// Execute a transaction code. Returns the set verifiers addresses requested by /// the transaction. #[allow(clippy::too_many_arguments)] @@ -101,7 +127,7 @@ pub fn tx( tx_wasm_cache: &mut TxCache, ) -> Result> where - S: StateRead + State, + S: StateRead + State + StorageRead, CA: 'static + WasmCacheAccess, { let tx_code = tx @@ -109,6 +135,11 @@ where .and_then(|x| Section::code_sec(x.as_ref())) .ok_or(Error::MissingSection(tx.code_sechash().to_string()))?; + // Check if the tx code is allowed (to be done after the check on the code + // section commitment to let the replay protection mechanism run some + // optimizations) + check_tx_allowed(tx, state)?; + // If the transaction code has a tag, ensure that the tag hash equals the // transaction code's hash. if let Some(tag) = &tx_code.tag { @@ -624,7 +655,8 @@ mod tests { use itertools::Either; use namada_state::StorageWrite; use namada_test_utils::TestWasms; - use namada_tx::data::TxType; + use namada_token::DenominatedAmount; + use namada_tx::data::{Fee, TxType}; use namada_tx::{Code, Data}; use test_log::test; use wasmer_vm::TrapCode; @@ -1283,6 +1315,73 @@ mod tests { assert!(!passed); } + #[test] + fn test_apply_wasm_tx_allowlist() { + let mut state = TestState::default(); + + let tx_read_key = TestWasms::TxReadStorageKey.read_bytes(); + // store the wasm code + let read_code_hash = Hash::sha256(&tx_read_key); + let code_len = (tx_read_key.len() as u64).serialize_to_vec(); + let key = Key::wasm_code(&read_code_hash); + let len_key = Key::wasm_code_len(&read_code_hash); + state.write_bytes(&key, tx_read_key).unwrap(); + state.write_bytes(&len_key, code_len).unwrap(); + + let mut tx = Tx::new(state.in_mem().chain_id.clone(), None); + let mut wrapper_tx = Tx::from_type(TxType::Wrapper(Box::new( + namada_tx::data::WrapperTx::new( + Fee { + amount_per_gas_unit: DenominatedAmount::native(1.into()), + token: state.in_mem().native_token.clone(), + }, + namada_core::key::testing::common_sk_from_simple_seed(0) + .to_public(), + namada_state::Epoch(0), + 0.into(), + None, + ), + ))); + tx.add_code_from_hash(read_code_hash, None); + wrapper_tx.add_code_from_hash(read_code_hash, None); + tx.add_serialized_data(vec![]); + wrapper_tx.add_serialized_data(vec![]); + + // Check that using a disallowed wrapper tx leads to an error, but a raw + // tx is ok even if not allowlisted + { + let allowlist = vec![format!("{}-bad", read_code_hash)]; + crate::parameters::update_tx_allowlist_parameter( + &mut state, allowlist, + ) + .unwrap(); + state.commit_tx(); + + let result = check_tx_allowed(&wrapper_tx, &state); + assert_matches!(result.unwrap_err(), Error::DisallowedTx); + let result = check_tx_allowed(&tx, &state); + if let Err(result) = result { + assert!(!matches!(result, Error::DisallowedTx)); + } + } + + // Check that using an allowed wrapper tx doesn't lead to + // `Error::DisallowedTx` + { + let allowlist = vec![read_code_hash.to_string()]; + crate::parameters::update_tx_allowlist_parameter( + &mut state, allowlist, + ) + .unwrap(); + state.commit_tx(); + + let result = check_tx_allowed(&wrapper_tx, &state); + if let Err(result) = result { + assert!(!matches!(result, Error::DisallowedTx)); + } + } + } + fn execute_tx_with_code(tx_code: Vec) -> Result> { let tx_data = vec![]; let tx_index = TxIndex::default(); diff --git a/crates/sdk/src/events/log.rs b/crates/sdk/src/events/log.rs index 3eb19f89e7..f68d27b469 100644 --- a/crates/sdk/src/events/log.rs +++ b/crates/sdk/src/events/log.rs @@ -91,16 +91,16 @@ mod tests { "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"; /// An accepted tx hash query. - macro_rules! accepted { + macro_rules! applied { ($hash:expr) => { - dumb_queries::QueryMatcher::accepted(Hash::try_from($hash).unwrap()) + dumb_queries::QueryMatcher::applied(Hash::try_from($hash).unwrap()) }; } /// Return a vector of mock `FinalizeBlock` events. fn mock_tx_events(hash: &str) -> Vec { let event_1 = Event { - event_type: EventType::Accepted, + event_type: EventType::Applied, level: EventLevel::Block, attributes: { let mut attrs = std::collections::HashMap::new(); @@ -109,7 +109,7 @@ mod tests { }, }; let event_2 = Event { - event_type: EventType::Applied, + event_type: EventType::Proposal, level: EventLevel::Block, attributes: { let mut attrs = std::collections::HashMap::new(); @@ -137,7 +137,7 @@ mod tests { // inspect log let events_in_log: Vec<_> = - log.iter_with_matcher(accepted!(HASH)).cloned().collect(); + log.iter_with_matcher(applied!(HASH)).cloned().collect(); assert_eq!(events_in_log.len(), NUM_HEIGHTS); @@ -176,7 +176,7 @@ mod tests { // inspect log - it should be full let events_in_log: Vec<_> = - log.iter_with_matcher(accepted!(HASH)).cloned().collect(); + log.iter_with_matcher(applied!(HASH)).cloned().collect(); assert_eq!(events_in_log.len(), MATCHED_EVENTS); @@ -184,12 +184,12 @@ mod tests { assert_eq!(events[0], event); } - // add a new APPLIED event to the log, - // pruning the first ACCEPTED event we added + // add a new PROPOSAL event to the log, + // pruning the first APPLIED event we added log.log_events(Some(events[1].clone())); let events_in_log: Vec<_> = - log.iter_with_matcher(accepted!(HASH)).cloned().collect(); + log.iter_with_matcher(applied!(HASH)).cloned().collect(); const ACCEPTED_EVENTS: usize = MATCHED_EVENTS - 1; assert_eq!(events_in_log.len(), ACCEPTED_EVENTS); diff --git a/crates/sdk/src/events/log/dumb_queries.rs b/crates/sdk/src/events/log/dumb_queries.rs index 1d2b0527a2..2d2896b3da 100644 --- a/crates/sdk/src/events/log/dumb_queries.rs +++ b/crates/sdk/src/events/log/dumb_queries.rs @@ -41,16 +41,6 @@ impl QueryMatcher { }) } - /// Returns a query matching the given accepted transaction hash. - pub fn accepted(tx_hash: Hash) -> Self { - let mut attributes = HashMap::new(); - attributes.insert("hash".to_string(), tx_hash.to_string()); - Self { - event_type: EventType::Accepted, - attributes, - } - } - /// Returns a query matching the given applied transaction hash. pub fn applied(tx_hash: Hash) -> Self { let mut attributes = HashMap::new(); @@ -132,13 +122,13 @@ mod tests { let mut attributes = HashMap::new(); attributes.insert("hash".to_string(), HASH.to_string()); let matcher = QueryMatcher { - event_type: EventType::Accepted, + event_type: EventType::Proposal, attributes, }; let tests = { let event_1 = Event { - event_type: EventType::Accepted, + event_type: EventType::Proposal, level: EventLevel::Block, attributes: { let mut attrs = std::collections::HashMap::new(); diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 71dc57aa46..ee49388709 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -965,7 +965,6 @@ pub mod testing { // Generate an arbitrary transaction type pub fn arb_tx_type()(tx_type in prop_oneof![ Just(TxType::Raw), - arb_decrypted_tx().prop_map(TxType::Decrypted), arb_wrapper_tx().prop_map(|x| TxType::Wrapper(Box::new(x))), ]) -> TxType { tx_type diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 35315ea39a..1b846184e6 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -150,6 +150,12 @@ pub enum TransferErr { General(#[from] Error), } +#[derive(Debug, Clone)] +struct ExtractedMaspTx { + fee_unshielding: Option<(BTreeSet, Transaction)>, + inner_tx: Option<(BTreeSet, Transaction)>, +} + /// MASP verifying keys pub struct PVKs { /// spend verifying key @@ -844,21 +850,36 @@ impl ShieldedContext { for (idx, tx_event) in txs_results { let tx = Tx::try_from(block[idx.0 as usize].as_ref()) .map_err(|e| Error::Other(e.to_string()))?; - let (changed_keys, masp_transaction) = Self::extract_masp_tx( + let ExtractedMaspTx { + fee_unshielding, + inner_tx, + } = Self::extract_masp_tx( &tx, ExtractShieldedActionArg::Event::(&tx_event), true, ) .await?; - - // Collect the current transaction - shielded_txs.insert( - IndexedTx { - height: height.into(), - index: idx, - }, - (epoch, changed_keys, masp_transaction), - ); + // Collect the current transaction(s) + fee_unshielding.and_then(|(changed_keys, masp_transaction)| { + shielded_txs.insert( + IndexedTx { + height: height.into(), + index: idx, + is_wrapper: true, + }, + (epoch, changed_keys, masp_transaction), + ) + }); + inner_tx.and_then(|(changed_keys, masp_transaction)| { + shielded_txs.insert( + IndexedTx { + height: height.into(), + index: idx, + is_wrapper: false, + }, + (epoch, changed_keys, masp_transaction), + ) + }); } } @@ -870,88 +891,70 @@ impl ShieldedContext { tx: &Tx, action_arg: ExtractShieldedActionArg<'args, C>, check_header: bool, - ) -> Result<(BTreeSet, Transaction), Error> { - let maybe_transaction = if check_header { - let tx_header = tx.header(); - // NOTE: simply looking for masp sections attached to the tx - // is not safe. We don't validate the sections attached to a - // transaction se we could end up with transactions carrying - // an unnecessary masp section. We must instead look for the - // required masp sections in the signed commitments (hashes) - // of the transactions' headers/data sections - if let Some(wrapper_header) = tx_header.wrapper() { - let hash = - wrapper_header.unshield_section_hash.ok_or_else(|| { - Error::Other( - "Missing expected fee unshielding section hash" - .to_string(), - ) - })?; - - let masp_transaction = tx - .get_section(&hash) + ) -> Result { + // We use the changed keys instead of the Transfer object + // because those are what the masp validity predicate works on + let (wrapper_changed_keys, changed_keys) = + if let ExtractShieldedActionArg::Event(tx_event) = action_arg { + let tx_result_str = tx_event + .attributes + .iter() + .find_map(|attr| { + if attr.key == "inner_tx" { + Some(&attr.value) + } else { + None + } + }) .ok_or_else(|| { Error::Other( - "Missing expected masp section".to_string(), + "Missing required tx result in event".to_string(), ) - })? - .masp_tx() - .ok_or_else(|| { - Error::Other("Missing masp transaction".to_string()) })?; + let result = TxResult::from_str(tx_result_str) + .map_err(|e| Error::Other(e.to_string()))?; + (result.wrapper_changed_keys, result.changed_keys) + } else { + (Default::default(), Default::default()) + }; - // We use the changed keys instead of the Transfer object - // because those are what the masp validity predicate works on - let changed_keys = - if let ExtractShieldedActionArg::Event(tx_event) = - action_arg - { - let tx_result_str = tx_event - .attributes - .iter() - .find_map(|attr| { - if attr.key == "inner_tx" { - Some(&attr.value) - } else { - None - } - }) - .ok_or_else(|| { - Error::Other( - "Missing required tx result in event" - .to_string(), - ) - })?; - TxResult::from_str(tx_result_str) - .map_err(|e| Error::Other(e.to_string()))? - .changed_keys - } else { - BTreeSet::default() - }; + let tx_header = tx.header(); + // NOTE: simply looking for masp sections attached to the tx + // is not safe. We don't validate the sections attached to a + // transaction se we could end up with transactions carrying + // an unnecessary masp section. We must instead look for the + // required masp sections in the signed commitments (hashes) + // of the transactions' headers/data sections + let wrapper_header = tx_header + .wrapper() + .expect("All transactions must have a wrapper"); + let maybe_fee_unshield = if let (Some(hash), true) = + (wrapper_header.unshield_section_hash, check_header) + { + let masp_transaction = tx + .get_section(&hash) + .ok_or_else(|| { + Error::Other("Missing expected masp section".to_string()) + })? + .masp_tx() + .ok_or_else(|| { + Error::Other("Missing masp transaction".to_string()) + })?; - Some((changed_keys, masp_transaction)) - } else { - None - } + Some((wrapper_changed_keys, masp_transaction)) } else { None }; - let result = if let Some(tx) = maybe_transaction { - tx - } else { - // Expect decrypted transaction - let tx_data = tx.data().ok_or_else(|| { - Error::Other("Missing data section".to_string()) - })?; - match Transfer::try_from_slice(&tx_data) { - Ok(transfer) => { - let masp_transaction = tx - .get_section(&transfer.shielded.ok_or_else(|| { - Error::Other( - "Missing masp section hash".to_string(), - ) - })?) + // Expect transaction + let tx_data = tx + .data() + .ok_or_else(|| Error::Other("Missing data section".to_string()))?; + let maybe_masp_tx = match Transfer::try_from_slice(&tx_data) { + Ok(transfer) => { + if let Some(hash) = transfer.shielded { + let masp_tx = tx + .get_section(&hash) .ok_or_else(|| { Error::Other( "Missing masp section in transaction" @@ -963,50 +966,25 @@ impl ShieldedContext { Error::Other("Missing masp transaction".to_string()) })?; - // We use the changed keys instead of the Transfer object - // because those are what the masp validity predicate works - // on - let changed_keys = - if let ExtractShieldedActionArg::Event(tx_event) = - action_arg - { - let tx_result_str = tx_event - .attributes - .iter() - .find_map(|attr| { - if attr.key == "inner_tx" { - Some(&attr.value) - } else { - None - } - }) - .ok_or_else(|| { - Error::Other( - "Missing required tx result in event" - .to_string(), - ) - })?; - TxResult::from_str(tx_result_str) - .map_err(|e| Error::Other(e.to_string()))? - .changed_keys - } else { - BTreeSet::default() - }; - (changed_keys, masp_transaction) - } - Err(_) => { - // This should be a MASP over IBC transaction, it - // could be a ShieldedTransfer or an Envelope - // message, need to try both - - extract_payload_from_shielded_action::( - &tx_data, action_arg, - ) - .await? + Some((changed_keys, masp_tx)) + } else { + None } } + Err(_) => { + // This should be a MASP over IBC transaction, it + // could be a ShieldedTransfer or an Envelope + // message, need to try both + extract_payload_from_shielded_action::(&tx_data, action_arg) + .await + .ok() + } }; - Ok(result) + + Ok(ExtractedMaspTx { + fee_unshielding: maybe_fee_unshield, + inner_tx: maybe_masp_tx, + }) } /// Applies the given transaction to the supplied context. More precisely, @@ -1768,7 +1746,11 @@ impl ShieldedContext { )), false, ) - .await?; + .await? + .inner_tx + .ok_or_else(|| { + Error::Other("Missing shielded inner portion of pinned tx".into()) + })?; // Accumulate the combined output note value into this Amount let mut val_acc = I128Sum::zero(); @@ -2349,10 +2331,12 @@ impl ShieldedContext { || IndexedTx { height: BlockHeight::first(), index: TxIndex(0), + is_wrapper: false, }, |indexed| IndexedTx { height: indexed.height, index: indexed.index + 1, + is_wrapper: false, }, ); self.sync_status = ContextSyncStatus::Speculative; @@ -2445,9 +2429,14 @@ impl ShieldedContext { let idx = TxIndex(response_tx.index); // Only process yet unprocessed transactions which have // been accepted by node VPs - let should_process = !transfers - .contains_key(&IndexedTx { height, index: idx }) - && block_results[u64::from(height) as usize] + // TODO: Check that wrappers shouldn't be considered + // here + let should_process = + !transfers.contains_key(&IndexedTx { + height, + index: idx, + is_wrapper: false, + }) && block_results[u64::from(height) as usize] .is_accepted(idx.0 as usize); if !should_process { continue; @@ -2482,7 +2471,11 @@ impl ShieldedContext { // No shielded accounts are affected by this // Transfer transfers.insert( - IndexedTx { height, index: idx }, + IndexedTx { + height, + index: idx, + is_wrapper: false, + }, (epoch, delta, TransactionDelta::new()), ); } diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index d4bcca3409..94fd40e36b 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -98,9 +98,6 @@ router! {SHELL, // Block results access - read bit-vec ( "results" ) -> Vec = read_results, - // was the transaction accepted? - ( "accepted" / [tx_hash: Hash] ) -> Option = accepted, - // was the transaction applied? ( "applied" / [tx_hash: Hash] ) -> Option = applied, @@ -507,23 +504,6 @@ where Ok(data) } -fn accepted( - ctx: RequestCtx<'_, D, H, V, T>, - tx_hash: Hash, -) -> namada_storage::Result> -where - D: 'static + DB + for<'iter> DBIter<'iter> + Sync, - H: 'static + StorageHasher + Sync, -{ - let matcher = dumb_queries::QueryMatcher::accepted(tx_hash); - Ok(ctx - .event_log - .iter_with_matcher(matcher) - .by_ref() - .next() - .cloned()) -} - fn applied( ctx: RequestCtx<'_, D, H, V, T>, tx_hash: Hash, diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index 9b7de172fb..85bf82f19a 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -103,9 +103,6 @@ pub async fn query_tx_status( "Transaction status query deadline of {deadline:?} exceeded" ); match status { - TxEventQuery::Accepted(_) => { - Error::Tx(TxSubmitError::AcceptTimeout) - } TxEventQuery::Applied(_) => { Error::Tx(TxSubmitError::AppliedTimeout) } @@ -460,8 +457,6 @@ pub async fn query_has_storage_key( /// Represents a query for an event pertaining to the specified transaction #[derive(Debug, Copy, Clone)] pub enum TxEventQuery<'a> { - /// Queries whether transaction with given hash was accepted - Accepted(&'a str), /// Queries whether transaction with given hash was applied Applied(&'a str), } @@ -470,7 +465,6 @@ impl<'a> TxEventQuery<'a> { /// The event type to which this event query pertains pub fn event_type(self) -> &'static str { match self { - TxEventQuery::Accepted(_) => "accepted", TxEventQuery::Applied(_) => "applied", } } @@ -478,7 +472,6 @@ impl<'a> TxEventQuery<'a> { /// The transaction to which this event query pertains pub fn tx_hash(self) -> &'a str { match self { - TxEventQuery::Accepted(tx_hash) => tx_hash, TxEventQuery::Applied(tx_hash) => tx_hash, } } @@ -488,9 +481,6 @@ impl<'a> TxEventQuery<'a> { impl<'a> From> for Query { fn from(tx_query: TxEventQuery<'a>) -> Self { match tx_query { - TxEventQuery::Accepted(tx_hash) => { - Query::default().and_eq("accepted.hash", tx_hash) - } TxEventQuery::Applied(tx_hash) => { Query::default().and_eq("applied.hash", tx_hash) } @@ -506,15 +496,9 @@ pub async fn query_tx_events( ) -> std::result::Result, ::Error> { let tx_hash: Hash = tx_event_query.tx_hash().try_into().unwrap(); match tx_event_query { - TxEventQuery::Accepted(_) => { - RPC.shell().accepted(client, &tx_hash).await - } - /*.wrap_err_with(|| { - eyre!("Failed querying whether a transaction was accepted") - })*/, - TxEventQuery::Applied(_) => RPC.shell().applied(client, &tx_hash).await, /*.wrap_err_with(|| { - eyre!("Error querying whether a transaction was applied") - })*/ + TxEventQuery::Applied(_) => RPC.shell().applied(client, &tx_hash).await, /* .wrap_err_with(|| { + * eyre!("Error querying whether a transaction was applied") + * }) */ } } @@ -561,10 +545,8 @@ pub enum TxBroadcastData { Live { /// Transaction to broadcast tx: Tx, - /// Hash of the wrapper transaction - wrapper_hash: String, - /// Hash of decrypted transaction - decrypted_hash: String, + /// Hash of the transaction + tx_hash: String, }, } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 15b69575d0..53ea588f1e 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -220,15 +220,10 @@ pub async fn process_tx( expect_dry_broadcast(TxBroadcastData::DryRun(tx), context).await } else { // We use this to determine when the wrapper tx makes it on-chain - let wrapper_hash = tx.header_hash().to_string(); + let tx_hash = tx.header_hash().to_string(); // We use this to determine when the decrypted inner tx makes it // on-chain - let decrypted_hash = tx.raw_header_hash().to_string(); - let to_broadcast = TxBroadcastData::Live { - tx, - wrapper_hash, - decrypted_hash, - }; + let to_broadcast = TxBroadcastData::Live { tx, tx_hash }; // TODO: implement the code to resubmit the wrapper if it fails because // of masp epoch Either broadcast or submit transaction and // collect result into sum type @@ -313,12 +308,8 @@ pub async fn broadcast_tx( context: &impl Namada, to_broadcast: &TxBroadcastData, ) -> Result { - let (tx, wrapper_tx_hash, decrypted_tx_hash) = match to_broadcast { - TxBroadcastData::Live { - tx, - wrapper_hash, - decrypted_hash, - } => Ok((tx, wrapper_hash, decrypted_hash)), + let (tx, tx_hash) = match to_broadcast { + TxBroadcastData::Live { tx, tx_hash } => Ok((tx, tx_hash)), TxBroadcastData::DryRun(tx) => { Err(TxSubmitError::ExpectLiveRun(tx.clone())) } @@ -342,14 +333,7 @@ pub async fn broadcast_tx( // Print the transaction identifiers to enable the extraction of // acceptance/application results later { - display_line!( - context.io(), - "Wrapper transaction hash: {wrapper_tx_hash}", - ); - display_line!( - context.io(), - "Inner transaction hash: {decrypted_tx_hash}", - ); + display_line!(context.io(), "Transaction hash: {tx_hash}",); } Ok(response) } else { @@ -373,12 +357,8 @@ pub async fn submit_tx( context: &impl Namada, to_broadcast: TxBroadcastData, ) -> Result { - let (_, wrapper_hash, decrypted_hash) = match &to_broadcast { - TxBroadcastData::Live { - tx, - wrapper_hash, - decrypted_hash, - } => Ok((tx, wrapper_hash, decrypted_hash)), + let (_, tx_hash) = match &to_broadcast { + TxBroadcastData::Live { tx, tx_hash } => Ok((tx, tx_hash)), TxBroadcastData::DryRun(tx) => { Err(TxSubmitError::ExpectLiveRun(tx.clone())) } @@ -398,36 +378,12 @@ pub async fn submit_tx( "Awaiting transaction approval", ); - let response = { - let wrapper_query = rpc::TxEventQuery::Accepted(wrapper_hash.as_str()); - let event = - rpc::query_tx_status(context, wrapper_query, deadline).await?; - let wrapper_resp = TxResponse::from_event(event); - - if display_wrapper_resp_and_get_result(context, &wrapper_resp) { - display_line!( - context.io(), - "Waiting for inner transaction result..." - ); - // The transaction is now on chain. We wait for it to be decrypted - // and applied - // We also listen to the event emitted when the encrypted - // payload makes its way onto the blockchain - let decrypted_query = - rpc::TxEventQuery::Applied(decrypted_hash.as_str()); - let event = - rpc::query_tx_status(context, decrypted_query, deadline) - .await?; - let inner_resp = TxResponse::from_event(event); - - display_inner_resp(context, &inner_resp); - Ok(inner_resp) - } else { - Ok(wrapper_resp) - } - }; - - response + // The transaction is now on chain. We wait for it to be applied + let tx_query = rpc::TxEventQuery::Applied(tx_hash.as_str()); + let event = rpc::query_tx_status(context, tx_query, deadline).await?; + let response = TxResponse::from_event(event); + display_inner_resp(context, &response); + Ok(response) } /// Display a result of a wrapper tx. @@ -2983,11 +2939,9 @@ async fn expect_dry_broadcast( let result = rpc::dry_run_tx(context, tx.to_bytes()).await?; Ok(ProcessTxResponse::DryRun(result)) } - TxBroadcastData::Live { - tx, - wrapper_hash: _, - decrypted_hash: _, - } => Err(Error::from(TxSubmitError::ExpectDryRun(tx))), + TxBroadcastData::Live { tx, tx_hash: _ } => { + Err(Error::from(TxSubmitError::ExpectDryRun(tx))) + } } } diff --git a/crates/shielded_token/src/utils.rs b/crates/shielded_token/src/utils.rs index 42fc6413dd..b4253e76e2 100644 --- a/crates/shielded_token/src/utils.rs +++ b/crates/shielded_token/src/utils.rs @@ -76,6 +76,7 @@ pub fn handle_masp_tx( IndexedTx { height: ctx.get_block_height()?, index: ctx.get_tx_index()?, + is_wrapper: false, }, )?; } diff --git a/crates/state/src/in_memory.rs b/crates/state/src/in_memory.rs index 2d53b92b7c..b00a496c4e 100644 --- a/crates/state/src/in_memory.rs +++ b/crates/state/src/in_memory.rs @@ -7,7 +7,7 @@ use namada_gas::MEMORY_ACCESS_GAS_PER_BYTE; use namada_merkle_tree::{MerkleRoot, MerkleTree}; use namada_parameters::{EpochDuration, Parameters}; use namada_storage::conversion_state::ConversionState; -use namada_storage::tx_queue::{ExpiredTxsQueue, TxQueue}; +use namada_storage::tx_queue::ExpiredTxsQueue; use namada_storage::{ BlockHash, BlockHeight, BlockResults, Epoch, Epochs, EthEventsQueue, Header, Key, KeySeg, StorageHasher, TxIndex, BLOCK_HASH_LENGTH, @@ -54,8 +54,6 @@ where pub tx_index: TxIndex, /// The currently saved conversion state pub conversion_state: ConversionState, - /// Wrapper txs to be decrypted in the next block proposal - pub tx_queue: TxQueue, /// Queue of expired transactions that need to be retransmitted. /// /// These transactions do not need to be persisted, as they are @@ -139,7 +137,6 @@ where update_epoch_blocks_delay: None, tx_index: TxIndex::default(), conversion_state: ConversionState::default(), - tx_queue: TxQueue::default(), expired_txs_queue: ExpiredTxsQueue::default(), native_token, ethereum_height: None, diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index afe728c4fa..05d66ab980 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -585,7 +585,7 @@ pub mod testing { use namada_core::address::EstablishedAddressGen; use namada_core::chain::ChainId; use namada_core::time::DateTimeUtc; - use namada_storage::tx_queue::{ExpiredTxsQueue, TxQueue}; + use namada_storage::tx_queue::ExpiredTxsQueue; use super::mockdb::MockDB; use super::*; @@ -632,7 +632,6 @@ pub mod testing { update_epoch_blocks_delay: None, tx_index: TxIndex::default(), conversion_state: ConversionState::default(), - tx_queue: TxQueue::default(), expired_txs_queue: ExpiredTxsQueue::default(), native_token: address::testing::nam(), ethereum_height: None, diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index c90403ea3c..59ebce8cea 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -13,7 +13,7 @@ use namada_storage::{BlockHeight, BlockStateRead, BlockStateWrite, ResultExt}; use crate::in_memory::InMemory; use crate::write_log::{ - self, ReProtStorageModification, StorageModification, WriteLog, + ReProtStorageModification, StorageModification, WriteLog, }; use crate::{ is_pending_transfer_key, DBIter, Epoch, Error, Hash, Key, LastBlock, @@ -467,7 +467,6 @@ where results, address_gen, conversion_state, - tx_queue, ethereum_height, eth_events_queue, }) = self @@ -500,7 +499,6 @@ where let in_mem = &mut self.0.in_mem; in_mem.block.tree = tree; in_mem.conversion_state = conversion_state; - in_mem.tx_queue = tx_queue; in_mem.ethereum_height = ethereum_height; in_mem.eth_events_queue = eth_events_queue; tracing::debug!("Loaded storage from DB"); @@ -554,7 +552,6 @@ where update_epoch_blocks_delay: self.in_mem.update_epoch_blocks_delay, address_gen: &self.in_mem.address_gen, conversion_state: &self.in_mem.conversion_state, - tx_queue: &self.in_mem.tx_queue, ethereum_height: self.in_mem.ethereum_height.as_ref(), eth_events_queue: &self.in_mem.eth_events_queue, }; @@ -628,8 +625,8 @@ where } /// Delete the provided transaction's hash from storage. - pub fn delete_tx_hash(&mut self, hash: Hash) -> write_log::Result<()> { - self.write_log.delete_tx_hash(hash) + pub fn delete_tx_hash(&mut self, hash: Hash) { + self.write_log.delete_tx_hash(hash); } #[inline] diff --git a/crates/state/src/wl_storage.rs b/crates/state/src/wl_storage.rs new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/state/src/write_log.rs b/crates/state/src/write_log.rs index 6ce1f31d54..6529c57580 100644 --- a/crates/state/src/write_log.rs +++ b/crates/state/src/write_log.rs @@ -607,24 +607,9 @@ impl WriteLog { } /// Remove the transaction hash - pub fn delete_tx_hash(&mut self, hash: Hash) -> Result<()> { - match self - .replay_protection - .insert(hash, ReProtStorageModification::Delete) - { - None => Ok(()), - // Allow overwriting a previous finalize request - Some(ReProtStorageModification::Finalize) => Ok(()), - Some(_) => - // Cannot delete an hash that still has to be written to - // storage or has already been deleted - { - Err(Error::ReplayProtection(format!( - "Requested a delete on hash {hash} not yet committed to \ - storage" - ))) - } - } + pub(crate) fn delete_tx_hash(&mut self, hash: Hash) { + self.replay_protection + .insert(hash, ReProtStorageModification::Delete); } /// Move the transaction hash of the previous block to the list of all @@ -915,9 +900,7 @@ mod tests { .unwrap(); // delete previous hash - write_log - .delete_tx_hash(Hash::sha256("tx1".as_bytes())) - .unwrap(); + write_log.delete_tx_hash(Hash::sha256("tx1".as_bytes())); // finalize previous hashes for tx in ["tx2", "tx3"] { @@ -947,8 +930,7 @@ mod tests { // try to delete finalized hash which shouldn't work state .write_log - .delete_tx_hash(Hash::sha256("tx2".as_bytes())) - .unwrap(); + .delete_tx_hash(Hash::sha256("tx2".as_bytes())); // commit a block state.commit_block().expect("commit failed"); diff --git a/crates/storage/src/db.rs b/crates/storage/src/db.rs index 5ce22e85db..a8d51f7da4 100644 --- a/crates/storage/src/db.rs +++ b/crates/storage/src/db.rs @@ -15,7 +15,6 @@ use namada_merkle_tree::{ use thiserror::Error; use crate::conversion_state::ConversionState; -use crate::tx_queue::TxQueue; #[allow(missing_docs)] #[derive(Error, Debug)] @@ -69,8 +68,6 @@ pub struct BlockStateRead { pub results: BlockResults, /// The conversion state pub conversion_state: ConversionState, - /// Wrapper txs to be decrypted in the next block proposal - pub tx_queue: TxQueue, /// The latest block height on Ethereum processed, if /// the bridge is enabled. pub ethereum_height: Option, @@ -106,8 +103,6 @@ pub struct BlockStateWrite<'a> { pub results: &'a BlockResults, /// The conversion state pub conversion_state: &'a ConversionState, - /// Wrapper txs to be decrypted in the next block proposal - pub tx_queue: &'a TxQueue, /// The latest block height on Ethereum processed, if /// the bridge is enabled. pub ethereum_height: Option<&'a ethereum_structs::BlockHeight>, diff --git a/crates/storage/src/mockdb.rs b/crates/storage/src/mockdb.rs index 4128321e54..e1943f6749 100644 --- a/crates/storage/src/mockdb.rs +++ b/crates/storage/src/mockdb.rs @@ -24,7 +24,6 @@ use crate::conversion_state::ConversionState; use crate::db::{ BlockStateRead, BlockStateWrite, DBIter, DBWriteBatch, Error, Result, DB, }; -use crate::tx_queue::TxQueue; use crate::types::{KVBytes, PrefixIterator}; const SUBSPACE_CF: &str = "subspace"; @@ -98,10 +97,6 @@ impl DB for MockDB { Some(bytes) => decode(bytes).map_err(Error::CodingError)?, None => return Ok(None), }; - let tx_queue: TxQueue = match self.0.borrow().get("tx_queue") { - Some(bytes) => decode(bytes).map_err(Error::CodingError)?, - None => return Ok(None), - }; let ethereum_height: Option = match self.0.borrow().get("ethereum_height") { @@ -214,7 +209,6 @@ impl DB for MockDB { address_gen, results, conversion_state, - tx_queue, ethereum_height, eth_events_queue, })), @@ -247,7 +241,6 @@ impl DB for MockDB { conversion_state, ethereum_height, eth_events_queue, - tx_queue, }: BlockStateWrite = state; // Epoch start height and time @@ -269,9 +262,6 @@ impl DB for MockDB { self.0 .borrow_mut() .insert("eth_events_queue".into(), encode(ð_events_queue)); - self.0 - .borrow_mut() - .insert("tx_queue".into(), encode(&tx_queue)); self.0 .borrow_mut() .insert("conversion_state".into(), encode(conversion_state)); diff --git a/crates/storage/src/tx_queue.rs b/crates/storage/src/tx_queue.rs index 3d5d9c7d87..87768dcbbe 100644 --- a/crates/storage/src/tx_queue.rs +++ b/crates/storage/src/tx_queue.rs @@ -1,52 +1,5 @@ use namada_core::borsh::{BorshDeserialize, BorshSerialize}; use namada_core::ethereum_events::EthereumEvent; -use namada_gas::Gas; -use namada_tx::Tx; - -/// A wrapper for `crate::types::transaction::WrapperTx` to conditionally -/// add `has_valid_pow` flag for only used in testnets. -#[derive(Debug, Clone, BorshDeserialize, BorshSerialize)] -pub struct TxInQueue { - /// Wrapper tx - pub tx: Tx, - /// The available gas remaining for the inner tx (for gas accounting). - /// This allows for a more detailed logging about the gas used by the - /// wrapper and that used by the inner - pub gas: Gas, -} - -#[derive(Default, Debug, Clone, BorshDeserialize, BorshSerialize)] -/// Wrapper txs to be decrypted in the next block proposal -pub struct TxQueue(std::collections::VecDeque); - -impl TxQueue { - /// Add a new wrapper at the back of the queue - pub fn push(&mut self, wrapper: TxInQueue) { - self.0.push_back(wrapper); - } - - /// Remove the wrapper at the head of the queue - pub fn pop(&mut self) -> Option { - self.0.pop_front() - } - - /// Get an iterator over the queue - pub fn iter(&self) -> impl std::iter::Iterator { - self.0.iter() - } - - /// Check if there are any txs in the queue - #[allow(dead_code)] - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - /// Get reference to the element at the given index. - /// Returns [`None`] if index exceeds the queue lenght. - pub fn get(&self, index: usize) -> Option<&TxInQueue> { - self.0.get(index) - } -} /// Expired transaction kinds. #[derive(Clone, Debug, BorshSerialize, BorshDeserialize)] diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index b0872c600f..a391ea70e3 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -19,6 +19,7 @@ pub const WASM_FOR_TESTS_DIR: &str = "wasm_for_tests"; pub enum TestWasms { TxMemoryLimit, TxNoOp, + TxInvalidData, TxProposalCode, TxReadStorageKey, TxWriteStorageKey, @@ -37,6 +38,7 @@ impl TestWasms { let filename = match self { TestWasms::TxMemoryLimit => "tx_memory_limit.wasm", TestWasms::TxNoOp => "tx_no_op.wasm", + TestWasms::TxInvalidData => "tx_invalid_data.wasm", TestWasms::TxProposalCode => "tx_proposal_code.wasm", TestWasms::TxReadStorageKey => "tx_read_storage_key.wasm", TestWasms::TxWriteStorageKey => "tx_write.wasm", diff --git a/crates/tests/src/e2e/eth_bridge_tests.rs b/crates/tests/src/e2e/eth_bridge_tests.rs index 9d7eee8ed8..9c77ab85b3 100644 --- a/crates/tests/src/e2e/eth_bridge_tests.rs +++ b/crates/tests/src/e2e/eth_bridge_tests.rs @@ -826,7 +826,6 @@ async fn test_wdai_transfer_established_unauthorized() -> Result<()> { &bertha_addr.to_string(), &token::DenominatedAmount::new(token::Amount::from(10_000), 0u8.into()), )?; - cmd.exp_string(TX_ACCEPTED)?; cmd.exp_string(TX_REJECTED)?; cmd.assert_success(); diff --git a/crates/tests/src/e2e/helpers.rs b/crates/tests/src/e2e/helpers.rs index 6381bdb753..09761833e7 100644 --- a/crates/tests/src/e2e/helpers.rs +++ b/crates/tests/src/e2e/helpers.rs @@ -36,7 +36,7 @@ use super::setup::{ ENV_VAR_USE_PREBUILT_BINARIES, }; use crate::e2e::setup::{Bin, Who, APPS_PACKAGE}; -use crate::strings::{LEDGER_STARTED, TX_ACCEPTED, TX_APPLIED_SUCCESS}; +use crate::strings::{LEDGER_STARTED, TX_APPLIED_SUCCESS}; use crate::{run, run_as}; /// Instantiate a new [`HttpClient`] to perform RPC requests with. @@ -100,7 +100,6 @@ pub fn init_established_account( rpc_addr, ]; let mut cmd = run!(test, Bin::Client, init_account_args, Some(40))?; - cmd.exp_string(TX_ACCEPTED)?; cmd.exp_string(TX_APPLIED_SUCCESS)?; cmd.assert_success(); Ok(()) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index c39a4d9bbe..4e079b8c38 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -94,7 +94,7 @@ use crate::e2e::setup::{ self, run_hermes_cmd, setup_hermes, sleep, Bin, NamadaCmd, Test, Who, }; use crate::strings::{ - LEDGER_STARTED, TX_ACCEPTED, TX_APPLIED_SUCCESS, TX_FAILED, VALIDATOR_NODE, + LEDGER_STARTED, TX_APPLIED_SUCCESS, TX_FAILED, VALIDATOR_NODE, }; use crate::{run, run_as}; @@ -1148,7 +1148,6 @@ fn transfer_on_chain( &rpc, ]; let mut client = run!(test, Bin::Client, tx_args, Some(120))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); @@ -1662,7 +1661,6 @@ fn delegate_token(test: &Test) -> Result<()> { &rpc, ]; let mut client = run!(test, Bin::Client, tx_args, Some(40))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); Ok(()) @@ -1710,7 +1708,6 @@ fn propose_funding( &rpc_a, ]; let mut client = run!(test_a, Bin::Client, submit_proposal_args, Some(40))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); Ok(start_epoch.into()) @@ -1738,7 +1735,6 @@ fn submit_votes(test: &Test) -> Result<()> { submit_proposal_vote, Some(40) )?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); @@ -1756,7 +1752,6 @@ fn submit_votes(test: &Test) -> Result<()> { ]; let mut client = run!(test, Bin::Client, submit_proposal_vote_delagator, Some(40))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); Ok(()) diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 17c85d5fd7..ac3b77bcb7 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -58,8 +58,8 @@ use crate::e2e::setup::{ Who, }; use crate::strings::{ - LEDGER_SHUTDOWN, LEDGER_STARTED, NON_VALIDATOR_NODE, TX_ACCEPTED, - TX_APPLIED_SUCCESS, TX_FAILED, TX_REJECTED, VALIDATOR_NODE, + LEDGER_SHUTDOWN, LEDGER_STARTED, NON_VALIDATOR_NODE, TX_APPLIED_SUCCESS, + TX_FAILED, TX_REJECTED, VALIDATOR_NODE, }; use crate::{run, run_as}; @@ -579,10 +579,6 @@ fn ledger_txs_and_queries() -> Result<()> { tx_args.clone() }; let mut client = run!(test, Bin::Client, tx_args, Some(40))?; - - if !dry_run { - client.exp_string(TX_ACCEPTED)?; - } client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); } @@ -753,7 +749,6 @@ fn wrapper_disposable_signer() -> Result<()> { &validator_one_rpc, ]; let mut client = run!(test, Bin::Client, tx_args, Some(720))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; } @@ -792,7 +787,6 @@ fn wrapper_disposable_signer() -> Result<()> { ]; let mut client = run!(test, Bin::Client, tx_args, Some(720))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; let _ep1 = epoch_sleep(&test, &validator_one_rpc, 720)?; let tx_args = vec!["shielded-sync", "--node", &validator_one_rpc]; @@ -849,7 +843,6 @@ fn wrapper_disposable_signer() -> Result<()> { ]; let mut client = run!(test, Bin::Client, tx_args, Some(720))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; Ok(()) } @@ -899,7 +892,6 @@ fn invalid_transactions() -> Result<()> { ]; let mut client = run!(test, Bin::Client, tx_args, Some(40))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_REJECTED)?; client.assert_success(); @@ -951,7 +943,6 @@ fn invalid_transactions() -> Result<()> { ]; let mut client = run!(test, Bin::Client, tx_args, Some(40))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_FAILED)?; client.assert_success(); Ok(()) @@ -1803,7 +1794,6 @@ fn ledger_many_txs_in_a_block() -> Result<()> { let mut args = (*tx_args).clone(); args.push(&*validator_one_rpc); let mut client = run!(*test, Bin::Client, args, Some(80))?; - client.exp_string(TX_ACCEPTED)?; client.exp_string(TX_APPLIED_SUCCESS)?; client.assert_success(); let res: Result<()> = Ok(()); @@ -1821,20 +1811,24 @@ fn ledger_many_txs_in_a_block() -> Result<()> { Ok(()) } -/// In this test we: -/// 1. Run the ledger node -/// 2. Submit a valid proposal -/// 3. Query the proposal -/// 4. Query token balance (submitted funds) -/// 5. Query governance address balance -/// 6. Submit an invalid proposal -/// 7. Check invalid proposal was not accepted -/// 8. Query token balance (funds shall not be submitted) -/// 9. Send a yay vote from a validator -/// 10. Send a yay vote from a normal user -/// 11. Query the proposal and check the result -/// 12. Wait proposal grace and check proposal author funds -/// 13. Check governance address funds are 0 +// In this test we: +// 1. Run the ledger node +// 2. Submit a valid proposal +// 3. Query the proposal +// 4. Query token balance (submitted funds) +// 5. Query governance address balance +// 6. Submit an invalid proposal +// 7. Check invalid proposal was not accepted +// 8. Query token balance (funds shall not be submitted) +// 9. Send a yay vote from a validator +// 10. Send a yay vote from a normal user +// 11. Query the proposal and check the result +// 12. Wait proposal grace and check proposal author funds +// 13. Check governance address funds are 0 +// 14. Query the new parameters +// 15. Try to initialize a new account which should fail +// 16. Submit a tx that triggers an already existing account which should +// succeed #[test] fn proposal_submission() -> Result<()> { let test = setup::network( @@ -2133,7 +2127,7 @@ fn proposal_submission() -> Result<()> { client.exp_string("nam: 0")?; client.assert_success(); - // // 14. Query parameters + // 14. Query parameters let query_protocol_parameters = vec!["query-protocol-parameters", "--node", &validator_one_rpc]; @@ -2142,6 +2136,49 @@ fn proposal_submission() -> Result<()> { client.exp_regex(".*Min. proposal grace epochs: 9.*")?; client.assert_success(); + // 15. Try to initialize a new account with the no more allowlisted vp + let init_account = vec![ + "init-account", + "--public-keys", + // Value obtained from `namada::core::key::ed25519::tests::gen_keypair` + "tpknam1qpqfzxu3gt05jx2mvg82f4anf90psqerkwqhjey4zlqv0qfgwuvkzt5jhkp", + "--threshold", + "1", + "--code-path", + VP_USER_WASM, + "--alias", + "Test-Account", + "--signing-keys", + BERTHA_KEY, + "--node", + &validator_one_rpc, + ]; + let mut client = run!(test, Bin::Client, init_account, Some(30))?; + client.exp_regex("VP code is not allowed in allowlist parameter")?; + client.assert_success(); + + // 16. Submit a tx touching a previous account with the no more allowlisted + // vp and verify that the transaction succeeds, i.e. the non allowlisted + // vp can still run + let transfer = [ + "transfer", + "--source", + BERTHA, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "10", + "--signing-keys", + BERTHA_KEY, + "--node", + &validator_one_rpc, + ]; + let mut client = run!(test, Bin::Client, transfer, Some(40))?; + client.exp_string(TX_APPLIED_SUCCESS)?; + client.assert_success(); + Ok(()) } @@ -3303,6 +3340,13 @@ fn deactivate_and_reactivate_validator() -> Result<()> { ethereum_bridge::ledger::Mode::Off, None, ); + set_ethereum_bridge_mode( + &test, + &test.net.chain_id, + Who::Validator(1), + ethereum_bridge::ledger::Mode::Off, + None, + ); // 1. Run the ledger node let _bg_validator_0 = @@ -3815,6 +3859,7 @@ fn change_consensus_key() -> Result<()> { Ok(()) } + #[test] fn proposal_change_shielded_reward() -> Result<()> { let test = setup::network( diff --git a/crates/tests/src/strings.rs b/crates/tests/src/strings.rs index f0b1f8ea82..0a9223dcc5 100644 --- a/crates/tests/src/strings.rs +++ b/crates/tests/src/strings.rs @@ -21,9 +21,6 @@ pub const TX_REJECTED: &str = "Transaction was rejected by VPs"; /// Inner transaction failed in execution (no VPs ran). pub const TX_FAILED: &str = "Transaction failed"; -/// Wrapper transaction accepted. -pub const TX_ACCEPTED: &str = "Wrapper transaction accepted"; - pub const WALLET_HD_PASSPHRASE_PROMPT: &str = "Enter BIP39 passphrase (empty for none): "; diff --git a/crates/tx/src/data/decrypted.rs b/crates/tx/src/data/decrypted.rs index d764f03d00..99c19bc6d9 100644 --- a/crates/tx/src/data/decrypted.rs +++ b/crates/tx/src/data/decrypted.rs @@ -17,7 +17,6 @@ pub mod decrypted_tx { serde::Serialize, serde::Deserialize, )] - #[allow(clippy::large_enum_variant)] /// Holds the result of attempting to decrypt /// a transaction and the data necessary for /// other validators to verify diff --git a/crates/tx/src/data/mod.rs b/crates/tx/src/data/mod.rs index e187bdd9fb..ccc9850041 100644 --- a/crates/tx/src/data/mod.rs +++ b/crates/tx/src/data/mod.rs @@ -60,30 +60,24 @@ pub enum ResultCode { InvalidTx = 2, /// Invalid signature InvalidSig = 3, - /// Tx is in invalid order - InvalidOrder = 4, - /// Tx wasn't expected - ExtraTxs = 5, - /// Undecryptable - Undecryptable = 6, /// The block is full - AllocationError = 7, + AllocationError = 4, /// Replayed tx - ReplayTx = 8, + ReplayTx = 5, /// Invalid chain ID - InvalidChainId = 9, + InvalidChainId = 6, /// Expired tx - ExpiredTx = 10, + ExpiredTx = 7, /// Exceeded gas limit - TxGasLimit = 11, + TxGasLimit = 8, /// Error in paying tx fee - FeeError = 12, + FeeError = 9, /// Invalid vote extension - InvalidVoteExtension = 13, + InvalidVoteExtension = 10, /// Tx is too large - TooLarge = 14, - /// Decrypted tx is expired - ExpiredDecryptedTx = 15, + TooLarge = 11, + /// Tx code is not allowlisted + TxNotAllowlisted = 12, // ========================================================================= // WARN: These codes shouldn't be changed between version! } @@ -96,11 +90,10 @@ impl ResultCode { // NOTE: pattern match on all `ResultCode` variants, in order // to catch potential bugs when adding new codes match self { - Ok | WasmRuntimeError | ExpiredDecryptedTx => true, - InvalidTx | InvalidSig | InvalidOrder | ExtraTxs - | Undecryptable | AllocationError | ReplayTx | InvalidChainId - | ExpiredTx | TxGasLimit | FeeError | InvalidVoteExtension - | TooLarge => false, + Ok | WasmRuntimeError => true, + InvalidTx | InvalidSig | AllocationError | ReplayTx + | InvalidChainId | ExpiredTx | TxGasLimit | FeeError + | InvalidVoteExtension | TooLarge | TxNotAllowlisted => false, } } @@ -175,6 +168,8 @@ pub fn hash_tx(tx_bytes: &[u8]) -> Hash { pub struct TxResult { /// Total gas used by the transaction (includes the gas used by VPs) pub gas_used: Gas, + /// Storage keys touched by the wrapper transaction + pub wrapper_changed_keys: BTreeSet, /// Storage keys touched by the transaction pub changed_keys: BTreeSet, /// The results of all the triggered validity predicates by the transaction @@ -299,8 +294,6 @@ pub enum TxType { Raw, /// A Tx that contains an encrypted raw tx Wrapper(Box), - /// An attempted decryption of a wrapper tx - Decrypted(DecryptedTx), /// Txs issued by validators as part of internal protocols Protocol(Box), } @@ -477,74 +470,3 @@ mod test_process_tx { assert_matches!(result, TxError::SigError(_)); } } - -/// Test that process_tx correctly identifies a DecryptedTx -/// with some unsigned data and returns an identical copy -#[test] -fn test_process_tx_decrypted_unsigned() { - use crate::{Code, Data, Tx}; - let mut tx = Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); - let code_sec = tx - .set_code(Code::new("transaction data".as_bytes().to_owned(), None)) - .clone(); - let data_sec = tx - .set_data(Data::new("transaction data".as_bytes().to_owned())) - .clone(); - tx.validate_tx().expect("Test failed"); - match tx.header().tx_type { - TxType::Decrypted(DecryptedTx::Decrypted) => { - assert_eq!(tx.header().code_hash, code_sec.get_hash(),); - assert_eq!(tx.header().data_hash, data_sec.get_hash(),); - } - _ => panic!("Test failed"), - } -} - -/// Test that process_tx correctly identifies a DecryptedTx -/// with some signed data and extracts it without checking -/// signature -#[test] -fn test_process_tx_decrypted_signed() { - use namada_core::key::*; - - use crate::{Code, Data, Section, Signature, Tx}; - - fn gen_keypair() -> common::SecretKey { - use rand::prelude::ThreadRng; - use rand::thread_rng; - - let mut rng: ThreadRng = thread_rng(); - ed25519::SigScheme::generate(&mut rng).try_to_sk().unwrap() - } - - use namada_core::key::Signature as S; - let mut decrypted = - Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); - // Invalid signed data - let ed_sig = - ed25519::Signature::try_from_slice([0u8; 64].as_ref()).unwrap(); - let mut sig_sec = Signature::new( - vec![decrypted.header_hash()], - [(0, gen_keypair())].into_iter().collect(), - None, - ); - sig_sec - .signatures - .insert(0, common::Signature::try_from_sig(&ed_sig).unwrap()); - decrypted.add_section(Section::Signature(sig_sec)); - // create the tx with signed decrypted data - let code_sec = decrypted - .set_code(Code::new("transaction data".as_bytes().to_owned(), None)) - .clone(); - let data_sec = decrypted - .set_data(Data::new("transaction data".as_bytes().to_owned())) - .clone(); - decrypted.validate_tx().expect("Test failed"); - match decrypted.header().tx_type { - TxType::Decrypted(DecryptedTx::Decrypted) => { - assert_eq!(decrypted.header.code_hash, code_sec.get_hash()); - assert_eq!(decrypted.header.data_hash, data_sec.get_hash()); - } - _ => panic!("Test failed"), - } -} diff --git a/crates/tx/src/data/wrapper.rs b/crates/tx/src/data/wrapper.rs index a70ebaa39d..eb9ecaa3f2 100644 --- a/crates/tx/src/data/wrapper.rs +++ b/crates/tx/src/data/wrapper.rs @@ -22,7 +22,7 @@ pub mod wrapper_tx { use sha2::{Digest, Sha256}; use thiserror::Error; - use crate::data::{DecryptedTx, TxType}; + use crate::data::TxType; use crate::{Code, Data, Section, Tx}; /// TODO: Determine a sane number for this @@ -194,6 +194,7 @@ pub mod wrapper_tx { pub pk: common::PublicKey, /// The epoch in which the tx is to be submitted. This determines /// which decryption key will be used + // TODO: Is this still necessary without the DKG? Seems not pub epoch: Epoch, /// Max amount of gas that can be used when executing the inner tx pub gas_limit: GasLimit, @@ -296,8 +297,7 @@ pub mod wrapper_tx { transfer_code_tag: Option, unshield: Transaction, ) -> Result { - let mut tx = - Tx::from_type(TxType::Decrypted(DecryptedTx::Decrypted)); + let mut tx = Tx::from_type(TxType::Raw); let masp_section = tx.add_section(Section::MaspTx(unshield)); let masp_hash = Hash( masp_section diff --git a/crates/tx/src/lib.rs b/crates/tx/src/lib.rs index 6c866e8513..f67e0c1a5a 100644 --- a/crates/tx/src/lib.rs +++ b/crates/tx/src/lib.rs @@ -21,25 +21,12 @@ pub use types::{ pub fn new_tx_event(tx: &Tx, height: u64) -> Event { let mut event = match tx.header().tx_type { TxType::Wrapper(_) => { - let mut event = Event { - event_type: EventType::Accepted, - level: EventLevel::Tx, - attributes: HashMap::new(), - }; - event["hash"] = tx.header_hash().to_string(); - event - } - TxType::Decrypted(_) => { let mut event = Event { event_type: EventType::Applied, level: EventLevel::Tx, attributes: HashMap::new(), }; - event["hash"] = tx - .clone() - .update_header(TxType::Raw) - .header_hash() - .to_string(); + event["hash"] = tx.header_hash().to_string(); event } TxType::Protocol(_) => { diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 9533209425..b5b6dab7a8 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -27,7 +27,7 @@ use sha2::{Digest, Sha256}; use thiserror::Error; use crate::data::protocol::ProtocolTx; -use crate::data::{hash_tx, DecryptedTx, Fee, GasLimit, TxType, WrapperTx}; +use crate::data::{hash_tx, Fee, GasLimit, TxType, WrapperTx}; use crate::proto; /// Represents an error in signature verification @@ -904,15 +904,6 @@ impl Header { } } - /// Get the decrypted header if it is present - pub fn decrypted(&self) -> Option { - if let TxType::Decrypted(decrypted) = &self.tx_type { - Some(decrypted.clone()) - } else { - None - } - } - /// Get the protocol header if it is present pub fn protocol(&self) -> Option { if let TxType::Protocol(protocol) = &self.tx_type { @@ -1341,8 +1332,6 @@ impl Tx { err )) }), - // we extract the signed data, but don't check the signature - TxType::Decrypted(_) => Ok(None), // return as is TxType::Raw => Ok(None), } diff --git a/wasm_for_tests/tx_fail.wasm b/wasm_for_tests/tx_fail.wasm index 3c6b6f2634..d34fcf5772 100755 Binary files a/wasm_for_tests/tx_fail.wasm and b/wasm_for_tests/tx_fail.wasm differ diff --git a/wasm_for_tests/tx_invalid_data.wasm b/wasm_for_tests/tx_invalid_data.wasm new file mode 100755 index 0000000000..9ae23573fc Binary files /dev/null and b/wasm_for_tests/tx_invalid_data.wasm differ diff --git a/wasm_for_tests/tx_memory_limit.wasm b/wasm_for_tests/tx_memory_limit.wasm index c77848a694..7c39ba4413 100755 Binary files a/wasm_for_tests/tx_memory_limit.wasm and b/wasm_for_tests/tx_memory_limit.wasm differ diff --git a/wasm_for_tests/tx_mint_tokens.wasm b/wasm_for_tests/tx_mint_tokens.wasm index 03736d7e45..a835370612 100755 Binary files a/wasm_for_tests/tx_mint_tokens.wasm and b/wasm_for_tests/tx_mint_tokens.wasm differ diff --git a/wasm_for_tests/tx_no_op.wasm b/wasm_for_tests/tx_no_op.wasm index 817cb21b02..56d2614eca 100755 Binary files a/wasm_for_tests/tx_no_op.wasm and b/wasm_for_tests/tx_no_op.wasm differ diff --git a/wasm_for_tests/tx_proposal_code.wasm b/wasm_for_tests/tx_proposal_code.wasm index 349818a9df..254c60c143 100755 Binary files a/wasm_for_tests/tx_proposal_code.wasm and b/wasm_for_tests/tx_proposal_code.wasm differ diff --git a/wasm_for_tests/tx_proposal_masp_reward.wasm b/wasm_for_tests/tx_proposal_masp_reward.wasm index 7adaa5a2ce..59e4bde8fb 100755 Binary files a/wasm_for_tests/tx_proposal_masp_reward.wasm and b/wasm_for_tests/tx_proposal_masp_reward.wasm differ diff --git a/wasm_for_tests/tx_read_storage_key.wasm b/wasm_for_tests/tx_read_storage_key.wasm index 78edf6da4a..751e62576e 100755 Binary files a/wasm_for_tests/tx_read_storage_key.wasm and b/wasm_for_tests/tx_read_storage_key.wasm differ diff --git a/wasm_for_tests/tx_write.wasm b/wasm_for_tests/tx_write.wasm index 100caa864b..6ea562fd9c 100755 Binary files a/wasm_for_tests/tx_write.wasm and b/wasm_for_tests/tx_write.wasm differ diff --git a/wasm_for_tests/tx_write_storage_key.wasm b/wasm_for_tests/tx_write_storage_key.wasm index 5822b157de..aa036451e1 100755 Binary files a/wasm_for_tests/tx_write_storage_key.wasm and b/wasm_for_tests/tx_write_storage_key.wasm differ diff --git a/wasm_for_tests/vp_always_false.wasm b/wasm_for_tests/vp_always_false.wasm index f4a92884fa..e13711eb58 100755 Binary files a/wasm_for_tests/vp_always_false.wasm and b/wasm_for_tests/vp_always_false.wasm differ diff --git a/wasm_for_tests/vp_always_true.wasm b/wasm_for_tests/vp_always_true.wasm index 43da86f12b..fbecf604d0 100755 Binary files a/wasm_for_tests/vp_always_true.wasm and b/wasm_for_tests/vp_always_true.wasm differ diff --git a/wasm_for_tests/vp_eval.wasm b/wasm_for_tests/vp_eval.wasm index bfe3fd54f2..28de9aefeb 100755 Binary files a/wasm_for_tests/vp_eval.wasm and b/wasm_for_tests/vp_eval.wasm differ diff --git a/wasm_for_tests/vp_memory_limit.wasm b/wasm_for_tests/vp_memory_limit.wasm index 2be9bf6a59..9e2949874d 100755 Binary files a/wasm_for_tests/vp_memory_limit.wasm and b/wasm_for_tests/vp_memory_limit.wasm differ diff --git a/wasm_for_tests/vp_read_storage_key.wasm b/wasm_for_tests/vp_read_storage_key.wasm index 90f1187c29..aac8cfbb0e 100755 Binary files a/wasm_for_tests/vp_read_storage_key.wasm and b/wasm_for_tests/vp_read_storage_key.wasm differ diff --git a/wasm_for_tests/wasm_source/Cargo.toml b/wasm_for_tests/wasm_source/Cargo.toml index 257d8db8f6..c7f0ddcca8 100644 --- a/wasm_for_tests/wasm_source/Cargo.toml +++ b/wasm_for_tests/wasm_source/Cargo.toml @@ -22,6 +22,7 @@ vp_always_true = [] vp_eval = [] vp_memory_limit = [] vp_read_storage_key = [] +tx_invalid_data = [] tx_proposal_code = [] tx_proposal_masp_reward = [] diff --git a/wasm_for_tests/wasm_source/Makefile b/wasm_for_tests/wasm_source/Makefile index c8783dc001..688a39668b 100644 --- a/wasm_for_tests/wasm_source/Makefile +++ b/wasm_for_tests/wasm_source/Makefile @@ -15,6 +15,7 @@ wasms += vp_always_true wasms += vp_eval wasms += vp_memory_limit wasms += vp_read_storage_key +wasms += tx_invalid_data wasms += tx_proposal_code wasms += tx_proposal_masp_reward diff --git a/wasm_for_tests/wasm_source/src/lib.rs b/wasm_for_tests/wasm_source/src/lib.rs index 1345a5634c..1c9876c01f 100644 --- a/wasm_for_tests/wasm_source/src/lib.rs +++ b/wasm_for_tests/wasm_source/src/lib.rs @@ -49,7 +49,7 @@ pub mod main { ctx.write(&target_key, 9_u64)?; // parameters - let target_key = parameters_storage::get_tx_allowlist_storage_key(); + let target_key = parameters_storage::get_vp_allowlist_storage_key(); ctx.write(&target_key, vec!["hash"])?; Ok(()) } @@ -158,6 +158,22 @@ pub mod main { } } +#[cfg(feature = "tx_invalid_data")] +pub mod main { + use namada_tx_prelude::*; + + #[transaction(gas = 1000)] + fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { + let signed = tx_data; + let _data = + signed.data().ok_or_err_msg("Missing data").map_err(|err| { + ctx.set_commitment_sentinel(); + err + })?; + Ok(()) + } +} + /// A VP that always returns `true`. #[cfg(feature = "vp_always_true")] pub mod main {