From fb4ae44becaa764bed19269a1f31ec1f918139f3 Mon Sep 17 00:00:00 2001 From: Fraser Hutchison Date: Sat, 7 Sep 2024 17:56:13 +0100 Subject: [PATCH 01/11] exclusively use borsh encoding for stored data --- Cargo.lock | 2 + .../astria-cli/src/commands/bridge/submit.rs | 2 +- crates/astria-cli/src/commands/sequencer.rs | 2 +- .../astria-composer/src/executor/builder.rs | 2 +- .../src/celestia/reconstruct.rs | 24 +- .../astria-conductor/src/celestia/verify.rs | 8 +- crates/astria-conductor/src/executor/mod.rs | 9 +- .../tests/blackbox/helpers/mod.rs | 4 +- crates/astria-core/src/celestia.rs | 2 +- crates/astria-core/src/crypto.rs | 6 +- .../src/primitive/v1/asset/denom.rs | 44 +- crates/astria-core/src/primitive/v1/mod.rs | 68 +-- .../src/protocol/transaction/v1alpha1/mod.rs | 28 +- .../src/sequencerblock/v1alpha1/block.rs | 141 ++++-- .../src/sequencerblock/v1alpha1/celestia.rs | 14 +- crates/astria-merkle/src/audit.rs | 18 +- crates/astria-sequencer/Cargo.toml | 3 +- .../astria-sequencer/src/accounts/action.rs | 20 +- .../src/accounts/component.rs | 2 +- crates/astria-sequencer/src/accounts/mod.rs | 36 +- crates/astria-sequencer/src/accounts/query.rs | 10 +- .../src/accounts/state_ext.rs | 238 ++++----- .../astria-sequencer/src/address/state_ext.rs | 46 +- crates/astria-sequencer/src/api_state_ext.rs | 455 ++++++++---------- .../src/app/action_handler.rs | 2 +- crates/astria-sequencer/src/app/mod.rs | 83 ++-- ...ransaction_with_every_action_snapshot.snap | 61 +-- ..._changes__app_finalize_block_snapshot.snap | 57 +-- ...reaking_changes__app_genesis_snapshot.snap | 61 +-- crates/astria-sequencer/src/app/test_utils.rs | 44 +- .../src/app/tests_app/mempool.rs | 4 +- .../astria-sequencer/src/app/tests_app/mod.rs | 27 +- .../src/app/tests_block_fees.rs | 30 +- .../src/app/tests_breaking_changes.rs | 8 +- .../src/app/tests_execute_transaction.rs | 163 +++++-- crates/astria-sequencer/src/assets/query.rs | 2 +- .../astria-sequencer/src/assets/state_ext.rs | 173 ++++--- .../astria-sequencer/src/authority/action.rs | 38 +- .../src/authority/component.rs | 2 +- crates/astria-sequencer/src/authority/mod.rs | 46 +- .../src/authority/state_ext.rs | 64 ++- .../src/bridge/bridge_lock_action.rs | 33 +- .../src/bridge/bridge_sudo_change_action.rs | 41 +- .../src/bridge/bridge_unlock_action.rs | 40 +- .../astria-sequencer/src/bridge/component.rs | 8 +- .../src/bridge/init_bridge_account_action.rs | 26 +- crates/astria-sequencer/src/bridge/query.rs | 28 +- .../astria-sequencer/src/bridge/state_ext.rs | 420 ++++++++++------ .../astria-sequencer/src/fee_asset_change.rs | 4 +- crates/astria-sequencer/src/grpc/sequencer.rs | 26 +- crates/astria-sequencer/src/ibc/component.rs | 3 +- .../src/ibc/ibc_relayer_change.rs | 4 +- .../src/ibc/ics20_transfer.rs | 123 +++-- .../src/ibc/ics20_withdrawal.rs | 82 ++-- crates/astria-sequencer/src/ibc/state_ext.rs | 131 ++--- crates/astria-sequencer/src/lib.rs | 1 + .../src/mempool/benchmarks.rs | 10 +- .../src/mempool/mempool_state.rs | 20 +- crates/astria-sequencer/src/mempool/mod.rs | 65 +-- .../src/mempool/transactions_container.rs | 75 ++- .../astria-sequencer/src/sequence/action.rs | 4 +- .../src/sequence/component.rs | 6 +- .../src/sequence/state_ext.rs | 46 +- .../astria-sequencer/src/service/consensus.rs | 2 +- .../astria-sequencer/src/service/info/mod.rs | 22 +- .../astria-sequencer/src/service/mempool.rs | 6 +- crates/astria-sequencer/src/state_ext.rs | 133 ++--- crates/astria-sequencer/src/storage/mod.rs | 26 + .../src/storage/stored_value/address_bytes.rs | 46 ++ .../storage/stored_value/address_prefix.rs | 34 ++ .../src/storage/stored_value/balance.rs | 32 ++ .../src/storage/stored_value/block_hash.rs | 34 ++ .../src/storage/stored_value/block_height.rs | 32 ++ .../storage/stored_value/block_timestamp.rs | 70 +++ .../src/storage/stored_value/chain_id.rs | 53 ++ .../src/storage/stored_value/deposit.rs | 107 ++++ .../src/storage/stored_value/fee.rs | 32 ++ .../stored_value/ibc_prefixed_denom.rs | 35 ++ .../src/storage/stored_value/mod.rs | 118 +++++ .../src/storage/stored_value/nonce.rs | 32 ++ .../src/storage/stored_value/proof.rs | 53 ++ .../storage/stored_value/revision_number.rs | 32 ++ .../src/storage/stored_value/rollup_id.rs | 35 ++ .../src/storage/stored_value/rollup_ids.rs | 36 ++ .../stored_value/rollup_transactions.rs | 55 +++ .../stored_value/sequencer_block_header.rs | 67 +++ .../storage/stored_value/storage_version.rs | 32 ++ .../stored_value/trace_prefixed_denom.rs | 50 ++ .../storage/stored_value/transaction_hash.rs | 34 ++ .../src/storage/stored_value/validator_set.rs | 84 ++++ .../src/transaction/checks.rs | 54 ++- .../astria-sequencer/src/transaction/mod.rs | 13 +- .../astria-sequencer/src/transaction/query.rs | 2 +- .../src/transaction/state_ext.rs | 4 +- .../astria-telemetry/src/metrics/builders.rs | 2 +- 95 files changed, 3056 insertions(+), 1556 deletions(-) create mode 100644 crates/astria-sequencer/src/storage/mod.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/address_bytes.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/address_prefix.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/balance.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/block_hash.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/block_height.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/block_timestamp.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/chain_id.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/deposit.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/fee.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/ibc_prefixed_denom.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/mod.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/nonce.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/proof.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/revision_number.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/rollup_id.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/rollup_ids.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/rollup_transactions.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/sequencer_block_header.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/storage_version.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/trace_prefixed_denom.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/transaction_hash.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value/validator_set.rs diff --git a/Cargo.lock b/Cargo.lock index b6e24ae1f4..35463c28c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -807,6 +807,7 @@ dependencies = [ "ibc-proto", "ibc-types", "insta", + "itertools 0.12.1", "matchit", "penumbra-ibc", "penumbra-proto", @@ -1398,6 +1399,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6362ed55def622cddc70a4746a68554d7b687713770de539e59a739b249f8ed" dependencies = [ "borsh-derive", + "bytes", "cfg_aliases", ] diff --git a/crates/astria-cli/src/commands/bridge/submit.rs b/crates/astria-cli/src/commands/bridge/submit.rs index c332a5a6d1..56d007c947 100644 --- a/crates/astria-cli/src/commands/bridge/submit.rs +++ b/crates/astria-cli/src/commands/bridge/submit.rs @@ -119,7 +119,7 @@ async fn submit_transaction( actions: Vec, ) -> eyre::Result { let from_address = Address::builder() - .array(signing_key.verification_key().address_bytes()) + .array(*signing_key.verification_key().address_bytes()) .prefix(prefix) .try_build() .wrap_err("failed constructing a valid from address from the provided prefix")?; diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index 46259e4763..c652548cfa 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -464,7 +464,7 @@ async fn submit_transaction( let sequencer_key = SigningKey::from(private_key_bytes); let from_address = Address::builder() - .array(sequencer_key.verification_key().address_bytes()) + .array(*sequencer_key.verification_key().address_bytes()) .prefix(prefix) .try_build() .wrap_err("failed constructing a valid from address from the provided prefix")?; diff --git a/crates/astria-composer/src/executor/builder.rs b/crates/astria-composer/src/executor/builder.rs index 871a4f1b33..7234600f05 100644 --- a/crates/astria-composer/src/executor/builder.rs +++ b/crates/astria-composer/src/executor/builder.rs @@ -58,7 +58,7 @@ impl Builder { let sequencer_address = Address::builder() .prefix(sequencer_address_prefix) - .array(sequencer_key.verification_key().address_bytes()) + .array(*sequencer_key.verification_key().address_bytes()) .try_build() .wrap_err("failed constructing a sequencer address from private key")?; diff --git a/crates/astria-conductor/src/celestia/reconstruct.rs b/crates/astria-conductor/src/celestia/reconstruct.rs index 9deae218e1..24c2af7a3d 100644 --- a/crates/astria-conductor/src/celestia/reconstruct.rs +++ b/crates/astria-conductor/src/celestia/reconstruct.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use astria_core::{ primitive::v1::RollupId, sequencerblock::v1alpha1::{ + celestia::UncheckedSubmittedMetadata, SubmittedMetadata, SubmittedRollupData, }, @@ -52,14 +53,19 @@ pub(super) fn reconstruct_blocks_from_verified_blobs( if let Some(header_blob) = remove_header_blob_matching_rollup_blob(&mut header_blobs, &rollup) { + let UncheckedSubmittedMetadata { + block_hash, + header, + .. + } = header_blob.into_unchecked(); reconstructed_blocks.push(ReconstructedBlock { celestia_height, - block_hash: header_blob.block_hash(), - header: header_blob.into_unchecked().header, + block_hash, + header, transactions: rollup.into_unchecked().transactions, }); } else { - let reason = if header_blobs.contains_key(&rollup.sequencer_block_hash()) { + let reason = if header_blobs.contains_key(rollup.sequencer_block_hash()) { "sequencer header blobs with the same block hash as the rollup blob found, but the \ rollup's Merkle proof did not lead any Merkle roots" } else { @@ -77,13 +83,13 @@ pub(super) fn reconstruct_blocks_from_verified_blobs( for header_blob in header_blobs.into_values() { if header_blob.contains_rollup_id(rollup_id) { warn!( - block_hash = %base64(&header_blob.block_hash()), + block_hash = %base64(header_blob.block_hash()), "sequencer header blob contains the target rollup ID, but no matching rollup blob was found; dropping it", ); } else { reconstructed_blocks.push(ReconstructedBlock { celestia_height, - block_hash: header_blob.block_hash(), + block_hash: *header_blob.block_hash(), header: header_blob.into_unchecked().header, transactions: vec![], }); @@ -98,11 +104,11 @@ fn remove_header_blob_matching_rollup_blob( ) -> Option { // chaining methods and returning () to use the ? operator and to not bind the value headers - .get(&rollup.sequencer_block_hash()) + .get(rollup.sequencer_block_hash()) .and_then(|header| { verify_rollup_blob_against_sequencer_blob(rollup, header).then_some(()) })?; - headers.remove(&rollup.sequencer_block_hash()) + headers.remove(rollup.sequencer_block_hash()) } fn verify_rollup_blob_against_sequencer_blob( @@ -112,9 +118,9 @@ fn verify_rollup_blob_against_sequencer_blob( rollup_blob .proof() .audit() - .with_root(sequencer_blob.rollup_transactions_root()) + .with_root(*sequencer_blob.rollup_transactions_root()) .with_leaf_builder() - .write(&rollup_blob.rollup_id().get()) + .write(rollup_blob.rollup_id().get()) .write(&merkle::Tree::from_leaves(rollup_blob.transactions()).root()) .finish_leaf() .perform() diff --git a/crates/astria-conductor/src/celestia/verify.rs b/crates/astria-conductor/src/celestia/verify.rs index 67b964ed9c..ae66d5b672 100644 --- a/crates/astria-conductor/src/celestia/verify.rs +++ b/crates/astria-conductor/src/celestia/verify.rs @@ -121,7 +121,7 @@ pub(super) async fn verify_metadata( verification_tasks.spawn( VerificationTaskKey { index, - block_hash: blob.block_hash(), + block_hash: *blob.block_hash(), sequencer_height: blob.height(), }, blob_verifier @@ -136,10 +136,10 @@ pub(super) async fn verify_metadata( match verification_result { Ok(Some(verified_blob)) => { if let Some(dropped_entry) = - verified_header_blobs.insert(verified_blob.block_hash(), verified_blob) + verified_header_blobs.insert(*verified_blob.block_hash(), verified_blob) { let accepted_entry = verified_header_blobs - .get(&dropped_entry.block_hash()) + .get(dropped_entry.block_hash()) .expect("must exist; just inserted an item under the same key"); info!( block_hash = %base64(&dropped_entry.block_hash()), @@ -291,7 +291,7 @@ impl BlobVerifier { .and_then(|()| { ensure_block_hashes_match( cached.commit_header.commit.block_id.hash.as_bytes(), - &metadata.block_hash(), + metadata.block_hash(), ) }) { info!(reason = %error, "failed to verify metadata retrieved from Celestia; dropping it"); diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 916a951e28..aa55b76dd8 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -718,19 +718,20 @@ impl ExecutableBlock { } fn from_sequencer(block: FilteredSequencerBlock, id: RollupId) -> Self { - let hash = block.block_hash(); - let height = block.height(); - let timestamp = convert_tendermint_time_to_protobuf_timestamp(block.header().time()); let FilteredSequencerBlockParts { + block_hash, + header, mut rollup_transactions, .. } = block.into_parts(); + let height = header.height(); + let timestamp = convert_tendermint_time_to_protobuf_timestamp(header.time()); let transactions = rollup_transactions .swap_remove(&id) .map(|txs| txs.transactions().to_vec()) .unwrap_or_default(); Self { - hash, + hash: block_hash, height, timestamp, transactions, diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index 3dfdf28c4c..35f1202855 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -44,7 +44,7 @@ use tracing::debug; pub const CELESTIA_BEARER_TOKEN: &str = "ABCDEFGH"; pub const ROLLUP_ID: RollupId = RollupId::new([42; 32]); -pub static ROLLUP_ID_BYTES: Bytes = Bytes::from_static(&RollupId::get(ROLLUP_ID)); +pub static ROLLUP_ID_BYTES: Bytes = Bytes::from_static(ROLLUP_ID.get()); pub const SEQUENCER_CHAIN_ID: &str = "test_sequencer-1000"; @@ -594,7 +594,7 @@ pub fn make_commit(height: u32) -> tendermint::block::Commit { let signing_key = signing_key(); let validator = validator(); - let block_hash = make_sequencer_block(height).block_hash(); + let block_hash = *make_sequencer_block(height).block_hash(); let timestamp = tendermint::Time::from_unix_timestamp(1, 1).unwrap(); let canonical_vote = tendermint::vote::CanonicalVote { diff --git a/crates/astria-core/src/celestia.rs b/crates/astria-core/src/celestia.rs index 9fc448c521..33f565ce1d 100644 --- a/crates/astria-core/src/celestia.rs +++ b/crates/astria-core/src/celestia.rs @@ -21,7 +21,7 @@ pub const fn namespace_v0_from_first_10_bytes(bytes: &[u8]) -> Namespace { /// 10 bytes of [`crate::primitive::v1::RollupId`]. #[must_use = "a celestia namespace must be used in order to be useful"] pub const fn namespace_v0_from_rollup_id(rollup_id: crate::primitive::v1::RollupId) -> Namespace { - namespace_v0_from_first_10_bytes(&rollup_id.get()) + namespace_v0_from_first_10_bytes(rollup_id.get()) } /// Constructs a [`celestia_types::nmt::Namespace`] from the first 10 bytes of the sha256 hash of diff --git a/crates/astria-core/src/crypto.rs b/crates/astria-core/src/crypto.rs index a0d7290f51..d721ee9049 100644 --- a/crates/astria-core/src/crypto.rs +++ b/crates/astria-core/src/crypto.rs @@ -86,7 +86,7 @@ impl SigningKey { /// Returns the address bytes of the verification key associated with this signing key. #[must_use] pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - self.verification_key().address_bytes() + *self.verification_key().address_bytes() } /// Attempts to create an Astria bech32m `[Address]` with the given prefix. @@ -167,8 +167,8 @@ impl VerificationKey { /// /// The address is the first 20 bytes of the sha256 hash of the verification key. #[must_use] - pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - *self.address_bytes.get_or_init(|| { + pub fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { + self.address_bytes.get_or_init(|| { fn first_20(array: [u8; 32]) -> [u8; ADDRESS_LEN] { [ array[0], array[1], array[2], array[3], array[4], array[5], array[6], array[7], diff --git a/crates/astria-core/src/primitive/v1/asset/denom.rs b/crates/astria-core/src/primitive/v1/asset/denom.rs index 2e8380ae42..1630cb8ad1 100644 --- a/crates/astria-core/src/primitive/v1/asset/denom.rs +++ b/crates/astria-core/src/primitive/v1/asset/denom.rs @@ -125,6 +125,12 @@ impl<'a> From<&'a Denom> for IbcPrefixed { } } +impl<'a> From<&'a IbcPrefixed> for IbcPrefixed { + fn from(value: &IbcPrefixed) -> Self { + *value + } +} + impl FromStr for Denom { type Err = ParseDenomError; @@ -297,6 +303,40 @@ impl TracePrefixed { pub fn push_trace_segment(&mut self, segment: PortAndChannel) { self.trace.push(segment); } + + pub fn trace(&self) -> impl Iterator { + self.trace + .inner + .iter() + .map(|segment| (segment.port.as_str(), segment.channel.as_str())) + } + + #[must_use] + pub fn base_denom(&self) -> &str { + &self.base_denom + } + + /// This should only be used where the inputs have been provided by a trusted entity, e.g. read + /// from our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts>( + trace: I, + base_denom: String, + ) -> Self { + Self { + trace: TraceSegments { + inner: trace + .into_iter() + .map(|(port, channel)| PortAndChannel { + port, + channel, + }) + .collect(), + }, + base_denom, + } + } } #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -518,8 +558,8 @@ impl IbcPrefixed { } #[must_use] - pub fn get(&self) -> [u8; 32] { - self.id + pub fn get(&self) -> &[u8; 32] { + &self.id } } diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index b544fa4241..038018c283 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -107,7 +107,7 @@ impl RollupId { /// use astria_core::primitive::v1::RollupId; /// let bytes = [42u8; 32]; /// let rollup_id = RollupId::new(bytes); - /// assert_eq!(bytes, rollup_id.get()); + /// assert_eq!(bytes, *rollup_id.get()); /// ``` #[must_use] pub const fn new(inner: [u8; ROLLUP_ID_LEN]) -> Self { @@ -116,18 +116,18 @@ impl RollupId { } } - /// Returns the 32 bytes array representing the rollup ID. + /// Returns a ref to the 32 bytes array representing the rollup ID. /// /// # Examples /// ``` /// use astria_core::primitive::v1::RollupId; /// let bytes = [42u8; 32]; /// let rollup_id = RollupId::new(bytes); - /// assert_eq!(bytes, rollup_id.get()); + /// assert_eq!(bytes, *rollup_id.get()); /// ``` #[must_use] - pub const fn get(self) -> [u8; 32] { - self.inner + pub const fn get(&self) -> &[u8; 32] { + &self.inner } /// Creates a new rollup ID by applying Sha256 to `bytes`. @@ -474,31 +474,6 @@ pub struct Address { format: PhantomData, } -// The serde impls need to be manually implemented for Address because they -// only work for Address which cannot be expressed using serde -// attributes. -#[cfg(feature = "serde")] -mod _serde_impls { - use serde::de::Error as _; - impl serde::Serialize for super::Address { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - self.to_raw().serialize(serializer) - } - } - impl<'de> serde::Deserialize<'de> for super::Address { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - super::raw::Address::deserialize(deserializer) - .and_then(|raw| raw.try_into().map_err(D::Error::custom)) - } - } -} - impl Clone for Address { fn clone(&self) -> Self { *self @@ -522,8 +497,8 @@ impl Address { } #[must_use] - pub fn bytes(self) -> [u8; ADDRESS_LEN] { - self.bytes + pub fn bytes(&self) -> &[u8; ADDRESS_LEN] { + &self.bytes } #[must_use] @@ -538,7 +513,7 @@ impl Address { /// The error conditions for this are the same as for [`AddressBuilder::try_build`]. pub fn to_prefix(&self, prefix: &str) -> Result { Self::builder() - .array(self.bytes()) + .array(*self.bytes()) .prefix(prefix) .try_build() } @@ -563,7 +538,7 @@ impl Address { #[must_use] pub fn to_raw(&self) -> raw::Address { let bech32m = - bech32::encode_lower::<::Checksum>(self.prefix, &self.bytes()) + bech32::encode_lower::<::Checksum>(self.prefix, self.bytes()) .expect( "should not fail because len(prefix) + len(bytes) <= 63 < BECH32M::CODELENGTH", ); @@ -590,6 +565,18 @@ impl Address { } = raw; bech32m.parse() } + + /// This should only be used where the inputs have been provided by a trusted entity, e.g. read + /// from our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts(bytes: [u8; ADDRESS_LEN], prefix: &str) -> Self { + Self { + bytes, + prefix: bech32::Hrp::parse_unchecked(prefix), + format: PhantomData, + } + } } impl From> for raw::Address { @@ -623,7 +610,7 @@ impl TryFrom for Address { impl std::fmt::Display for Address { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use bech32::EncodeError; - match bech32::encode_lower_to_fmt::(f, self.prefix, &self.bytes()) { + match bech32::encode_lower_to_fmt::(f, self.prefix, self.bytes()) { Ok(()) => Ok(()), Err(EncodeError::Fmt(err)) => Err(err), Err(err) => panic!( @@ -664,11 +651,6 @@ where } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr( - feature = "serde", - serde(try_from = "raw::TransactionId", into = "raw::TransactionId") -)] pub struct TransactionId { inner: [u8; TRANSACTION_ID_LEN], } @@ -684,8 +666,8 @@ impl TransactionId { /// Returns the 32-byte transaction hash. #[must_use] - pub fn get(self) -> [u8; TRANSACTION_ID_LEN] { - self.inner + pub fn get(&self) -> &[u8; TRANSACTION_ID_LEN] { + &self.inner } #[must_use] @@ -805,7 +787,7 @@ mod tests { .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .unwrap(); - insta::assert_json_snapshot!(&main_address); + insta::assert_json_snapshot!(&main_address.to_raw()); let compat_address = main_address .to_prefix(ASTRIA_COMPAT_ADDRESS_PREFIX) diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index d534c9c548..eb3936abd2 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -63,14 +63,6 @@ enum SignedTransactionErrorKind { Verification(crypto::Error), } -/// The individual parts of a [`SignedTransaction`]. -#[derive(Debug)] -pub struct SignedTransactionParts { - pub signature: Signature, - pub verification_key: VerificationKey, - pub transaction: UnsignedTransaction, -} - /// A signed transaction. /// /// [`SignedTransaction`] contains an [`UnsignedTransaction`] together @@ -85,7 +77,7 @@ pub struct SignedTransaction { } impl SignedTransaction { - pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { + pub fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { self.verification_key.address_bytes() } @@ -175,22 +167,6 @@ impl SignedTransaction { }) } - /// Converts a [`SignedTransaction`] into its [`SignedTransactionParts`]. - #[must_use] - pub fn into_parts(self) -> SignedTransactionParts { - let Self { - signature, - verification_key, - transaction, - .. - } = self; - SignedTransactionParts { - signature, - verification_key, - transaction, - } - } - #[must_use] pub fn into_unsigned(self) -> UnsignedTransaction { self.transaction @@ -610,7 +586,7 @@ mod test { transaction_bytes: unsigned.to_raw().encode_to_vec().into(), }; - insta::assert_json_snapshot!(tx.id()); + insta::assert_json_snapshot!(tx.id().to_raw()); } #[test] diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs index a1805c9532..bb781a67cd 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -1,5 +1,9 @@ use std::{ collections::HashMap, + fmt::{ + Display, + Formatter, + }, vec::IntoIter, }; @@ -92,8 +96,8 @@ pub struct RollupTransactions { impl RollupTransactions { /// Returns the [`RollupId`] identifying the rollup these transactions belong to. #[must_use] - pub fn rollup_id(&self) -> RollupId { - self.rollup_id + pub fn rollup_id(&self) -> &RollupId { + &self.rollup_id } /// Returns the block data for this rollup. @@ -169,6 +173,23 @@ impl RollupTransactions { proof, } } + + /// This should only be used where `parts` has been provided by a trusted entity, e.g. read from + /// our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts(parts: RollupTransactionsParts) -> Self { + let RollupTransactionsParts { + rollup_id, + transactions, + proof, + } = parts; + Self { + rollup_id, + transactions, + proof, + } + } } #[derive(Debug, thiserror::Error)] @@ -371,13 +392,13 @@ impl SequencerBlockHeader { } #[must_use] - pub fn rollup_transactions_root(&self) -> [u8; 32] { - self.rollup_transactions_root + pub fn rollup_transactions_root(&self) -> &[u8; 32] { + &self.rollup_transactions_root } #[must_use] - pub fn data_hash(&self) -> [u8; 32] { - self.data_hash + pub fn data_hash(&self) -> &[u8; 32] { + &self.data_hash } #[must_use] @@ -478,6 +499,29 @@ impl SequencerBlockHeader { proposer_address, }) } + + /// This should only be used where `parts` has been provided by a trusted entity, e.g. read from + /// our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts(parts: SequencerBlockHeaderParts) -> Self { + let SequencerBlockHeaderParts { + chain_id, + height, + time, + rollup_transactions_root, + data_hash, + proposer_address, + } = parts; + Self { + chain_id, + height, + time, + rollup_transactions_root, + data_hash, + proposer_address, + } + } } #[derive(Debug, thiserror::Error)] @@ -557,17 +601,17 @@ pub struct SequencerBlock { header: SequencerBlockHeader, /// The collection of rollup transactions that were included in this block. rollup_transactions: IndexMap, - // The proof that the rollup transactions are included in the `CometBFT` block this - // sequencer block is derived form. This proof together with - // `Sha256(MTH(rollup_transactions))` must match `header.data_hash`. - // `MTH(rollup_transactions)` is the Merkle Tree Hash derived from the - // rollup transactions. + /// The proof that the rollup transactions are included in the `CometBFT` block this + /// sequencer block is derived form. This proof together with + /// `Sha256(MTH(rollup_transactions))` must match `header.data_hash`. + /// `MTH(rollup_transactions)` is the Merkle Tree Hash derived from the + /// rollup transactions. rollup_transactions_proof: merkle::Proof, - // The proof that the rollup IDs listed in `rollup_transactions` are included - // in the `CometBFT` block this sequencer block is derived form. This proof together - // with `Sha256(MTH(rollup_ids))` must match `header.data_hash`. - // `MTH(rollup_ids)` is the Merkle Tree Hash derived from the rollup IDs listed in - // the rollup transactions. + /// The proof that the rollup IDs listed in `rollup_transactions` are included + /// in the `CometBFT` block this sequencer block is derived form. This proof together + /// with `Sha256(MTH(rollup_ids))` must match `header.data_hash`. + /// `MTH(rollup_ids)` is the Merkle Tree Hash derived from the rollup IDs listed in + /// the rollup transactions. rollup_ids_proof: merkle::Proof, } @@ -576,8 +620,8 @@ impl SequencerBlock { /// /// This is done by hashing the `CometBFT` header stored in this block. #[must_use] - pub fn block_hash(&self) -> [u8; 32] { - self.block_hash + pub fn block_hash(&self) -> &[u8; 32] { + &self.block_hash } #[must_use] @@ -596,6 +640,16 @@ impl SequencerBlock { &self.rollup_transactions } + #[must_use] + pub fn rollup_transactions_proof(&self) -> &merkle::Proof { + &self.rollup_transactions_proof + } + + #[must_use] + pub fn rollup_ids_proof(&self) -> &merkle::Proof { + &self.rollup_ids_proof + } + /// Converts a [`SequencerBlock`] into its [`SequencerBlockParts`]. #[must_use] pub fn into_parts(self) -> SequencerBlockParts { @@ -912,6 +966,27 @@ impl SequencerBlock { rollup_ids_proof, }) } + + /// This should only be used where `parts` has been provided by a trusted entity, e.g. read from + /// our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts(parts: SequencerBlockParts) -> Self { + let SequencerBlockParts { + block_hash, + header, + rollup_transactions, + rollup_transactions_proof, + rollup_ids_proof, + } = parts; + Self { + block_hash, + header, + rollup_transactions, + rollup_transactions_proof, + rollup_ids_proof, + } + } } fn rollup_transactions_and_ids_root_from_data( @@ -983,8 +1058,8 @@ pub struct FilteredSequencerBlock { impl FilteredSequencerBlock { #[must_use] - pub fn block_hash(&self) -> [u8; 32] { - self.block_hash + pub fn block_hash(&self) -> &[u8; 32] { + &self.block_hash } #[must_use] @@ -1003,8 +1078,8 @@ impl FilteredSequencerBlock { } #[must_use] - pub fn rollup_transactions_root(&self) -> [u8; 32] { - self.header.rollup_transactions_root + pub fn rollup_transactions_root(&self) -> &[u8; 32] { + &self.header.rollup_transactions_root } #[must_use] @@ -1147,7 +1222,7 @@ impl FilteredSequencerBlock { ) { return Err( FilteredSequencerBlockError::rollup_transaction_for_id_not_in_sequencer_block( - rollup_transactions.rollup_id(), + *rollup_transactions.rollup_id(), ), ); } @@ -1284,11 +1359,6 @@ impl FilteredSequencerBlockError { /// A [`Deposit`] is constructed whenever a [`BridgeLockAction`] is executed /// and stored as part of the block's events. #[derive(Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize))] -#[cfg_attr( - feature = "serde", - serde(into = "crate::generated::sequencerblock::v1alpha1::Deposit") -)] pub struct Deposit { // the address on the sequencer to which the funds were sent to. bridge_address: Address, @@ -1439,6 +1509,21 @@ impl Deposit { } } +impl Display for Deposit { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + write!( + formatter, + "Deposit[bridge address: {}, rollup id: {}, amount: {}, asset: {}, destination chain \ + address: {}]", + self.bridge_address, + self.rollup_id, + self.amount, + self.asset, + self.destination_chain_address, + ) + } +} + #[derive(Debug, thiserror::Error)] #[error(transparent)] pub struct DepositError(DepositErrorKind); diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs b/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs index 1646e9af3a..d6f179fac3 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs @@ -183,8 +183,8 @@ impl SubmittedRollupData { } #[must_use] - pub fn sequencer_block_hash(&self) -> [u8; 32] { - self.sequencer_block_hash + pub fn sequencer_block_hash(&self) -> &[u8; 32] { + &self.sequencer_block_hash } /// Converts from the unchecked representation of this type (its shadow). @@ -511,8 +511,8 @@ impl<'a> Iterator for RollupIdIter<'a> { impl SubmittedMetadata { /// Returns the block hash of the tendermint header stored in this blob. #[must_use] - pub fn block_hash(&self) -> [u8; 32] { - self.block_hash + pub fn block_hash(&self) -> &[u8; 32] { + &self.block_hash } /// Returns the sequencer's `CometBFT` chain ID. @@ -543,7 +543,7 @@ impl SubmittedMetadata { /// Returns the Merkle Tree Hash constructed from the rollup transactions of the original /// [`SequencerBlock`] this blob was derived from. #[must_use] - pub fn rollup_transactions_root(&self) -> [u8; 32] { + pub fn rollup_transactions_root(&self) -> &[u8; 32] { self.header.rollup_transactions_root() } @@ -588,7 +588,7 @@ impl SubmittedMetadata { if !rollup_transactions_proof.verify( &Sha256::digest(header.rollup_transactions_root()), - header.data_hash(), + *header.data_hash(), ) { return Err(SubmittedMetadataError::rollup_transactions_not_in_cometbft_block()); } @@ -596,7 +596,7 @@ impl SubmittedMetadata { if !super::are_rollup_ids_included( rollup_ids.iter().copied(), &rollup_ids_proof, - header.data_hash(), + *header.data_hash(), ) { return Err(SubmittedMetadataError::rollup_ids_not_in_cometbft_block()); } diff --git a/crates/astria-merkle/src/audit.rs b/crates/astria-merkle/src/audit.rs index 4ab383cbd5..6e6687219b 100644 --- a/crates/astria-merkle/src/audit.rs +++ b/crates/astria-merkle/src/audit.rs @@ -407,7 +407,6 @@ impl UncheckedProof { /// The proof is the concatenation of all sibling hashes required to reconstruct /// the Merkle tree from a leaf. This is also called the audit path. #[derive(Clone, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Proof { pub(super) audit_path: Vec, pub(super) leaf_index: usize, @@ -621,4 +620,21 @@ impl Proof { pub fn verify(&self, leaf: &[u8], root_hash: [u8; 32]) -> bool { self.audit().with_leaf(leaf).with_root(root_hash).perform() } + + /// This should only be used where `parts` has been provided by a trusted entity, e.g. read from + /// our own state store. + #[doc(hidden)] + #[must_use] + pub fn unchecked_from_parts(parts: UncheckedProof) -> Self { + let UncheckedProof { + audit_path, + leaf_index, + tree_size, + } = parts; + Proof { + audit_path, + leaf_index, + tree_size: NonZeroUsize::try_from(tree_size).expect("must be non-zero"), + } + } } diff --git a/crates/astria-sequencer/Cargo.toml b/crates/astria-sequencer/Cargo.toml index 24c5e82895..3d3501763e 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -24,7 +24,7 @@ telemetry = { package = "astria-telemetry", path = "../astria-telemetry", featur ] } anyhow = "1" -borsh = { version = "1", features = ["derive"] } +borsh = { version = "1.5.1", features = ["bytes", "derive"] } cnidarium = { git = "https://github.com/penumbra-zone/penumbra.git", rev = "87adc8d6b15f6081c1adf169daed4ca8873bd9f6", features = [ "metrics", ] } @@ -41,6 +41,7 @@ divan = { workspace = true, optional = true } futures = { workspace = true } hex = { workspace = true, features = ["serde"] } ibc-types = { workspace = true, features = ["with_serde"] } +itertools = { workspace = true } penumbra-ibc = { workspace = true, features = ["component", "rpc"] } penumbra-proto = { workspace = true } penumbra-tower-trace = { workspace = true } diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index d722bdd2e7..99313232ae 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -42,15 +42,15 @@ impl ActionHandler for TransferAction { ensure!( state - .get_bridge_account_rollup_id(from) + .get_bridge_account_rollup_id(&from) .await .context("failed to get bridge account rollup id")? .is_none(), "cannot transfer out of bridge account; BridgeUnlock must be used", ); - check_transfer(self, from, &state).await?; - execute_transfer(self, from, state).await?; + check_transfer(self, &from, &state).await?; + execute_transfer(self, &from, state).await?; Ok(()) } @@ -58,15 +58,13 @@ impl ActionHandler for TransferAction { pub(crate) async fn execute_transfer( action: &TransferAction, - from: TAddress, + from: &TAddress, mut state: S, ) -> anyhow::Result<()> where S: StateWrite, TAddress: AddressBytes, { - let from = from.address_bytes(); - let fee = state .get_transfer_base_fee() .await @@ -90,7 +88,7 @@ where .await .context("failed decreasing `from` account balance")?; state - .increase_balance(action.to, &action.asset, action.amount) + .increase_balance(&action.to, &action.asset, action.amount) .await .context("failed increasing `to` account balance")?; } else { @@ -101,7 +99,7 @@ where .await .context("failed decreasing `from` account balance")?; state - .increase_balance(action.to, &action.asset, action.amount) + .increase_balance(&action.to, &action.asset, action.amount) .await .context("failed increasing `to` account balance")?; @@ -116,7 +114,7 @@ where pub(crate) async fn check_transfer( action: &TransferAction, - from: TAddress, + from: &TAddress, state: &S, ) -> Result<()> where @@ -138,10 +136,10 @@ where .get_transfer_base_fee() .await .context("failed to get transfer base fee")?; - let transfer_asset = action.asset.clone(); + let transfer_asset = &action.asset; let from_fee_balance = state - .get_account_balance(&from, &action.fee_asset) + .get_account_balance(from, &action.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index e08d7b357a..42fef087f3 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -35,7 +35,7 @@ impl Component for AccountsComponent { .context("failed to read native asset from state")?; for account in app_state.accounts() { state - .put_account_balance(account.address, &native_asset, account.balance) + .put_account_balance(&account.address, &native_asset, account.balance) .context("failed writing account balance to state")?; } diff --git a/crates/astria-sequencer/src/accounts/mod.rs b/crates/astria-sequencer/src/accounts/mod.rs index ee7c84adf2..f4eb5355f1 100644 --- a/crates/astria-sequencer/src/accounts/mod.rs +++ b/crates/astria-sequencer/src/accounts/mod.rs @@ -4,10 +4,7 @@ pub(crate) mod query; mod state_ext; use astria_core::{ - crypto::{ - SigningKey, - VerificationKey, - }, + crypto::VerificationKey, primitive::v1::{ Address, ADDRESS_LEN, @@ -21,44 +18,29 @@ pub(crate) use state_ext::{ }; pub(crate) trait AddressBytes: Send + Sync { - fn address_bytes(&self) -> [u8; ADDRESS_LEN]; + fn address_bytes(&self) -> &[u8; ADDRESS_LEN]; } impl AddressBytes for Address { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { + fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { self.bytes() } } impl AddressBytes for [u8; ADDRESS_LEN] { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - *self + fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { + self } } -impl AddressBytes for SignedTransaction { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - self.address_bytes() - } -} - -impl AddressBytes for SigningKey { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - self.address_bytes() +impl<'a> AddressBytes for &'a SignedTransaction { + fn address_bytes(&self) -> &'a [u8; ADDRESS_LEN] { + SignedTransaction::address_bytes(self) } } impl AddressBytes for VerificationKey { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { + fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { self.address_bytes() } } - -impl<'a, T> AddressBytes for &'a T -where - T: AddressBytes, -{ - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - (*self).address_bytes() - } -} diff --git a/crates/astria-sequencer/src/accounts/query.rs b/crates/astria-sequencer/src/accounts/query.rs index 985bdfeca6..1ed05a23fc 100644 --- a/crates/astria-sequencer/src/accounts/query.rs +++ b/crates/astria-sequencer/src/accounts/query.rs @@ -34,7 +34,7 @@ use crate::{ async fn ibc_to_trace( state: S, - asset: asset::IbcPrefixed, + asset: &asset::IbcPrefixed, ) -> anyhow::Result { state .map_ibc_to_trace_prefixed_asset(asset) @@ -46,7 +46,7 @@ async fn ibc_to_trace( #[instrument(skip_all, fields(%address))] async fn get_trace_prefixed_account_balances( state: &S, - address: Address, + address: &Address, ) -> anyhow::Result> { let stream = state .account_asset_balances(address) @@ -59,7 +59,7 @@ async fn get_trace_prefixed_account_balances( let result_denom = if asset_balance.asset == native_asset.to_ibc_prefixed() { native_asset.into() } else { - ibc_to_trace(state, asset_balance.asset) + ibc_to_trace(state, &asset_balance.asset) .await .context("failed to map ibc prefixed asset to trace prefixed")? .into() @@ -84,7 +84,7 @@ pub(crate) async fn balance_request( Err(err_rsp) => return err_rsp, }; - let balances = match get_trace_prefixed_account_balances(&snapshot, address).await { + let balances = match get_trace_prefixed_account_balances(&snapshot, &address).await { Ok(balance) => balance, Err(err) => { return response::Query { @@ -122,7 +122,7 @@ pub(crate) async fn nonce_request( Ok(tup) => tup, Err(err_rsp) => return err_rsp, }; - let nonce = match snapshot.get_account_nonce(address).await { + let nonce = match snapshot.get_account_nonce(&address).await { Ok(nonce) => nonce, Err(err) => { return response::Query { diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index dd74ab3f18..b7fa326a3b 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -13,10 +13,6 @@ use anyhow::{ }; use astria_core::primitive::v1::asset; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, @@ -26,18 +22,10 @@ use pin_project_lite::pin_project; use tracing::instrument; use super::AddressBytes; - -/// Newtype wrapper to read and write a u32 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Nonce(u32); - -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Balance(u128); - -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Fee(u128); +use crate::storage::{ + self, + StoredValue, +}; const ACCOUNTS_PREFIX: &str = "accounts"; const TRANSFER_BASE_FEE_STORAGE_KEY: &str = "transferfee"; @@ -54,19 +42,21 @@ impl<'a, T: AddressBytes> std::fmt::Display for StorageKey<'a, T> { } } -fn balance_storage_key>( - address: TAddress, - asset: TAsset, -) -> String { +fn balance_storage_key<'a, TAddress, TAsset>(address: &TAddress, asset: &'a TAsset) -> String +where + TAddress: AddressBytes, + asset::IbcPrefixed: From<&'a TAsset>, +{ + let asset: asset::IbcPrefixed = asset.into(); format!( "{}/balance/{}", - StorageKey(&address), + StorageKey(address), crate::storage_keys::hunks::Asset::from(asset) ) } -fn nonce_storage_key(address: T) -> String { - format!("{}/nonce", StorageKey(&address)) +fn nonce_storage_key(address: &T) -> String { + format!("{}/nonce", StorageKey(address)) } pin_project! { @@ -133,12 +123,9 @@ where Err(e) => return Poll::Ready(Some(Err(e))), Ok(asset) => asset, }; - let Balance(balance) = match Balance::try_from_slice(&bytes).with_context(|| { - format!("failed decoding bytes read from state as balance for key `{key}`") - }) { - Err(e) => return Poll::Ready(Some(Err(e))), - Ok(balance) => balance, - }; + let balance = StoredValue::deserialize(&bytes) + .and_then(|value| storage::Balance::try_from(value).map(u128::from)) + .context("invalid balance bytes")?; Poll::Ready(Some(Ok(AssetBalance { asset, balance, @@ -158,22 +145,22 @@ fn extract_asset_from_key(s: &str) -> Result { #[async_trait] pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { #[instrument(skip_all)] - fn account_asset_keys( + fn account_asset_keys( &self, - address: impl AddressBytes, + address: &T, ) -> AccountAssetsStream { - let prefix = format!("{}/balance/", StorageKey(&address)); + let prefix = format!("{}/balance/", StorageKey(address)); AccountAssetsStream { underlying: self.prefix_keys(&prefix), } } #[instrument(skip_all)] - fn account_asset_balances( + fn account_asset_balances( &self, - address: impl AddressBytes, + address: &T, ) -> AccountAssetBalancesStream { - let prefix = format!("{}/balance/", StorageKey(&address)); + let prefix = format!("{}/balance/", StorageKey(address)); AccountAssetBalancesStream { underlying: self.prefix_raw(&prefix), } @@ -182,12 +169,13 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { #[instrument(skip_all)] async fn get_account_balance<'a, TAddress, TAsset>( &self, - address: TAddress, - asset: TAsset, + address: &TAddress, + asset: &'a TAsset, ) -> Result where TAddress: AddressBytes, - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset> + Send + Sync, { let Some(bytes) = self .get_raw(&balance_storage_key(address, asset)) @@ -196,12 +184,13 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { else { return Ok(0); }; - let Balance(balance) = Balance::try_from_slice(&bytes).context("invalid balance bytes")?; - Ok(balance) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Balance::try_from(value).map(u128::from)) + .context("invalid balance bytes") } #[instrument(skip_all)] - async fn get_account_nonce(&self, address: T) -> Result { + async fn get_account_nonce(&self, address: &T) -> Result { let bytes = self .get_raw(&nonce_storage_key(address)) .await @@ -210,9 +199,9 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { // the account has not yet been initialized; return 0 return Ok(0); }; - - let Nonce(nonce) = Nonce::try_from_slice(&bytes).context("invalid nonce bytes")?; - Ok(nonce) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Nonce::try_from(value).map(u32::from)) + .context("invalid nonce bytes") } #[instrument(skip_all)] @@ -224,9 +213,9 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { let Some(bytes) = bytes else { return Err(anyhow::anyhow!("transfer base fee not set")); }; - - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid fee bytes") } } @@ -235,46 +224,50 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_account_balance( + fn put_account_balance<'a, TAddress, TAsset>( &mut self, - address: TAddress, - asset: TAsset, + address: &TAddress, + asset: &'a TAsset, balance: u128, ) -> Result<()> where TAddress: AddressBytes, - TAsset: Into + std::fmt::Display + Send, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let bytes = borsh::to_vec(&Balance(balance)).context("failed to serialize balance")?; + let bytes = StoredValue::Balance(balance.into()) + .serialize() + .context("failed to serialize balance")?; self.put_raw(balance_storage_key(address, asset), bytes); Ok(()) } #[instrument(skip_all)] - fn put_account_nonce(&mut self, address: T, nonce: u32) -> Result<()> { - let bytes = borsh::to_vec(&Nonce(nonce)).context("failed to serialize nonce")?; + fn put_account_nonce(&mut self, address: &T, nonce: u32) -> Result<()> { + let bytes = StoredValue::Nonce(nonce.into()) + .serialize() + .context("failed to serialize nonce")?; self.put_raw(nonce_storage_key(address), bytes); Ok(()) } #[instrument(skip_all)] - async fn increase_balance( + async fn increase_balance<'a, TAddress, TAsset>( &mut self, - address: TAddress, - asset: TAsset, + address: &TAddress, + asset: &'a TAsset, amount: u128, ) -> Result<()> where TAddress: AddressBytes, - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let asset = asset.into(); let balance = self - .get_account_balance(&address, asset) + .get_account_balance(address, asset) .await .context("failed to get account balance")?; self.put_account_balance( - &address, + address, asset, balance .checked_add(amount) @@ -285,23 +278,23 @@ pub(crate) trait StateWriteExt: StateWrite { } #[instrument(skip_all)] - async fn decrease_balance( + async fn decrease_balance<'a, TAddress, TAsset>( &mut self, - address: TAddress, - asset: TAsset, + address: &TAddress, + asset: &'a TAsset, amount: u128, ) -> Result<()> where TAddress: AddressBytes, - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let asset = asset.into(); let balance = self - .get_account_balance(&address, asset) + .get_account_balance(address, asset) .await .context("failed to get account balance")?; self.put_account_balance( - &address, + address, asset, balance .checked_sub(amount) @@ -313,7 +306,9 @@ pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] fn put_transfer_base_fee(&mut self, fee: u128) -> Result<()> { - let bytes = borsh::to_vec(&Fee(fee)).context("failed to serialize fee")?; + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize fee")?; self.put_raw(TRANSFER_BASE_FEE_STORAGE_KEY.to_string(), bytes); Ok(()) } @@ -371,7 +366,7 @@ mod tests { // uninitialized accounts return zero assert_eq!( state - .get_account_nonce(address) + .get_account_nonce(&address) .await .expect("getting a non-initialized account's nonce should not fail"), nonce_expected, @@ -391,11 +386,11 @@ mod tests { // can write new state - .put_account_nonce(address, nonce_expected) + .put_account_nonce(&address, nonce_expected) .expect("putting an account nonce should not fail"); assert_eq!( state - .get_account_nonce(address) + .get_account_nonce(&address) .await .expect("a nonce was written and must exist inside the database"), nonce_expected, @@ -405,11 +400,11 @@ mod tests { // can rewrite with new value let nonce_expected = 1u32; state - .put_account_nonce(address, nonce_expected) + .put_account_nonce(&address, nonce_expected) .expect("putting an account nonce should not fail"); assert_eq!( state - .get_account_nonce(address) + .get_account_nonce(&address) .await .expect("a new nonce was written and must exist inside the database"), nonce_expected, @@ -429,11 +424,11 @@ mod tests { // can write new state - .put_account_nonce(address, nonce_expected) + .put_account_nonce(&address, nonce_expected) .expect("putting an account nonce should not fail"); assert_eq!( state - .get_account_nonce(address) + .get_account_nonce(&address) .await .expect("a nonce was written and must exist inside the database"), nonce_expected, @@ -445,11 +440,11 @@ mod tests { let nonce_expected_1 = 3u32; state - .put_account_nonce(address_1, nonce_expected_1) + .put_account_nonce(&address_1, nonce_expected_1) .expect("putting an account nonce should not fail"); assert_eq!( state - .get_account_nonce(address_1) + .get_account_nonce(&address_1) .await .expect("a new nonce was written and must exist inside the database"), nonce_expected_1, @@ -457,7 +452,7 @@ mod tests { ); assert_eq!( state - .get_account_nonce(address) + .get_account_nonce(&address) .await .expect("a new nonce was written and must exist inside the database"), nonce_expected, @@ -479,7 +474,7 @@ mod tests { // non-initialized accounts return zero assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(&address, &asset) .await .expect("getting a non-initialized asset balance should not fail"), amount_expected, @@ -499,13 +494,13 @@ mod tests { let mut amount_expected = 1u128; state - .put_account_balance(address, &asset, amount_expected) + .put_account_balance(&address, &asset, amount_expected) .expect("putting an account balance should not fail"); // can initialize assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -516,12 +511,12 @@ mod tests { amount_expected = 2u128; state - .put_account_balance(address, &asset, amount_expected) + .put_account_balance(&address, &asset, amount_expected) .expect("putting an asset balance for an account should not fail"); assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -541,13 +536,13 @@ mod tests { let amount_expected = 1u128; state - .put_account_balance(address, &asset, amount_expected) + .put_account_balance(&address, &asset, amount_expected) .expect("putting an account balance should not fail"); // able to write to account's storage assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -560,11 +555,11 @@ mod tests { let amount_expected_1 = 2u128; state - .put_account_balance(address_1, &asset, amount_expected_1) + .put_account_balance(&address_1, &asset, amount_expected_1) .expect("putting an account balance should not fail"); assert_eq!( state - .get_account_balance(address_1, &asset) + .get_account_balance(&address_1, &asset) .await .expect("getting an asset balance should not fail"), amount_expected_1, @@ -573,7 +568,7 @@ mod tests { ); assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -596,16 +591,16 @@ mod tests { let amount_expected_1 = 2u128; state - .put_account_balance(address, &asset_0, amount_expected_0) + .put_account_balance(&address, &asset_0, amount_expected_0) .expect("putting an account balance should not fail"); state - .put_account_balance(address, &asset_1, amount_expected_1) + .put_account_balance(&address, &asset_1, amount_expected_1) .expect("putting an account balance should not fail"); // wrote correct balances assert_eq!( state - .get_account_balance(address, &asset_0) + .get_account_balance(&address, &asset_0) .await .expect("getting an asset balance should not fail"), amount_expected_0, @@ -613,7 +608,7 @@ mod tests { ); assert_eq!( state - .get_account_balance(address, &asset_1) + .get_account_balance(&address, &asset_1) .await .expect("getting an asset balance should not fail"), amount_expected_1, @@ -631,7 +626,7 @@ mod tests { let address = astria_address(&[42u8; 20]); // see that call was ok - let stream = state.account_asset_balances(address); + let stream = state.account_asset_balances(&address); // Collect the stream into a vector let balances: Vec<_> = stream @@ -653,7 +648,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // native account should work with ibc too - state.put_native_asset(&nria()); + state.put_native_asset(nria()).unwrap(); let asset_0 = state.get_native_asset().await.unwrap(); let asset_1 = asset_1(); @@ -661,13 +656,13 @@ mod tests { // also need to add assets to the ibc state state - .put_ibc_asset(&asset_0.clone()) + .put_ibc_asset(asset_0.clone()) .expect("should be able to call other trait method on state object"); state - .put_ibc_asset(&asset_1.clone().unwrap_trace_prefixed()) + .put_ibc_asset(asset_1.clone().unwrap_trace_prefixed()) .expect("should be able to call other trait method on state object"); state - .put_ibc_asset(&asset_2.clone().unwrap_trace_prefixed()) + .put_ibc_asset(asset_2.clone().unwrap_trace_prefixed()) .expect("should be able to call other trait method on state object"); // create needed variables @@ -678,17 +673,17 @@ mod tests { // add balances to the account state - .put_account_balance(address, asset_0.clone(), amount_expected_0) + .put_account_balance(&address, &asset_0, amount_expected_0) .expect("putting an account balance should not fail"); state - .put_account_balance(address, &asset_1, amount_expected_1) + .put_account_balance(&address, &asset_1, amount_expected_1) .expect("putting an account balance should not fail"); state - .put_account_balance(address, &asset_2, amount_expected_2) + .put_account_balance(&address, &asset_2, amount_expected_2) .expect("putting an account balance should not fail"); let mut balances = state - .account_asset_balances(address) + .account_asset_balances(&address) .try_collect::>() .await .expect("should not fail"); @@ -724,14 +719,14 @@ mod tests { let amount_increase = 2u128; state - .increase_balance(address, &asset, amount_increase) + .increase_balance(&address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // correct balance was set assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_increase, @@ -739,13 +734,13 @@ mod tests { ); state - .increase_balance(address, &asset, amount_increase) + .increase_balance(&address, &asset, amount_increase) .await .expect("increasing account balance for initialized account should be ok"); assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_increase * 2, @@ -765,14 +760,14 @@ mod tests { let amount_increase = 2u128; state - .increase_balance(address, &asset, amount_increase) + .increase_balance(&address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // correct balance was set assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), amount_increase, @@ -781,13 +776,13 @@ mod tests { // decrease balance state - .decrease_balance(address, &asset, amount_increase) + .decrease_balance(&address, &asset, amount_increase) .await .expect("decreasing account balance for initialized account should be ok"); assert_eq!( state - .get_account_balance(address, &asset) + .get_account_balance(&address, &asset) .await .expect("getting an asset balance should not fail"), 0, @@ -808,17 +803,28 @@ mod tests { // give initial balance state - .increase_balance(address, &asset, amount_increase) + .increase_balance(&address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // decrease balance state - .decrease_balance(address, &asset, amount_increase + 1) + .decrease_balance(&address, &asset, amount_increase + 1) .await .expect_err("should not be able to subtract larger balance than what existed"); } + #[tokio::test] + async fn transfer_base_fee_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_transfer_base_fee(123).unwrap(); + let retrieved_fee = state.get_transfer_base_fee().await.unwrap(); + assert_eq!(retrieved_fee, 123); + } + #[test] fn storage_keys_have_not_changed() { let address: Address = "astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm" @@ -828,10 +834,10 @@ mod tests { .parse::() .unwrap(); assert_eq!( - balance_storage_key(address, &asset), - balance_storage_key(address, asset.to_ibc_prefixed()) + balance_storage_key(&address, &asset), + balance_storage_key(&address, &asset.to_ibc_prefixed()) ); - assert_snapshot!(balance_storage_key(address, asset)); - assert_snapshot!(nonce_storage_key(address)); + assert_snapshot!(balance_storage_key(&address, &asset)); + assert_snapshot!(nonce_storage_key(&address)); } } diff --git a/crates/astria-sequencer/src/address/state_ext.rs b/crates/astria-sequencer/src/address/state_ext.rs index 3016dc36f1..345417ea1c 100644 --- a/crates/astria-sequencer/src/address/state_ext.rs +++ b/crates/astria-sequencer/src/address/state_ext.rs @@ -15,13 +15,13 @@ use cnidarium::{ }; use tracing::instrument; -fn base_prefix_key() -> &'static str { - "prefixes/base" -} +use crate::storage::{ + self, + StoredValue, +}; -fn ibc_compat_prefix_key() -> &'static str { - "prefixes/ibc-compat" -} +const BASE_PREFIX_KEY: &str = "prefixes/base"; +const IBC_COMPAT_PREFIX_KEY: &str = "prefixes/ibc-compat"; #[async_trait] pub(crate) trait StateReadExt: StateRead { @@ -53,25 +53,29 @@ pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn get_base_prefix(&self) -> Result { let Some(bytes) = self - .get_raw(base_prefix_key()) + .get_raw(BASE_PREFIX_KEY) .await .context("failed reading address base prefix from state")? else { bail!("no base prefix found in state"); }; - String::from_utf8(bytes).context("prefix retrieved from storage is not valid utf8") + StoredValue::deserialize(&bytes) + .and_then(|value| storage::AddressPrefix::try_from(value).map(String::from)) + .context("invalid base prefix bytes") } #[instrument(skip_all)] async fn get_ibc_compat_prefix(&self) -> Result { let Some(bytes) = self - .get_raw(ibc_compat_prefix_key()) + .get_raw(IBC_COMPAT_PREFIX_KEY) .await .context("failed reading address ibc compat prefix from state")? else { bail!("no ibc compat prefix found in state") }; - String::from_utf8(bytes).context("prefix retrieved from storage is not valid utf8") + StoredValue::deserialize(&bytes) + .and_then(|value| storage::AddressPrefix::try_from(value).map(String::from)) + .context("invalid ibc compat prefix bytes") } } @@ -80,13 +84,21 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_base_prefix(&mut self, prefix: &str) { - self.put_raw(base_prefix_key().into(), prefix.into()); + fn put_base_prefix(&mut self, prefix: String) -> Result<()> { + let bytes = StoredValue::AddressPrefix(prefix.as_str().into()) + .serialize() + .context("failed to serialize base prefix")?; + self.put_raw(BASE_PREFIX_KEY.to_string(), bytes); + Ok(()) } #[instrument(skip_all)] - fn put_ibc_compat_prefix(&mut self, prefix: &str) { - self.put_raw(ibc_compat_prefix_key().into(), prefix.into()); + fn put_ibc_compat_prefix(&mut self, prefix: String) -> Result<()> { + let bytes = StoredValue::AddressPrefix(prefix.as_str().into()) + .serialize() + .context("failed to serialize ibc-compat prefix")?; + self.put_raw(IBC_COMPAT_PREFIX_KEY.to_string(), bytes); + Ok(()) } } @@ -107,7 +119,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix("astria"); + state.put_base_prefix("astria".to_string()).unwrap(); assert_eq!("astria", &state.get_base_prefix().await.unwrap()); } @@ -117,7 +129,9 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_ibc_compat_prefix("astriacompat"); + state + .put_ibc_compat_prefix("astriacompat".to_string()) + .unwrap(); assert_eq!( "astriacompat", &state.get_ibc_compat_prefix().await.unwrap() diff --git a/crates/astria-sequencer/src/api_state_ext.rs b/crates/astria-sequencer/src/api_state_ext.rs index fd2c836308..af80f1500d 100644 --- a/crates/astria-sequencer/src/api_state_ext.rs +++ b/crates/astria-sequencer/src/api_state_ext.rs @@ -1,14 +1,9 @@ use anyhow::{ - anyhow, bail, Context as _, Result, }; use astria_core::{ - generated::{ - primitive::v1 as primitiveRaw, - sequencerblock::v1alpha1 as raw, - }, primitive::v1::RollupId, sequencerblock::v1alpha1::block::{ RollupTransactions, @@ -16,244 +11,118 @@ use astria_core::{ SequencerBlockHeader, SequencerBlockParts, }, - Protobuf as _, }; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; -use bytes::Bytes; use cnidarium::{ StateRead, StateWrite, }; -use prost::Message; use tracing::instrument; +use crate::storage::{ + self, + StoredValue, +}; + fn block_hash_by_height_key(height: u64) -> String { format!("blockhash/{height}") } -fn sequencer_block_header_by_hash_key(hash: &[u8]) -> String { +fn sequencer_block_header_by_hash_key(hash: &[u8; 32]) -> String { format!("blockheader/{}", crate::utils::Hex(hash)) } -fn rollup_data_by_hash_and_rollup_id_key(hash: &[u8], rollup_id: &RollupId) -> String { +fn rollup_data_by_hash_and_rollup_id_key(hash: &[u8; 32], rollup_id: &RollupId) -> String { format!("rollupdata/{}/{}", crate::utils::Hex(hash), rollup_id) } -fn rollup_ids_by_hash_key(hash: &[u8]) -> String { +fn rollup_ids_by_hash_key(hash: &[u8; 32]) -> String { format!("rollupids/{}", crate::utils::Hex(hash)) } -fn rollup_transactions_proof_by_hash_key(hash: &[u8]) -> String { +fn rollup_transactions_proof_by_hash_key(hash: &[u8; 32]) -> String { format!("rolluptxsproof/{}", crate::utils::Hex(hash)) } -fn rollup_ids_proof_by_hash_key(hash: &[u8]) -> String { +fn rollup_ids_proof_by_hash_key(hash: &[u8; 32]) -> String { format!("rollupidsproof/{}", crate::utils::Hex(hash)) } -#[derive(BorshSerialize, BorshDeserialize)] -struct RollupIdSeq( - #[borsh( - deserialize_with = "rollup_id_impl::deserialize_many", - serialize_with = "rollup_id_impl::serialize_many" - )] - Vec, -); - -impl From> for RollupIdSeq { - fn from(value: Vec) -> Self { - RollupIdSeq(value) - } -} - -mod rollup_id_impl { - use super::{ - RollupId, - RollupIdSer, - }; - - pub(super) fn deserialize( - reader: &mut R, - ) -> ::core::result::Result { - let inner: [u8; 32] = borsh::BorshDeserialize::deserialize_reader(reader)?; - Ok(RollupId::from(inner)) - } - - pub(super) fn serialize( - obj: &RollupId, - writer: &mut W, - ) -> ::core::result::Result<(), borsh::io::Error> { - borsh::BorshSerialize::serialize(&obj.get(), writer)?; - Ok(()) - } - - pub(super) fn deserialize_many( - reader: &mut R, - ) -> ::core::result::Result, borsh::io::Error> { - let deser: Vec = borsh::BorshDeserialize::deserialize_reader(reader)?; - let ids = deser.into_iter().map(RollupIdSer::get).collect(); - Ok(ids) - } - - pub(super) fn serialize_many( - obj: &[RollupId], - writer: &mut W, - ) -> ::core::result::Result<(), borsh::io::Error> { - let inner: Vec<_> = obj.iter().copied().map(RollupIdSer::from).collect(); - borsh::BorshSerialize::serialize(&inner, writer)?; - Ok(()) - } -} - -#[derive(BorshSerialize, BorshDeserialize)] -struct RollupIdSer( - #[borsh( - deserialize_with = "rollup_id_impl::deserialize", - serialize_with = "rollup_id_impl::serialize" - )] - RollupId, -); - -impl RollupIdSer { - fn get(self) -> RollupId { - self.0 - } -} - -impl From for RollupIdSer { - fn from(value: RollupId) -> Self { - Self(value) - } -} - #[async_trait] pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn get_block_hash_by_height(&self, height: u64) -> Result<[u8; 32]> { - let key = block_hash_by_height_key(height); - let Some(hash) = self - .get_raw(&key) + let Some(bytes) = self + .get_raw(&block_hash_by_height_key(height)) .await .context("failed to read block hash by height from state")? else { bail!("block hash not found for given height"); }; - - let hash: [u8; 32] = hash.try_into().map_err(|bytes: Vec<_>| { - anyhow!("expected 32 bytes block hash, but got {}", bytes.len()) - })?; - Ok(hash) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::BlockHash::try_from(value).map(<[u8; 32]>::from)) + .context("invalid block hash bytes") } #[instrument(skip_all)] async fn get_sequencer_block_header_by_hash( &self, - hash: &[u8], + hash: &[u8; 32], ) -> Result { - let key = sequencer_block_header_by_hash_key(hash); - let Some(header_bytes) = self - .get_raw(&key) + let Some(bytes) = self + .get_raw(&sequencer_block_header_by_hash_key(hash)) .await .context("failed to read raw sequencer block from state")? else { bail!("header not found for given block hash"); }; - - let raw = raw::SequencerBlockHeader::decode(header_bytes.as_slice()) - .context("failed to decode sequencer block from raw bytes")?; - let header = SequencerBlockHeader::try_from_raw(raw) - .context("failed to convert raw sequencer block to sequencer block")?; - Ok(header) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::SequencerBlockHeader::try_from(value).map(SequencerBlockHeader::from) + }) + .context("invalid sequencer block header bytes") } #[instrument(skip_all)] - async fn get_rollup_ids_by_block_hash(&self, hash: &[u8]) -> Result> { - let key = rollup_ids_by_hash_key(hash); - let Some(rollup_ids_bytes) = self - .get_raw(&key) + async fn get_rollup_ids_by_block_hash(&self, hash: &[u8; 32]) -> Result> { + let Some(bytes) = self + .get_raw(&rollup_ids_by_hash_key(hash)) .await .context("failed to read rollup IDs by block hash from state")? else { bail!("rollup IDs not found for given block hash"); }; - - let RollupIdSeq(rollup_ids) = RollupIdSeq::try_from_slice(&rollup_ids_bytes) - .context("failed to deserialize rollup IDs list")?; - Ok(rollup_ids) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::RollupIds::try_from(value).map(Vec::::from)) + .context("invalid rollup ids bytes") } #[instrument(skip_all)] - async fn get_sequencer_block_by_hash(&self, hash: &[u8]) -> Result { - let Some(header_bytes) = self - .get_raw(&sequencer_block_header_by_hash_key(hash)) - .await - .context("failed to read raw sequencer block from state")? - else { - bail!("header not found for given block hash"); + async fn get_sequencer_block_by_hash(&self, hash: &[u8; 32]) -> Result { + // No need to add context as these `get` methods already report sufficient context on error. + let header = self.get_sequencer_block_header_by_hash(hash).await?; + let rollup_ids = self.get_rollup_ids_by_block_hash(hash).await?; + let rollup_transactions_proof = self + .get_rollup_transactions_proof_by_block_hash(hash) + .await?; + let rollup_ids_proof = self.get_rollup_ids_proof_by_block_hash(hash).await?; + + // allow: want to avoid explicitly importing `index_map` crate to sequencer crate. + #[allow(clippy::default_trait_access)] + let mut parts = SequencerBlockParts { + block_hash: *hash, + header, + rollup_transactions: Default::default(), + rollup_transactions_proof, + rollup_ids_proof, }; - let header_raw = raw::SequencerBlockHeader::decode(header_bytes.as_slice()) - .context("failed to decode sequencer block from raw bytes")?; - - let rollup_ids = self - .get_rollup_ids_by_block_hash(hash) - .await - .context("failed to get rollup IDs by block hash")?; - - let mut rollup_transactions = Vec::with_capacity(rollup_ids.len()); - for id in &rollup_ids { - let key = rollup_data_by_hash_and_rollup_id_key(hash, id); - let raw = self - .get_raw(&key) - .await - .context("failed to read rollup data by block hash and rollup ID from state")?; - if let Some(raw) = raw { - let raw = raw.as_slice(); - let rollup_data = raw::RollupTransactions::decode(raw) - .context("failed to decode rollup data from raw bytes")?; - rollup_transactions.push(rollup_data); - } + for rollup_id in rollup_ids { + let rollup_txs = self.get_rollup_data(hash, &rollup_id).await?; + let _ = parts.rollup_transactions.insert(rollup_id, rollup_txs); } - let Some(rollup_transactions_proof) = self - .get_raw(&rollup_transactions_proof_by_hash_key(hash)) - .await - .context("failed to read rollup transactions proof by block hash from state")? - else { - bail!("rollup transactions proof not found for given block hash"); - }; - - let rollup_transactions_proof = - primitiveRaw::Proof::decode(rollup_transactions_proof.as_slice()) - .context("failed to decode rollup transactions proof from raw bytes")?; - - let Some(rollup_ids_proof) = self - .get_raw(&rollup_ids_proof_by_hash_key(hash)) - .await - .context("failed to read rollup IDs proof by block hash from state")? - else { - bail!("rollup IDs proof not found for given block hash"); - }; - - let rollup_ids_proof = primitiveRaw::Proof::decode(rollup_ids_proof.as_slice()) - .context("failed to decode rollup IDs proof from raw bytes")?; - - let raw = raw::SequencerBlock { - block_hash: Bytes::copy_from_slice(hash), - header: header_raw.into(), - rollup_transactions, - rollup_transactions_proof: rollup_transactions_proof.into(), - rollup_ids_proof: rollup_ids_proof.into(), - }; - - let block = SequencerBlock::try_from_raw(raw) - .context("failed to convert raw sequencer block to sequencer block")?; - - Ok(block) + Ok(SequencerBlock::unchecked_from_parts(parts)) } #[instrument(skip_all)] @@ -270,55 +139,52 @@ pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn get_rollup_data( &self, - hash: &[u8], + hash: &[u8; 32], rollup_id: &RollupId, ) -> Result { - let key = rollup_data_by_hash_and_rollup_id_key(hash, rollup_id); let Some(bytes) = self - .get_raw(&key) + .get_raw(&rollup_data_by_hash_and_rollup_id_key(hash, rollup_id)) .await - .context("failed to read rollup data by block hash and rollup ID from state")? + .context("failed to read rollup transactions by block hash and rollup ID from state")? else { - bail!("rollup data not found for given block hash and rollup ID"); + bail!("rollup transactions not found for given block hash and rollup ID"); }; - let raw = raw::RollupTransactions::decode(bytes.as_slice()) - .context("failed to decode rollup data from raw bytes")?; - - let rollup_transactions = RollupTransactions::try_from_raw(raw) - .context("failed to convert raw rollup transaction to rollup transaction")?; - - Ok(rollup_transactions) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::RollupTransactions::try_from(value).map(RollupTransactions::from) + }) + .context("invalid rollup transactions bytes") } #[instrument(skip_all)] - async fn get_block_proofs_by_block_hash( + async fn get_rollup_transactions_proof_by_block_hash( &self, - hash: &[u8], - ) -> Result<(primitiveRaw::Proof, primitiveRaw::Proof)> { - let Some(rollup_transactions_proof) = self + hash: &[u8; 32], + ) -> Result { + let Some(bytes) = self .get_raw(&rollup_transactions_proof_by_hash_key(hash)) .await .context("failed to read rollup transactions proof by block hash from state")? else { bail!("rollup transactions proof not found for given block hash"); }; + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Proof::try_from(value).map(merkle::Proof::from)) + .context("invalid rollup transactions proof bytes") + } - let rollup_transactions_proof = - primitiveRaw::Proof::decode(rollup_transactions_proof.as_slice()) - .context("failed to decode rollup transactions proof from raw bytes")?; - - let Some(rollup_ids_proof) = self + #[instrument(skip_all)] + async fn get_rollup_ids_proof_by_block_hash(&self, hash: &[u8; 32]) -> Result { + let Some(bytes) = self .get_raw(&rollup_ids_proof_by_hash_key(hash)) .await .context("failed to read rollup IDs proof by block hash from state")? else { bail!("rollup IDs proof not found for given block hash"); }; - - let rollup_ids_proof = primitiveRaw::Proof::decode(rollup_ids_proof.as_slice()) - .context("failed to decode rollup IDs proof from raw bytes")?; - - Ok((rollup_transactions_proof, rollup_ids_proof)) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Proof::try_from(value).map(merkle::Proof::from)) + .context("invalid rollup IDs proof bytes") } } @@ -327,7 +193,7 @@ impl StateReadExt for T {} pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] fn put_sequencer_block(&mut self, block: SequencerBlock) -> Result<()> { - // split up and write the sequencer block to state in the following order: + // write the sequencer block to state in the following order: // 1. height to block hash // 2. block hash to rollup IDs // 3. block hash to block header @@ -335,25 +201,6 @@ pub(crate) trait StateWriteExt: StateWrite { // 5. block hash to rollup transactions proof // 6. block hash to rollup IDs proof - let key = block_hash_by_height_key(block.height().into()); - self.put_raw(key, block.block_hash().to_vec()); - - let rollup_ids = block - .rollup_transactions() - .keys() - .copied() - .map(From::from) - .collect::>(); - - let key = rollup_ids_by_hash_key(&block.block_hash()); - - self.put_raw( - key, - borsh::to_vec(&RollupIdSeq(rollup_ids)) - .context("failed to serialize rollup IDs list")?, - ); - - let key = sequencer_block_header_by_hash_key(&block.block_hash()); let SequencerBlockParts { block_hash, header, @@ -361,22 +208,100 @@ pub(crate) trait StateWriteExt: StateWrite { rollup_transactions_proof, rollup_ids_proof, } = block.into_parts(); - let header = header.into_raw(); - self.put_raw(key, header.encode_to_vec()); - for (id, rollup_data) in rollup_transactions { - let key = rollup_data_by_hash_and_rollup_id_key(&block_hash, &id); - self.put_raw(key, rollup_data.into_raw().encode_to_vec()); - } + put_block_hash(self, header.height(), block_hash)?; + put_rollup_ids(self, &block_hash, rollup_transactions.keys().copied())?; + put_block_header(self, &block_hash, header)?; + put_rollups_transactions(self, &block_hash, rollup_transactions.into_iter())?; + put_rollups_transactions_proof(self, &block_hash, rollup_transactions_proof)?; + put_rollup_ids_proof(self, &block_hash, rollup_ids_proof) + } +} - let key = rollup_transactions_proof_by_hash_key(&block_hash); - self.put_raw(key, rollup_transactions_proof.into_raw().encode_to_vec()); +fn put_block_hash( + state: &mut S, + block_height: tendermint::block::Height, + block_hash: [u8; 32], +) -> Result<()> { + let bytes = StoredValue::BlockHash((&block_hash).into()) + .serialize() + .context("failed to serialize block hash")?; + state.put_raw(block_hash_by_height_key(block_height.into()), bytes); + Ok(()) +} - let key = rollup_ids_proof_by_hash_key(&block_hash); - self.put_raw(key, rollup_ids_proof.into_raw().encode_to_vec()); +fn put_rollup_ids>( + state: &mut S, + block_hash: &[u8; 32], + rollup_ids: I, +) -> Result<()> { + let rollup_ids: Vec<_> = rollup_ids.collect(); + let bytes = StoredValue::RollupIds(rollup_ids.iter().into()) + .serialize() + .context("failed to serialize rollup ids")?; + state.put_raw(rollup_ids_by_hash_key(block_hash), bytes); + Ok(()) +} +// allow: `block_header` will be consumed in upcoming PR. +#[allow(clippy::needless_pass_by_value)] +fn put_block_header( + state: &mut S, + block_hash: &[u8; 32], + block_header: SequencerBlockHeader, +) -> Result<()> { + let bytes = StoredValue::SequencerBlockHeader((&block_header).into()) + .serialize() + .context("failed to serialize sequencer block header")?; + state.put_raw(sequencer_block_header_by_hash_key(block_hash), bytes); + Ok(()) +} + +fn put_rollups_transactions( + state: &mut S, + block_hash: &[u8; 32], + all_rollups_txs: I, +) -> Result<()> +where + S: StateWrite + ?Sized, + I: Iterator, +{ + let all_rollups_txs: Vec<_> = all_rollups_txs.collect(); + all_rollups_txs.iter().try_for_each(|(id, rollup_txs)| { + let bytes = StoredValue::RollupTransactions(rollup_txs.into()) + .serialize() + .context("failed to serialize rollup transactions")?; + state.put_raw(rollup_data_by_hash_and_rollup_id_key(block_hash, id), bytes); Ok(()) - } + }) +} + +// allow: `proof` will be consumed in upcoming PR. +#[allow(clippy::needless_pass_by_value)] +fn put_rollups_transactions_proof( + state: &mut S, + block_hash: &[u8; 32], + proof: merkle::Proof, +) -> Result<()> { + let bytes = StoredValue::Proof((&proof).into()) + .serialize() + .context("failed to serialize rollups transactions proof")?; + state.put_raw(rollup_transactions_proof_by_hash_key(block_hash), bytes); + Ok(()) +} + +// allow: `proof` will be consumed in upcoming PR. +#[allow(clippy::needless_pass_by_value)] +fn put_rollup_ids_proof( + state: &mut S, + block_hash: &[u8; 32], + proof: merkle::Proof, +) -> Result<()> { + let bytes = StoredValue::Proof((&proof).into()) + .serialize() + .context("failed to serialize rollup ids proof")?; + state.put_raw(rollup_ids_proof_by_hash_key(block_hash), bytes); + Ok(()) } impl StateWriteExt for T {} @@ -530,7 +455,7 @@ mod test { "a block was written to the database and we should be able to query its block \ hash by height" ), - block.block_hash(), + *block.block_hash(), "stored block hash does not match expected" ); } @@ -550,7 +475,7 @@ mod test { // grab block header by block hash assert_eq!( state - .get_sequencer_block_header_by_hash(block.block_hash().as_ref()) + .get_sequencer_block_header_by_hash(block.block_hash()) .await .expect( "a block was written to the database and we should be able to query its block \ @@ -575,7 +500,7 @@ mod test { // grab rollup ids by block hash let stored_rollup_ids = state - .get_rollup_ids_by_block_hash(block.block_hash().as_ref()) + .get_rollup_ids_by_block_hash(block.block_hash()) .await .expect( "a block was written to the database and we should be able to query its rollup ids", @@ -603,7 +528,7 @@ mod test { // grab block by block hash assert_eq!( state - .get_sequencer_block_by_hash(block.block_hash().as_ref()) + .get_sequencer_block_by_hash(block.block_hash()) .await .expect( "a block was written to the database and we should be able to query its block \ @@ -636,7 +561,7 @@ mod test { // grab rollup's data by block hash let stored_rollup_data = state - .get_rollup_data(block.block_hash().as_ref(), &rollup_id) + .get_rollup_data(block.block_hash(), &rollup_id) .await .expect( "a block was written to the database and we should be able to query the data for \ @@ -649,36 +574,38 @@ mod test { } #[tokio::test] - async fn get_block_proofs_by_block_hash() { + async fn get_rollup_transactions_proof_by_block_hash() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - // write block let block = make_test_sequencer_block(2u32); state .put_sequencer_block(block.clone()) .expect("writing block to database should work"); - // get written proofs - let transactions_proof = block - .clone() - .into_parts() - .rollup_transactions_proof - .into_raw(); - let ids_proof = block.clone().into_parts().rollup_ids_proof.into_raw(); - - // grab rollup's stored proofs - let stored_proofs = state - .get_block_proofs_by_block_hash(block.block_hash().as_ref()) + let transactions_proof = state + .get_rollup_transactions_proof_by_block_hash(block.block_hash()) .await - .expect( - "a block was written to the database and we should be able to query its proof data", - ); - assert_eq!( - (transactions_proof, ids_proof), - stored_proofs, - "stored proofs do not match expected" - ); + .expect("should have txs proof in state"); + assert_eq!(*block.rollup_transactions_proof(), transactions_proof); + } + + #[tokio::test] + async fn get_rollup_ids_proof_by_block_hash() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + let block = make_test_sequencer_block(2u32); + state + .put_sequencer_block(block.clone()) + .expect("writing block to database should work"); + + let ids_proof = state + .get_rollup_ids_proof_by_block_hash(block.block_hash()) + .await + .expect("should have ids proof in state"); + assert_eq!(*block.rollup_ids_proof(), ids_proof); } } diff --git a/crates/astria-sequencer/src/app/action_handler.rs b/crates/astria-sequencer/src/app/action_handler.rs index 180f592d39..157455ec64 100644 --- a/crates/astria-sequencer/src/app/action_handler.rs +++ b/crates/astria-sequencer/src/app/action_handler.rs @@ -9,7 +9,7 @@ use cnidarium::StateWrite; /// [1]: https://github.com/penumbra-zone/penumbra/blob/14959350abcb8cfbf33f9aedc7463fccfd8e3f9f/crates/cnidarium-component/src/action_handler.rs#L30 #[async_trait::async_trait] pub(crate) trait ActionHandler { - // Commenting out for the time being as this is currentl nonot being used. Leaving this in + // Commenting out for the time being as this is currently not being used. Leaving this in // for reference as this is copied from cnidarium_component. // ``` // type CheckStatelessContext: Clone + Send + Sync + 'static; diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index efb12c8a18..86f1f20d47 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -44,6 +44,7 @@ use cnidarium::{ StateRead, Storage, }; +use itertools::Itertools as _; use prost::Message as _; use sha2::{ Digest as _, @@ -223,26 +224,38 @@ impl App { .try_begin_transaction() .expect("state Arc should not be referenced elsewhere"); - state_tx.put_base_prefix(genesis_state.address_prefixes().base()); - state_tx.put_ibc_compat_prefix(genesis_state.address_prefixes().ibc_compat()); + state_tx + .put_base_prefix(genesis_state.address_prefixes().base().to_string()) + .context("failed to write base prefix to state")?; + state_tx + .put_ibc_compat_prefix(genesis_state.address_prefixes().ibc_compat().to_string()) + .context("failed to write ibc-compat prefix to state")?; let native_asset = genesis_state.native_asset_base_denomination(); - state_tx.put_native_asset(native_asset); state_tx - .put_ibc_asset(native_asset) + .put_native_asset(native_asset.clone()) + .context("failed to write native asset to state")?; + state_tx + .put_ibc_asset(native_asset.clone()) .context("failed to commit native asset as ibc asset to state")?; - state_tx.put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?); - state_tx.put_block_height(0); + state_tx + .put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?) + .context("failed to write chain id to state")?; + state_tx + .put_block_height(0) + .context("failed to write block height to state")?; for fee_asset in genesis_state.allowed_fee_assets() { - state_tx.put_allowed_fee_asset(fee_asset); + state_tx + .put_allowed_fee_asset(fee_asset) + .context("failed to write allowed fee asset to state")?; } // call init_chain on all components AccountsComponent::init_chain(&mut state_tx, &genesis_state) .await - .context("failed to call init_chain on AccountsComponent")?; + .context("init_chain failed on AccountsComponent")?; AuthorityComponent::init_chain( &mut state_tx, &AuthorityComponentAppState { @@ -251,16 +264,16 @@ impl App { }, ) .await - .context("failed to call init_chain on AuthorityComponent")?; + .context("init_chain failed on AuthorityComponent")?; BridgeComponent::init_chain(&mut state_tx, &genesis_state) .await - .context("failed to call init_chain on BridgeComponent")?; + .context("init_chain failed on BridgeComponent")?; IbcComponent::init_chain(&mut state_tx, &genesis_state) .await - .context("failed to call init_chain on IbcComponent")?; + .context("init_chain failed on IbcComponent")?; SequenceComponent::init_chain(&mut state_tx, &genesis_state) .await - .context("failed to call init_chain on SequenceComponent")?; + .context("init_chain failed on SequenceComponent")?; state_tx.apply(); @@ -753,7 +766,7 @@ impl App { self.begin_block(&begin_block) .await - .context("failed to call begin_block")?; + .context("begin_block failed")?; Ok(()) } @@ -861,7 +874,7 @@ impl App { tx_results.extend(execution_results); }; - let end_block = self.end_block(height.value(), sudo_address).await?; + let end_block = self.end_block(height.value(), &sudo_address).await?; // get and clear block deposits from state let mut state_tx = StateDelta::new(self.state.clone()); @@ -875,7 +888,12 @@ impl App { .await .context("failed to clear block deposits")?; debug!( - deposits = %telemetry::display::json(&deposits), + deposits = %deposits + .iter() + .map(|(rollup_id, deposits)| { + format!("[rollup {}: {}]", rollup_id, deposits.iter().join(", ")) + }) + .join(", "), "got block deposits from state" ); @@ -934,7 +952,8 @@ impl App { .get_block_height() .await .expect("block height must be set, as `put_block_height` was already called"); - state.put_storage_version_by_height(height, new_version); + // No need to add context as this method already reports sufficient context on error. + state.put_storage_version_by_height(height, new_version)?; debug!( height, version = new_version, @@ -962,28 +981,28 @@ impl App { ) -> anyhow::Result> { let mut state_tx = StateDelta::new(self.state.clone()); - // store the block height - state_tx.put_block_height(begin_block.header.height.into()); - // store the block time - state_tx.put_block_timestamp(begin_block.header.time); + // No need to add context as this method already reports sufficient context on error. + state_tx.put_block_height(begin_block.header.height.into())?; + // No need to add context as this method already reports sufficient context on error. + state_tx.put_block_timestamp(begin_block.header.time)?; // call begin_block on all components let mut arc_state_tx = Arc::new(state_tx); AccountsComponent::begin_block(&mut arc_state_tx, begin_block) .await - .context("failed to call begin_block on AccountsComponent")?; + .context("begin_block failed on AccountsComponent")?; AuthorityComponent::begin_block(&mut arc_state_tx, begin_block) .await - .context("failed to call begin_block on AuthorityComponent")?; + .context("begin_block failed on AuthorityComponent")?; BridgeComponent::begin_block(&mut arc_state_tx, begin_block) .await - .context("failed to call begin_block on BridgeComponent")?; + .context("begin_block failed on BridgeComponent")?; IbcComponent::begin_block(&mut arc_state_tx, begin_block) .await - .context("failed to call begin_block on IbcComponent")?; + .context("begin_block failed on IbcComponent")?; SequenceComponent::begin_block(&mut arc_state_tx, begin_block) .await - .context("failed to call begin_block on SequenceComponent")?; + .context("begin_block failed on SequenceComponent")?; let state_tx = Arc::try_unwrap(arc_state_tx) .expect("components should not retain copies of shared state"); @@ -1026,7 +1045,7 @@ impl App { async fn end_block( &mut self, height: u64, - fee_recipient: [u8; 20], + fee_recipient: &[u8; 20], ) -> anyhow::Result { let state_tx = StateDelta::new(self.state.clone()); let mut arc_state_tx = Arc::new(state_tx); @@ -1040,19 +1059,19 @@ impl App { // call end_block on all components AccountsComponent::end_block(&mut arc_state_tx, &end_block) .await - .context("failed to call end_block on AccountsComponent")?; + .context("end_block failed on AccountsComponent")?; AuthorityComponent::end_block(&mut arc_state_tx, &end_block) .await - .context("failed to call end_block on AuthorityComponent")?; + .context("end_block failed on AuthorityComponent")?; BridgeComponent::end_block(&mut arc_state_tx, &end_block) .await - .context("failed to call end_block on BridgeComponent")?; + .context("end_block failed on BridgeComponent")?; IbcComponent::end_block(&mut arc_state_tx, &end_block) .await - .context("failed to call end_block on IbcComponent")?; + .context("end_block failed on IbcComponent")?; SequenceComponent::end_block(&mut arc_state_tx, &end_block) .await - .context("failed to call end_block on SequenceComponent")?; + .context("end_block failed on SequenceComponent")?; let mut state_tx = Arc::try_unwrap(arc_state_tx) .expect("components should not retain copies of shared state"); @@ -1076,7 +1095,7 @@ impl App { for (asset, amount) in fees { state_tx - .increase_balance(fee_recipient, asset, amount) + .increase_balance(fee_recipient, &asset, amount) .await .context("failed to increase fee recipient balance")?; } diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap index 3e900c2423..0a24233978 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap @@ -1,38 +1,39 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs +assertion_line: 305 expression: app.app_hash.as_bytes() --- [ - 18, - 206, - 200, - 101, + 108, + 234, + 64, + 170, + 186, + 117, + 136, + 169, + 162, + 224, + 89, + 131, + 240, + 17, + 209, + 68, 160, - 129, - 124, - 188, - 249, - 181, - 44, + 121, + 172, + 68, 218, - 209, - 251, - 208, - 119, - 122, - 211, - 105, - 201, - 75, - 214, - 56, - 145, - 239, - 132, - 48, - 0, - 79, - 13, - 53, - 156 + 226, + 127, + 186, + 125, + 166, + 39, + 169, + 252, + 115, + 102, + 19 ] diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap index 2c96d3d3ba..ee0e47d1e1 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap @@ -1,38 +1,39 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs +assertion_line: 158 expression: app.app_hash.as_bytes() --- [ - 113, - 62, - 40, - 174, - 69, - 178, - 27, - 49, - 166, - 136, - 209, - 163, - 103, + 176, + 192, + 100, + 194, + 0, + 149, + 14, + 26, + 253, + 230, + 129, + 29, + 149, + 217, + 111, + 4, + 187, 135, - 254, - 99, - 63, + 101, + 20, + 30, 197, - 35, - 165, - 202, + 243, + 221, + 92, + 184, + 57, 177, - 78, - 238, - 106, - 144, - 250, - 69, - 90, + 24, + 22, 192, - 129, - 187 + 5 ] diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap index 6df05ffada..a9e0786df6 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap @@ -1,38 +1,39 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs +assertion_line: 77 expression: app.app_hash.as_bytes() --- [ - 230, - 203, - 68, - 175, - 166, - 230, - 219, - 247, - 217, - 191, - 121, + 244, + 103, + 78, + 192, + 119, + 25, + 221, + 242, + 158, + 157, + 138, + 58, + 248, + 66, + 163, + 100, + 167, 236, - 155, - 53, - 148, - 22, - 12, - 121, - 60, - 22, - 214, - 93, - 26, - 177, - 216, - 135, + 243, + 112, + 248, + 199, + 114, + 66, + 137, + 111, 217, - 212, - 93, - 40, - 173, - 94 + 69, + 89, + 17, + 164, + 43 ] diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index afe705fb6e..bac6638fc8 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -241,7 +241,6 @@ pub(crate) fn mock_tx( Arc::new(tx.into_signed(signer)) } -#[cfg_attr(feature = "test", allow(dead_code))] pub(crate) const MOCK_SEQUENCE_FEE: u128 = 10; pub(crate) fn denom_0() -> Denom { "denom_0".parse().unwrap() @@ -271,8 +270,6 @@ fn denom_6() -> Denom { "denom_6".parse().unwrap() } -#[cfg_attr(feature = "test", allow(dead_code))] - pub(crate) fn mock_balances( denom_0_balance: u128, denom_1_balance: u128, @@ -287,12 +284,11 @@ pub(crate) fn mock_balances( // we don't sanitize the balance inputs balances.insert(denom_3().to_ibc_prefixed(), 100); // balance transaction costs won't have entry for balances.insert(denom_4().to_ibc_prefixed(), 0); // zero balance not in transaction - balances.insert(denom_5().to_ibc_prefixed(), 0); // zero balance with corresponding zero cost + balances.insert(denom_5().to_ibc_prefixed(), 0); // zero balance with corresponding zero cost balances } -#[cfg_attr(feature = "test", allow(dead_code))] pub(crate) fn mock_tx_cost( denom_0_cost: u128, denom_1_cost: u128, @@ -304,8 +300,8 @@ pub(crate) fn mock_tx_cost( costs.insert(denom_2().to_ibc_prefixed(), denom_2_cost); // not present in balances // we don't sanitize the cost inputs - costs.insert(denom_5().to_ibc_prefixed(), 0); // zero in balances also - costs.insert(denom_6().to_ibc_prefixed(), 0); // not present in balances + costs.insert(denom_5().to_ibc_prefixed(), 0); // zero in balances also + costs.insert(denom_6().to_ibc_prefixed(), 0); // not present in balances costs } @@ -313,18 +309,18 @@ pub(crate) fn mock_tx_cost( #[cfg_attr(feature = "benchmark", allow(dead_code))] pub(crate) fn mock_state_put_account_balances( state: &mut StateDelta, - address: [u8; 20], + address: &[u8; 20], account_balances: HashMap, ) { for (denom, balance) in account_balances { - state.put_account_balance(address, denom, balance).unwrap(); + state.put_account_balance(address, &denom, balance).unwrap(); } } #[cfg_attr(feature = "benchmark", allow(dead_code))] pub(crate) fn mock_state_put_account_nonce( state: &mut StateDelta, - address: [u8; 20], + address: &[u8; 20], nonce: u32, ) { state.put_account_nonce(address, nonce).unwrap(); @@ -338,38 +334,40 @@ pub(crate) async fn mock_state_getter() -> StateDelta { // setup denoms state - .put_ibc_asset(denom_0().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_0().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_1().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_1().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_2().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_2().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_3().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_3().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_4().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_4().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_5().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_5().unwrap_trace_prefixed()) .unwrap(); state - .put_ibc_asset(denom_6().as_trace_prefixed().unwrap()) + .put_ibc_asset(denom_6().unwrap_trace_prefixed()) .unwrap(); // setup tx fees - state.put_sequence_action_base_fee(MOCK_SEQUENCE_FEE); - state.put_sequence_action_byte_cost_multiplier(0); + state + .put_sequence_action_base_fee(MOCK_SEQUENCE_FEE) + .unwrap(); + state.put_sequence_action_byte_cost_multiplier(0).unwrap(); state.put_transfer_base_fee(0).unwrap(); state.put_ics20_withdrawal_base_fee(0).unwrap(); - state.put_init_bridge_account_base_fee(0); - state.put_bridge_lock_byte_cost_multiplier(0); - state.put_bridge_sudo_change_base_fee(0); + state.put_init_bridge_account_base_fee(0).unwrap(); + state.put_bridge_lock_byte_cost_multiplier(0).unwrap(); + state.put_bridge_sudo_change_base_fee(0).unwrap(); // put denoms as allowed fee asset - state.put_allowed_fee_asset(denom_0()); + state.put_allowed_fee_asset(&denom_0()).unwrap(); state } diff --git a/crates/astria-sequencer/src/app/tests_app/mempool.rs b/crates/astria-sequencer/src/app/tests_app/mempool.rs index 65d327af80..6f1a448e46 100644 --- a/crates/astria-sequencer/src/app/tests_app/mempool.rs +++ b/crates/astria-sequencer/src/app/tests_app/mempool.rs @@ -366,7 +366,7 @@ async fn maintenance_recosting_promotes() { // see transfer went through assert_eq!( app.state - .get_account_balance(astria_address_from_hex_string(CAROL_ADDRESS), nria()) + .get_account_balance(&astria_address_from_hex_string(CAROL_ADDRESS), &nria()) .await .unwrap(), 1, @@ -541,7 +541,7 @@ async fn maintenance_funds_added_promotes() { // see transfer went through assert_eq!( app.state - .get_account_balance(astria_address_from_hex_string(BOB_ADDRESS), nria()) + .get_account_balance(&astria_address_from_hex_string(BOB_ADDRESS), &nria()) .await .unwrap(), 10, diff --git a/crates/astria-sequencer/src/app/tests_app/mod.rs b/crates/astria-sequencer/src/app/tests_app/mod.rs index 6869d0bf90..1737b56a14 100644 --- a/crates/astria-sequencer/src/app/tests_app/mod.rs +++ b/crates/astria-sequencer/src/app/tests_app/mod.rs @@ -103,7 +103,7 @@ async fn app_genesis_and_init_chain() { assert_eq!( balance, app.state - .get_account_balance(address, nria()) + .get_account_balance(&address, &nria()) .await .unwrap(), ); @@ -157,7 +157,7 @@ async fn app_begin_block_remove_byzantine_validators() { let misbehavior = types::Misbehavior { kind: types::MisbehaviorKind::Unknown, validator: types::Validator { - address: crate::test_utils::verification_key(1).address_bytes(), + address: *crate::test_utils::verification_key(1).address_bytes(), power: 0u32.into(), }, height: Height::default(), @@ -197,7 +197,7 @@ async fn app_commit() { assert_eq!( balance, app.state - .get_account_balance(address, nria()) + .get_account_balance(&address, &nria()) .await .unwrap() ); @@ -216,7 +216,10 @@ async fn app_commit() { } in default_genesis_accounts() { assert_eq!( - snapshot.get_account_balance(address, nria()).await.unwrap(), + snapshot + .get_account_balance(&address, &nria()) + .await + .unwrap(), balance ); } @@ -275,7 +278,7 @@ async fn app_transfer_block_fees_to_sudo() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(astria_address_from_hex_string(JUDY_ADDRESS), nria()) + .get_account_balance(&astria_address_from_hex_string(JUDY_ADDRESS), &nria()) .await .unwrap(), transfer_fee, @@ -300,9 +303,11 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); app.prepare_commit(storage.clone()).await.unwrap(); @@ -393,9 +398,11 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); app.apply(state_tx); app.prepare_commit(storage.clone()).await.unwrap(); @@ -764,7 +771,7 @@ async fn app_end_block_validator_updates() { .unwrap(); app.apply(state_tx); - let resp = app.end_block(1, proposer_address).await.unwrap(); + let resp = app.end_block(1, &proposer_address).await.unwrap(); // we only assert length here as the ordering of the updates is not guaranteed // and validator::Update does not implement Ord assert_eq!(resp.validator_updates.len(), validator_updates.len()); diff --git a/crates/astria-sequencer/src/app/tests_block_fees.rs b/crates/astria-sequencer/src/app/tests_block_fees.rs index 29c2292c0e..f894cc5b10 100644 --- a/crates/astria-sequencer/src/app/tests_block_fees.rs +++ b/crates/astria-sequencer/src/app/tests_block_fees.rs @@ -139,8 +139,10 @@ async fn ensure_correct_block_fees_transfer() { async fn ensure_correct_block_fees_sequence() { let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_sequence_action_base_fee(1); - state_tx.put_sequence_action_byte_cost_multiplier(1); + state_tx.put_sequence_action_base_fee(1).unwrap(); + state_tx + .put_sequence_action_byte_cost_multiplier(1) + .unwrap(); app.apply(state_tx); let alice = get_alice_signing_key(); @@ -182,7 +184,9 @@ async fn ensure_correct_block_fees_init_bridge_acct() { let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); let init_bridge_account_base_fee = 1; - state_tx.put_init_bridge_account_base_fee(init_bridge_account_base_fee); + state_tx + .put_init_bridge_account_base_fee(init_bridge_account_base_fee) + .unwrap(); app.apply(state_tx); let alice = get_alice_signing_key(); @@ -234,10 +238,14 @@ async fn ensure_correct_block_fees_bridge_lock() { let bridge_lock_byte_cost_multiplier = 1; state_tx.put_transfer_base_fee(transfer_base_fee).unwrap(); - state_tx.put_bridge_lock_byte_cost_multiplier(bridge_lock_byte_cost_multiplier); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_lock_byte_cost_multiplier(bridge_lock_byte_cost_multiplier) + .unwrap(); + state_tx + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -296,10 +304,14 @@ async fn ensure_correct_block_fees_bridge_sudo_change() { let mut state_tx = StateDelta::new(app.state.clone()); let sudo_change_base_fee = 1; - state_tx.put_bridge_sudo_change_base_fee(sudo_change_base_fee); - state_tx.put_bridge_account_sudo_address(bridge_address, alice_address); state_tx - .increase_balance(bridge_address, nria(), 1) + .put_bridge_sudo_change_base_fee(sudo_change_base_fee) + .unwrap(); + state_tx + .put_bridge_account_sudo_address(&bridge_address, alice_address) + .unwrap(); + state_tx + .increase_balance(&bridge_address, &nria(), 1) .await .unwrap(); app.apply(state_tx); diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 473b4d5eaa..143dae44f0 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -87,9 +87,11 @@ async fn app_finalize_block_snapshot() { let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -295,7 +297,7 @@ async fn app_execute_transaction_with_every_action_snapshot() { app.execute_transaction(signed_tx).await.unwrap(); let sudo_address = app.state.get_sudo_address().await.unwrap(); - app.end_block(1, sudo_address).await.unwrap(); + app.end_block(1, &sudo_address).await.unwrap(); app.prepare_commit(storage.clone()).await.unwrap(); app.commit(storage.clone()).await; diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 0be551232a..bd1a007b2e 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -124,7 +124,7 @@ async fn app_execute_transaction_transfer() { assert_eq!( app.state - .get_account_balance(bob_address, nria()) + .get_account_balance(&bob_address, &nria()) .await .unwrap(), value + 10u128.pow(19) @@ -132,13 +132,16 @@ async fn app_execute_transaction_transfer() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), 10u128.pow(19) - (value + transfer_fee), ); - assert_eq!(app.state.get_account_nonce(bob_address).await.unwrap(), 0); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!(app.state.get_account_nonce(&bob_address).await.unwrap(), 0); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); } #[tokio::test] @@ -154,7 +157,7 @@ async fn app_execute_transaction_transfer_not_native_token() { let mut state_tx = StateDelta::new(app.state.clone()); state_tx - .put_account_balance(alice_address, &test_asset(), value) + .put_account_balance(&alice_address, &test_asset(), value) .unwrap(); app.apply(state_tx); @@ -181,14 +184,14 @@ async fn app_execute_transaction_transfer_not_native_token() { assert_eq!( app.state - .get_account_balance(bob_address, nria()) + .get_account_balance(&bob_address, &nria()) .await .unwrap(), 10u128.pow(19), // genesis balance ); assert_eq!( app.state - .get_account_balance(bob_address, test_asset()) + .get_account_balance(&bob_address, &test_asset()) .await .unwrap(), value, // transferred amount @@ -197,21 +200,24 @@ async fn app_execute_transaction_transfer_not_native_token() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), 10u128.pow(19) - transfer_fee, // genesis balance - fee ); assert_eq!( app.state - .get_account_balance(alice_address, test_asset()) + .get_account_balance(&alice_address, &test_asset()) .await .unwrap(), 0, // 0 since all funds of `asset` were transferred ); - assert_eq!(app.state.get_account_nonce(bob_address).await.unwrap(), 0); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!(app.state.get_account_nonce(&bob_address).await.unwrap(), 0); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); } #[tokio::test] @@ -257,8 +263,10 @@ async fn app_execute_transaction_sequence() { let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_sequence_action_base_fee(0); - state_tx.put_sequence_action_byte_cost_multiplier(1); + state_tx.put_sequence_action_base_fee(0).unwrap(); + state_tx + .put_sequence_action_byte_cost_multiplier(1) + .unwrap(); app.apply(state_tx); let alice = get_alice_signing_key(); @@ -283,11 +291,14 @@ async fn app_execute_transaction_sequence() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), 10u128.pow(19) - fee, @@ -342,7 +353,10 @@ async fn app_execute_transaction_validator_update() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); let validator_updates = app.state.get_validator_updates().await.unwrap(); assert_eq!(validator_updates.len(), 1); @@ -369,7 +383,10 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); assert!(app.state.is_ibc_relayer(alice_address).await.unwrap()); } @@ -397,7 +414,10 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); assert!(!app.state.is_ibc_relayer(alice_address).await.unwrap()); } @@ -450,10 +470,13 @@ async fn app_execute_transaction_sudo_address_change() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); let sudo_address = app.state.get_sudo_address().await.unwrap(); - assert_eq!(sudo_address, new_address.bytes()); + assert_eq!(sudo_address, *new_address.bytes()); } #[tokio::test] @@ -517,7 +540,10 @@ async fn app_execute_transaction_fee_asset_change_addition() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); assert!(app.state.is_allowed_fee_asset(&test_asset()).await.unwrap()); } @@ -550,9 +576,12 @@ async fn app_execute_transaction_fee_asset_change_removal() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); - assert!(!app.state.is_allowed_fee_asset(test_asset()).await.unwrap()); + assert!(!app.state.is_allowed_fee_asset(&test_asset()).await.unwrap()); } #[tokio::test] @@ -593,7 +622,7 @@ async fn app_execute_transaction_init_bridge_account_ok() { let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); let fee = 12; // arbitrary - state_tx.put_init_bridge_account_base_fee(fee); + state_tx.put_init_bridge_account_base_fee(fee).unwrap(); app.apply(state_tx); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); @@ -616,14 +645,17 @@ async fn app_execute_transaction_init_bridge_account_ok() { let before_balance = app .state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); assert_eq!( app.state - .get_bridge_account_rollup_id(alice_address) + .get_bridge_account_rollup_id(&alice_address) .await .unwrap() .unwrap(), @@ -631,14 +663,14 @@ async fn app_execute_transaction_init_bridge_account_ok() { ); assert_eq!( app.state - .get_bridge_account_ibc_asset(alice_address) + .get_bridge_account_ibc_asset(&alice_address) .await .unwrap(), nria().to_ibc_prefixed(), ); assert_eq!( app.state - .get_account_balance(alice_address, &nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), before_balance - fee, @@ -702,9 +734,11 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -728,17 +762,20 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let alice_before_balance = app .state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(); let bridge_before_balance = app .state - .get_account_balance(bridge_address, nria()) + .get_account_balance(&bridge_address, &nria()) .await .unwrap(); app.execute_transaction(signed_tx.clone()).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); let expected_deposit = Deposit::new( bridge_address, @@ -759,14 +796,14 @@ async fn app_execute_transaction_bridge_lock_action_ok() { * crate::bridge::get_deposit_byte_len(&expected_deposit); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), alice_before_balance - (amount + fee) ); assert_eq!( app.state - .get_account_balance(bridge_address, nria()) + .get_account_balance(&bridge_address, &nria()) .await .unwrap(), bridge_before_balance + amount @@ -835,10 +872,13 @@ async fn app_execute_transaction_invalid_nonce() { let response = app.execute_transaction(signed_tx).await; // check that tx was not executed by checking nonce and balance are unchanged - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 0 + ); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), 10u128.pow(19), @@ -882,10 +922,13 @@ async fn app_execute_transaction_invalid_chain_id() { let response = app.execute_transaction(signed_tx).await; // check that tx was not executed by checking nonce and balance are unchanged - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 0 + ); assert_eq!( app.state - .get_account_balance(alice_address, nria()) + .get_account_balance(&alice_address, &nria()) .await .unwrap(), 10u128.pow(19), @@ -911,7 +954,7 @@ async fn app_stateful_check_fails_insufficient_total_balance() { // create a new key; will have 0 balance let keypair = SigningKey::new(OsRng); - let keypair_address = astria_address(&keypair.verification_key().address_bytes()); + let keypair_address = astria_address(keypair.verification_key().address_bytes()); // figure out needed fee for a single transfer let data = Bytes::from_static(b"hello world"); @@ -1013,15 +1056,19 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { // unlock transfer action let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); state_tx - .put_account_balance(bridge_address, nria(), transfer_fee) + .put_account_balance(&bridge_address, &nria(), transfer_fee) .unwrap(); // create bridge account - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) + .unwrap(); + state_tx + .put_bridge_account_withdrawer_address(&bridge_address, bridge_address) .unwrap(); - state_tx.put_bridge_account_withdrawer_address(bridge_address, bridge_address); app.apply(state_tx); let amount = 100; @@ -1043,7 +1090,10 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); // see can unlock through bridge unlock let action = BridgeUnlockAction { @@ -1070,7 +1120,7 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { .expect("executing bridge unlock action should succeed"); assert_eq!( app.state - .get_account_balance(bridge_address, nria()) + .get_account_balance(&bridge_address, &nria()) .await .expect("executing bridge unlock action should succeed"), 0, @@ -1089,9 +1139,11 @@ async fn app_execute_transaction_action_index_correctly_increments() { let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, nria()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -1113,7 +1165,10 @@ async fn app_execute_transaction_action_index_correctly_increments() { let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx.clone()).await.unwrap(); - assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + assert_eq!( + app.state.get_account_nonce(&alice_address).await.unwrap(), + 1 + ); let deposits = app.state.get_deposit_events(&rollup_id).await.unwrap(); assert_eq!(deposits.len(), 2); @@ -1134,10 +1189,12 @@ async fn transaction_execution_records_deposit_event() { let alice = get_alice_signing_key(); let bob_address = astria_address_from_hex_string(BOB_ADDRESS); - state_tx.put_bridge_account_rollup_id(bob_address, &[0; 32].into()); - state_tx.put_allowed_fee_asset(nria()); state_tx - .put_bridge_account_ibc_asset(bob_address, nria()) + .put_bridge_account_rollup_id(&bob_address, [0; 32].into()) + .unwrap(); + state_tx.put_allowed_fee_asset(&nria()).unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bob_address, nria()) .unwrap(); let tx = UnsignedTransaction { params: TransactionParams::builder() diff --git a/crates/astria-sequencer/src/assets/query.rs b/crates/astria-sequencer/src/assets/query.rs index f4f88afa1d..5db2781a7a 100644 --- a/crates/astria-sequencer/src/assets/query.rs +++ b/crates/astria-sequencer/src/assets/query.rs @@ -50,7 +50,7 @@ pub(crate) async fn denom_request( } }; - let maybe_denom = match snapshot.map_ibc_to_trace_prefixed_asset(asset).await { + let maybe_denom = match snapshot.map_ibc_to_trace_prefixed_asset(&asset).await { Ok(maybe_denom) => maybe_denom, Err(err) => { return response::Query { diff --git a/crates/astria-sequencer/src/assets/state_ext.rs b/crates/astria-sequencer/src/assets/state_ext.rs index 48deeaf6a0..baa51f344d 100644 --- a/crates/astria-sequencer/src/assets/state_ext.rs +++ b/crates/astria-sequencer/src/assets/state_ext.rs @@ -5,10 +5,6 @@ use anyhow::{ }; use astria_core::primitive::v1::asset; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, @@ -20,26 +16,36 @@ use tendermint::abci::{ }; use tracing::instrument; -/// Newtype wrapper to read and write a denomination trace from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct DenominationTrace(String); +use crate::storage::{ + self, + StoredValue, +}; const BLOCK_FEES_PREFIX: &str = "block_fees/"; const FEE_ASSET_PREFIX: &str = "fee_asset/"; const NATIVE_ASSET_KEY: &[u8] = b"nativeasset"; -fn asset_storage_key>(asset: TAsset) -> String { +fn asset_storage_key<'a, TAsset>(asset: &'a TAsset) -> String +where + asset::IbcPrefixed: From<&'a TAsset>, +{ format!("asset/{}", crate::storage_keys::hunks::Asset::from(asset)) } -fn block_fees_key>(asset: TAsset) -> String { +fn block_fees_key<'a, TAsset>(asset: &'a TAsset) -> String +where + asset::IbcPrefixed: From<&'a TAsset>, +{ format!( "{BLOCK_FEES_PREFIX}{}", crate::storage_keys::hunks::Asset::from(asset) ) } -fn fee_asset_key>(asset: TAsset) -> String { +fn fee_asset_key<'a, TAsset>(asset: &'a TAsset) -> String +where + asset::IbcPrefixed: From<&'a TAsset>, +{ format!( "{FEE_ASSET_PREFIX}{}", crate::storage_keys::hunks::Asset::from(asset) @@ -73,18 +79,18 @@ pub(crate) trait StateReadExt: StateRead { else { bail!("native asset denom not found in state"); }; - - let asset = std::str::from_utf8(&bytes) - .context("bytes stored in state not utf8 encoded")? - .parse::() - .context("failed to parse bytes retrieved from state as trace prefixed IBC asset")?; - Ok(asset) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::TracePrefixedDenom::try_from(value).map(asset::TracePrefixed::from) + }) + .context("invalid native asset bytes") } #[instrument(skip_all)] - async fn has_ibc_asset(&self, asset: TAsset) -> Result + async fn has_ibc_asset<'a, TAsset>(&self, asset: &'a TAsset) -> Result where - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset>, { Ok(self .get_raw(&asset_storage_key(asset)) @@ -96,7 +102,7 @@ pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn map_ibc_to_trace_prefixed_asset( &self, - asset: asset::IbcPrefixed, + asset: &asset::IbcPrefixed, ) -> Result> { let Some(bytes) = self .get_raw(&asset_storage_key(asset)) @@ -105,13 +111,12 @@ pub(crate) trait StateReadExt: StateRead { else { return Ok(None); }; - - let DenominationTrace(denom_str) = - DenominationTrace::try_from_slice(&bytes).context("invalid asset bytes")?; - let denom = denom_str - .parse() - .context("failed to parse retrieved denom string as a Denom")?; - Ok(Some(denom)) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::TracePrefixedDenom::try_from(value) + .map(|stored_denom| Some(asset::TracePrefixed::from(stored_denom))) + }) + .context("invalid ibc asset bytes") } #[instrument(skip_all)] @@ -120,7 +125,7 @@ pub(crate) trait StateReadExt: StateRead { let mut stream = std::pin::pin!(self.nonverifiable_prefix_raw(BLOCK_FEES_PREFIX.as_bytes())); - while let Some(Ok((key, value))) = stream.next().await { + while let Some(Ok((key, bytes))) = stream.next().await { // if the key isn't of the form `block_fees/{asset_id}`, then we have a bug // in `put_block_fees` let suffix = key @@ -132,20 +137,21 @@ pub(crate) trait StateReadExt: StateRead { .context("failed to parse storage key suffix as address hunk")? .get(); - let Ok(bytes): Result<[u8; 16], _> = value.try_into() else { - bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); - }; + let fee = StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid block fees bytes")?; - fees.push((asset, u128::from_be_bytes(bytes))); + fees.push((asset, fee)); } Ok(fees) } #[instrument(skip_all)] - async fn is_allowed_fee_asset(&self, asset: TAsset) -> Result + async fn is_allowed_fee_asset<'a, TAsset>(&self, asset: &'a TAsset) -> Result where - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset>, { Ok(self .nonverifiable_get_raw(fee_asset_key(asset).as_bytes()) @@ -182,30 +188,37 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_native_asset(&mut self, asset: &asset::TracePrefixed) { - self.nonverifiable_put_raw(NATIVE_ASSET_KEY.to_vec(), asset.to_string().into_bytes()); + fn put_native_asset(&mut self, asset: asset::TracePrefixed) -> Result<()> { + let bytes = StoredValue::TracePrefixedDenom((&asset).into()) + .serialize() + .context("failed to serialize native asset")?; + self.nonverifiable_put_raw(NATIVE_ASSET_KEY.to_vec(), bytes); + Ok(()) } #[instrument(skip_all)] - fn put_ibc_asset(&mut self, asset: &asset::TracePrefixed) -> Result<()> { - let bytes = borsh::to_vec(&DenominationTrace(asset.to_string())) - .context("failed to serialize asset")?; - self.put_raw(asset_storage_key(asset), bytes); + fn put_ibc_asset(&mut self, asset: asset::TracePrefixed) -> Result<()> { + let key = asset_storage_key(&asset); + let bytes = StoredValue::TracePrefixedDenom((&asset).into()) + .serialize() + .context("failed to serialize ibc asset")?; + self.put_raw(key, bytes); Ok(()) } /// Adds `amount` to the block fees for `asset`. #[instrument(skip_all)] - async fn get_and_increase_block_fees( + async fn get_and_increase_block_fees<'a, TAsset>( &mut self, - asset: TAsset, + asset: &'a TAsset, amount: u128, action_type: String, ) -> Result<()> where - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync + std::fmt::Display, + asset::IbcPrefixed: From<&'a TAsset>, { - let tx_fee_event = construct_tx_fee_event(&asset, amount, action_type); + let tx_fee_event = construct_tx_fee_event(asset, amount, action_type); let block_fees_key = block_fees_key(asset); let current_amount = self @@ -213,11 +226,9 @@ pub(crate) trait StateWriteExt: StateWrite { .await .context("failed to read raw block fees from state")? .map(|bytes| { - let Ok(bytes): Result<[u8; 16], _> = bytes.try_into() else { - // this shouldn't happen - bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); - }; - Ok(u128::from_be_bytes(bytes)) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid block fees bytes") }) .transpose()? .unwrap_or_default(); @@ -225,8 +236,10 @@ pub(crate) trait StateWriteExt: StateWrite { let new_amount = current_amount .checked_add(amount) .context("block fees overflowed u128")?; - - self.nonverifiable_put_raw(block_fees_key.into(), new_amount.to_be_bytes().to_vec()); + let bytes = StoredValue::Fee(new_amount.into()) + .serialize() + .context("failed to serialize block fees")?; + self.nonverifiable_put_raw(block_fees_key.into(), bytes); self.record(tx_fee_event); @@ -243,19 +256,23 @@ pub(crate) trait StateWriteExt: StateWrite { } #[instrument(skip_all)] - fn delete_allowed_fee_asset(&mut self, asset: TAsset) + fn delete_allowed_fee_asset<'a, TAsset>(&mut self, asset: &'a TAsset) where - TAsset: Into + std::fmt::Display, + asset::IbcPrefixed: From<&'a TAsset>, { self.nonverifiable_delete(fee_asset_key(asset).into()); } #[instrument(skip_all)] - fn put_allowed_fee_asset(&mut self, asset: TAsset) + fn put_allowed_fee_asset<'a, TAsset>(&mut self, asset: &'a TAsset) -> Result<()> where - TAsset: Into + std::fmt::Display + Send, + asset::IbcPrefixed: From<&'a TAsset>, { - self.nonverifiable_put_raw(fee_asset_key(asset).into(), vec![]); + let bytes = StoredValue::Unit + .serialize() + .context("failed to serialize unit for allowed fee asset")?; + self.nonverifiable_put_raw(fee_asset_key(asset).into(), bytes); + Ok(()) } } @@ -303,8 +320,8 @@ mod tests { .expect_err("no native asset denom should exist at first"); // can write - let denom_orig = "denom_orig".parse().unwrap(); - state.put_native_asset(&denom_orig); + let denom_orig: asset::TracePrefixed = "denom_orig".parse().unwrap(); + state.put_native_asset(denom_orig.clone()).unwrap(); assert_eq!( state.get_native_asset().await.expect( "a native asset denomination was written and must exist inside the database" @@ -314,8 +331,8 @@ mod tests { ); // can write new value - let denom_update = "denom_update".parse().unwrap(); - state.put_native_asset(&denom_update); + let denom_update: asset::TracePrefixed = "denom_update".parse().unwrap(); + state.put_native_asset(denom_update.clone()).unwrap(); assert_eq!( state.get_native_asset().await.expect( "a native asset denomination update was written and must exist inside the database" @@ -404,7 +421,7 @@ mod tests { // gets for non existing assets should return none assert_eq!( state - .map_ibc_to_trace_prefixed_asset(asset.to_ibc_prefixed()) + .map_ibc_to_trace_prefixed_asset(&asset.to_ibc_prefixed()) .await .expect("getting non existing asset should not fail"), None @@ -429,7 +446,7 @@ mod tests { ); state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .put_ibc_asset(denom.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); // existing calls are ok for 'has' @@ -451,11 +468,11 @@ mod tests { // can write new let denom = asset(); state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .put_ibc_asset(denom.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .map_ibc_to_trace_prefixed_asset(&denom.to_ibc_prefixed()) .await .unwrap() .expect("an ibc asset was written and must exist inside the database"), @@ -473,11 +490,11 @@ mod tests { // can write new let denom = asset_0(); state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .put_ibc_asset(denom.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .map_ibc_to_trace_prefixed_asset(&denom.to_ibc_prefixed()) .await .unwrap() .expect("an ibc asset was written and must exist inside the database"), @@ -488,11 +505,11 @@ mod tests { // can write another without affecting original let denom_1 = asset_1(); state - .put_ibc_asset(&denom_1.clone().unwrap_trace_prefixed()) + .put_ibc_asset(denom_1.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .map_ibc_to_trace_prefixed_asset(denom_1.to_ibc_prefixed()) + .map_ibc_to_trace_prefixed_asset(&denom_1.to_ibc_prefixed()) .await .unwrap() .expect("an additional ibc asset was written and must exist inside the database"), @@ -501,7 +518,7 @@ mod tests { ); assert_eq!( state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .map_ibc_to_trace_prefixed_asset(&denom.to_ibc_prefixed()) .await .unwrap() .expect("an ibc asset was written and must exist inside the database"), @@ -527,7 +544,7 @@ mod tests { ); // existent fee assets return true - state.put_allowed_fee_asset(&asset); + state.put_allowed_fee_asset(&asset).unwrap(); assert!( state .is_allowed_fee_asset(&asset) @@ -545,7 +562,7 @@ mod tests { // setup fee asset let asset = asset_0(); - state.put_allowed_fee_asset(&asset); + state.put_allowed_fee_asset(&asset).unwrap(); assert!( state .is_allowed_fee_asset(&asset) @@ -578,7 +595,7 @@ mod tests { // setup fee assets let asset_first = asset_0(); - state.put_allowed_fee_asset(&asset_first); + state.put_allowed_fee_asset(&asset_first).unwrap(); assert!( state .is_allowed_fee_asset(&asset_first) @@ -587,7 +604,7 @@ mod tests { "fee asset was expected to be allowed" ); let asset_second = asset_1(); - state.put_allowed_fee_asset(&asset_second); + state.put_allowed_fee_asset(&asset_second).unwrap(); assert!( state .is_allowed_fee_asset(&asset_second) @@ -596,7 +613,7 @@ mod tests { "fee asset was expected to be allowed" ); let asset_third = asset_2(); - state.put_allowed_fee_asset(&asset_third); + state.put_allowed_fee_asset(&asset_third).unwrap(); assert!( state .is_allowed_fee_asset(&asset_third) @@ -627,23 +644,23 @@ mod tests { .unwrap(); assert_eq!( asset_storage_key(&asset), - asset_storage_key(asset.to_ibc_prefixed()), + asset_storage_key(&asset.to_ibc_prefixed()), ); - insta::assert_snapshot!(asset_storage_key(asset)); + insta::assert_snapshot!(asset_storage_key(&asset)); let trace_prefixed = "a/denom/with/a/prefix" .parse::() .unwrap(); assert_eq!( block_fees_key(&trace_prefixed), - block_fees_key(trace_prefixed.to_ibc_prefixed()), + block_fees_key(&trace_prefixed.to_ibc_prefixed()), ); insta::assert_snapshot!(block_fees_key(&trace_prefixed)); assert_eq!( fee_asset_key(&trace_prefixed), - fee_asset_key(trace_prefixed.to_ibc_prefixed()), + fee_asset_key(&trace_prefixed.to_ibc_prefixed()), ); - insta::assert_snapshot!(fee_asset_key(trace_prefixed)); + insta::assert_snapshot!(fee_asset_key(&trace_prefixed)); } } diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 1997c30f0a..c93fa68568 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -125,33 +125,27 @@ impl ActionHandler for FeeChangeAction { .context("failed to get sudo address from state")?; ensure!(sudo_address == from, "signer is not the sudo key"); + // No need to add context to any of these `put` calls, as they already report sufficient + // context on error. match self.fee_change { - FeeChange::TransferBaseFee => { - state - .put_transfer_base_fee(self.new_value) - .context("failed to put transfer base fee in state")?; - } + FeeChange::TransferBaseFee => state.put_transfer_base_fee(self.new_value), FeeChange::SequenceBaseFee => state.put_sequence_action_base_fee(self.new_value), FeeChange::SequenceByteCostMultiplier => { - state.put_sequence_action_byte_cost_multiplier(self.new_value); + state.put_sequence_action_byte_cost_multiplier(self.new_value) } FeeChange::InitBridgeAccountBaseFee => { - state.put_init_bridge_account_base_fee(self.new_value); + state.put_init_bridge_account_base_fee(self.new_value) } FeeChange::BridgeLockByteCostMultiplier => { - state.put_bridge_lock_byte_cost_multiplier(self.new_value); + state.put_bridge_lock_byte_cost_multiplier(self.new_value) } FeeChange::BridgeSudoChangeBaseFee => { - state.put_bridge_sudo_change_base_fee(self.new_value); + state.put_bridge_sudo_change_base_fee(self.new_value) } FeeChange::Ics20WithdrawalBaseFee => { - state - .put_ics20_withdrawal_base_fee(self.new_value) - .context("failed to put ics20 withdrawal base fee in state")?; + state.put_ics20_withdrawal_base_fee(self.new_value) } } - - Ok(()) } } @@ -197,7 +191,9 @@ mod test { assert_eq!(state.get_transfer_base_fee().await.unwrap(), 10); let sequence_base_fee = 5; - state.put_sequence_action_base_fee(sequence_base_fee); + state + .put_sequence_action_base_fee(sequence_base_fee) + .unwrap(); let fee_change = FeeChangeAction { fee_change: FeeChange::SequenceBaseFee, @@ -208,7 +204,9 @@ mod test { assert_eq!(state.get_sequence_action_base_fee().await.unwrap(), 3); let sequence_byte_cost_multiplier = 2; - state.put_sequence_action_byte_cost_multiplier(sequence_byte_cost_multiplier); + state + .put_sequence_action_byte_cost_multiplier(sequence_byte_cost_multiplier) + .unwrap(); let fee_change = FeeChangeAction { fee_change: FeeChange::SequenceByteCostMultiplier, @@ -225,7 +223,9 @@ mod test { ); let init_bridge_account_base_fee = 1; - state.put_init_bridge_account_base_fee(init_bridge_account_base_fee); + state + .put_init_bridge_account_base_fee(init_bridge_account_base_fee) + .unwrap(); let fee_change = FeeChangeAction { fee_change: FeeChange::InitBridgeAccountBaseFee, @@ -236,7 +236,9 @@ mod test { assert_eq!(state.get_init_bridge_account_base_fee().await.unwrap(), 2); let bridge_lock_byte_cost_multiplier = 1; - state.put_bridge_lock_byte_cost_multiplier(bridge_lock_byte_cost_multiplier); + state + .put_bridge_lock_byte_cost_multiplier(bridge_lock_byte_cost_multiplier) + .unwrap(); let fee_change = FeeChangeAction { fee_change: FeeChange::BridgeLockByteCostMultiplier, diff --git a/crates/astria-sequencer/src/authority/component.rs b/crates/astria-sequencer/src/authority/component.rs index 91d8d71c7b..8bdf0b5c64 100644 --- a/crates/astria-sequencer/src/authority/component.rs +++ b/crates/astria-sequencer/src/authority/component.rs @@ -58,7 +58,7 @@ impl Component for AuthorityComponent { .context("failed getting validator set")?; for misbehaviour in &begin_block.byzantine_validators { - current_set.remove(misbehaviour.validator.address); + current_set.remove(&misbehaviour.validator.address); } let state = Arc::get_mut(state) diff --git a/crates/astria-sequencer/src/authority/mod.rs b/crates/astria-sequencer/src/authority/mod.rs index 5dba34b9a1..6db5009ccf 100644 --- a/crates/astria-sequencer/src/authority/mod.rs +++ b/crates/astria-sequencer/src/authority/mod.rs @@ -6,64 +6,56 @@ use std::collections::BTreeMap; use anyhow::Context as _; use astria_core::{ - crypto::VerificationKey, primitive::v1::ADDRESS_LEN, protocol::transaction::v1alpha1::action::ValidatorUpdate, }; -use serde::{ - Deserialize, - Serialize, -}; pub(crate) use state_ext::{ StateReadExt, StateWriteExt, }; -#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Ord, PartialOrd)] -pub(crate) struct ValidatorSetKey(#[serde(with = "::hex::serde")] [u8; ADDRESS_LEN]); - -impl From<[u8; ADDRESS_LEN]> for ValidatorSetKey { - fn from(value: [u8; ADDRESS_LEN]) -> Self { - Self(value) - } -} - -impl From<&VerificationKey> for ValidatorSetKey { - fn from(value: &VerificationKey) -> Self { - Self(value.address_bytes()) - } -} +use crate::accounts::AddressBytes; /// Newtype wrapper to read and write a validator set or set of updates from rocksdb. /// /// Contains a map of hex-encoded public keys to validator updates. -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] -pub(crate) struct ValidatorSet(BTreeMap); +#[derive(Debug, PartialEq, Eq)] +#[cfg_attr(test, derive(Clone))] +pub(crate) struct ValidatorSet(BTreeMap<[u8; ADDRESS_LEN], ValidatorUpdate>); impl ValidatorSet { + pub(crate) fn new(inner: BTreeMap<[u8; ADDRESS_LEN], ValidatorUpdate>) -> Self { + Self(inner) + } + pub(crate) fn new_from_updates(updates: Vec) -> Self { Self( updates .into_iter() - .map(|update| ((&update.verification_key).into(), update)) + .map(|update| (*update.verification_key.address_bytes(), update)) .collect::>(), ) } + pub(crate) fn updates(&self) -> impl Iterator { + self.0.values() + } + pub(crate) fn len(&self) -> usize { self.0.len() } - pub(crate) fn get>(&self, address: T) -> Option<&ValidatorUpdate> { - self.0.get(&address.into()) + pub(crate) fn get(&self, address: &T) -> Option<&ValidatorUpdate> { + self.0.get(address.address_bytes()) } pub(super) fn push_update(&mut self, update: ValidatorUpdate) { - self.0.insert((&update.verification_key).into(), update); + self.0 + .insert(*update.verification_key.address_bytes(), update); } - pub(super) fn remove>(&mut self, address: T) { - self.0.remove(&address.into()); + pub(super) fn remove(&mut self, address: &T) { + self.0.remove(address.address_bytes()); } /// Apply updates to the validator set. diff --git a/crates/astria-sequencer/src/authority/state_ext.rs b/crates/astria-sequencer/src/authority/state_ext.rs index 2219b4ca49..7aa1de719c 100644 --- a/crates/astria-sequencer/src/authority/state_ext.rs +++ b/crates/astria-sequencer/src/authority/state_ext.rs @@ -7,10 +7,6 @@ use anyhow::{ }; use astria_core::primitive::v1::ADDRESS_LEN; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, @@ -18,11 +14,13 @@ use cnidarium::{ use tracing::instrument; use super::ValidatorSet; -use crate::accounts::AddressBytes; - -/// Newtype wrapper to read and write an address from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct SudoAddress([u8; ADDRESS_LEN]); +use crate::{ + accounts::AddressBytes, + storage::{ + self, + StoredValue, + }, +}; const SUDO_STORAGE_KEY: &str = "sudo"; const VALIDATOR_SET_STORAGE_KEY: &str = "valset"; @@ -40,9 +38,9 @@ pub(crate) trait StateReadExt: StateRead { // return error because sudo key must be set bail!("sudo key not found"); }; - let SudoAddress(address_bytes) = - SudoAddress::try_from_slice(&bytes).context("invalid sudo key bytes")?; - Ok(address_bytes) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::AddressBytes::try_from(value).map(<[u8; ADDRESS_LEN]>::from)) + .context("invalid sudo key bytes") } #[instrument(skip_all)] @@ -55,10 +53,9 @@ pub(crate) trait StateReadExt: StateRead { // return error because validator set must be set bail!("validator set not found") }; - - let ValidatorSet(validator_set) = - serde_json::from_slice(&bytes).context("invalid validator set bytes")?; - Ok(ValidatorSet(validator_set)) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::ValidatorSet::try_from(value).map(ValidatorSet::from)) + .context("invalid validator set bytes") } #[instrument(skip_all)] @@ -71,10 +68,9 @@ pub(crate) trait StateReadExt: StateRead { // return empty set because validator updates are optional return Ok(ValidatorSet(BTreeMap::new())); }; - - let validator_updates: ValidatorSet = - serde_json::from_slice(&bytes).context("invalid validator updates bytes")?; - Ok(validator_updates) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::ValidatorSet::try_from(value).map(ValidatorSet::from)) + .context("invalid validator update bytes") } } @@ -84,30 +80,28 @@ impl StateReadExt for T {} pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] fn put_sudo_address(&mut self, address: T) -> Result<()> { - self.put_raw( - SUDO_STORAGE_KEY.to_string(), - borsh::to_vec(&SudoAddress(address.address_bytes())) - .context("failed to convert sudo address to vec")?, - ); + let bytes = StoredValue::AddressBytes((&address).into()) + .serialize() + .context("failed to serialize sudo address")?; + self.put_raw(SUDO_STORAGE_KEY.to_string(), bytes); Ok(()) } #[instrument(skip_all)] fn put_validator_set(&mut self, validator_set: ValidatorSet) -> Result<()> { - self.put_raw( - VALIDATOR_SET_STORAGE_KEY.to_string(), - serde_json::to_vec(&validator_set).context("failed to serialize validator set")?, - ); + let bytes = StoredValue::ValidatorSet((&validator_set).into()) + .serialize() + .context("failed to serialize validator set")?; + self.put_raw(VALIDATOR_SET_STORAGE_KEY.to_string(), bytes); Ok(()) } #[instrument(skip_all)] fn put_validator_updates(&mut self, validator_updates: ValidatorSet) -> Result<()> { - self.nonverifiable_put_raw( - VALIDATOR_UPDATES_KEY.to_vec(), - serde_json::to_vec(&validator_updates) - .context("failed to serialize validator updates")?, - ); + let bytes = StoredValue::ValidatorSet((&validator_updates).into()) + .serialize() + .context("failed to serialize validator updates")?; + self.nonverifiable_put_raw(VALIDATOR_UPDATES_KEY.to_vec(), bytes); Ok(()) } @@ -150,7 +144,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // doesn't exist at first state diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index da913778d6..90f1c5e538 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -50,13 +50,13 @@ impl ActionHandler for BridgeLockAction { .context("failed check for base prefix of destination address")?; // ensure the recipient is a bridge account. let rollup_id = state - .get_bridge_account_rollup_id(self.to) + .get_bridge_account_rollup_id(&self.to) .await .context("failed to get bridge account rollup id")? .ok_or_else(|| anyhow::anyhow!("bridge lock must be sent to a bridge account"))?; let allowed_asset = state - .get_bridge_account_ibc_asset(self.to) + .get_bridge_account_ibc_asset(&self.to) .await .context("failed to get bridge account asset ID")?; ensure!( @@ -65,7 +65,7 @@ impl ActionHandler for BridgeLockAction { ); let from_balance = state - .get_account_balance(from, &self.fee_asset) + .get_account_balance(&from, &self.fee_asset) .await .context("failed to get sender account balance")?; let transfer_fee = state @@ -109,17 +109,16 @@ impl ActionHandler for BridgeLockAction { fee_asset: self.fee_asset.clone(), }; - check_transfer(&transfer_action, from, &state).await?; + check_transfer(&transfer_action, &from, &state).await?; // Executes the transfer and deducts transfer feeds. // FIXME: This is a very roundabout way of paying for fees. IMO it would be // better to just duplicate this entire logic here so that we don't call out // to the transfer-action logic. - execute_transfer(&transfer_action, from, &mut state).await?; + execute_transfer(&transfer_action, &from, &mut state).await?; - // the transfer fee is already deducted in `execute_transfer() above, // so we just deduct the bridge lock byte multiplier fee. // FIXME: similar to what is mentioned there: this should be reworked so that - // the fee deducation logic for these actions are defined fully independently + // the fee deduction logic for these actions are defined fully independently // (even at the cost of duplicating code). let byte_cost_multiplier = state .get_bridge_lock_byte_cost_multiplier() @@ -131,7 +130,7 @@ impl ActionHandler for BridgeLockAction { .await .context("failed to add to block fees")?; state - .decrease_balance(from, &self.fee_asset, fee) + .decrease_balance(&from, &self.fee_asset, fee) .await .context("failed to deduct fee from account balance")?; @@ -188,14 +187,14 @@ mod tests { let from_address = astria_address(&[2; 20]); let transaction_id = TransactionId::new([0; 32]); state.put_transaction_context(TransactionContext { - address_bytes: from_address.bytes(), + address_bytes: *from_address.bytes(), transaction_id, source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); state.put_transfer_base_fee(transfer_fee).unwrap(); - state.put_bridge_lock_byte_cost_multiplier(2); + state.put_bridge_lock_byte_cost_multiplier(2).unwrap(); let bridge_address = astria_address(&[1; 20]); let asset = test_asset(); @@ -208,15 +207,17 @@ mod tests { }; let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); - state.put_bridge_account_rollup_id(bridge_address, &rollup_id); state - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) .unwrap(); - state.put_allowed_fee_asset(&asset); + state + .put_bridge_account_ibc_asset(&bridge_address, asset.clone()) + .unwrap(); + state.put_allowed_fee_asset(&asset).unwrap(); // not enough balance; should fail state - .put_account_balance(from_address, &asset, 100 + transfer_fee) + .put_account_balance(&from_address, &asset, 100 + transfer_fee) .unwrap(); assert_anyhow_error( &bridge_lock.check_and_execute(&mut state).await.unwrap_err(), @@ -235,7 +236,7 @@ mod tests { 0, )) * 2; state - .put_account_balance(from_address, &asset, 100 + expected_deposit_fee) + .put_account_balance(&from_address, &asset, 100 + expected_deposit_fee) .unwrap(); bridge_lock.check_and_execute(&mut state).await.unwrap(); } diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs index a7390df048..16f79d14bf 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -61,7 +61,7 @@ impl ActionHandler for BridgeSudoChangeAction { // check that the sender of this tx is the authorized sudo address for the bridge account let Some(sudo_address) = state - .get_bridge_account_sudo_address(self.bridge_address) + .get_bridge_account_sudo_address(&self.bridge_address) .await .context("failed to get bridge account sudo address")? else { @@ -84,16 +84,19 @@ impl ActionHandler for BridgeSudoChangeAction { .await .context("failed to add to block fees")?; state - .decrease_balance(self.bridge_address, &self.fee_asset, fee) + .decrease_balance(&self.bridge_address, &self.fee_asset, fee) .await .context("failed to decrease balance for bridge sudo change fee")?; if let Some(sudo_address) = self.new_sudo_address { - state.put_bridge_account_sudo_address(self.bridge_address, sudo_address); + // No need to add context as this method already reports sufficient context on error. + state.put_bridge_account_sudo_address(&self.bridge_address, sudo_address)?; } if let Some(withdrawer_address) = self.new_withdrawer_address { - state.put_bridge_account_withdrawer_address(self.bridge_address, withdrawer_address); + // No need to add context as this method already reports sufficient context on error. + state + .put_bridge_account_withdrawer_address(&self.bridge_address, withdrawer_address)?; } Ok(()) @@ -136,14 +139,16 @@ mod tests { transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); let asset = test_asset(); - state.put_allowed_fee_asset(&asset); + state.put_allowed_fee_asset(&asset).unwrap(); let bridge_address = astria_address(&[99; 20]); let sudo_address = astria_address(&[98; 20]); - state.put_bridge_account_sudo_address(bridge_address, sudo_address); + state + .put_bridge_account_sudo_address(&bridge_address, sudo_address) + .unwrap(); let action = BridgeSudoChangeAction { bridge_address, @@ -170,24 +175,26 @@ mod tests { let sudo_address = astria_address(&[98; 20]); state.put_transaction_context(TransactionContext { - address_bytes: sudo_address.bytes(), + address_bytes: *sudo_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); - state.put_bridge_sudo_change_base_fee(10); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); + state.put_bridge_sudo_change_base_fee(10).unwrap(); let fee_asset = test_asset(); - state.put_allowed_fee_asset(&fee_asset); + state.put_allowed_fee_asset(&fee_asset).unwrap(); let bridge_address = astria_address(&[99; 20]); - state.put_bridge_account_sudo_address(bridge_address, sudo_address); + state + .put_bridge_account_sudo_address(&bridge_address, sudo_address) + .unwrap(); let new_sudo_address = astria_address(&[98; 20]); let new_withdrawer_address = astria_address(&[97; 20]); state - .put_account_balance(bridge_address, &fee_asset, 10) + .put_account_balance(&bridge_address, &fee_asset, 10) .unwrap(); let action = BridgeSudoChangeAction { @@ -201,17 +208,17 @@ mod tests { assert_eq!( state - .get_bridge_account_sudo_address(bridge_address) + .get_bridge_account_sudo_address(&bridge_address) .await .unwrap(), - Some(new_sudo_address.bytes()), + Some(*new_sudo_address.bytes()), ); assert_eq!( state - .get_bridge_account_withdrawer_address(bridge_address) + .get_bridge_account_withdrawer_address(&bridge_address) .await .unwrap(), - Some(new_withdrawer_address.bytes()), + Some(*new_withdrawer_address.bytes()), ); } } diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index 505671f2cb..1abf25792e 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -60,13 +60,13 @@ impl ActionHandler for BridgeUnlockAction { .context("failed check for base prefix of bridge address")?; let asset = state - .get_bridge_account_ibc_asset(self.bridge_address) + .get_bridge_account_ibc_asset(&self.bridge_address) .await .context("failed to get bridge's asset id, must be a bridge account")?; // check that the sender of this tx is the authorized withdrawer for the bridge account let Some(withdrawer_address) = state - .get_bridge_account_withdrawer_address(self.bridge_address) + .get_bridge_account_withdrawer_address(&self.bridge_address) .await .context("failed to get bridge account withdrawer address")? else { @@ -85,16 +85,16 @@ impl ActionHandler for BridgeUnlockAction { fee_asset: self.fee_asset.clone(), }; - check_transfer(&transfer_action, self.bridge_address, &state).await?; + check_transfer(&transfer_action, &self.bridge_address, &state).await?; state .check_and_set_withdrawal_event_block_for_bridge_account( - self.bridge_address, + &self.bridge_address, &self.rollup_withdrawal_event_id, self.rollup_block_number, ) .await .context("withdrawal event already processed")?; - execute_transfer(&transfer_action, self.bridge_address, state).await?; + execute_transfer(&transfer_action, &self.bridge_address, state).await?; Ok(()) } @@ -144,7 +144,7 @@ mod tests { transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); let asset = test_asset(); let transfer_amount = 100; @@ -152,7 +152,7 @@ mod tests { let to_address = astria_address(&[2; 20]); let bridge_address = astria_address(&[3; 20]); state - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); let bridge_unlock = BridgeUnlockAction { @@ -183,7 +183,7 @@ mod tests { transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); let asset = test_asset(); let transfer_amount = 100; @@ -191,9 +191,11 @@ mod tests { let to_address = astria_address(&[2; 20]); let bridge_address = astria_address(&[3; 20]); let withdrawer_address = astria_address(&[4; 20]); - state.put_bridge_account_withdrawer_address(bridge_address, withdrawer_address); state - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_bridge_account_withdrawer_address(&bridge_address, withdrawer_address) + .unwrap(); + state + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); let bridge_unlock = BridgeUnlockAction { @@ -221,11 +223,11 @@ mod tests { let bridge_address = astria_address(&[1; 20]); state.put_transaction_context(TransactionContext { - address_bytes: bridge_address.bytes(), + address_bytes: *bridge_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); let asset = test_asset(); let transfer_fee = 10; @@ -235,15 +237,19 @@ mod tests { let to_address = astria_address(&[2; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); - state.put_bridge_account_rollup_id(bridge_address, &rollup_id); state - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state + .put_bridge_account_ibc_asset(&bridge_address, &asset) + .unwrap(); + state + .put_bridge_account_withdrawer_address(&bridge_address, bridge_address) .unwrap(); - state.put_bridge_account_withdrawer_address(bridge_address, bridge_address); - state.put_allowed_fee_asset(&asset); + state.put_allowed_fee_asset(&asset).unwrap(); // Put plenty of balance state - .put_account_balance(bridge_address, &asset, 3 * transfer_amount) + .put_account_balance(&bridge_address, &asset, 3 * transfer_amount) .unwrap(); let bridge_unlock_first = BridgeUnlockAction { diff --git a/crates/astria-sequencer/src/bridge/component.rs b/crates/astria-sequencer/src/bridge/component.rs index 88c105e9d0..550a773d61 100644 --- a/crates/astria-sequencer/src/bridge/component.rs +++ b/crates/astria-sequencer/src/bridge/component.rs @@ -20,12 +20,12 @@ impl Component for BridgeComponent { #[instrument(name = "BridgeComponent::init_chain", skip_all)] async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> { - state.put_init_bridge_account_base_fee(app_state.fees().init_bridge_account_base_fee); + // No need to add context as these `put` methods already report sufficient context on error. + state.put_init_bridge_account_base_fee(app_state.fees().init_bridge_account_base_fee)?; state.put_bridge_lock_byte_cost_multiplier( app_state.fees().bridge_lock_byte_cost_multiplier, - ); - state.put_bridge_sudo_change_base_fee(app_state.fees().bridge_sudo_change_fee); - Ok(()) + )?; + state.put_bridge_sudo_change_base_fee(app_state.fees().bridge_sudo_change_fee) } #[instrument(name = "BridgeComponent::begin_block", skip_all)] diff --git a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs index 5a9c1575a1..2010d299f1 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs @@ -5,7 +5,6 @@ use anyhow::{ Result, }; use astria_core::{ - primitive::v1::Address, protocol::transaction::v1alpha1::action::InitBridgeAccountAction, Protobuf as _, }; @@ -75,7 +74,7 @@ impl ActionHandler for InitBridgeAccountAction { // after the account becomes a bridge account, it can no longer receive funds // via `TransferAction`, only via `BridgeLockAction`. if state - .get_bridge_account_rollup_id(from) + .get_bridge_account_rollup_id(&from) .await .context("failed getting rollup ID of bridge account")? .is_some() @@ -84,7 +83,7 @@ impl ActionHandler for InitBridgeAccountAction { } let balance = state - .get_account_balance(from, &self.fee_asset) + .get_account_balance(&from, &self.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; @@ -93,21 +92,28 @@ impl ActionHandler for InitBridgeAccountAction { "insufficient funds for bridge account initialization", ); - state.put_bridge_account_rollup_id(from, &self.rollup_id); + // No need to add context as this method already reports sufficient context on error. + state.put_bridge_account_rollup_id(&from, self.rollup_id)?; state - .put_bridge_account_ibc_asset(from, &self.asset) + .put_bridge_account_ibc_asset(&from, &self.asset) .context("failed to put asset ID")?; - state.put_bridge_account_sudo_address(from, self.sudo_address.map_or(from, Address::bytes)); + // No need to add context as this method already reports sufficient context on error. + state.put_bridge_account_sudo_address( + &from, + self.sudo_address.map_or(from, |address| *address.bytes()), + )?; + // No need to add context as this method already reports sufficient context on error. state.put_bridge_account_withdrawer_address( - from, - self.withdrawer_address.map_or(from, Address::bytes), - ); + &from, + self.withdrawer_address + .map_or(from, |address| *address.bytes()), + )?; state .get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name()) .await .context("failed to get and increase block fees")?; state - .decrease_balance(from, &self.fee_asset, fee) + .decrease_balance(&from, &self.fee_asset, fee) .await .context("failed to deduct fee from account balance")?; Ok(()) diff --git a/crates/astria-sequencer/src/bridge/query.rs b/crates/astria-sequencer/src/bridge/query.rs index 25f4fdf6f3..5c6484e0a4 100644 --- a/crates/astria-sequencer/src/bridge/query.rs +++ b/crates/astria-sequencer/src/bridge/query.rs @@ -43,7 +43,7 @@ fn error_query_response( #[allow(clippy::too_many_lines)] async fn get_bridge_account_info( snapshot: cnidarium::Snapshot, - address: Address, + address: &Address, ) -> anyhow::Result, response::Query> { let rollup_id = match snapshot.get_bridge_account_rollup_id(address).await { Ok(Some(rollup_id)) => rollup_id, @@ -70,7 +70,7 @@ async fn get_bridge_account_info( } }; - let trace_asset = match snapshot.map_ibc_to_trace_prefixed_asset(ibc_asset).await { + let trace_asset = match snapshot.map_ibc_to_trace_prefixed_asset(&ibc_asset).await { Ok(Some(trace_asset)) => trace_asset, Ok(None) => { return Err(error_query_response( @@ -183,7 +183,7 @@ pub(crate) async fn bridge_account_info_request( } }; - let info = match get_bridge_account_info(snapshot, address).await { + let info = match get_bridge_account_info(snapshot, &address).await { Ok(info) => info, Err(err) => { return err; @@ -238,7 +238,7 @@ pub(crate) async fn bridge_account_last_tx_hash_request( { Ok(Some(tx_id)) => BridgeAccountLastTxHashResponse { height, - tx_hash: Some(tx_id.get()), + tx_hash: Some(*tx_id.get()), }, Ok(None) => BridgeAccountLastTxHashResponse { height, @@ -314,23 +314,29 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); let asset: astria_core::primitive::v1::asset::Denom = "test".parse().unwrap(); let rollup_id = RollupId::from_unhashed_bytes("test"); let bridge_address = astria_address(&[0u8; 20]); let sudo_address = astria_address(&[1u8; 20]); let withdrawer_address = astria_address(&[2u8; 20]); - state.put_block_height(1); - state.put_bridge_account_rollup_id(bridge_address, &rollup_id); + state.put_block_height(1).unwrap(); state - .put_ibc_asset(asset.as_trace_prefixed().unwrap()) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) .unwrap(); state - .put_bridge_account_ibc_asset(bridge_address, &asset) + .put_ibc_asset(asset.as_trace_prefixed().unwrap().clone()) + .unwrap(); + state + .put_bridge_account_ibc_asset(&bridge_address, &asset) + .unwrap(); + state + .put_bridge_account_sudo_address(&bridge_address, sudo_address) + .unwrap(); + state + .put_bridge_account_withdrawer_address(&bridge_address, withdrawer_address) .unwrap(); - state.put_bridge_account_sudo_address(bridge_address, sudo_address); - state.put_bridge_account_withdrawer_address(bridge_address, withdrawer_address); storage.commit(state).await.unwrap(); let query = request::Query { diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index 846eb5f846..34a5c46144 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -10,10 +10,8 @@ use anyhow::{ Result, }; use astria_core::{ - generated::sequencerblock::v1alpha1::Deposit as RawDeposit, primitive::v1::{ asset, - Address, RollupId, TransactionId, ADDRESS_LEN, @@ -21,17 +19,12 @@ use astria_core::{ sequencerblock::v1alpha1::block::Deposit, }; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, }; use futures::StreamExt as _; use hex::ToHex as _; -use prost::Message as _; use tracing::{ debug, instrument, @@ -40,24 +33,12 @@ use tracing::{ use crate::{ accounts::AddressBytes, address, + storage::{ + self, + StoredValue, + }, }; -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Balance(u128); - -/// Newtype wrapper to read and write a u32 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Nonce(u32); - -/// Newtype wrapper to read and write a Vec<[u8; 32]> from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct AssetId([u8; 32]); - -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Fee(u128); - const BRIDGE_ACCOUNT_PREFIX: &str = "bridgeacc"; const BRIDGE_ACCOUNT_SUDO_PREFIX: &str = "bsudo"; const BRIDGE_ACCOUNT_WITHDRAWER_PREFIX: &str = "bwithdrawer"; @@ -166,7 +147,7 @@ fn last_transaction_id_for_bridge_account_storage_key(address: #[async_trait] pub(crate) trait StateReadExt: StateRead + address::StateReadExt { #[instrument(skip_all)] - async fn is_a_bridge_account(&self, address: T) -> anyhow::Result { + async fn is_a_bridge_account(&self, address: &T) -> anyhow::Result { let maybe_id = self.get_bridge_account_rollup_id(address).await?; Ok(maybe_id.is_some()) } @@ -174,67 +155,71 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { #[instrument(skip_all)] async fn get_bridge_account_rollup_id( &self, - address: T, + address: &T, ) -> Result> { - let Some(rollup_id_bytes) = self - .get_raw(&rollup_id_storage_key(&address)) + let Some(bytes) = self + .get_raw(&rollup_id_storage_key(address)) .await .context("failed reading raw account rollup ID from state")? else { debug!("account rollup ID not found, returning None"); return Ok(None); }; - - let rollup_id = - RollupId::try_from_slice(&rollup_id_bytes).context("invalid rollup ID bytes")?; - Ok(Some(rollup_id)) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::RollupId::try_from(value) + .map(|stored_rollup_id| Some(RollupId::from(stored_rollup_id))) + }) + .context("invalid rollup ID bytes") } #[instrument(skip_all)] async fn get_bridge_account_ibc_asset( &self, - address: T, + address: &T, ) -> Result { let bytes = self - .get_raw(&asset_id_storage_key(&address)) + .get_raw(&asset_id_storage_key(address)) .await - .context("failed reading raw asset ID from state")? - .ok_or_else(|| anyhow!("asset ID not found"))?; - let id = borsh::from_slice::(&bytes) - .context("failed to reconstruct asset ID from storage")?; - Ok(asset::IbcPrefixed::new(id.0)) + .context("failed reading raw bridge account asset ID from state")? + .ok_or_else(|| anyhow!("bridge account asset ID not found"))?; + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::IbcPrefixedDenom::try_from(value).map(asset::IbcPrefixed::from) + }) + .context("invalid bridge account asset ID bytes") } #[instrument(skip_all)] async fn get_bridge_account_sudo_address( &self, - bridge_address: T, + bridge_address: &T, ) -> Result> { - let Some(sudo_address_bytes) = self - .get_raw(&bridge_account_sudo_address_storage_key(&bridge_address)) + let Some(bytes) = self + .get_raw(&bridge_account_sudo_address_storage_key(bridge_address)) .await .context("failed reading raw bridge account sudo address from state")? else { debug!("bridge account sudo address not found, returning None"); return Ok(None); }; - let sudo_address = sudo_address_bytes.try_into().map_err(|bytes: Vec<_>| { - anyhow::format_err!( - "failed to convert address `{}` bytes read from state to fixed length address", - bytes.len() - ) - })?; - Ok(Some(sudo_address)) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::AddressBytes::try_from(value).map(|stored_address_bytes| { + Some(<[u8; ADDRESS_LEN]>::from(stored_address_bytes)) + }) + }) + .context("invalid bridge account sudo address bytes") } #[instrument(skip_all)] async fn get_bridge_account_withdrawer_address( &self, - bridge_address: T, + bridge_address: &T, ) -> Result> { - let Some(withdrawer_address_bytes) = self + let Some(bytes) = self .get_raw(&bridge_account_withdrawer_address_storage_key( - &bridge_address, + bridge_address, )) .await .context("failed reading raw bridge account withdrawer address from state")? @@ -242,15 +227,13 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { debug!("bridge account withdrawer address not found, returning None"); return Ok(None); }; - let addr = withdrawer_address_bytes - .try_into() - .map_err(|bytes: Vec<_>| { - anyhow::Error::msg(format!( - "failed converting `{}` bytes retrieved from storage to fixed address length", - bytes.len() - )) - })?; - Ok(Some(addr)) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::AddressBytes::try_from(value).map(|stored_address_bytes| { + Some(<[u8; ADDRESS_LEN]>::from(stored_address_bytes)) + }) + }) + .context("invalid bridge account withdrawer address bytes") } #[instrument(skip_all)] @@ -263,12 +246,9 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { // no deposits for this rollup id yet; return 0 return Ok(0); }; - - let Nonce(nonce) = - Nonce(u32::from_be_bytes(bytes.try_into().expect( - "all deposit nonces stored should be 4 bytes; this is a bug", - ))); - Ok(nonce) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Nonce::try_from(value).map(u32::from)) + .context("invalid deposit nonce bytes") } #[instrument(skip_all)] @@ -298,9 +278,10 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { self.nonverifiable_prefix_raw(deposit_storage_key_prefix(rollup_id).as_bytes()) ); let mut deposits = Vec::new(); - while let Some(Ok((_, value))) = stream.next().await { - let raw = RawDeposit::decode(value.as_ref()).context("invalid deposit bytes")?; - let deposit = Deposit::try_from_raw(raw).context("invalid deposit raw proto")?; + while let Some(Ok((_, bytes))) = stream.next().await { + let deposit = StoredValue::deserialize(&bytes) + .and_then(|value| storage::Deposit::try_from(value).map(Deposit::from)) + .context("invalid deposit bytes")?; deposits.push(deposit); } Ok(deposits) @@ -330,8 +311,9 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { .await .context("failed reading raw init bridge account base fee from state")? .ok_or_else(|| anyhow!("init bridge account base fee not found"))?; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid fee bytes") } #[instrument(skip_all)] @@ -341,8 +323,9 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { .await .context("failed reading raw bridge lock byte cost multiplier from state")? .ok_or_else(|| anyhow!("bridge lock byte cost multiplier not found"))?; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid bridge lock byte cost multiplier bytes") } #[instrument(skip_all)] @@ -352,28 +335,30 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { .await .context("failed reading raw bridge sudo change fee from state")? .ok_or_else(|| anyhow!("bridge sudo change fee not found"))?; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid bridge sudo change fee bytes") } #[instrument(skip_all)] - async fn get_last_transaction_id_for_bridge_account( + async fn get_last_transaction_id_for_bridge_account( &self, - address: &Address, + address: &T, ) -> Result> { - let Some(tx_hash_bytes) = self + let Some(bytes) = self .nonverifiable_get_raw(&last_transaction_id_for_bridge_account_storage_key(address)) .await .context("failed reading raw last transaction hash for bridge account from state")? else { return Ok(None); }; - - let tx_hash: [u8; 32] = tx_hash_bytes - .try_into() - .expect("all transaction hashes stored should be 32 bytes; this is a bug"); - - Ok(Some(TransactionId::new(tx_hash))) + StoredValue::deserialize(&bytes) + .and_then(|value| { + storage::TransactionHash::try_from(value).map(|stored_tx_hash_bytes| { + Some(TransactionId::new(<[u8; 32]>::from(stored_tx_hash_bytes))) + }) + }) + .context("invalid bridge account transaction hash bytes") } } @@ -382,66 +367,84 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_bridge_account_rollup_id(&mut self, address: T, rollup_id: &RollupId) { - self.put_raw(rollup_id_storage_key(&address), rollup_id.to_vec()); + fn put_bridge_account_rollup_id( + &mut self, + address: &T, + rollup_id: RollupId, + ) -> Result<()> { + let bytes = StoredValue::RollupId((&rollup_id).into()) + .serialize() + .context("failed to serialize bridge account rollup id")?; + self.put_raw(rollup_id_storage_key(address), bytes); + Ok(()) } #[instrument(skip_all)] fn put_bridge_account_ibc_asset( &mut self, - address: TAddress, + address: &TAddress, asset: TAsset, ) -> Result<()> where TAddress: AddressBytes, - TAsset: Into + std::fmt::Display, + TAsset: Into, { let ibc = asset.into(); - self.put_raw( - asset_id_storage_key(&address), - borsh::to_vec(&AssetId(ibc.get())).context("failed to serialize asset IDs")?, - ); + let bytes = StoredValue::IbcPrefixedDenom((&ibc).into()) + .serialize() + .context("failed to serialize asset ids")?; + self.put_raw(asset_id_storage_key(address), bytes); Ok(()) } #[instrument(skip_all)] fn put_bridge_account_sudo_address( &mut self, - bridge_address: TBridgeAddress, + bridge_address: &TBridgeAddress, sudo_address: TSudoAddress, - ) where + ) -> Result<()> + where TBridgeAddress: AddressBytes, TSudoAddress: AddressBytes, { + let bytes = StoredValue::AddressBytes((&sudo_address).into()) + .serialize() + .context("failed to serialize bridge account sudo address")?; self.put_raw( - bridge_account_sudo_address_storage_key(&bridge_address), - sudo_address.address_bytes().to_vec(), + bridge_account_sudo_address_storage_key(bridge_address), + bytes, ); + Ok(()) } #[instrument(skip_all)] fn put_bridge_account_withdrawer_address( &mut self, - bridge_address: TBridgeAddress, + bridge_address: &TBridgeAddress, withdrawer_address: TWithdrawerAddress, - ) where + ) -> Result<()> + where TBridgeAddress: AddressBytes, TWithdrawerAddress: AddressBytes, { + let bytes = StoredValue::AddressBytes((&withdrawer_address).into()) + .serialize() + .context("failed to serialize bridge account sudo address")?; self.put_raw( - bridge_account_withdrawer_address_storage_key(&bridge_address), - withdrawer_address.address_bytes().to_vec(), + bridge_account_withdrawer_address_storage_key(bridge_address), + bytes, ); + Ok(()) } #[instrument(skip_all)] async fn check_and_set_withdrawal_event_block_for_bridge_account( &mut self, - address: T, + address: &T, withdrawal_event_id: &str, block_num: u64, ) -> Result<()> { - let key = bridge_account_withdrawal_event_storage_key(&address, withdrawal_event_id); + let key = bridge_account_withdrawal_event_storage_key(address, withdrawal_event_id); // Check if the withdrawal ID has already been used, if so return an error. let bytes = self @@ -449,19 +452,19 @@ pub(crate) trait StateWriteExt: StateWrite { .await .context("failed reading raw withdrawal event from state")?; if let Some(bytes) = bytes { - let existing_block_num = u64::from_be_bytes( - bytes - .try_into() - .expect("all block numbers stored should be 8 bytes; this is a bug"), - ); - + let existing_block_num = StoredValue::deserialize(&bytes) + .and_then(|value| storage::BlockHeight::try_from(value).map(u64::from)) + .context("invalid withdrawal event block height bytes")?; bail!( "withdrawal event ID {withdrawal_event_id} used by block number \ {existing_block_num}" ); } - self.put_raw(key, block_num.to_be_bytes().to_vec()); + let bytes = StoredValue::BlockHeight(block_num.into()) + .serialize() + .context("failed to serialize withdrawal event block height")?; + self.put_raw(key, bytes); Ok(()) } @@ -469,11 +472,12 @@ pub(crate) trait StateWriteExt: StateWrite { // this is only used to generate storage keys for each of the deposits within a block, // and is reset to 0 at the beginning of each block. #[instrument(skip_all)] - fn put_deposit_nonce(&mut self, rollup_id: &RollupId, nonce: u32) { - self.nonverifiable_put_raw( - deposit_nonce_storage_key(rollup_id), - nonce.to_be_bytes().to_vec(), - ); + fn put_deposit_nonce(&mut self, rollup_id: &RollupId, nonce: u32) -> Result<()> { + let bytes = StoredValue::Nonce(nonce.into()) + .serialize() + .context("failed to serialize deposit nonce")?; + self.nonverifiable_put_raw(deposit_nonce_storage_key(rollup_id), bytes); + Ok(()) } #[instrument(skip_all)] @@ -481,15 +485,18 @@ pub(crate) trait StateWriteExt: StateWrite { let nonce = self.get_deposit_nonce(deposit.rollup_id()).await?; self.put_deposit_nonce( deposit.rollup_id(), - nonce.checked_add(1).context("nonce overflowed")?, - ); + nonce.checked_add(1).context("deposit nonce overflowed")?, + )?; let key = deposit_storage_key(deposit.rollup_id(), nonce); - self.nonverifiable_put_raw(key, deposit.into_raw().encode_to_vec()); + let bytes = StoredValue::Deposit((&deposit).into()) + .serialize() + .context("failed to serialize bridge deposit")?; + self.nonverifiable_put_raw(key, bytes); Ok(()) } - // clears the deposit nonce and all deposits for for a given rollup ID. + /// Clears the deposit nonce and all deposits for a given rollup ID. #[instrument(skip_all)] async fn clear_deposit_info(&mut self, rollup_id: &RollupId) { self.nonverifiable_delete(deposit_nonce_storage_key(rollup_id)); @@ -514,39 +521,49 @@ pub(crate) trait StateWriteExt: StateWrite { } #[instrument(skip_all)] - fn put_init_bridge_account_base_fee(&mut self, fee: u128) { - self.put_raw( - INIT_BRIDGE_ACCOUNT_BASE_FEE_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).expect("failed to serialize fee"), - ); + fn put_init_bridge_account_base_fee(&mut self, fee: u128) -> Result<()> { + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize bridge account base fee")?; + self.put_raw(INIT_BRIDGE_ACCOUNT_BASE_FEE_STORAGE_KEY.to_string(), bytes); + Ok(()) } #[instrument(skip_all)] - fn put_bridge_lock_byte_cost_multiplier(&mut self, fee: u128) { + fn put_bridge_lock_byte_cost_multiplier(&mut self, fee: u128) -> Result<()> { + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize bridge lock byte cost multiplier")?; self.put_raw( BRIDGE_LOCK_BYTE_COST_MULTIPLIER_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).expect("failed to serialize fee"), + bytes, ); + Ok(()) } #[instrument(skip_all)] - fn put_bridge_sudo_change_base_fee(&mut self, fee: u128) { - self.put_raw( - BRIDGE_SUDO_CHANGE_FEE_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).expect("failed to serialize fee"), - ); + fn put_bridge_sudo_change_base_fee(&mut self, fee: u128) -> Result<()> { + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize bridge sudo change base fee")?; + self.put_raw(BRIDGE_SUDO_CHANGE_FEE_STORAGE_KEY.to_string(), bytes); + Ok(()) } #[instrument(skip_all)] fn put_last_transaction_id_for_bridge_account( &mut self, - address: T, - tx_id: &TransactionId, - ) { + address: &T, + tx_id: TransactionId, + ) -> Result<()> { + let bytes = StoredValue::TransactionHash(tx_id.get().into()) + .serialize() + .context("failed to serialize transaction hash for bridge account")?; self.nonverifiable_put_raw( - last_transaction_id_for_bridge_account_storage_key(&address), - tx_id.get().to_vec(), + last_transaction_id_for_bridge_account_storage_key(address), + bytes, ); + Ok(()) } } @@ -594,7 +611,7 @@ mod test { // uninitialized ok assert_eq!( - state.get_bridge_account_rollup_id(address).await.expect( + state.get_bridge_account_rollup_id(&address).await.expect( "call to get bridge account rollup id should not fail for uninitialized addresses" ), Option::None, @@ -612,10 +629,12 @@ mod test { let address = astria_address(&[42u8; 20]); // can write new - state.put_bridge_account_rollup_id(address, &rollup_id); + state + .put_bridge_account_rollup_id(&address, rollup_id) + .unwrap(); assert_eq!( state - .get_bridge_account_rollup_id(address) + .get_bridge_account_rollup_id(&address) .await .expect("a rollup ID was written and must exist inside the database") .expect("expecting return value"), @@ -625,10 +644,12 @@ mod test { // can rewrite with new value rollup_id = RollupId::new([2u8; 32]); - state.put_bridge_account_rollup_id(address, &rollup_id); + state + .put_bridge_account_rollup_id(&address, rollup_id) + .unwrap(); assert_eq!( state - .get_bridge_account_rollup_id(address) + .get_bridge_account_rollup_id(&address) .await .expect("a rollup ID was written and must exist inside the database") .expect("expecting return value"), @@ -639,10 +660,12 @@ mod test { // can write additional account and both valid let rollup_id_1 = RollupId::new([2u8; 32]); let address_1 = astria_address(&[41u8; 20]); - state.put_bridge_account_rollup_id(address_1, &rollup_id_1); + state + .put_bridge_account_rollup_id(&address_1, rollup_id_1) + .unwrap(); assert_eq!( state - .get_bridge_account_rollup_id(address_1) + .get_bridge_account_rollup_id(&address_1) .await .expect("a rollup ID was written and must exist inside the database") .expect("expecting return value"), @@ -652,7 +675,7 @@ mod test { assert_eq!( state - .get_bridge_account_rollup_id(address) + .get_bridge_account_rollup_id(&address) .await .expect("a rollup ID was written and must exist inside the database") .expect("expecting return value"), @@ -669,7 +692,7 @@ mod test { let address = astria_address(&[42u8; 20]); state - .get_bridge_account_ibc_asset(address) + .get_bridge_account_ibc_asset(&address) .await .expect_err("call to get bridge account asset ids should fail if no assets"); } @@ -685,10 +708,10 @@ mod test { // can write state - .put_bridge_account_ibc_asset(address, &asset) + .put_bridge_account_ibc_asset(&address, asset.clone()) .expect("storing bridge account asset should not fail"); let mut result = state - .get_bridge_account_ibc_asset(address) + .get_bridge_account_ibc_asset(&address) .await .expect("bridge asset id was written and must exist inside the database"); assert_eq!( @@ -700,10 +723,10 @@ mod test { // can update asset = "asset_2".parse::().unwrap(); state - .put_bridge_account_ibc_asset(address, &asset) + .put_bridge_account_ibc_asset(&address, &asset) .expect("storing bridge account assets should not fail"); result = state - .get_bridge_account_ibc_asset(address) + .get_bridge_account_ibc_asset(&address) .await .expect("bridge asset id was written and must exist inside the database"); assert_eq!( @@ -716,18 +739,18 @@ mod test { let address_1 = astria_address(&[41u8; 20]); let asset_1 = asset_1(); state - .put_bridge_account_ibc_asset(address_1, &asset_1) + .put_bridge_account_ibc_asset(&address_1, &asset_1) .expect("storing bridge account assets should not fail"); assert_eq!( state - .get_bridge_account_ibc_asset(address_1) + .get_bridge_account_ibc_asset(&address_1) .await .expect("bridge asset id was written and must exist inside the database"), asset_1.into(), "second bridge account asset not what was expected" ); result = state - .get_bridge_account_ibc_asset(address) + .get_bridge_account_ibc_asset(&address) .await .expect("original bridge asset id was written and must exist inside the database"); assert_eq!( @@ -738,6 +761,42 @@ mod test { ); } + #[tokio::test] + async fn bridge_account_sudo_address_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + let bridge_address = [1; 20]; + let sudo_address = [2; 20]; + state + .put_bridge_account_sudo_address(&bridge_address, sudo_address) + .unwrap(); + let retrieved_sudo_address = state + .get_bridge_account_sudo_address(&bridge_address) + .await + .unwrap(); + assert_eq!(retrieved_sudo_address, Some(sudo_address)); + } + + #[tokio::test] + async fn bridge_account_withdrawer_address_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + let bridge_address = [1; 20]; + let withdrawer_address = [2; 20]; + state + .put_bridge_account_withdrawer_address(&bridge_address, withdrawer_address) + .unwrap(); + let retrieved_withdrawer_address = state + .get_bridge_account_withdrawer_address(&bridge_address) + .await + .unwrap(); + assert_eq!(retrieved_withdrawer_address, Some(withdrawer_address)); + } + #[tokio::test] async fn get_deposit_nonce_uninitialized_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); @@ -767,7 +826,7 @@ mod test { let mut nonce = 1u32; // can write - state.put_deposit_nonce(&rollup_id, nonce); + state.put_deposit_nonce(&rollup_id, nonce).unwrap(); assert_eq!( state .get_deposit_nonce(&rollup_id) @@ -779,7 +838,7 @@ mod test { // can update nonce = 2u32; - state.put_deposit_nonce(&rollup_id, nonce); + state.put_deposit_nonce(&rollup_id, nonce).unwrap(); assert_eq!( state .get_deposit_nonce(&rollup_id) @@ -792,7 +851,7 @@ mod test { // writing to different account is ok let rollup_id_1 = RollupId::new([3u8; 32]); let nonce_1 = 3u32; - state.put_deposit_nonce(&rollup_id_1, nonce_1); + state.put_deposit_nonce(&rollup_id_1, nonce_1).unwrap(); assert_eq!( state .get_deposit_nonce(&rollup_id_1) @@ -978,7 +1037,7 @@ mod test { // writing to same rollup id does not create duplicates state - .put_deposit_event(deposit.clone()) + .put_deposit_event(deposit) .await .expect("writing deposit events should be ok"); @@ -1260,6 +1319,57 @@ mod test { ); } + #[tokio::test] + async fn init_bridge_account_base_fee_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_init_bridge_account_base_fee(123).unwrap(); + let retrieved_fee = state.get_init_bridge_account_base_fee().await.unwrap(); + assert_eq!(retrieved_fee, 123); + } + + #[tokio::test] + async fn bridge_lock_byte_cost_multiplier_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_bridge_lock_byte_cost_multiplier(123).unwrap(); + let retrieved_fee = state.get_bridge_lock_byte_cost_multiplier().await.unwrap(); + assert_eq!(retrieved_fee, 123); + } + + #[tokio::test] + async fn bridge_sudo_change_base_fee_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_bridge_sudo_change_base_fee(123).unwrap(); + let retrieved_fee = state.get_bridge_sudo_change_base_fee().await.unwrap(); + assert_eq!(retrieved_fee, 123); + } + + #[tokio::test] + async fn last_transaction_id_for_bridge_account_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + let bridge_address = [1; 20]; + let tx_hash = TransactionId::new([2; 32]); + state + .put_last_transaction_id_for_bridge_account(&bridge_address, tx_hash) + .unwrap(); + let retrieved_tx_hash = state + .get_last_transaction_id_for_bridge_account(&bridge_address) + .await + .unwrap(); + assert_eq!(retrieved_tx_hash, Some(tx_hash)); + } + #[test] fn storage_keys_have_not_changed() { let address: Address = "astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm" diff --git a/crates/astria-sequencer/src/fee_asset_change.rs b/crates/astria-sequencer/src/fee_asset_change.rs index e51cd3461f..11a57db489 100644 --- a/crates/astria-sequencer/src/fee_asset_change.rs +++ b/crates/astria-sequencer/src/fee_asset_change.rs @@ -39,7 +39,9 @@ impl ActionHandler for FeeAssetChangeAction { ); match self { FeeAssetChangeAction::Addition(asset) => { - state.put_allowed_fee_asset(asset); + state + .put_allowed_fee_asset(asset) + .context("failed to write allowed fee asset to state")?; } FeeAssetChangeAction::Removal(asset) => { state.delete_allowed_fee_asset(asset); diff --git a/crates/astria-sequencer/src/grpc/sequencer.rs b/crates/astria-sequencer/src/grpc/sequencer.rs index 5263a09da6..cb81673d25 100644 --- a/crates/astria-sequencer/src/grpc/sequencer.rs +++ b/crates/astria-sequencer/src/grpc/sequencer.rs @@ -11,6 +11,7 @@ use astria_core::{ SequencerBlock as RawSequencerBlock, }, primitive::v1::RollupId, + Protobuf, }; use bytes::Bytes; use cnidarium::Storage; @@ -117,15 +118,22 @@ impl SequencerService for SequencerServer { )) })?; - let (rollup_transactions_proof, rollup_ids_proof) = snapshot - .get_block_proofs_by_block_hash(&block_hash) + let rollup_transactions_proof = snapshot + .get_rollup_transactions_proof_by_block_hash(&block_hash) .await .map_err(|e| { Status::internal(format!( - "failed to get sequencer block proofs from storage: {e}" + "failed to get rollup transactions proof from storage: {e}" )) })?; + let rollup_ids_proof = snapshot + .get_rollup_ids_proof_by_block_hash(&block_hash) + .await + .map_err(|e| { + Status::internal(format!("failed to get rollup ids proof from storage: {e}")) + })?; + let mut all_rollup_ids = snapshot .get_rollup_ids_by_block_hash(&block_hash) .await @@ -158,8 +166,8 @@ impl SequencerService for SequencerServer { block_hash: Bytes::copy_from_slice(&block_hash), header: Some(header.into_raw()), rollup_transactions, - rollup_transactions_proof: rollup_transactions_proof.into(), - rollup_ids_proof: rollup_ids_proof.into(), + rollup_transactions_proof: Some(rollup_transactions_proof.into_raw()), + rollup_ids_proof: Some(rollup_ids_proof.into_raw()), all_rollup_ids, }; @@ -200,7 +208,7 @@ impl SequencerService for SequencerServer { // nonce wasn't in mempool, so just look it up from storage let snapshot = self.storage.latest_snapshot(); - let nonce = snapshot.get_account_nonce(address).await.map_err(|e| { + let nonce = snapshot.get_account_nonce(&address).await.map_err(|e| { error!( error = AsRef::::as_ref(&e), "failed to parse get account nonce from storage", @@ -248,8 +256,8 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mempool = Mempool::new(); let mut state_tx = StateDelta::new(storage.latest_snapshot()); - state_tx.put_block_height(1); - state_tx.put_sequencer_block(block.clone()).unwrap(); + state_tx.put_block_height(1).unwrap(); + state_tx.put_sequencer_block(block).unwrap(); storage.commit(state_tx).await.unwrap(); let server = Arc::new(SequencerServer::new(storage.clone(), mempool)); @@ -311,7 +319,7 @@ mod test { let mut state_tx = StateDelta::new(storage.latest_snapshot()); let alice = get_alice_signing_key(); let alice_address = astria_address(&alice.address_bytes()); - state_tx.put_account_nonce(alice_address, 99).unwrap(); + state_tx.put_account_nonce(&alice_address, 99).unwrap(); storage.commit(state_tx).await.unwrap(); let server = Arc::new(SequencerServer::new(storage.clone(), mempool)); diff --git a/crates/astria-sequencer/src/ibc/component.rs b/crates/astria-sequencer/src/ibc/component.rs index 6f0ac0d8db..b9eae0419c 100644 --- a/crates/astria-sequencer/src/ibc/component.rs +++ b/crates/astria-sequencer/src/ibc/component.rs @@ -45,7 +45,8 @@ impl Component for IbcComponent { .context("failed to set IBC sudo key")?; for address in app_state.ibc_relayer_addresses() { - state.put_ibc_relayer_address(address); + // No need to add context as this method already reports sufficient context on error. + state.put_ibc_relayer_address(address)?; } state diff --git a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs index 81f06b050d..f97d5a1f87 100644 --- a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs +++ b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs @@ -47,7 +47,9 @@ impl ActionHandler for IbcRelayerChangeAction { match self { IbcRelayerChangeAction::Addition(address) => { - state.put_ibc_relayer_address(address); + // No need to add context as this method already reports sufficient context on + // error. + state.put_ibc_relayer_address(address)?; } IbcRelayerChangeAction::Removal(address) => { state.delete_ibc_relayer_address(address); diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index f6b716a5e0..f25e2d802d 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -219,7 +219,7 @@ async fn refund_tokens_check( // // check if escrow account has enough balance to refund user let balance = state - .get_ibc_channel_balance(source_channel, denom) + .get_ibc_channel_balance(source_channel, &denom) .await .context("failed to get channel balance in refund_tokens_check")?; @@ -344,7 +344,7 @@ async fn convert_denomination_if_ibc_prefixed( let denom = match packet_denom { Denom::TracePrefixed(trace) => trace, Denom::IbcPrefixed(ibc) => state - .map_ibc_to_trace_prefixed_asset(ibc) + .map_ibc_to_trace_prefixed_asset(&ibc) .await .context("failed to get denom trace from asset id")? .context("denom for given asset id not found in state")?, @@ -444,7 +444,7 @@ async fn execute_ics20_transfer( // execute relevant state changes if it is execute_ics20_transfer_bridge_lock( state, - recipient, + &recipient, &trace_with_dest, packet_amount, packet_data.memo.clone(), @@ -487,23 +487,23 @@ async fn execute_ics20_transfer( .context("failed to update escrow account balance in execute_ics20_transfer")?; state - .increase_balance(recipient, &denom_trace, packet_amount) + .increase_balance(&recipient, &denom_trace, packet_amount) .await .context("failed to update user account balance in execute_ics20_transfer")?; } else { // register denomination in global ID -> denom map if it's not already there if !state - .has_ibc_asset(&*trace_with_dest) + .has_ibc_asset(trace_with_dest.as_ref()) .await .context("failed to check if ibc asset exists in state")? { state - .put_ibc_asset(&trace_with_dest) + .put_ibc_asset(trace_with_dest.as_ref().to_owned()) .context("failed to put IBC asset in storage")?; } state - .increase_balance(recipient, &*trace_with_dest, packet_amount) + .increase_balance(&recipient, trace_with_dest.as_ref(), packet_amount) .await .context("failed to update user account balance in execute_ics20_transfer")?; } @@ -558,20 +558,20 @@ async fn execute_withdrawal_refund_to_rollup( bridge_address, &denom, amount, - bridge_address.to_string(), + bridge_address.to_string().as_str(), ) .await .context("failed to emit deposit")?; state - .increase_balance(bridge_address, denom, amount) + .increase_balance(&bridge_address, &denom, amount) .await .context("failed to update bridge account account balance")?; Ok(()) } -async fn parse_refund_sender(state: &S, sender: &str) -> anyhow::Result
{ +async fn parse_refund_sender(state: &S, sender: &str) -> Result
{ use futures::TryFutureExt as _; let (base_prefix, compat_prefix) = match try_join!( state @@ -620,7 +620,7 @@ async fn parse_refund_sender(state: &S, sender: &str) -> anyhow::R /// this function is a no-op. async fn execute_ics20_transfer_bridge_lock( state: &mut S, - recipient: Address, + recipient: &Address, denom: &denom::TracePrefixed, amount: u128, memo: String, @@ -666,10 +666,10 @@ async fn execute_ics20_transfer_bridge_lock( execute_deposit( state, - recipient, + *recipient, denom, amount, - deposit_memo.rollup_deposit_address, + &deposit_memo.rollup_deposit_address, ) .await } @@ -679,13 +679,13 @@ async fn execute_deposit( bridge_address: Address, denom: &denom::TracePrefixed, amount: u128, - destination_address: String, + destination_address: &str, ) -> Result<()> { // check if the recipient is a bridge account and // ensure that the asset ID being transferred // to it is allowed. let Some(rollup_id) = state - .get_bridge_account_rollup_id(bridge_address) + .get_bridge_account_rollup_id(&bridge_address) .await .context("failed to get bridge account rollup ID from state")? else { @@ -693,7 +693,7 @@ async fn execute_deposit( }; let allowed_asset = state - .get_bridge_account_ibc_asset(bridge_address) + .get_bridge_account_ibc_asset(&bridge_address) .await .context("failed to get bridge account asset ID")?; ensure!( @@ -712,7 +712,7 @@ async fn execute_deposit( rollup_id, amount, denom.into(), - destination_address, + destination_address.to_string(), transaction_id, index_of_action, ); @@ -788,8 +788,8 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let denom_trace = "asset".parse().unwrap(); - state_tx.put_ibc_asset(&denom_trace).unwrap(); + let denom_trace: TracePrefixed = "asset".parse().unwrap(); + state_tx.put_ibc_asset(denom_trace.clone()).unwrap(); let expected = denom_trace.clone(); let packet_denom = denom_trace.to_ibc_prefixed().into(); @@ -842,10 +842,13 @@ mod test { .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); - let balance = state_tx.get_account_balance(recipient, denom).await.expect( - "ics20 transfer to user account should succeed and balance should be minted to this \ - account", - ); + let balance = state_tx + .get_account_balance(&recipient, &denom) + .await + .expect( + "ics20 transfer to user account should succeed and balance should be minted to \ + this account", + ); assert_eq!(balance, 100); } @@ -860,14 +863,16 @@ mod test { let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); state_tx.put_transaction_context(TransactionContext { - address_bytes: bridge_address.bytes(), + address_bytes: *bridge_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let memo = memos::v1alpha1::Ics20TransferDeposit { @@ -897,7 +902,7 @@ mod test { let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); let balance = state_tx - .get_account_balance(bridge_address, denom) + .get_account_balance(&bridge_address, &denom) .await .expect( "ics20 transfer from sender to bridge account should have updated funds in the \ @@ -922,9 +927,11 @@ mod test { let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); // use invalid memo, which should fail @@ -960,9 +967,11 @@ mod test { let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); // use invalid asset, which should fail @@ -1027,7 +1036,7 @@ mod test { .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(recipient_address, &base_denom) + .get_account_balance(&recipient_address, &base_denom) .await .expect("ics20 transfer to user account should succeed"); assert_eq!(balance, amount); @@ -1077,7 +1086,7 @@ mod test { .expect("valid ics20 refund to user account; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(recipient_address, &base_denom) + .get_account_balance(&recipient_address, &base_denom) .await .expect("ics20 refund to user account should succeed"); assert_eq!(balance, amount); @@ -1094,8 +1103,10 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - state_tx.put_base_prefix(ASTRIA_PREFIX); - state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + state_tx.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); + state_tx + .put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX.to_string()) + .unwrap(); let bridge_address = astria_address(&[99u8; 20]); @@ -1105,14 +1116,16 @@ mod test { .unwrap(); state_tx.put_transaction_context(TransactionContext { - address_bytes: bridge_address.bytes(), + address_bytes: *bridge_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let amount = 100; @@ -1130,7 +1143,7 @@ mod test { .expect("valid rollup withdrawal refund"); let balance = state_tx - .get_account_balance(bridge_address, denom) + .get_account_balance(&bridge_address, &denom) .await .expect("rollup withdrawal refund should have updated funds in the bridge address"); assert_eq!(balance, 100); @@ -1148,8 +1161,10 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - state_tx.put_base_prefix(ASTRIA_PREFIX); - state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + state_tx.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); + state_tx + .put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX.to_string()) + .unwrap(); let bridge_address = astria_address(&[99u8; 20]); let destination_chain_address = bridge_address.to_string(); @@ -1157,14 +1172,16 @@ mod test { let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); state_tx.put_transaction_context(TransactionContext { - address_bytes: bridge_address.bytes(), + address_bytes: *bridge_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let packet = FungibleTokenPacketData { @@ -1195,7 +1212,7 @@ mod test { .expect("valid ics20 transfer refund; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(bridge_address, &denom) + .get_account_balance(&bridge_address, &denom) .await .expect( "ics20 transfer refunding to rollup should succeed and balance should be added to \ @@ -1228,17 +1245,21 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - state_tx.put_base_prefix(ASTRIA_PREFIX); - state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + state_tx.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); + state_tx + .put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX.to_string()) + .unwrap(); let bridge_address = astria_address(&[99u8; 20]); let bridge_address_compat = astria_compat_address(&[99u8; 20]); let denom = "nootasset".parse::().unwrap(); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(bridge_address, &denom) + .put_bridge_account_rollup_id(&bridge_address, rollup_id) + .unwrap(); + state_tx + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let packet = FungibleTokenPacketData { @@ -1257,7 +1278,7 @@ mod test { let packet_bytes = serde_json::to_vec(&packet).unwrap(); let transaction_context = TransactionContext { - address_bytes: bridge_address.bytes(), + address_bytes: *bridge_address.bytes(), transaction_id: TransactionId::new([0; 32]), source_action_index: 0, }; @@ -1276,7 +1297,7 @@ mod test { .expect("valid ics20 transfer refund; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(bridge_address, &denom) + .get_account_balance(&bridge_address, &denom) .await .expect( "ics20 transfer refunding to rollup should succeed and balance should be added to \ diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index 327194185b..8cf56c5ad3 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -101,14 +101,14 @@ async fn create_ibc_packet_from_withdrawal( /// 1. Errors reading from DB /// 2. `action.bridge_address` is set, but `from` is not the withdrawer address. /// 3. `action.bridge_address` is unset, but `from` is a bridge account. -async fn establish_withdrawal_target( - action: &action::Ics20Withdrawal, +async fn establish_withdrawal_target<'a, S: StateRead>( + action: &'a action::Ics20Withdrawal, state: &S, - from: [u8; 20], -) -> Result<[u8; 20]> { + from: &'a [u8; 20], +) -> Result<&'a [u8; 20]> { // If the bridge address is set, the withdrawer on that address must match // the from address. - if let Some(bridge_address) = action.bridge_address { + if let Some(bridge_address) = &action.bridge_address { let Some(withdrawer) = state .get_bridge_account_withdrawer_address(bridge_address) .await @@ -118,7 +118,7 @@ async fn establish_withdrawal_target( }; ensure!( - withdrawer == from.address_bytes(), + withdrawer == *from.address_bytes(), "sender does not match bridge withdrawer address; unauthorized" ); @@ -195,7 +195,7 @@ impl ActionHandler for action::Ics20Withdrawal { state .check_and_set_withdrawal_event_block_for_bridge_account( - self.bridge_address.map_or(from, Address::bytes), + self.bridge_address.as_ref().map_or(&from, Address::bytes), &parsed_bridge_memo.rollup_withdrawal_event_id, parsed_bridge_memo.rollup_block_number, ) @@ -203,7 +203,7 @@ impl ActionHandler for action::Ics20Withdrawal { .context("withdrawal event already processed")?; } - let withdrawal_target = establish_withdrawal_target(self, &state, from) + let withdrawal_target = establish_withdrawal_target(self, &state, &from) .await .context("failed establishing which account to withdraw funds from")?; @@ -237,7 +237,7 @@ impl ActionHandler for action::Ics20Withdrawal { .context("failed to decrease sender or bridge balance")?; state - .decrease_balance(from, self.fee_asset(), fee) + .decrease_balance(&from, self.fee_asset(), fee) .await .context("failed to subtract fee from sender balance")?; @@ -312,7 +312,7 @@ mod tests { }; assert_eq!( - establish_withdrawal_target(&action, &state, from) + *establish_withdrawal_target(&action, &state, &from) .await .unwrap(), from @@ -325,15 +325,19 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // sender is a bridge address, which is also the withdrawer, so it's ok let bridge_address = [1u8; 20]; - state.put_bridge_account_rollup_id( - bridge_address, - &RollupId::from_unhashed_bytes("testrollupid"), - ); - state.put_bridge_account_withdrawer_address(bridge_address, bridge_address); + state + .put_bridge_account_rollup_id( + &bridge_address, + RollupId::from_unhashed_bytes("testrollupid"), + ) + .unwrap(); + state + .put_bridge_account_withdrawer_address(&bridge_address, bridge_address) + .unwrap(); let denom = "test".parse::().unwrap(); let action = action::Ics20Withdrawal { @@ -351,7 +355,7 @@ mod tests { }; assert_anyhow_error( - &establish_withdrawal_target(&action, &state, bridge_address) + &establish_withdrawal_target(&action, &state, &bridge_address) .await .unwrap_err(), "sender cannot be a bridge address if bridge address is not set", @@ -390,20 +394,24 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // withdraw is *not* the bridge address, Ics20Withdrawal must be sent by the withdrawer - state.put_bridge_account_rollup_id( - bridge_address(), - &RollupId::from_unhashed_bytes("testrollupid"), - ); - state.put_bridge_account_withdrawer_address( - bridge_address(), - astria_address(&[2u8; 20]), - ); + state + .put_bridge_account_rollup_id( + &bridge_address(), + RollupId::from_unhashed_bytes("testrollupid"), + ) + .unwrap(); + state + .put_bridge_account_withdrawer_address( + &bridge_address(), + astria_address(&[2u8; 20]), + ) + .unwrap(); assert_anyhow_error( - &establish_withdrawal_target(&action, &state, bridge_address()) + &establish_withdrawal_target(&action, &state, &bridge_address()) .await .unwrap_err(), "sender does not match bridge withdrawer address; unauthorized", @@ -424,16 +432,20 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // sender the withdrawer address, so it's ok let bridge_address = [1u8; 20]; let withdrawer_address = [2u8; 20]; - state.put_bridge_account_rollup_id( - bridge_address, - &RollupId::from_unhashed_bytes("testrollupid"), - ); - state.put_bridge_account_withdrawer_address(bridge_address, withdrawer_address); + state + .put_bridge_account_rollup_id( + &bridge_address, + RollupId::from_unhashed_bytes("testrollupid"), + ) + .unwrap(); + state + .put_bridge_account_withdrawer_address(&bridge_address, withdrawer_address) + .unwrap(); let denom = "test".parse::().unwrap(); let action = action::Ics20Withdrawal { @@ -451,7 +463,7 @@ mod tests { }; assert_eq!( - establish_withdrawal_target(&action, &state, withdrawer_address) + *establish_withdrawal_target(&action, &state, &withdrawer_address) .await .unwrap(), bridge_address, @@ -483,7 +495,7 @@ mod tests { }; assert_anyhow_error( - &establish_withdrawal_target(&action, &state, not_bridge_address) + &establish_withdrawal_target(&action, &state, ¬_bridge_address) .await .unwrap_err(), "bridge address must have a withdrawer address set", diff --git a/crates/astria-sequencer/src/ibc/state_ext.rs b/crates/astria-sequencer/src/ibc/state_ext.rs index b5c230bf98..58e8c47fbd 100644 --- a/crates/astria-sequencer/src/ibc/state_ext.rs +++ b/crates/astria-sequencer/src/ibc/state_ext.rs @@ -8,10 +8,6 @@ use astria_core::primitive::v1::{ ADDRESS_LEN, }; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, @@ -22,19 +18,13 @@ use tracing::{ instrument, }; -use crate::accounts::AddressBytes; - -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Balance(u128); - -/// Newtype wrapper to read and write an address from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct SudoAddress([u8; ADDRESS_LEN]); - -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Fee(u128); +use crate::{ + accounts::AddressBytes, + storage::{ + self, + StoredValue, + }, +}; const IBC_SUDO_STORAGE_KEY: &str = "ibcsudo"; const ICS20_WITHDRAWAL_BASE_FEE_STORAGE_KEY: &str = "ics20withdrawalfee"; @@ -52,10 +42,10 @@ impl<'a, T: AddressBytes> std::fmt::Display for IbcRelayerKey<'a, T> { } } -fn channel_balance_storage_key>( - channel: &ChannelId, - asset: TAsset, -) -> String { +fn channel_balance_storage_key<'a, TAsset>(channel: &ChannelId, asset: &'a TAsset) -> String +where + asset::IbcPrefixed: From<&'a TAsset>, +{ format!( "ibc-data/{channel}/balance/{}", crate::storage_keys::hunks::Asset::from(asset), @@ -69,13 +59,14 @@ fn ibc_relayer_key(address: &T) -> String { #[async_trait] pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] - async fn get_ibc_channel_balance( + async fn get_ibc_channel_balance<'a, TAsset>( &self, channel: &ChannelId, - asset: TAsset, + asset: &'a TAsset, ) -> Result where - TAsset: Into + std::fmt::Display + Send, + TAsset: Sync, + asset::IbcPrefixed: From<&'a TAsset>, { let Some(bytes) = self .get_raw(&channel_balance_storage_key(channel, asset)) @@ -85,8 +76,9 @@ pub(crate) trait StateReadExt: StateRead { debug!("ibc channel balance not found, returning 0"); return Ok(0); }; - let Balance(balance) = Balance::try_from_slice(&bytes).context("invalid balance bytes")?; - Ok(balance) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Balance::try_from(value).map(u128::from)) + .context("invalid ibc channel balance bytes") } #[instrument(skip_all)] @@ -94,14 +86,14 @@ pub(crate) trait StateReadExt: StateRead { let Some(bytes) = self .get_raw(IBC_SUDO_STORAGE_KEY) .await - .context("failed reading raw ibc sudo key from state")? + .context("failed reading raw ibc sudo address from state")? else { // ibc sudo key must be set - bail!("ibc sudo key not found"); + bail!("ibc sudo address not found"); }; - let SudoAddress(address_bytes) = - SudoAddress::try_from_slice(&bytes).context("invalid ibc sudo key bytes")?; - Ok(address_bytes) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::AddressBytes::try_from(value).map(<[u8; ADDRESS_LEN]>::from)) + .context("invalid ibc sudo address bytes") } #[instrument(skip_all)] @@ -122,8 +114,9 @@ pub(crate) trait StateReadExt: StateRead { else { bail!("ics20 withdrawal fee not found"); }; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid ics20 withdrawal base fee bytes") } } @@ -132,46 +125,51 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_ibc_channel_balance( + fn put_ibc_channel_balance<'a, TAsset>( &mut self, channel: &ChannelId, - asset: TAsset, + asset: &'a TAsset, balance: u128, ) -> Result<()> where - TAsset: Into + std::fmt::Display + Send, + asset::IbcPrefixed: From<&'a TAsset>, { - let bytes = borsh::to_vec(&Balance(balance)).context("failed to serialize balance")?; + let bytes = StoredValue::Balance(balance.into()) + .serialize() + .context("failed to serialize ibc channel balance")?; self.put_raw(channel_balance_storage_key(channel, asset), bytes); Ok(()) } #[instrument(skip_all)] fn put_ibc_sudo_address(&mut self, address: T) -> Result<()> { - self.put_raw( - IBC_SUDO_STORAGE_KEY.to_string(), - borsh::to_vec(&SudoAddress(address.address_bytes())) - .context("failed to convert sudo address to vec")?, - ); + let bytes = StoredValue::AddressBytes((&address).into()) + .serialize() + .context("failed to serialize ibc sudo address")?; + self.put_raw(IBC_SUDO_STORAGE_KEY.to_string(), bytes); Ok(()) } #[instrument(skip_all)] - fn put_ibc_relayer_address(&mut self, address: T) { - self.put_raw(ibc_relayer_key(&address), vec![]); + fn put_ibc_relayer_address(&mut self, address: &T) -> Result<()> { + let bytes = StoredValue::Unit + .serialize() + .context("failed to serialize unit for ibc relayer address")?; + self.put_raw(ibc_relayer_key(address), bytes); + Ok(()) } #[instrument(skip_all)] - fn delete_ibc_relayer_address(&mut self, address: T) { - self.delete(ibc_relayer_key(&address)); + fn delete_ibc_relayer_address(&mut self, address: &T) { + self.delete(ibc_relayer_key(address)); } #[instrument(skip_all)] fn put_ics20_withdrawal_base_fee(&mut self, fee: u128) -> Result<()> { - self.put_raw( - ICS20_WITHDRAWAL_BASE_FEE_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).context("failed to serialize fee")?, - ); + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize ics20 withdrawal base fee")?; + self.put_raw(ICS20_WITHDRAWAL_BASE_FEE_STORAGE_KEY.to_string(), bytes); Ok(()) } } @@ -227,7 +225,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // can write new let mut address = [42u8; 20]; @@ -264,7 +262,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // unset address returns false let address = astria_address(&[42u8; 20]); @@ -283,11 +281,11 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // can write let address = astria_address(&[42u8; 20]); - state.put_ibc_relayer_address(address); + state.put_ibc_relayer_address(&address).unwrap(); assert!( state .is_ibc_relayer(address) @@ -297,7 +295,7 @@ mod tests { ); // can delete - state.delete_ibc_relayer_address(address); + state.delete_ibc_relayer_address(&address); assert!( !state .is_ibc_relayer(address) @@ -313,11 +311,11 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX); + state.put_base_prefix(ASTRIA_PREFIX.to_string()).unwrap(); // can write let address = astria_address(&[42u8; 20]); - state.put_ibc_relayer_address(address); + state.put_ibc_relayer_address(&address).unwrap(); assert!( state .is_ibc_relayer(address) @@ -328,7 +326,7 @@ mod tests { // can write multiple let address_1 = astria_address(&[41u8; 20]); - state.put_ibc_relayer_address(address_1); + state.put_ibc_relayer_address(&address_1).unwrap(); assert!( state .is_ibc_relayer(address_1) @@ -356,7 +354,7 @@ mod tests { assert_eq!( state - .get_ibc_channel_balance(&channel, asset) + .get_ibc_channel_balance(&channel, &asset) .await .expect("retrieving asset balance for channel should not fail"), 0u128, @@ -468,7 +466,7 @@ mod tests { ); assert_eq!( state - .get_ibc_channel_balance(&channel_1, asset) + .get_ibc_channel_balance(&channel_1, &asset) .await .expect("retrieving asset balance for channel should not fail"), amount_1, @@ -476,6 +474,17 @@ mod tests { ); } + #[tokio::test] + async fn ics20_withdrawal_base_fee_round_trip() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_ics20_withdrawal_base_fee(123).unwrap(); + let retrieved_fee = state.get_ics20_withdrawal_base_fee().await.unwrap(); + assert_eq!(retrieved_fee, 123); + } + #[test] fn storage_keys_have_not_changed() { let channel = ChannelId::new(5); @@ -490,7 +499,7 @@ mod tests { .unwrap(); assert_eq!( channel_balance_storage_key(&channel, &asset), - channel_balance_storage_key(&channel, asset.to_ibc_prefixed()), + channel_balance_storage_key(&channel, &asset.to_ibc_prefixed()), ); assert_snapshot!(channel_balance_storage_key(&channel, &asset)); } diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index d35e9e0782..bb98d35409 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -20,6 +20,7 @@ pub(crate) mod sequence; mod sequencer; pub(crate) mod service; pub(crate) mod state_ext; +pub(crate) mod storage; pub(crate) mod storage_keys; #[cfg(any(test, feature = "benchmark"))] pub(crate) mod test_utils; diff --git a/crates/astria-sequencer/src/mempool/benchmarks.rs b/crates/astria-sequencer/src/mempool/benchmarks.rs index d8df9dc718..3f87f06bf2 100644 --- a/crates/astria-sequencer/src/mempool/benchmarks.rs +++ b/crates/astria-sequencer/src/mempool/benchmarks.rs @@ -197,7 +197,7 @@ fn builder_queue(bencher: divan::Bencher) { for i in 0..SIGNER_COUNT { let signing_key = SigningKey::from([i; 32]); let signing_address = signing_key.address_bytes(); - mock_state_put_account_nonce(&mut mock_state, signing_address, 0); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 0); } bencher @@ -295,8 +295,8 @@ fn run_maintenance(bencher: divan::Bencher) { for i in 0..SIGNER_COUNT { let signing_key = SigningKey::from([i; 32]); let signing_address = signing_key.address_bytes(); - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances.clone()); - mock_state_put_account_nonce(&mut mock_state, signing_address, new_nonce); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances.clone()); + mock_state_put_account_nonce(&mut mock_state, &signing_address, new_nonce); } bencher @@ -335,8 +335,8 @@ fn run_maintenance_tx_recosting(bencher: divan::Bencher) { for i in 0..SIGNER_COUNT { let signing_key = SigningKey::from([i; 32]); let signing_address = signing_key.address_bytes(); - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances.clone()); - mock_state_put_account_nonce(&mut mock_state, signing_address, new_nonce); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances.clone()); + mock_state_put_account_nonce(&mut mock_state, &signing_address, new_nonce); } bencher diff --git a/crates/astria-sequencer/src/mempool/mempool_state.rs b/crates/astria-sequencer/src/mempool/mempool_state.rs index 767479637d..1e755d6d64 100644 --- a/crates/astria-sequencer/src/mempool/mempool_state.rs +++ b/crates/astria-sequencer/src/mempool/mempool_state.rs @@ -11,9 +11,9 @@ use crate::accounts::{ }; #[instrument(skip_all)] -pub(crate) async fn get_account_balances( +pub(crate) async fn get_account_balances( state: S, - address: impl AddressBytes, + address: &T, ) -> Result> { use futures::TryStreamExt as _; state @@ -54,7 +54,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // native account should work with ibc too - state.put_native_asset(&nria()); + state.put_native_asset(nria()).unwrap(); let asset_0 = state.get_native_asset().await.unwrap(); let asset_1: Denom = "asset_0".parse().unwrap(); @@ -62,13 +62,13 @@ mod tests { // also need to add assets to the ibc state state - .put_ibc_asset(&asset_0.clone()) + .put_ibc_asset(asset_0.clone()) .expect("should be able to call other trait method on state object"); state - .put_ibc_asset(&asset_1.clone().unwrap_trace_prefixed()) + .put_ibc_asset(asset_1.clone().unwrap_trace_prefixed()) .expect("should be able to call other trait method on state object"); state - .put_ibc_asset(&asset_2.clone().unwrap_trace_prefixed()) + .put_ibc_asset(asset_2.clone().unwrap_trace_prefixed()) .expect("should be able to call other trait method on state object"); // create needed variables @@ -79,16 +79,16 @@ mod tests { // add balances to the account state - .put_account_balance(address, asset_0.clone(), amount_expected_0) + .put_account_balance(&address, &asset_0, amount_expected_0) .expect("putting an account balance should not fail"); state - .put_account_balance(address, &asset_1, amount_expected_1) + .put_account_balance(&address, &asset_1, amount_expected_1) .expect("putting an account balance should not fail"); state - .put_account_balance(address, &asset_2, amount_expected_2) + .put_account_balance(&address, &asset_2, amount_expected_2) .expect("putting an account balance should not fail"); - let balances = get_account_balances(state, address).await.unwrap(); + let balances = get_account_balances(state, &address).await.unwrap(); assert_eq!( balances.get(&asset_0.to_ibc_prefixed()).unwrap(), diff --git a/crates/astria-sequencer/src/mempool/mod.rs b/crates/astria-sequencer/src/mempool/mod.rs index 7de2e6576c..bc6d63aaf3 100644 --- a/crates/astria-sequencer/src/mempool/mod.rs +++ b/crates/astria-sequencer/src/mempool/mod.rs @@ -204,7 +204,7 @@ impl Mempool { .checked_add(1) .expect("failed to increment nonce in promotion"), &pending.subtract_contained_costs( - *timemarked_tx.address(), + timemarked_tx.address(), current_account_balances.clone(), ), ); @@ -244,8 +244,8 @@ impl Mempool { signed_tx: Arc, reason: RemovalReason, ) { - let tx_hash = signed_tx.id().get(); - let address = signed_tx.verification_key().address_bytes(); + let tx_hash = *signed_tx.id().get(); + let address = *signed_tx.verification_key().address_bytes(); // Try to remove from pending. let removed_txs = match self.pending.write().await.remove(signed_tx) { @@ -301,12 +301,12 @@ impl Mempool { let addresses: HashSet<[u8; 20]> = pending .addresses() - .into_iter() .chain(parked.addresses()) + .copied() .collect(); // TODO: Make this concurrent, all account state is separate with IO bound disk reads. - for address in addresses { + for address in &addresses { // get current account state let current_nonce = match state.get_account_nonce(address).await { Ok(res) => res, @@ -322,7 +322,7 @@ impl Mempool { Ok(res) => res, Err(error) => { error!( - address = %telemetry::display::base64(&address), + address = %telemetry::display::base64(address), "failed to fetch account balances when cleaning accounts: {error:#}" ); continue; @@ -352,7 +352,7 @@ impl Mempool { let remaining_balances = pending.subtract_contained_costs(address, current_balances.clone()); let promtion_txs = - parked.find_promotables(&address, highest_pending_nonce, &remaining_balances); + parked.find_promotables(address, highest_pending_nonce, &remaining_balances); for tx in promtion_txs { if let Err(error) = pending.add(tx, current_nonce, ¤t_balances) { @@ -391,7 +391,7 @@ impl Mempool { /// pending queue for an account has nonces [0,1] and the parked queue has [3], [1] will be /// returned. #[instrument(skip_all)] - pub(crate) async fn pending_nonce(&self, address: [u8; 20]) -> Option { + pub(crate) async fn pending_nonce(&self, address: &[u8; 20]) -> Option { self.pending.read().await.pending_nonce(address) } @@ -486,7 +486,7 @@ mod test { let mempool = Mempool::new(); let signing_key = SigningKey::from([1; 32]); - let signing_address = signing_key.verification_key().address_bytes(); + let signing_address = *signing_key.verification_key().address_bytes(); let account_balances = mock_balances(100, 100); let tx_cost = mock_tx_cost(10, 10, 0); @@ -536,7 +536,7 @@ mod test { // mock state with nonce at 1 let mut mock_state = mock_state_getter().await; - mock_state_put_account_nonce(&mut mock_state, signing_address, 1); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 1); // grab building queue, should return transactions [1,2] since [0] was below and [4] is // gapped @@ -556,8 +556,8 @@ mod test { // to pending // setup state - mock_state_put_account_nonce(&mut mock_state, signing_address, 4); - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances(100, 100)); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 4); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances(100, 100)); mempool.run_maintenance(&mock_state, false).await; @@ -577,7 +577,7 @@ mod test { async fn run_maintenance_promotion() { let mempool = Mempool::new(); let signing_key = SigningKey::from([1; 32]); - let signing_address = signing_key.verification_key().address_bytes(); + let signing_address = signing_key.address_bytes(); // create transaction setup to trigger promotions // @@ -608,7 +608,7 @@ mod test { // see pending only has one transaction let mut mock_state = mock_state_getter().await; - mock_state_put_account_nonce(&mut mock_state, signing_address, 1); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 1); let builder_queue = mempool .builder_queue(&mock_state) @@ -623,7 +623,7 @@ mod test { // run maintenance with account containing balance for two more transactions // setup state - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances(3, 0)); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances(3, 0)); mempool.run_maintenance(&mock_state, false).await; @@ -644,7 +644,7 @@ mod test { async fn run_maintenance_demotion() { let mempool = Mempool::new(); let signing_key = SigningKey::from([1; 32]); - let signing_address = signing_key.verification_key().address_bytes(); + let signing_address = signing_key.address_bytes(); // create transaction setup to trigger demotions // @@ -676,7 +676,7 @@ mod test { // see pending only has all transactions let mut mock_state = mock_state_getter().await; - mock_state_put_account_nonce(&mut mock_state, signing_address, 1); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 1); let builder_queue = mempool .builder_queue(&mock_state) @@ -689,7 +689,7 @@ mod test { ); // setup state - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances(1, 0)); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances(1, 0)); mempool.run_maintenance(&mock_state, false).await; @@ -704,8 +704,8 @@ mod test { "builder queue should contain single transaction" ); - mock_state_put_account_nonce(&mut mock_state, signing_address, 1); - mock_state_put_account_balances(&mut mock_state, signing_address, mock_balances(3, 0)); + mock_state_put_account_nonce(&mut mock_state, &signing_address, 1); + mock_state_put_account_balances(&mut mock_state, &signing_address, mock_balances(3, 0)); mempool.run_maintenance(&mock_state, false).await; @@ -802,23 +802,23 @@ mod test { // assert that all were added to the cometbft removal cache // and the expected reasons were tracked assert!(matches!( - mempool.check_removed_comet_bft(tx0.id().get()).await, + mempool.check_removed_comet_bft(*tx0.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool.check_removed_comet_bft(tx1.id().get()).await, + mempool.check_removed_comet_bft(*tx1.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool.check_removed_comet_bft(tx3.id().get()).await, + mempool.check_removed_comet_bft(*tx3.id().get()).await, Some(RemovalReason::LowerNonceInvalidated) )); assert!(matches!( - mempool.check_removed_comet_bft(tx4.id().get()).await, + mempool.check_removed_comet_bft(*tx4.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool.check_removed_comet_bft(tx5.id().get()).await, + mempool.check_removed_comet_bft(*tx5.id().get()).await, Some(RemovalReason::LowerNonceInvalidated) )); } @@ -829,9 +829,9 @@ mod test { let signing_key_0 = SigningKey::from([1; 32]); let signing_key_1 = SigningKey::from([2; 32]); let signing_key_2 = SigningKey::from([3; 32]); - let signing_address_0 = signing_key_0.verification_key().address_bytes(); - let signing_address_1 = signing_key_1.verification_key().address_bytes(); - let signing_address_2 = signing_key_2.verification_key().address_bytes(); + let signing_address_0 = *signing_key_0.verification_key().address_bytes(); + let signing_address_1 = *signing_key_1.verification_key().address_bytes(); + let signing_address_2 = *signing_key_2.verification_key().address_bytes(); let account_balances = mock_balances(100, 100); let tx_cost = mock_tx_cost(10, 10, 0); @@ -884,11 +884,14 @@ mod test { assert_eq!(mempool.len().await, 4); // Check the pending nonces - assert_eq!(mempool.pending_nonce(signing_address_0).await.unwrap(), 1); - assert_eq!(mempool.pending_nonce(signing_address_1).await.unwrap(), 101); + assert_eq!(mempool.pending_nonce(&signing_address_0).await.unwrap(), 1); + assert_eq!( + mempool.pending_nonce(&signing_address_1).await.unwrap(), + 101 + ); // Check the pending nonce for an address with no txs is `None`. - assert!(mempool.pending_nonce(signing_address_2).await.is_none()); + assert!(mempool.pending_nonce(&signing_address_2).await.is_none()); } #[tokio::test] diff --git a/crates/astria-sequencer/src/mempool/transactions_container.rs b/crates/astria-sequencer/src/mempool/transactions_container.rs index f642b5e51b..ef3c19e353 100644 --- a/crates/astria-sequencer/src/mempool/transactions_container.rs +++ b/crates/astria-sequencer/src/mempool/transactions_container.rs @@ -4,7 +4,6 @@ use std::{ hash_map, BTreeMap, HashMap, - HashSet, }, fmt, mem, @@ -49,8 +48,8 @@ pub(super) struct TimemarkedTransaction { impl TimemarkedTransaction { pub(super) fn new(signed_tx: Arc, cost: HashMap) -> Self { Self { - tx_hash: signed_tx.id().get(), - address: signed_tx.verification_key().address_bytes(), + tx_hash: *signed_tx.id().get(), + address: *signed_tx.verification_key().address_bytes(), signed_tx, time_first_seen: Instant::now(), cost, @@ -500,8 +499,8 @@ impl TransactionsContainer { } /// Returns all of the currently tracked addresses. - pub(super) fn addresses(&self) -> HashSet<[u8; 20]> { - self.txs.keys().copied().collect() + pub(super) fn addresses(&self) -> impl Iterator { + self.txs.keys() } /// Recosts transactions for an account. @@ -509,10 +508,10 @@ impl TransactionsContainer { /// Logs an error if fails to recost a transaction. pub(super) async fn recost_transactions( &mut self, - address: [u8; 20], + address: &[u8; 20], state: &S, ) { - let Some(account) = self.txs.get_mut(&address) else { + let Some(account) = self.txs.get_mut(address) else { return; }; @@ -523,7 +522,7 @@ impl TransactionsContainer { Ok(res) => res, Err(error) => { error!( - address = %telemetry::display::base64(&address), + address = %telemetry::display::base64(address), "failed to calculate new transaction cost when cleaning accounts: {error:#}" ); continue; @@ -572,7 +571,7 @@ impl TransactionsContainer { let address = signed_tx.verification_key().address_bytes(); // Take the collection for this account out of `self` temporarily. - let Some(mut account_txs) = self.txs.remove(&address) else { + let Some(mut account_txs) = self.txs.remove(address) else { return Err(signed_tx); }; @@ -580,7 +579,7 @@ impl TransactionsContainer { // Re-add the collection to `self` if it's not empty. if !account_txs.txs().is_empty() { - let _ = self.txs.insert(address, account_txs); + let _ = self.txs.insert(*address, account_txs); } if removed.is_empty() { @@ -602,11 +601,11 @@ impl TransactionsContainer { /// Cleans the specified account of stale and expired transactions. pub(super) fn clean_account_stale_expired( &mut self, - address: [u8; 20], + address: &[u8; 20], current_account_nonce: u32, ) -> Vec<([u8; 32], RemovalReason)> { // Take the collection for this account out of `self` temporarily if it exists. - let Some(mut account_txs) = self.txs.remove(&address) else { + let Some(mut account_txs) = self.txs.remove(address) else { return Vec::new(); }; @@ -635,7 +634,7 @@ impl TransactionsContainer { // Re-add the collection to `self` if it's not empty. if !account_txs.txs().is_empty() { - let _ = self.txs.insert(address, account_txs); + let _ = self.txs.insert(*address, account_txs); } removed_txs @@ -662,11 +661,11 @@ impl TransactionsContainer { /// based on the specified account's current balances. pub(super) fn find_demotables( &mut self, - address: [u8; 20], + address: &[u8; 20], current_balances: &HashMap, ) -> Vec { // Take the collection for this account out of `self` temporarily if it exists. - let Some(mut account) = self.txs.remove(&address) else { + let Some(mut account) = self.txs.remove(address) else { return Vec::new(); }; @@ -674,7 +673,7 @@ impl TransactionsContainer { // Re-add the collection to `self` if it's not empty. if !account.txs().is_empty() { - let _ = self.txs.insert(address, account); + let _ = self.txs.insert(*address, account); } demoted @@ -684,19 +683,19 @@ impl TransactionsContainer { /// transactions' costs. pub(super) fn subtract_contained_costs( &self, - address: [u8; 20], + address: &[u8; 20], mut current_balances: HashMap, ) -> HashMap { - if let Some(account) = self.txs.get(&address) { + if let Some(account) = self.txs.get(address) { account.subtract_contained_costs(&mut current_balances); }; current_balances } /// Returns the highest nonce for an account. - pub(super) fn pending_nonce(&self, address: [u8; 20]) -> Option { + pub(super) fn pending_nonce(&self, address: &[u8; 20]) -> Option { self.txs - .get(&address) + .get(address) .and_then(PendingTransactionsForAccount::highest_nonce) } @@ -717,7 +716,7 @@ impl TransactionsContainer { // Add all transactions to the queue. for (address, account_txs) in &self.txs { let current_account_nonce = state - .get_account_nonce(*address) + .get_account_nonce(address) .await .context("failed to fetch account nonce for builder queue")?; for ttx in account_txs.txs.values() { @@ -1428,7 +1427,7 @@ mod test { // recost transactions with mock state's tx costs let state = mock_state_getter().await; pending_txs - .recost_transactions(signing_address, &state) + .recost_transactions(&signing_address, &state) .await; // transaction should have been recosted @@ -1502,9 +1501,9 @@ mod test { // clean accounts // should pop none from signing_address_0, one from signing_address_1, and all from // signing_address_2 - let mut removed_txs = pending_txs.clean_account_stale_expired(signing_address_0, 0); - removed_txs.extend(pending_txs.clean_account_stale_expired(signing_address_1, 1)); - removed_txs.extend(pending_txs.clean_account_stale_expired(signing_address_2, 4)); + let mut removed_txs = pending_txs.clean_account_stale_expired(&signing_address_0, 0); + removed_txs.extend(pending_txs.clean_account_stale_expired(&signing_address_1, 1)); + removed_txs.extend(pending_txs.clean_account_stale_expired(&signing_address_2, 4)); assert_eq!( removed_txs.len(), @@ -1569,8 +1568,8 @@ mod test { .unwrap(); // clean accounts, all nonces should be valid - let mut removed_txs = pending_txs.clean_account_stale_expired(signing_address_0, 0); - removed_txs.extend(pending_txs.clean_account_stale_expired(signing_address_1, 0)); + let mut removed_txs = pending_txs.clean_account_stale_expired(&signing_address_0, 0); + removed_txs.extend(pending_txs.clean_account_stale_expired(&signing_address_1, 0)); assert_eq!( removed_txs.len(), @@ -1620,13 +1619,13 @@ mod test { // empty account returns zero assert!( - pending_txs.pending_nonce(signing_address_1).is_none(), + pending_txs.pending_nonce(&signing_address_1).is_none(), "empty account should return None" ); // non empty account returns highest nonce assert_eq!( - pending_txs.pending_nonce(signing_address_0), + pending_txs.pending_nonce(&signing_address_0), Some(1), "should return highest nonce" ); @@ -1663,8 +1662,8 @@ mod test { // should return all transactions from signing_key_0 and last two from signing_key_1 let mut mock_state = mock_state_getter().await; - mock_state_put_account_nonce(&mut mock_state, signing_address_0, 1); - mock_state_put_account_nonce(&mut mock_state, signing_address_1, 2); + mock_state_put_account_nonce(&mut mock_state, &signing_address_0, 1); + mock_state_put_account_nonce(&mut mock_state, &signing_address_1, 2); // get builder queue let builder_queue = pending_txs @@ -1744,7 +1743,7 @@ mod test { // remove last parked_txs.find_promotables(&signing_address, 3, &remaining_balances); assert_eq!( - parked_txs.addresses().len(), + parked_txs.addresses().count(), 0, "empty account should've been removed from container" ); @@ -1779,30 +1778,30 @@ mod test { // demote none let demotables: Vec = - pending_txs.find_demotables(signing_address, &account_balances_full); + pending_txs.find_demotables(&signing_address, &account_balances_full); assert_eq!(demotables.len(), 0); // demote last let account_balances_demotion = mock_balances(100, 9); - let demotables = pending_txs.find_demotables(signing_address, &account_balances_demotion); + let demotables = pending_txs.find_demotables(&signing_address, &account_balances_demotion); assert_eq!(demotables.len(), 1); assert_eq!(demotables[0].nonce(), 4); // demote multiple let account_balances_demotion = mock_balances(100, 4); - let demotables = pending_txs.find_demotables(signing_address, &account_balances_demotion); + let demotables = pending_txs.find_demotables(&signing_address, &account_balances_demotion); assert_eq!(demotables.len(), 2); assert_eq!(demotables[0].nonce(), 2); // demote rest let account_balances_demotion = mock_balances(0, 5); - let demotables = pending_txs.find_demotables(signing_address, &account_balances_demotion); + let demotables = pending_txs.find_demotables(&signing_address, &account_balances_demotion); assert_eq!(demotables.len(), 1); assert_eq!(demotables[0].nonce(), 1); // empty account removed assert_eq!( - pending_txs.addresses().len(), + pending_txs.addresses().count(), 0, "empty account should've been removed from container" ); @@ -1837,7 +1836,7 @@ mod test { // get balances let remaining_balances = - pending_txs.subtract_contained_costs(signing_address, account_balances_full); + pending_txs.subtract_contained_costs(&signing_address, account_balances_full); assert_eq!( remaining_balances .get(&denom_0().to_ibc_prefixed()) diff --git a/crates/astria-sequencer/src/sequence/action.rs b/crates/astria-sequencer/src/sequence/action.rs index 04b3d8eb67..2192e0218a 100644 --- a/crates/astria-sequencer/src/sequence/action.rs +++ b/crates/astria-sequencer/src/sequence/action.rs @@ -50,7 +50,7 @@ impl ActionHandler for SequenceAction { ); let curr_balance = state - .get_account_balance(from, &self.fee_asset) + .get_account_balance(&from, &self.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; let fee = calculate_fee_from_state(&self.data, &state) @@ -63,7 +63,7 @@ impl ActionHandler for SequenceAction { .await .context("failed to add to block fees")?; state - .decrease_balance(from, &self.fee_asset, fee) + .decrease_balance(&from, &self.fee_asset, fee) .await .context("failed updating `from` account balance")?; Ok(()) diff --git a/crates/astria-sequencer/src/sequence/component.rs b/crates/astria-sequencer/src/sequence/component.rs index 742fb6ea1f..49d88c01ef 100644 --- a/crates/astria-sequencer/src/sequence/component.rs +++ b/crates/astria-sequencer/src/sequence/component.rs @@ -20,11 +20,11 @@ impl Component for SequenceComponent { #[instrument(name = "SequenceComponent::init_chain", skip_all)] async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> { - state.put_sequence_action_base_fee(app_state.fees().sequence_base_fee); + // No need to add context as these `put` methods already report sufficient context on error. + state.put_sequence_action_base_fee(app_state.fees().sequence_base_fee)?; state.put_sequence_action_byte_cost_multiplier( app_state.fees().sequence_byte_cost_multiplier, - ); - Ok(()) + ) } #[instrument(name = "SequenceComponent::begin_block", skip_all)] diff --git a/crates/astria-sequencer/src/sequence/state_ext.rs b/crates/astria-sequencer/src/sequence/state_ext.rs index ad17e4ef72..da76ce8ba1 100644 --- a/crates/astria-sequencer/src/sequence/state_ext.rs +++ b/crates/astria-sequencer/src/sequence/state_ext.rs @@ -4,23 +4,20 @@ use anyhow::{ Result, }; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, }; use tracing::instrument; +use crate::storage::{ + self, + StoredValue, +}; + const SEQUENCE_ACTION_BASE_FEE_STORAGE_KEY: &str = "seqbasefee"; const SEQUENCE_ACTION_BYTE_COST_MULTIPLIER_STORAGE_KEY: &str = "seqmultiplier"; -/// Newtype wrapper to read and write a u128 from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct Fee(u128); - #[async_trait] pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] @@ -30,8 +27,9 @@ pub(crate) trait StateReadExt: StateRead { .await .context("failed reading raw sequence action base fee from state")? .ok_or_else(|| anyhow!("sequence action base fee not found"))?; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid sequence action base fee bytes") } #[instrument(skip_all)] @@ -41,8 +39,9 @@ pub(crate) trait StateReadExt: StateRead { .await .context("failed reading raw sequence action byte cost multiplier from state")? .ok_or_else(|| anyhow!("sequence action byte cost multiplier not found"))?; - let Fee(fee) = Fee::try_from_slice(&bytes).context("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .context("invalid sequence action byte cost multiplier bytes") } } @@ -51,19 +50,24 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_sequence_action_base_fee(&mut self, fee: u128) { - self.put_raw( - SEQUENCE_ACTION_BASE_FEE_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).expect("failed to serialize fee"), - ); + fn put_sequence_action_base_fee(&mut self, fee: u128) -> Result<()> { + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize sequence action base fee")?; + self.put_raw(SEQUENCE_ACTION_BASE_FEE_STORAGE_KEY.to_string(), bytes); + Ok(()) } #[instrument(skip_all)] - fn put_sequence_action_byte_cost_multiplier(&mut self, fee: u128) { + fn put_sequence_action_byte_cost_multiplier(&mut self, fee: u128) -> Result<()> { + let bytes = StoredValue::Fee(fee.into()) + .serialize() + .context("failed to serialize sequence action byte cost multiplier")?; self.put_raw( SEQUENCE_ACTION_BYTE_COST_MULTIPLIER_STORAGE_KEY.to_string(), - borsh::to_vec(&Fee(fee)).expect("failed to serialize fee"), + bytes, ); + Ok(()) } } @@ -85,7 +89,7 @@ mod test { let mut state = StateDelta::new(snapshot); let fee = 42; - state.put_sequence_action_base_fee(fee); + state.put_sequence_action_base_fee(fee).unwrap(); assert_eq!(state.get_sequence_action_base_fee().await.unwrap(), fee); } @@ -96,7 +100,7 @@ mod test { let mut state = StateDelta::new(snapshot); let fee = 42; - state.put_sequence_action_byte_cost_multiplier(fee); + state.put_sequence_action_byte_cost_multiplier(fee).unwrap(); assert_eq!( state .get_sequence_action_byte_cost_multiplier() diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index a5384c9359..ca65577273 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -457,7 +457,7 @@ mod test { vec![ astria_core::generated::protocol::genesis::v1alpha1::Account { address: Some( - crate::test_utils::astria_address(&funded_key.address_bytes()).to_raw(), + crate::test_utils::astria_address(funded_key.address_bytes()).to_raw(), ), balance: Some(10u128.pow(19).into()), }, diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 11e1348036..7527d5e486 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -214,10 +214,12 @@ mod test { let height = 99; let version = storage.latest_version().wrapping_add(1); let mut state = StateDelta::new(storage.latest_snapshot()); - state.put_storage_version_by_height(height, version); + state + .put_storage_version_by_height(height, version) + .unwrap(); - state.put_base_prefix("astria"); - state.put_native_asset(&crate::test_utils::nria()); + state.put_base_prefix("astria".to_string()).unwrap(); + state.put_native_asset(crate::test_utils::nria()).unwrap(); let address = state .try_base_prefixed(&hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap()) @@ -226,9 +228,9 @@ mod test { let balance = 1000; state - .put_account_balance(address, crate::test_utils::nria(), balance) + .put_account_balance(&address, &crate::test_utils::nria(), balance) .unwrap(); - state.put_block_height(height); + state.put_block_height(height).unwrap(); storage.commit(state).await.unwrap(); let info_request = InfoRequest::Query(request::Query { @@ -273,10 +275,10 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mut state = StateDelta::new(storage.latest_snapshot()); - let denom = "some/ibc/asset".parse().unwrap(); + let denom: asset::TracePrefixed = "some/ibc/asset".parse().unwrap(); let height = 99; - state.put_block_height(height); - state.put_ibc_asset(&denom).unwrap(); + state.put_block_height(height).unwrap(); + state.put_ibc_asset(denom.clone()).unwrap(); storage.commit(state).await.unwrap(); let info_request = InfoRequest::Query(request::Query { @@ -322,7 +324,7 @@ mod test { let height = 99; for asset in &assets { - state.put_allowed_fee_asset(asset); + state.put_allowed_fee_asset(asset).unwrap(); assert!( state .is_allowed_fee_asset(asset) @@ -331,7 +333,7 @@ mod test { "fee asset was expected to be allowed" ); } - state.put_block_height(height); + state.put_block_height(height).unwrap(); storage.commit(state).await.unwrap(); let info_request = InfoRequest::Query(request::Query { diff --git a/crates/astria-sequencer/src/service/mempool.rs b/crates/astria-sequencer/src/service/mempool.rs index 4e027e0ce7..594dccd3a2 100644 --- a/crates/astria-sequencer/src/service/mempool.rs +++ b/crates/astria-sequencer/src/service/mempool.rs @@ -266,7 +266,7 @@ async fn handle_check_tx = - match get_account_balances(&state, address) + match get_account_balances(&state, &address) .await .with_context(|| "failed fetching balances for account `{address}`") { diff --git a/crates/astria-sequencer/src/state_ext.rs b/crates/astria-sequencer/src/state_ext.rs index 78eb4c32cd..b99a4fc442 100644 --- a/crates/astria-sequencer/src/state_ext.rs +++ b/crates/astria-sequencer/src/state_ext.rs @@ -11,7 +11,15 @@ use cnidarium::{ use tendermint::Time; use tracing::instrument; +use crate::storage::{ + self, + StoredValue, +}; + +const CHAIN_ID_KEY: &str = "chain_id"; const REVISION_NUMBER_KEY: &str = "revision_number"; +const BLOCK_HEIGHT_KEY: &str = "block_height"; +const BLOCK_TIMESTAMP_KEY: &str = "block_timestamp"; fn storage_version_by_height_key(height: u64) -> Vec { format!("storage_version/{height}").into() @@ -22,17 +30,15 @@ pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn get_chain_id(&self) -> Result { let Some(bytes) = self - .get_raw("chain_id") + .get_raw(CHAIN_ID_KEY) .await .context("failed to read raw chain_id from state")? else { bail!("chain id not found in state"); }; - - Ok(String::from_utf8(bytes) - .context("failed to parse chain id from raw bytes")? - .try_into() - .expect("only valid chain ids should be stored in the state")) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::ChainId::try_from(value).map(tendermint::chain::Id::from)) + .context("invalid chain id bytes") } #[instrument(skip_all)] @@ -44,44 +50,37 @@ pub(crate) trait StateReadExt: StateRead { else { bail!("revision number not found in state"); }; - - let bytes = TryInto::<[u8; 8]>::try_into(bytes).map_err(|b| { - anyhow::anyhow!( - "expected 8 revision number bytes but got {}; this is a bug", - b.len() - ) - })?; - - Ok(u64::from_be_bytes(bytes)) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::RevisionNumber::try_from(value).map(u64::from)) + .context("invalid revision number bytes") } #[instrument(skip_all)] async fn get_block_height(&self) -> Result { let Some(bytes) = self - .get_raw("block_height") + .get_raw(BLOCK_HEIGHT_KEY) .await .context("failed to read raw block_height from state")? else { bail!("block height not found state"); }; - let Ok(bytes): Result<[u8; 8], _> = bytes.try_into() else { - bail!("failed turning raw block height bytes into u64; not 8 bytes?"); - }; - Ok(u64::from_be_bytes(bytes)) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::BlockHeight::try_from(value).map(u64::from)) + .context("invalid block height bytes") } #[instrument(skip_all)] async fn get_block_timestamp(&self) -> Result