From 7723173e4b1fc6724a964b11232449db4a54e362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Aug 2024 13:06:19 +0100 Subject: [PATCH 1/2] use typed ChainId instead of string in all `fn get_chain_id` --- crates/governance/src/finalize_block.rs | 5 ++--- crates/ibc/src/actions.rs | 3 ++- crates/ibc/src/vp/context.rs | 6 +++--- crates/ibc/src/vp/mod.rs | 2 +- crates/state/src/in_memory.rs | 4 ++-- crates/state/src/lib.rs | 4 ++-- crates/storage/src/lib.rs | 7 ++++--- crates/tx_prelude/src/lib.rs | 10 +++++++--- crates/vm/src/host_env.rs | 4 ++-- crates/vp/src/native_vp.rs | 8 ++++---- crates/vp/src/vp_host_fns.rs | 4 ++-- crates/vp_env/src/lib.rs | 3 ++- crates/vp_prelude/src/lib.rs | 16 +++++++++------- 13 files changed, 42 insertions(+), 34 deletions(-) diff --git a/crates/governance/src/finalize_block.rs b/crates/governance/src/finalize_block.rs index 6f1666a7f3..6eea729bc9 100644 --- a/crates/governance/src/finalize_block.rs +++ b/crates/governance/src/finalize_block.rs @@ -1,11 +1,10 @@ //! Governance logic applied on an end of a block. use std::collections::BTreeSet; -use std::str::FromStr; use borsh::BorshDeserialize; use namada_core::address::Address; -use namada_core::chain::{ChainId, Epoch}; +use namada_core::chain::Epoch; use namada_core::collections::HashMap; use namada_core::encode; use namada_core::ibc::PGFIbcTarget; @@ -374,7 +373,7 @@ where state.write(&pending_execution_key, ())?; let mut tx = Tx::from_type(TxType::Raw); - tx.header.chain_id = ChainId::from_str(&state.get_chain_id()?).unwrap(); + tx.header.chain_id = state.get_chain_id()?; tx.set_data(Data::new(encode(&id))); tx.set_code(Code::new(proposal_code, None)); diff --git a/crates/ibc/src/actions.rs b/crates/ibc/src/actions.rs index 8c793017a0..f5ce305ace 100644 --- a/crates/ibc/src/actions.rs +++ b/crates/ibc/src/actions.rs @@ -13,6 +13,7 @@ use ibc::apps::transfer::types::PrefixedCoin; use ibc::core::channel::types::timeout::TimeoutHeight; use namada_core::address::Address; use namada_core::borsh::{BorshSerialize, BorshSerializeExt}; +use namada_core::chain::ChainId; use namada_core::ibc::PGFIbcTarget; use namada_core::tendermint::Time as TmTime; use namada_core::token::Amount; @@ -65,7 +66,7 @@ where self.state.iter_next(iter) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { self.state.get_chain_id() } diff --git a/crates/ibc/src/vp/context.rs b/crates/ibc/src/vp/context.rs index 40f319b0a2..e3a4fec615 100644 --- a/crates/ibc/src/vp/context.rs +++ b/crates/ibc/src/vp/context.rs @@ -5,7 +5,7 @@ use std::marker::PhantomData; use namada_core::address::Address; use namada_core::arith::checked; -use namada_core::chain::{BlockHeader, BlockHeight, Epoch, Epochs}; +use namada_core::chain::{BlockHeader, BlockHeight, ChainId, Epoch, Epochs}; use namada_core::collections::{HashMap, HashSet}; use namada_core::storage::{Key, TxIndex}; use namada_events::Event; @@ -142,7 +142,7 @@ Self: 'iter; self.ctx.iter_next(iter) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { self.ctx.get_chain_id() } @@ -332,7 +332,7 @@ where self.ctx.iter_next(iter) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { self.ctx.get_chain_id() } diff --git a/crates/ibc/src/vp/mod.rs b/crates/ibc/src/vp/mod.rs index a5d5886d45..c6eeb1b0d2 100644 --- a/crates/ibc/src/vp/mod.rs +++ b/crates/ibc/src/vp/mod.rs @@ -322,7 +322,7 @@ where let unbonding_period_secs = checked!(pipeline_len * epoch_duration.min_duration.0)?; Ok(ValidationParams { - chain_id: IbcChainId::from_str(&chain_id) + chain_id: IbcChainId::from_str(chain_id.as_str()) .map_err(ActionError::ChainId)?, proof_specs: proof_specs .try_into() diff --git a/crates/state/src/in_memory.rs b/crates/state/src/in_memory.rs index 1f669420f6..fdc9175dd8 100644 --- a/crates/state/src/in_memory.rs +++ b/crates/state/src/in_memory.rs @@ -207,11 +207,11 @@ where } /// Get the chain ID as a raw string - pub fn get_chain_id(&self) -> (String, u64) { + pub fn get_chain_id(&self) -> (ChainId, u64) { // Adding consts that cannot overflow #[allow(clippy::arithmetic_side_effects)] ( - self.chain_id.to_string(), + self.chain_id.clone(), CHAIN_ID_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, ) } diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index 70de19f402..74b5b84844 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -32,6 +32,7 @@ pub use in_memory::{ }; use namada_core::address::Address; use namada_core::arith::checked; +use namada_core::chain::ChainId; pub use namada_core::chain::{ BlockHash, BlockHeader, BlockHeight, Epoch, Epochs, BLOCK_HASH_LENGTH, BLOCK_HEIGHT_LENGTH, @@ -290,7 +291,7 @@ macro_rules! impl_storage_read { fn get_chain_id( &self, - ) -> std::result::Result { + ) -> std::result::Result { let (chain_id, gas) = self.in_mem().get_chain_id(); self.charge_gas(gas).into_storage_result()?; Ok(chain_id) @@ -587,7 +588,6 @@ pub mod testing { use clru::CLruCache; use namada_core::address; use namada_core::address::EstablishedAddressGen; - use namada_core::chain::ChainId; use namada_core::time::DateTimeUtc; pub use namada_storage::testing::{PrefixIter, *}; use namada_storage::tx_queue::ExpiredTxsQueue; diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index fad41bdd21..7c923f9255 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -36,6 +36,7 @@ pub use db::{Error as DbError, Result as DbResult, *}; pub use error::{CustomError, Error, OptionExt, Result, ResultExt}; use namada_core::address::Address; use namada_core::borsh::{BorshDeserialize, BorshSerialize, BorshSerializeExt}; +use namada_core::chain::ChainId; pub use namada_core::chain::{ BlockHash, BlockHeader, BlockHeight, Epoch, Epochs, }; @@ -85,7 +86,7 @@ pub trait StorageRead { ) -> Result)>>; /// Getting the chain ID. - fn get_chain_id(&self) -> Result; + fn get_chain_id(&self) -> Result; /// Getting the block height. The height is that of the block to which the /// current transaction is being applied. @@ -385,8 +386,8 @@ pub mod testing { Ok(iter.next()) } - fn get_chain_id(&self) -> Result { - Ok(self.chain_id.to_string()) + fn get_chain_id(&self) -> Result { + Ok(self.chain_id.clone()) } fn get_block_height(&self) -> Result { diff --git a/crates/tx_prelude/src/lib.rs b/crates/tx_prelude/src/lib.rs index d7767e9892..6297857ce8 100644 --- a/crates/tx_prelude/src/lib.rs +++ b/crates/tx_prelude/src/lib.rs @@ -21,7 +21,9 @@ pub mod token; use core::slice; use std::marker::PhantomData; +use std::str::FromStr; +use chain::ChainId; use namada_account::AccountPublicKeysMap; pub use namada_core::address::Address; pub use namada_core::borsh::{ @@ -156,15 +158,17 @@ impl StorageRead for Ctx { Ok(HostEnvResult::is_success(found)) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { let result = Vec::with_capacity(CHAIN_ID_LENGTH); unsafe { namada_tx_get_chain_id(result.as_ptr() as _); } let slice = unsafe { slice::from_raw_parts(result.as_ptr(), CHAIN_ID_LENGTH) }; - Ok(String::from_utf8(slice.to_vec()) - .expect("Cannot convert the ID string")) + Ok(ChainId::from_str( + std::str::from_utf8(slice).expect("Chain ID must be valid utf8"), + ) + .expect("Chain ID must be valid")) } fn get_block_height(&self) -> Result { diff --git a/crates/vm/src/host_env.rs b/crates/vm/src/host_env.rs index 027ecbcbfb..02288e7779 100644 --- a/crates/vm/src/host_env.rs +++ b/crates/vm/src/host_env.rs @@ -1563,7 +1563,7 @@ where tx_charge_gas::(env, gas)?; let gas = env .memory - .write_string(result_ptr, chain_id) + .write_string(result_ptr, chain_id.to_string()) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas) } @@ -1748,7 +1748,7 @@ where let chain_id = vp_host_fns::get_chain_id(gas_meter, &state)?; let gas = env .memory - .write_string(result_ptr, chain_id) + .write_string(result_ptr, chain_id.to_string()) .map_err(Into::into)?; vp_host_fns::add_gas(gas_meter, gas) } diff --git a/crates/vp/src/native_vp.rs b/crates/vp/src/native_vp.rs index 4f01ab4f01..97fbdef331 100644 --- a/crates/vp/src/native_vp.rs +++ b/crates/vp/src/native_vp.rs @@ -9,7 +9,7 @@ use std::marker::PhantomData; use namada_core::borsh; use namada_core::borsh::BorshDeserialize; -use namada_core::chain::Epochs; +use namada_core::chain::{ChainId, Epochs}; use namada_gas::{GasMetering, VpGasMeter}; use namada_tx::{BatchedTxRef, Tx, TxCommitments}; @@ -202,7 +202,7 @@ where .into_storage_result() } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { self.ctx.get_chain_id() } @@ -277,7 +277,7 @@ where .into_storage_result() } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { self.ctx.get_chain_id() } @@ -341,7 +341,7 @@ where .into_storage_result() } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { vp_host_fns::get_chain_id(self.gas_meter, self.state) .into_storage_result() } diff --git a/crates/vp/src/vp_host_fns.rs b/crates/vp/src/vp_host_fns.rs index 48c12099c6..ad028b25df 100644 --- a/crates/vp/src/vp_host_fns.rs +++ b/crates/vp/src/vp_host_fns.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use namada_core::address::{Address, ESTABLISHED_ADDRESS_BYTES_LEN}; use namada_core::arith::checked; -use namada_core::chain::{BlockHeader, BlockHeight, Epoch, Epochs}; +use namada_core::chain::{BlockHeader, BlockHeight, ChainId, Epoch, Epochs}; use namada_core::hash::{Hash, HASH_LENGTH}; use namada_core::storage::{Key, TxIndex, TX_INDEX_LENGTH}; use namada_events::{Event, EventTypeBuilder}; @@ -201,7 +201,7 @@ where pub fn get_chain_id( gas_meter: &RefCell, state: &S, -) -> Result +) -> Result where S: StateRead + Debug, { diff --git a/crates/vp_env/src/lib.rs b/crates/vp_env/src/lib.rs index 4a957a7064..5e19e9a51b 100644 --- a/crates/vp_env/src/lib.rs +++ b/crates/vp_env/src/lib.rs @@ -22,6 +22,7 @@ pub mod collection_validation; use namada_core::address::Address; use namada_core::borsh::BorshDeserialize; +use namada_core::chain::ChainId; pub use namada_core::chain::{BlockHeader, BlockHeight, Epoch, Epochs}; use namada_core::hash::Hash; pub use namada_core::storage::{Key, TxIndex}; @@ -67,7 +68,7 @@ where ) -> Result>, namada_storage::Error>; /// Getting the chain ID. - fn get_chain_id(&self) -> Result; + fn get_chain_id(&self) -> Result; /// Getting the block height. The height is that of the block to which the /// current transaction is being applied. diff --git a/crates/vp_prelude/src/lib.rs b/crates/vp_prelude/src/lib.rs index 46d52ab619..0b4ed9cff4 100644 --- a/crates/vp_prelude/src/lib.rs +++ b/crates/vp_prelude/src/lib.rs @@ -23,7 +23,9 @@ pub mod ibc { use core::slice; pub use std::collections::BTreeSet; use std::marker::PhantomData; +use std::str::FromStr; +use chain::ChainId; pub use namada_core::address::Address; pub use namada_core::borsh::{ BorshDeserialize, BorshSerialize, BorshSerializeExt, @@ -296,7 +298,7 @@ impl<'view> VpEnv<'view> for Ctx { Ok(read_from_buffer(read_result, namada_vp_result_buffer)) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { // Both `CtxPreStorageRead` and `CtxPostStorageRead` have the same impl get_chain_id() } @@ -447,7 +449,7 @@ impl StorageRead for CtxPreStorageRead<'_> { )) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { get_chain_id() } @@ -517,7 +519,7 @@ impl StorageRead for CtxPostStorageRead<'_> { )) } - fn get_chain_id(&self) -> Result { + fn get_chain_id(&self) -> Result { get_chain_id() } @@ -569,17 +571,17 @@ fn iter_prefix_post_impl( Ok(KeyValIterator(iter_id, PhantomData)) } -fn get_chain_id() -> Result { +fn get_chain_id() -> Result { let result = Vec::with_capacity(CHAIN_ID_LENGTH); unsafe { namada_vp_get_chain_id(result.as_ptr() as _); } let slice = unsafe { slice::from_raw_parts(result.as_ptr(), CHAIN_ID_LENGTH) }; - Ok( - String::from_utf8(slice.to_vec()) - .expect("Cannot convert the ID string"), + Ok(ChainId::from_str( + std::str::from_utf8(slice).expect("Chain ID must be valid utf8"), ) + .expect("Chain ID must be valid")) } fn get_block_height() -> Result { From 82718057d4bbfabcdbd2915de28db33a3cc7edd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Aug 2024 13:10:44 +0100 Subject: [PATCH 2/2] changelog: add #3733 --- .changelog/unreleased/improvements/3733-typed-chain-id.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3733-typed-chain-id.md diff --git a/.changelog/unreleased/improvements/3733-typed-chain-id.md b/.changelog/unreleased/improvements/3733-typed-chain-id.md new file mode 100644 index 0000000000..2e45cbd8da --- /dev/null +++ b/.changelog/unreleased/improvements/3733-typed-chain-id.md @@ -0,0 +1,2 @@ +- Switched to use typed ChainId instead of a string in all `fn get_chain_id`. + ([\#3733](https://github.com/anoma/namada/pull/3733)) \ No newline at end of file