diff --git a/.changelog/unreleased/improvements/4690-wasm-name-lookups.md b/.changelog/unreleased/improvements/4690-wasm-name-lookups.md new file mode 100644 index 00000000000..9a376c6dfae --- /dev/null +++ b/.changelog/unreleased/improvements/4690-wasm-name-lookups.md @@ -0,0 +1 @@ +- Indexers fail to identify the types of older txs since these types use wasm hashes. If those hashes were overwritten by governance proposals, the indexer is unable to determine how to deserialize these txs. This PR adds a new key to storage which allows looking up a tx type from a wasm hash, even if that hash is of code no longer in use. ([\#4690](https://github.com/anoma/namada/pull/4690)) diff --git a/.changelog/unreleased/improvements/4691-wasm-code-events.md b/.changelog/unreleased/improvements/4691-wasm-code-events.md new file mode 100644 index 00000000000..f5f37e3020b --- /dev/null +++ b/.changelog/unreleased/improvements/4691-wasm-code-events.md @@ -0,0 +1 @@ + - For each inner tx, an event is emitted giving the human readable wasm name of the code contained in said tx.([\#4691](https://github.com/anoma/namada/pull/4691)) \ No newline at end of file diff --git a/crates/core/src/storage.rs b/crates/core/src/storage.rs index 2ac9959df1f..91c66dbe488 100644 --- a/crates/core/src/storage.rs +++ b/crates/core/src/storage.rs @@ -551,7 +551,7 @@ impl Key { } /// Returns a key of wasm code's hash of the given name - pub fn wasm_code_name(code_name: String) -> Self { + pub fn wasm_code_hash(code_name: String) -> Self { let mut segments = Self::from(PARAMETERS.to_owned().to_db_key()).segments; segments.push(DbKeySeg::StringSeg(WASM_KEY_PREFIX.to_owned())); @@ -580,6 +580,16 @@ impl Key { Key { segments } } + /// Returns a key of the wasm name of the given code hash + pub fn wasm_code_name(code_hash: &Hash) -> Self { + let mut segments = + Self::from(PARAMETERS.to_owned().to_db_key()).segments; + segments.push(DbKeySeg::StringSeg(WASM_KEY_PREFIX.to_owned())); + segments.push(DbKeySeg::StringSeg(WASM_HASH_PREFIX.to_owned())); + segments.push(DbKeySeg::StringSeg(code_hash.to_string())); + Key { segments } + } + /// Returns a key of the validity predicate of the given address /// Only this function can push "?" segment for validity predicate pub fn validity_predicate(addr: &Address) -> Self { diff --git a/crates/events/src/extend.rs b/crates/events/src/extend.rs index 0ff64abd747..a2282a04312 100644 --- a/crates/events/src/extend.rs +++ b/crates/events/src/extend.rs @@ -493,6 +493,20 @@ impl EventAttributeEntry<'static> for Height { } } +/// Extend an [`Event`] with the name of the wasm code. +pub struct CodeName(pub String); + +impl EventAttributeEntry<'static> for CodeName { + type Value = String; + type ValueOwned = Self::Value; + + const KEY: &'static str = "code-name"; + + fn into_value(self) -> Self::Value { + self.0 + } +} + /// Extend an [`Event`] with transaction hash information. pub struct TxHash(pub Hash); diff --git a/crates/migrations/src/foreign_types.rs b/crates/migrations/src/foreign_types.rs index 9f31ecfbd08..7b7dd50c0b8 100644 --- a/crates/migrations/src/foreign_types.rs +++ b/crates/migrations/src/foreign_types.rs @@ -8,6 +8,7 @@ use namada_macros::derive_typehash; use crate::TypeHash; +derive_typehash!(String); derive_typehash!(Vec::); derive_typehash!(Vec::); derive_typehash!(u64); diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index db8eed3d1dc..c84bca6f265 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -21,8 +21,8 @@ use namada_sdk::state::{ use namada_sdk::storage::{BlockHeader, BlockResults, Epoch}; use namada_sdk::tx::data::protocol::ProtocolTxType; use namada_sdk::tx::data::{VpStatusFlags, compute_inner_tx_hash}; -use namada_sdk::tx::event::{Batch, Code}; -use namada_sdk::tx::new_tx_event; +use namada_sdk::tx::event::{Batch, Code, TxWasmEvent}; +use namada_sdk::tx::{TxCommitments, new_tx_event}; use namada_sdk::{ibc, proof_of_stake}; use namada_vote_ext::ethereum_events::MultiSignedEthEvent; use namada_vote_ext::ethereum_tx_data_variants; @@ -363,12 +363,18 @@ where tx_data.tx.wrapper_hash().as_ref(), Either::Right(cmt), ); + self.emit_wasm_name_event( + &tx_data, + cmt, + inner_tx_hash, + response, + ); if let Some(Ok(batched_result)) = tx_result.get_mut(&inner_tx_hash) { if batched_result.is_accepted() { // Take the events from the batch result to - // avoid emitting them again after the exection + // avoid emitting them again after the execution // of the entire batch response.events.emit_many( std::mem::take(&mut batched_result.events) @@ -470,6 +476,40 @@ where None } + // a special event is emitted for each inner tx giving the human readable + // name of its wasm code. + fn emit_wasm_name_event( + &self, + tx_data: &TxData<'_>, + cmt: &TxCommitments, + inner_tx_hash: Hash, + response: &mut shim::response::FinalizeBlock, + ) { + // emit an event that gives the human readable name + // of the wasm payload. + let code_name = match tx_data + .tx + .get_section(&cmt.code_hash) + .unwrap() + .code_sec() + .map(|c| c.code.hash()) + { + None => "".to_string(), + Some(hash) => self + .state + .read(&Key::wasm_code_name(&hash)) + .expect("Reading wasm name from storage should not fail") + .unwrap_or("".to_string()), + }; + + let code_name_event = TxWasmEvent { + hash: tx_data.tx.wrapper_hash(), + inner_tx_hash, + name: code_name, + }; + response.events.emit(code_name_event); + } + // Evaluate the results of all the transactions of the batch. Commit or drop // the storage changes, update stats and event, manage replay protection. fn handle_inner_tx_results( @@ -1302,7 +1342,7 @@ mod test_finalize_block { }; use namada_sdk::ethereum_events::{EthAddress, Uint as ethUint}; use namada_sdk::events::Event; - use namada_sdk::events::extend::Log; + use namada_sdk::events::extend::{CodeName, Log}; use namada_sdk::gas::VpGasMeter; use namada_sdk::governance::storage::keys::get_proposal_execution_key; use namada_sdk::governance::storage::proposal::ProposalType; @@ -1519,6 +1559,7 @@ mod test_finalize_block { }) .expect("Test failed") .iter() + .filter(|e| e.read_attribute::().is_err()) { assert_eq!(*event.kind(), APPLIED_TX); let hash = event.read_attribute::().expect("Test failed"); @@ -3166,7 +3207,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed")[0]; + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>()[0]; assert_eq!(*event.kind(), APPLIED_TX); let code = event.read_attribute::().expect("Test failed"); assert_eq!(code, ResultCode::Ok); @@ -3325,7 +3369,10 @@ mod test_finalize_block { txs: processed_txs, ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); // the merkle tree root should not change after finalize_block let root_post = shell.shell.state.in_mem().block.tree.root(); @@ -3453,7 +3500,10 @@ mod test_finalize_block { txs: processed_txs, ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); // the merkle tree root should not change after finalize_block let root_post = shell.shell.state.in_mem().block.tree.root(); @@ -3582,7 +3632,10 @@ mod test_finalize_block { txs: processed_txs, ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); // the merkle tree root should not change after finalize_block let root_post = shell.shell.state.in_mem().block.tree.root(); @@ -3814,7 +3867,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed")[0]; + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>()[0]; // Check balance of fee payer assert_eq!(*event.kind(), APPLIED_TX); @@ -3974,7 +4030,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed")[0]; + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>()[0]; // Check fee payment assert_eq!(*event.kind(), APPLIED_TX); @@ -4059,7 +4118,10 @@ mod test_finalize_block { proposer_address, ..Default::default() }) - .expect("Test failed")[0]; + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>()[0]; // Check fee payment assert_eq!(*event.kind(), APPLIED_TX); @@ -5791,7 +5853,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); let code = event[0].read_attribute::().unwrap(); assert_eq!(code, ResultCode::Ok); @@ -5839,7 +5904,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); let code = event[0].read_attribute::().unwrap(); assert_eq!(code, ResultCode::WasmRuntimeError); @@ -5898,7 +5966,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); let code = event[0].read_attribute::().unwrap(); assert_eq!(code, ResultCode::Ok); @@ -5975,7 +6046,10 @@ mod test_finalize_block { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); let code = event[0].read_attribute::().unwrap(); assert_eq!(code, ResultCode::WasmRuntimeError); @@ -6028,12 +6102,15 @@ mod test_finalize_block { let (batch, processed_tx) = mk_tx_batch(&shell, &sk, false, false, true); - let event = &shell + let event = shell .finalize_block(FinalizeBlock { txs: vec![processed_tx], ..Default::default() }) - .expect("Test failed"); + .expect("Test failed") + .into_iter() + .filter(|e| e.read_attribute::().is_err()) + .collect::>(); let code = event[0].read_attribute::().unwrap(); assert_eq!(code, ResultCode::WasmRuntimeError); @@ -6121,7 +6198,27 @@ mod test_finalize_block { batch }; + let commitments = batch_tx.commitments().first().expect("Test failed"); + shell + .state + .write( + &Key::wasm_code_name( + &batch_tx + .get_section(&commitments.code_hash) + .unwrap() + .code_sec() + .map(|c| c.code.hash()) + .unwrap(), + ), + TestWasms::TxNoOpEvent + .path() + .file_name() + .unwrap() + .to_string_lossy() + .to_string(), + ) + .expect("Test failed"); let processed_txs = vec![ProcessedTx { tx: batch_tx.to_bytes().into(), result: TxResult { @@ -6136,14 +6233,19 @@ mod test_finalize_block { ..Default::default() }) .expect("Test failed"); - - // three top level events - assert_eq!(events.len(), 3); + // five top level events + assert_eq!(events.len(), 5); // tx events. Check that they are present and in the correct order // TODO(namada#3856): right now we lose events ordering in a batch // because of the BTreeSet we use in BatchedTxResult so we can only // check the presence of the events but not the order + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, "tx_no_op_event.wasm"); + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, "tx_no_op_event.wasm"); let mut unordered_events = vec![]; let event = events.remove(0); let msg = event.read_attribute::().unwrap(); @@ -6220,7 +6322,7 @@ mod test_finalize_block { .expect("Test failed"); // three top level events - assert_eq!(events.len(), 3); + assert_eq!(events.len(), 5); // tx events. Check that they are both present even if they are // identical. This is important because some inner txs of a single batch @@ -6228,6 +6330,12 @@ mod test_finalize_block { // overshadow each other which could deceive external tools like // indexers let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); + let event = events.remove(0); let msg = event.read_attribute::().unwrap(); assert_eq!(&msg, EVENT_MSG); @@ -6297,11 +6405,17 @@ mod test_finalize_block { .expect("Test failed"); // two top level events - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 4); // tx events. Check the expected ones are present and in the correct // order let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); + let event = events.remove(0); let msg = event.read_attribute::().unwrap(); assert_eq!(&msg, "bing"); @@ -6368,9 +6482,15 @@ mod test_finalize_block { .expect("Test failed"); // one top level event (no tx events, only tx result) - assert_eq!(events.len(), 1); + assert_eq!(events.len(), 3); // multiple tx results (2) + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); + let event = events.remove(0); + let code_name = event.read_attribute::().unwrap(); + assert_eq!(code_name, ""); let tx_event = events.remove(0); let tx_results = tx_event.read_attribute::>().unwrap(); assert_eq!(tx_results.len(), 2); diff --git a/crates/node/src/shell/init_chain.rs b/crates/node/src/shell/init_chain.rs index e14d0d7ac87..171c375ffad 100644 --- a/crates/node/src/shell/init_chain.rs +++ b/crates/node/src/shell/init_chain.rs @@ -418,7 +418,8 @@ where let code_key = Key::wasm_code(&code_hash); let code_len_key = Key::wasm_code_len(&code_hash); let hash_key = Key::wasm_hash(name); - let code_name_key = Key::wasm_code_name(name.to_owned()); + let code_hash_key = Key::wasm_code_hash(name.to_owned()); + let code_name_key = Key::wasm_code_name(&code_hash); self.state.write(&code_key, code).unwrap(); self.state.write(&code_len_key, code_len).unwrap(); @@ -426,7 +427,8 @@ where if &Some(code_hash) == implicit_vp_code_hash { is_implicit_vp_stored = true; } - self.state.write(&code_name_key, code_hash).unwrap(); + self.state.write(&code_hash_key, code_hash).unwrap(); + self.state.write(&code_name_key, name).unwrap(); } else { tracing::warn!("The wasm {name} isn't allowed."); self.warn(Warning::DisallowedWasm(name.to_string())); diff --git a/crates/tx/src/data/mod.rs b/crates/tx/src/data/mod.rs index 5121d8bf152..433f54f9401 100644 --- a/crates/tx/src/data/mod.rs +++ b/crates/tx/src/data/mod.rs @@ -772,7 +772,7 @@ mod test_process_tx { let batched_result = BatchedTxResult { changed_keys: [ - namada_account::Key::wasm_code_name("test-name".to_string()), + namada_account::Key::wasm_code_hash("test-name".to_string()), namada_account::Key::wasm_hash("test-name"), ] .into(), diff --git a/crates/tx/src/event.rs b/crates/tx/src/event.rs index 4240076b4d3..df6afaa9aac 100644 --- a/crates/tx/src/event.rs +++ b/crates/tx/src/event.rs @@ -4,10 +4,12 @@ use std::fmt::Display; use std::str::FromStr; use namada_core::borsh::{BorshDeserialize, BorshSerialize}; +use namada_core::hash::Hash; use namada_core::ibc::IbcTxDataHash; use namada_core::masp::MaspTxId; use namada_events::extend::{ - ComposeEvent, EventAttributeEntry, Height, Log, TxHash, + CodeName, ComposeEvent, EventAttributeEntry, Height, InnerTxHash, Log, + TxHash, }; use namada_events::{Event, EventLevel, EventToEmit, EventType}; use namada_macros::BorshDeserializer; @@ -204,6 +206,33 @@ impl From for Event { } } +/// An event that indicates the wasm payload of a tx +pub struct TxWasmEvent { + /// Hash of wrapper tx + pub hash: Option, + /// Hash of inner tx + pub inner_tx_hash: Hash, + /// The name of the wasm payload + pub name: String, +} + +impl EventToEmit for TxWasmEvent { + const DOMAIN: &'static str = "tx"; +} + +impl From for Event { + fn from(tx_wasm_event: TxWasmEvent) -> Self { + let composite = + Self::new(EventType::new("tx-wasm-name"), EventLevel::Tx) + .with(CodeName(tx_wasm_event.name)) + .with(InnerTxHash(tx_wasm_event.inner_tx_hash)); + match tx_wasm_event.hash { + None => composite.into(), + Some(hash) => composite.with(TxHash(hash)).into(), + } + } +} + impl EventAttributeEntry<'static> for MaspTxRef { type Value = MaspTxRef; type ValueOwned = Self::Value; diff --git a/examples/make-db-migration.rs b/examples/make-db-migration.rs index a86828f7eea..e9d767f9283 100644 --- a/examples/make-db-migration.rs +++ b/examples/make-db-migration.rs @@ -592,7 +592,8 @@ pub fn wasm_migration(updates: &mut Vec) { let code_key = Key::wasm_code(&new_code_hash); let code_len_key = Key::wasm_code_len(&new_code_hash); let hash_key = Key::wasm_hash(name); - let code_name_key = Key::wasm_code_name(name.to_owned()); + let code_hash_key = Key::wasm_code_hash(name.to_owned()); + let code_name_key = Key::wasm_code_name(&new_code_hash); updates.push(migrations::DbUpdateType::Add { key: code_key, @@ -613,11 +614,17 @@ pub fn wasm_migration(updates: &mut Vec) { force: false, }); updates.push(migrations::DbUpdateType::Add { - key: code_name_key, + key: code_hash_key, cf: DbColFam::SUBSPACE, value: new_code_hash.into(), force: false, }); + updates.push(migrations::DbUpdateType::Add { + key: code_name_key, + cf: DbColFam::SUBSPACE, + value: name.to_string().into(), + force: false, + }); } // Put the allow list in storage updates.push(migrations::DbUpdateType::Add { diff --git a/wasm_for_tests/tx_proposal_code/src/lib.rs b/wasm_for_tests/tx_proposal_code/src/lib.rs index 3996e41fbec..8221b7e4230 100644 --- a/wasm_for_tests/tx_proposal_code/src/lib.rs +++ b/wasm_for_tests/tx_proposal_code/src/lib.rs @@ -26,8 +26,11 @@ fn apply_tx(ctx: &mut Ctx, _tx_data: BatchedTx) -> TxResult { let wasm_code_len_key = Key::wasm_code_len(&wasm_code_hash); ctx.write(&wasm_code_len_key, 30.serialize_to_vec())?; - let wasm_code_name_key = Key::wasm_code_name("test".to_string()); - ctx.write_bytes(&wasm_code_name_key, wasm_code_name.clone())?; + let wasm_code_hash_key = Key::wasm_code_hash("test".to_string()); + ctx.write_bytes(&wasm_code_hash_key, wasm_code_hash)?; + + let wasm_code_name_key = Key::wasm_code_name(&wasm_code_hash); + ctx.write_bytes(&wasm_code_name_key, &wasm_code_name)?; let wasm_hash_key = Key::wasm_hash("test"); ctx.write_bytes(&wasm_hash_key, wasm_code_name)?;