From 6d9eb288efc071402078db258f9146b93e1918c4 Mon Sep 17 00:00:00 2001 From: Fraser Hutchison <190532+Fraser999@users.noreply.github.com> Date: Tue, 1 Oct 2024 19:46:38 +0100 Subject: [PATCH] chore(sequencer)!: exclusively use Borsh encoding for stored data (ENG-768) (#1492) ## Summary This PR primarily changes the encoding format of all data being written to storage to Borsh. ## Background We currently have a variety of different encoding formats, and this can be confusing, sub-optimal (in terms of storage space consumed and serialization/deserialization performance) and potentially problematic as e.g. JSON-encoding leaves a lot of flexibility in how the actual serialized data can look. This PR is part one of three which aim to improve the performance and quality of the storage component. As such, the APIs of the various `StateReadExt` and `StateWriteExt` extension traits were updated slightly in preparation for the upcoming changes. In broad terms, for getters this meant having ref parameters rather than value ones (even for copyable types like `[u8; 32]` this is significantly more performant), and for "putters", parameters which are used for DB keys are refs, while the DB value parameters become values, since in the next PR these values will be added to a cache. ## Changes - Added a new `storage` module. This will ultimately contain our own equivalents of the `cnidarium` types, but for now consists only of a collection of submodules for types currently being written to storage. There is a top-level enum `StoredValue` which becomes the only type being written to storage by our own code. - To accommodate for converting between the storage types and the corresponding domain types defined in `astria-core` and `astria-merkle`, some of these have been provided with constructors named `unchecked_from_parts`. This allows the type to be constructed from a trusted source like our own DB, skipping any validation steps which might otherwise be done. - Updated all `StateReadExt` and `StateWriteExt` traits to use the new `StoredValue`, which internally uses Borsh encoding. - Updated the APIs of all these extension traits as described above. This change resulted in a slew of similar updates to avoid needless copying `[u8; 32]` and `[u8; 20]` arrays, and that has unfortunately made the PR larger than expected. - A few core types which currently derive or implement `serde` traits have had that removed, since it only existed to support JSON-encoding the type for storing. In one case it was for JSON-encoding to a debug log line, but I replaced that with a `Display` impl. ## Testing - Existing unit tests of the `StateReadExt` and `StateWriteExt` traits already had almost full coverage. Where coverage was missing, I added round trip tests. ## Breaking Changelist - The on-disk format of stored data has changed. ## Related Issues Closes #1434. --- Cargo.lock | 1 + .../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 +- crates/astria-conductor/src/lib.rs | 1 + .../tests/blackbox/helpers/mod.rs | 4 +- crates/astria-core/Cargo.toml | 4 + crates/astria-core/src/celestia.rs | 2 +- crates/astria-core/src/crypto.rs | 6 +- .../src/primitive/v1/asset/denom.rs | 48 +- crates/astria-core/src/primitive/v1/mod.rs | 50 +- .../src/protocol/transaction/v1alpha1/mod.rs | 28 +- .../src/sequencerblock/v1alpha1/block.rs | 129 ++++- .../src/sequencerblock/v1alpha1/celestia.rs | 14 +- crates/astria-merkle/Cargo.toml | 4 + crates/astria-merkle/src/audit.rs | 21 + crates/astria-sequencer/Cargo.toml | 15 +- .../astria-sequencer/src/accounts/action.rs | 20 +- .../src/accounts/component.rs | 2 +- crates/astria-sequencer/src/accounts/mod.rs | 39 +- crates/astria-sequencer/src/accounts/query.rs | 12 +- ...ests__storage_keys_have_not_changed-2.snap | 3 +- ..._tests__storage_keys_have_not_changed.snap | 3 +- .../src/accounts/state_ext.rs | 248 ++++----- .../src/accounts/storage/mod.rs | 8 + .../src/accounts/storage/values.rs | 112 ++++ crates/astria-sequencer/src/address/mod.rs | 2 + .../astria-sequencer/src/address/state_ext.rs | 44 +- .../src/address/storage/mod.rs | 4 + .../src/address/storage/values.rs | 48 ++ .../src/app/action_handler.rs | 2 +- crates/astria-sequencer/src/app/mod.rs | 95 ++-- ...ransaction_with_every_action_snapshot.snap | 65 +-- ..._changes__app_finalize_block_snapshot.snap | 64 +-- ...reaking_changes__app_genesis_snapshot.snap | 59 +- .../src/{ => app}/state_ext.rs | 132 +++-- .../astria-sequencer/src/app/storage/mod.rs | 10 + .../src/app/storage/values/block_height.rs | 43 ++ .../src/app/storage/values/block_timestamp.rs | 82 +++ .../src/app/storage/values/chain_id.rs | 79 +++ .../src/app/storage/values/mod.rs | 30 + .../src/app/storage/values/revision_number.rs | 44 ++ .../src/app/storage/values/storage_version.rs | 44 ++ crates/astria-sequencer/src/app/test_utils.rs | 40 +- .../src/app/tests_app/mempool.rs | 4 +- .../astria-sequencer/src/app/tests_app/mod.rs | 30 +- .../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/mod.rs | 1 + crates/astria-sequencer/src/assets/query.rs | 4 +- .../astria-sequencer/src/assets/state_ext.rs | 171 +++--- .../src/assets/storage/mod.rs | 7 + .../src/assets/storage/values.rs | 100 ++++ .../astria-sequencer/src/authority/action.rs | 62 ++- .../src/authority/component.rs | 2 +- crates/astria-sequencer/src/authority/mod.rs | 49 +- .../src/authority/state_ext.rs | 66 +-- .../src/authority/storage/mod.rs | 7 + .../src/authority/storage/values.rs | 155 ++++++ .../src/bridge/bridge_lock_action.rs | 31 +- .../src/bridge/bridge_sudo_change_action.rs | 36 +- .../src/bridge/bridge_unlock_action.rs | 38 +- .../astria-sequencer/src/bridge/component.rs | 20 +- .../src/bridge/init_bridge_account_action.rs | 26 +- crates/astria-sequencer/src/bridge/mod.rs | 1 + crates/astria-sequencer/src/bridge/query.rs | 30 +- .../astria-sequencer/src/bridge/state_ext.rs | 408 ++++++++------ .../src/bridge/storage/mod.rs | 12 + .../bridge/storage/values/address_bytes.rs | 61 +++ .../src/bridge/storage/values/block_height.rs | 56 ++ .../src/bridge/storage/values/deposits.rs | 156 ++++++ .../src/bridge/storage/values/fee.rs | 42 ++ .../storage/values/ibc_prefixed_denom.rs | 62 +++ .../src/bridge/storage/values/mod.rs | 36 ++ .../src/bridge/storage/values/rollup_id.rs | 60 ++ .../bridge/storage/values/transaction_id.rs | 63 +++ .../astria-sequencer/src/fee_asset_change.rs | 4 +- crates/astria-sequencer/src/grpc/mod.rs | 7 + crates/astria-sequencer/src/grpc/sequencer.rs | 46 +- .../{api_state_ext.rs => grpc/state_ext.rs} | 511 ++++++++---------- .../astria-sequencer/src/grpc/storage/mod.rs | 10 + .../src/grpc/storage/values/block_hash.rs | 59 ++ .../src/grpc/storage/values/mod.rs | 30 + .../src/grpc/storage/values/proof.rs | 63 +++ .../src/grpc/storage/values/rollup_ids.rs | 75 +++ .../storage/values/rollup_transactions.rs | 89 +++ .../storage/values/sequencer_block_header.rs | 201 +++++++ crates/astria-sequencer/src/ibc/component.rs | 6 +- .../src/ibc/host_interface.rs | 2 +- .../src/ibc/ibc_relayer_change.rs | 4 +- .../src/ibc/ics20_transfer.rs | 144 +++-- .../src/ibc/ics20_withdrawal.rs | 94 ++-- crates/astria-sequencer/src/ibc/mod.rs | 1 + crates/astria-sequencer/src/ibc/state_ext.rs | 167 +++--- .../astria-sequencer/src/ibc/storage/mod.rs | 8 + .../src/ibc/storage/values.rs | 131 +++++ crates/astria-sequencer/src/lib.rs | 3 +- .../src/mempool/benchmarks.rs | 10 +- .../src/mempool/mempool_state.rs | 20 +- crates/astria-sequencer/src/mempool/mod.rs | 38 +- .../src/mempool/transactions_container.rs | 101 ++-- .../astria-sequencer/src/sequence/action.rs | 4 +- .../src/sequence/component.rs | 18 +- crates/astria-sequencer/src/sequence/mod.rs | 1 + .../src/sequence/state_ext.rs | 44 +- .../src/sequence/storage/mod.rs | 4 + .../src/sequence/storage/values.rs | 45 ++ .../astria-sequencer/src/service/consensus.rs | 2 +- .../astria-sequencer/src/service/info/mod.rs | 31 +- .../astria-sequencer/src/service/mempool.rs | 6 +- crates/astria-sequencer/src/storage/mod.rs | 3 + .../src/storage/stored_value.rs | 32 ++ crates/astria-sequencer/src/storage_keys.rs | 2 +- .../src/transaction/checks.rs | 56 +- .../astria-sequencer/src/transaction/mod.rs | 22 +- .../astria-sequencer/src/transaction/query.rs | 4 +- .../src/transaction/state_ext.rs | 4 +- 121 files changed, 4167 insertions(+), 1640 deletions(-) create mode 100644 crates/astria-sequencer/src/accounts/storage/mod.rs create mode 100644 crates/astria-sequencer/src/accounts/storage/values.rs create mode 100644 crates/astria-sequencer/src/address/storage/mod.rs create mode 100644 crates/astria-sequencer/src/address/storage/values.rs rename crates/astria-sequencer/src/{ => app}/state_ext.rs (75%) create mode 100644 crates/astria-sequencer/src/app/storage/mod.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/block_height.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/block_timestamp.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/chain_id.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/mod.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/revision_number.rs create mode 100644 crates/astria-sequencer/src/app/storage/values/storage_version.rs create mode 100644 crates/astria-sequencer/src/assets/storage/mod.rs create mode 100644 crates/astria-sequencer/src/assets/storage/values.rs create mode 100644 crates/astria-sequencer/src/authority/storage/mod.rs create mode 100644 crates/astria-sequencer/src/authority/storage/values.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/mod.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/address_bytes.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/block_height.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/deposits.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/fee.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/ibc_prefixed_denom.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/mod.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/rollup_id.rs create mode 100644 crates/astria-sequencer/src/bridge/storage/values/transaction_id.rs rename crates/astria-sequencer/src/{api_state_ext.rs => grpc/state_ext.rs} (55%) create mode 100644 crates/astria-sequencer/src/grpc/storage/mod.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/block_hash.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/mod.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/proof.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/rollup_ids.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/rollup_transactions.rs create mode 100644 crates/astria-sequencer/src/grpc/storage/values/sequencer_block_header.rs create mode 100644 crates/astria-sequencer/src/ibc/storage/mod.rs create mode 100644 crates/astria-sequencer/src/ibc/storage/values.rs create mode 100644 crates/astria-sequencer/src/sequence/storage/mod.rs create mode 100644 crates/astria-sequencer/src/sequence/storage/values.rs create mode 100644 crates/astria-sequencer/src/storage/mod.rs create mode 100644 crates/astria-sequencer/src/storage/stored_value.rs diff --git a/Cargo.lock b/Cargo.lock index 2630d1141e..d116d2712a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1394,6 +1394,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 cd0085b54c..c7a2a00e71 100644 --- a/crates/astria-cli/src/commands/bridge/submit.rs +++ b/crates/astria-cli/src/commands/bridge/submit.rs @@ -118,7 +118,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 95c274f534..33bade4b4b 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -463,7 +463,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..f84a2fd428 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().as_bytes()) .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 a63254bec6..2b3e535984 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 b923728e88..464235aa5a 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -714,19 +714,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/src/lib.rs b/crates/astria-conductor/src/lib.rs index 4bda194839..ee26570938 100644 --- a/crates/astria-conductor/src/lib.rs +++ b/crates/astria-conductor/src/lib.rs @@ -1,6 +1,7 @@ //! # Astria Conductor //! //! The Astria conductor connects the shared sequencer layer and the execution layer. +//! //! When a block is received from the sequencer layer, the conductor pushes it to the execution //! layer. There are two ways for a block to be received: //! diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index 8e95499ceb..061dfac16a 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -48,7 +48,7 @@ use wiremock::MockServer; 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.as_bytes()); pub const SEQUENCER_CHAIN_ID: &str = "test_sequencer-1000"; pub const CELESTIA_CHAIN_ID: &str = "test_celestia-1000"; @@ -634,7 +634,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/Cargo.toml b/crates/astria-core/Cargo.toml index cd08623c13..a7a45d5ba0 100644 --- a/crates/astria-core/Cargo.toml +++ b/crates/astria-core/Cargo.toml @@ -55,6 +55,10 @@ server = ["dep:tonic"] test-utils = [] base64-serde = ["dep:base64-serde"] brotli = ["dep:brotli"] +# When enabled, this adds constructors for some types that skip the normal constructor validity +# checks. It supports the case where the inputs are already deemed valid, e.g. having read them from +# local storage. +unchecked-constructors = [] [dev-dependencies] astria-core = { path = ".", features = ["serde"] } diff --git a/crates/astria-core/src/celestia.rs b/crates/astria-core/src/celestia.rs index b9762e6736..f14be40d8c 100644 --- a/crates/astria-core/src/celestia.rs +++ b/crates/astria-core/src/celestia.rs @@ -20,7 +20,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.as_bytes()) } /// 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 65b5499854..2ce819d875 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 412f557ecf..ec685dba7d 100644 --- a/crates/astria-core/src/primitive/v1/asset/denom.rs +++ b/crates/astria-core/src/primitive/v1/asset/denom.rs @@ -134,6 +134,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; @@ -282,6 +288,44 @@ impl TracePrefixed { } len + self.base_denom.len() } + + 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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)] @@ -507,8 +551,8 @@ impl IbcPrefixed { } #[must_use] - pub fn get(&self) -> [u8; 32] { - self.id + pub fn as_bytes(&self) -> &[u8; 32] { + &self.id } #[must_use] diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 29cd265b40..531ebe8f72 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.as_bytes()); /// ``` #[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.as_bytes()); /// ``` #[must_use] - pub const fn get(self) -> [u8; 32] { - self.inner + pub const fn as_bytes(&self) -> &[u8; 32] { + &self.inner } /// Creates a new rollup ID by applying Sha256 to `bytes`. @@ -526,6 +526,11 @@ impl Address { self.bytes } + #[must_use] + pub fn as_bytes(&self) -> &[u8; ADDRESS_LEN] { + &self.bytes + } + #[must_use] pub fn prefix(&self) -> &str { self.prefix.as_str() @@ -538,7 +543,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.as_bytes()) .prefix(prefix) .try_build() } @@ -565,7 +570,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.as_bytes()) .expect( "should not fail because len(prefix) + len(bytes) <= 63 < BECH32M::CODELENGTH", ); @@ -590,6 +595,22 @@ 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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 +644,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.as_bytes()) { Ok(()) => Ok(()), Err(EncodeError::Fmt(err)) => Err(err), Err(err) => panic!( @@ -682,12 +703,18 @@ impl TransactionId { } } - /// Returns the 32-byte transaction hash. + /// Consumes `self` and returns the 32-byte transaction hash. #[must_use] pub fn get(self) -> [u8; TRANSACTION_ID_LEN] { self.inner } + /// Returns a reference to the 32-byte transaction hash. + #[must_use] + pub fn as_bytes(&self) -> &[u8; TRANSACTION_ID_LEN] { + &self.inner + } + #[must_use] pub fn to_raw(&self) -> raw::TransactionId { raw::TransactionId { @@ -805,7 +832,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) @@ -896,6 +923,7 @@ mod tests { let _ = address.into_raw(); } + #[cfg(feature = "unchecked-constructors")] #[test] fn address_to_unchecked_roundtrip() { let bytes = [42u8; ADDRESS_LEN]; @@ -907,7 +935,7 @@ mod tests { let unchecked = input.into_raw(); let roundtripped = Address::try_from_raw(&unchecked).unwrap(); assert_eq!(input, roundtripped); - assert_eq!(input.bytes(), roundtripped.bytes()); + assert_eq!(input.as_bytes(), roundtripped.as_bytes()); assert_eq!("astria", input.prefix()); } } diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 67f86add44..c47c6ac684 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -65,14 +65,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 @@ -86,7 +78,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() } @@ -176,22 +168,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 @@ -645,7 +621,7 @@ mod tests { 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 6df6428e0e..bd8a1a4dc8 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -92,8 +92,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 +169,27 @@ 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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,33 @@ 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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)] @@ -559,17 +607,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, } @@ -578,8 +626,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] @@ -598,6 +646,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 { @@ -914,6 +972,31 @@ 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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( @@ -987,8 +1070,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] @@ -1007,8 +1090,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] @@ -1151,7 +1234,7 @@ impl FilteredSequencerBlock { ) { return Err( FilteredSequencerBlockError::rollup_transaction_for_id_not_in_sequencer_block( - rollup_transactions.rollup_id(), + *rollup_transactions.rollup_id(), ), ); } diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs b/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs index 3e6f456d02..9ae3a0aab7 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs @@ -181,8 +181,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). @@ -507,8 +507,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. @@ -539,7 +539,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() } @@ -584,7 +584,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()); } @@ -592,7 +592,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/Cargo.toml b/crates/astria-merkle/Cargo.toml index 9f2a4c4b5e..f12b263682 100644 --- a/crates/astria-merkle/Cargo.toml +++ b/crates/astria-merkle/Cargo.toml @@ -19,6 +19,10 @@ hex-literal = { workspace = true } [features] # Used to enable allocation data in benchmarks. bench_include_allocs = [] +# When enabled, this adds constructors for some types that skip the normal constructor validity +# checks. It supports the case where the inputs are already deemed valid, e.g. having read them from +# local storage. +unchecked-constructors = [] [[bench]] name = "benchmark" diff --git a/crates/astria-merkle/src/audit.rs b/crates/astria-merkle/src/audit.rs index 4ab383cbd5..ce524e0a74 100644 --- a/crates/astria-merkle/src/audit.rs +++ b/crates/astria-merkle/src/audit.rs @@ -621,4 +621,25 @@ 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. + /// + /// Note that this function is not considered part of the public API and is subject to breaking + /// change at any time. + #[cfg(feature = "unchecked-constructors")] + #[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 373e0e8dee..a31c86ac27 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -15,20 +15,27 @@ name = "astria-sequencer" benchmark = ["divan"] [dependencies] -astria-core = { path = "../astria-core", features = ["server", "serde"] } +astria-core = { path = "../astria-core", features = [ + "server", + "serde", + "unchecked-constructors", +] } astria-build-info = { path = "../astria-build-info", features = ["runtime"] } -# The "anyhow" feature is only included because it is necessary for the implementation of +# The "anyhow" feature is only included because it is necessary for the implementation of # `penumbra_ibc::component::HostInterface` in `crates/astria-sequencer/src/ibc/host_interface.rs`. # Avoid using "anyhow" results anywhere else. astria-eyre = { path = "../astria-eyre", features = ["anyhow"] } config = { package = "astria-config", path = "../astria-config" } -merkle = { package = "astria-merkle", path = "../astria-merkle" } +merkle = { package = "astria-merkle", path = "../astria-merkle", features = [ + "unchecked-constructors", +] } telemetry = { package = "astria-telemetry", path = "../astria-telemetry", features = [ "display", ] } -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", ] } diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index fea06788db..b02a77c1de 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -43,15 +43,15 @@ impl ActionHandler for TransferAction { ensure!( state - .get_bridge_account_rollup_id(from) + .get_bridge_account_rollup_id(&from) .await .wrap_err("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(()) } @@ -59,15 +59,13 @@ impl ActionHandler for TransferAction { pub(crate) async fn execute_transfer( action: &TransferAction, - from: TAddress, + from: &TAddress, mut state: S, ) -> Result<()> where S: StateWrite, TAddress: AddressBytes, { - let from = from.address_bytes(); - let fee = state .get_transfer_base_fee() .await @@ -91,7 +89,7 @@ where .await .wrap_err("failed decreasing `from` account balance")?; state - .increase_balance(action.to, &action.asset, action.amount) + .increase_balance(&action.to, &action.asset, action.amount) .await .wrap_err("failed increasing `to` account balance")?; } else { @@ -102,7 +100,7 @@ where .await .wrap_err("failed decreasing `from` account balance")?; state - .increase_balance(action.to, &action.asset, action.amount) + .increase_balance(&action.to, &action.asset, action.amount) .await .wrap_err("failed increasing `to` account balance")?; @@ -117,7 +115,7 @@ where pub(crate) async fn check_transfer( action: &TransferAction, - from: TAddress, + from: &TAddress, state: &S, ) -> Result<()> where @@ -139,10 +137,10 @@ where .get_transfer_base_fee() .await .wrap_err("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 .wrap_err("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 e197a1911f..6913eba06b 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 { .wrap_err("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) .wrap_err("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 ba4244fa97..545d6ff74e 100644 --- a/crates/astria-sequencer/src/accounts/mod.rs +++ b/crates/astria-sequencer/src/accounts/mod.rs @@ -2,12 +2,10 @@ pub(crate) mod action; pub(crate) mod component; pub(crate) mod query; mod state_ext; +pub(crate) mod storage; use astria_core::{ - crypto::{ - SigningKey, - VerificationKey, - }, + crypto::VerificationKey, primitive::v1::{ Address, ADDRESS_LEN, @@ -21,7 +19,7 @@ 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]; fn display_address(&self) -> impl std::fmt::Display { telemetry::display::base64(self.address_bytes()) @@ -29,8 +27,8 @@ pub(crate) trait AddressBytes: Send + Sync { } impl AddressBytes for Address { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - self.bytes() + fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { + self.as_bytes() } fn display_address(&self) -> impl std::fmt::Display { @@ -39,34 +37,19 @@ impl AddressBytes for Address { } impl AddressBytes for [u8; ADDRESS_LEN] { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - *self - } -} - -impl AddressBytes for SignedTransaction { - fn address_bytes(&self) -> [u8; ADDRESS_LEN] { - self.address_bytes() + fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { + self } } -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 1aaa5f6412..9189513d50 100644 --- a/crates/astria-sequencer/src/accounts/query.rs +++ b/crates/astria-sequencer/src/accounts/query.rs @@ -32,13 +32,13 @@ use tracing::instrument; use crate::{ accounts::StateReadExt as _, + app::StateReadExt as _, assets::StateReadExt as _, - state_ext::StateReadExt as _, }; async fn ibc_to_trace( state: S, - asset: asset::IbcPrefixed, + asset: &asset::IbcPrefixed, ) -> Result { state .map_ibc_to_trace_prefixed_asset(asset) @@ -50,7 +50,7 @@ async fn ibc_to_trace( #[instrument(skip_all, fields(%address))] async fn get_trace_prefixed_account_balances( state: &S, - address: Address, + address: &Address, ) -> Result> { let stream = state .account_asset_balances(address) @@ -63,7 +63,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() @@ -88,7 +88,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 { @@ -126,7 +126,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/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap index df1b0bf0c8..9b2ee0652b 100644 --- a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap +++ b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap @@ -1,5 +1,6 @@ --- source: crates/astria-sequencer/src/accounts/state_ext.rs -expression: nonce_storage_key(address) +assertion_line: 855 +expression: nonce_storage_key(&address) --- accounts/1c0c490f1b5528d8173c5de46d131160e4b2c0c3/nonce diff --git a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap index 2f7840b415..9125e1ba51 100644 --- a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap +++ b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap @@ -1,5 +1,6 @@ --- source: crates/astria-sequencer/src/accounts/state_ext.rs -expression: "balance_storage_key(address, asset)" +assertion_line: 854 +expression: "balance_storage_key(&address, &asset)" --- accounts/1c0c490f1b5528d8173c5de46d131160e4b2c0c3/balance/be429a02d00837245167a2616674a979a2ac6f9806468b48a975b156ad711320 diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index 268798bfac..1aca12616c 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -1,4 +1,5 @@ use std::{ + fmt::Display, pin::Pin, task::{ ready, @@ -18,10 +19,6 @@ use astria_eyre::{ }, }; use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use cnidarium::{ StateRead, StateWrite, @@ -30,19 +27,11 @@ use futures::Stream; 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 super::storage; +use crate::{ + accounts::AddressBytes, + storage::StoredValue, +}; const ACCOUNTS_PREFIX: &str = "accounts"; const TRANSFER_BASE_FEE_STORAGE_KEY: &str = "transferfee"; @@ -59,19 +48,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! { @@ -140,12 +131,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, @@ -165,22 +153,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), } @@ -189,12 +177,13 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { #[instrument(skip_all, fields(address = %address.display_address(), %asset), err)] 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 + Display, + asset::IbcPrefixed: From<&'a TAsset> + Send + Sync, { let Some(bytes) = self .get_raw(&balance_storage_key(address, asset)) @@ -204,12 +193,13 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { else { return Ok(0); }; - let Balance(balance) = Balance::try_from_slice(&bytes).wrap_err("invalid balance bytes")?; - Ok(balance) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Balance::try_from(value).map(u128::from)) + .wrap_err("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 @@ -219,9 +209,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).wrap_err("invalid nonce bytes")?; - Ok(nonce) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Nonce::try_from(value).map(u32::from)) + .wrap_err("invalid nonce bytes") } #[instrument(skip_all)] @@ -234,9 +224,9 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { let Some(bytes) = bytes else { return Err(eyre!("transfer base fee not set")); }; - - let Fee(fee) = Fee::try_from_slice(&bytes).wrap_err("invalid fee bytes")?; - Ok(fee) + StoredValue::deserialize(&bytes) + .and_then(|value| storage::Fee::try_from(value).map(u128::from)) + .wrap_err("invalid fee bytes") } } @@ -245,46 +235,51 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all, fields(address = %address.display_address(), %asset, balance), err)] - 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, + TAsset: Display, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let bytes = borsh::to_vec(&Balance(balance)).wrap_err("failed to serialize balance")?; + let bytes = StoredValue::from(storage::Balance::from(balance)) + .serialize() + .wrap_err("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)).wrap_err("failed to serialize nonce")?; + fn put_account_nonce(&mut self, address: &T, nonce: u32) -> Result<()> { + let bytes = StoredValue::from(storage::Nonce::from(nonce)) + .serialize() + .wrap_err("failed to serialize nonce")?; self.put_raw(nonce_storage_key(address), bytes); Ok(()) } #[instrument(skip_all, fields(address = %address.display_address(), %asset, amount), err)] - 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 + Display, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let asset = asset.into(); let balance = self - .get_account_balance(&address, asset) + .get_account_balance(address, asset) .await .wrap_err("failed to get account balance")?; self.put_account_balance( - &address, + address, asset, balance .checked_add(amount) @@ -295,23 +290,23 @@ pub(crate) trait StateWriteExt: StateWrite { } #[instrument(skip_all, fields(address = %address.display_address(), %asset, amount))] - 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 + Display, + asset::IbcPrefixed: From<&'a TAsset> + Send, { - let asset = asset.into(); let balance = self - .get_account_balance(&address, asset) + .get_account_balance(address, asset) .await .wrap_err("failed to get account balance")?; self.put_account_balance( - &address, + address, asset, balance .checked_sub(amount) @@ -323,7 +318,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)).wrap_err("failed to serialize fee")?; + let bytes = StoredValue::from(storage::Fee::from(fee)) + .serialize() + .wrap_err("failed to serialize fee")?; self.put_raw(TRANSFER_BASE_FEE_STORAGE_KEY.to_string(), bytes); Ok(()) } @@ -339,14 +336,12 @@ mod tests { use insta::assert_snapshot; use super::{ + balance_storage_key, + nonce_storage_key, StateReadExt as _, StateWriteExt as _, }; use crate::{ - accounts::state_ext::{ - balance_storage_key, - nonce_storage_key, - }, assets::{ StateReadExt as _, StateWriteExt as _, @@ -381,7 +376,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, @@ -401,11 +396,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, @@ -415,11 +410,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, @@ -439,11 +434,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, @@ -455,11 +450,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, @@ -467,7 +462,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, @@ -489,7 +484,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, @@ -509,13 +504,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, @@ -526,12 +521,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, @@ -551,13 +546,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, @@ -570,11 +565,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, @@ -583,7 +578,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, @@ -606,16 +601,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, @@ -623,7 +618,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, @@ -641,7 +636,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 @@ -663,7 +658,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(); @@ -671,13 +666,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 @@ -688,17 +683,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"); @@ -734,14 +729,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, @@ -749,13 +744,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, @@ -775,14 +770,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, @@ -791,13 +786,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, @@ -818,17 +813,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 let _ = 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" @@ -838,10 +844,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/accounts/storage/mod.rs b/crates/astria-sequencer/src/accounts/storage/mod.rs new file mode 100644 index 0000000000..5fb23de750 --- /dev/null +++ b/crates/astria-sequencer/src/accounts/storage/mod.rs @@ -0,0 +1,8 @@ +mod values; + +pub(crate) use values::Value; +pub(super) use values::{ + Balance, + Fee, + Nonce, +}; diff --git a/crates/astria-sequencer/src/accounts/storage/values.rs b/crates/astria-sequencer/src/accounts/storage/values.rs new file mode 100644 index 0000000000..88a73c3842 --- /dev/null +++ b/crates/astria-sequencer/src/accounts/storage/values.rs @@ -0,0 +1,112 @@ +use astria_eyre::eyre::bail; +use borsh::{ + BorshDeserialize, + BorshSerialize, +}; + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(crate) struct Value(ValueImpl); + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +enum ValueImpl { + Balance(Balance), + Nonce(Nonce), + Fee(Fee), +} + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(in crate::accounts) struct Balance(u128); + +impl From for Balance { + fn from(balance: u128) -> Self { + Balance(balance) + } +} + +impl From for u128 { + fn from(balance: Balance) -> Self { + balance.0 + } +} + +impl<'a> From for crate::storage::StoredValue<'a> { + fn from(balance: Balance) -> Self { + crate::storage::StoredValue::Accounts(Value(ValueImpl::Balance(balance))) + } +} + +impl<'a> TryFrom> for Balance { + type Error = astria_eyre::eyre::Error; + + fn try_from(value: crate::storage::StoredValue<'a>) -> Result { + let crate::storage::StoredValue::Accounts(Value(ValueImpl::Balance(balance))) = value + else { + bail!("accounts stored value type mismatch: expected balance, found {value:?}"); + }; + Ok(balance) + } +} + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(in crate::accounts) struct Nonce(u32); + +impl From for Nonce { + fn from(nonce: u32) -> Self { + Nonce(nonce) + } +} + +impl From for u32 { + fn from(nonce: Nonce) -> Self { + nonce.0 + } +} + +impl<'a> From for crate::storage::StoredValue<'a> { + fn from(nonce: Nonce) -> Self { + crate::storage::StoredValue::Accounts(Value(ValueImpl::Nonce(nonce))) + } +} + +impl<'a> TryFrom> for Nonce { + type Error = astria_eyre::eyre::Error; + + fn try_from(value: crate::storage::StoredValue<'a>) -> Result { + let crate::storage::StoredValue::Accounts(Value(ValueImpl::Nonce(nonce))) = value else { + bail!("accounts stored value type mismatch: expected nonce, found {value:?}"); + }; + Ok(nonce) + } +} + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(in crate::accounts) struct Fee(u128); + +impl From for Fee { + fn from(fee: u128) -> Self { + Fee(fee) + } +} + +impl From for u128 { + fn from(fee: Fee) -> Self { + fee.0 + } +} + +impl<'a> From for crate::storage::StoredValue<'a> { + fn from(fee: Fee) -> Self { + crate::storage::StoredValue::Accounts(Value(ValueImpl::Fee(fee))) + } +} + +impl<'a> TryFrom> for Fee { + type Error = astria_eyre::eyre::Error; + + fn try_from(value: crate::storage::StoredValue<'a>) -> Result { + let crate::storage::StoredValue::Accounts(Value(ValueImpl::Fee(fee))) = value else { + bail!("accounts stored value type mismatch: expected fee, found {value:?}"); + }; + Ok(fee) + } +} diff --git a/crates/astria-sequencer/src/address/mod.rs b/crates/astria-sequencer/src/address/mod.rs index 35bea31ada..8a6081651a 100644 --- a/crates/astria-sequencer/src/address/mod.rs +++ b/crates/astria-sequencer/src/address/mod.rs @@ -1,4 +1,6 @@ mod state_ext; +pub(crate) mod storage; + pub(crate) use state_ext::{ StateReadExt, StateWriteExt, diff --git a/crates/astria-sequencer/src/address/state_ext.rs b/crates/astria-sequencer/src/address/state_ext.rs index df4d0d33d3..a0c074777b 100644 --- a/crates/astria-sequencer/src/address/state_ext.rs +++ b/crates/astria-sequencer/src/address/state_ext.rs @@ -18,13 +18,11 @@ use cnidarium::{ }; use tracing::instrument; -fn base_prefix_key() -> &'static str { - "prefixes/base" -} +use super::storage; +use crate::storage::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 { @@ -56,27 +54,31 @@ pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all, err)] async fn get_base_prefix(&self) -> Result { let Some(bytes) = self - .get_raw(base_prefix_key()) + .get_raw(BASE_PREFIX_KEY) .await .map_err(anyhow_to_eyre) .wrap_err("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, err)] 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 .map_err(anyhow_to_eyre) .wrap_err("failed reading address ibc compat prefix from state")? else { bail!("no ibc compat prefix found in state") }; - String::from_utf8(bytes).wrap_err("prefix retrieved from storage is not valid utf8") + StoredValue::deserialize(&bytes) + .and_then(|value| storage::AddressPrefix::try_from(value).map(String::from)) + .wrap_err("invalid ibc compat prefix bytes") } } @@ -85,13 +87,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::from(storage::AddressPrefix::from(prefix.as_str())) + .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::from(storage::AddressPrefix::from(prefix.as_str())) + .serialize() + .context("failed to serialize ibc-compat prefix")?; + self.put_raw(IBC_COMPAT_PREFIX_KEY.to_string(), bytes); + Ok(()) } } @@ -112,7 +122,7 @@ mod tests { 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()); } @@ -122,7 +132,9 @@ mod tests { 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/address/storage/mod.rs b/crates/astria-sequencer/src/address/storage/mod.rs new file mode 100644 index 0000000000..9d97b02d12 --- /dev/null +++ b/crates/astria-sequencer/src/address/storage/mod.rs @@ -0,0 +1,4 @@ +mod values; + +pub(super) use values::AddressPrefix; +pub(crate) use values::Value; diff --git a/crates/astria-sequencer/src/address/storage/values.rs b/crates/astria-sequencer/src/address/storage/values.rs new file mode 100644 index 0000000000..35d83f524c --- /dev/null +++ b/crates/astria-sequencer/src/address/storage/values.rs @@ -0,0 +1,48 @@ +use std::borrow::Cow; + +use astria_eyre::eyre::bail; +use borsh::{ + BorshDeserialize, + BorshSerialize, +}; + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(crate) struct Value<'a>(ValueImpl<'a>); + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +enum ValueImpl<'a> { + AddressPrefix(AddressPrefix<'a>), +} + +#[derive(Debug, BorshSerialize, BorshDeserialize)] +pub(in crate::address) struct AddressPrefix<'a>(Cow<'a, str>); + +impl<'a> From<&'a str> for AddressPrefix<'a> { + fn from(prefix: &'a str) -> Self { + AddressPrefix(Cow::Borrowed(prefix)) + } +} + +impl<'a> From> for String { + fn from(prefix: AddressPrefix<'a>) -> Self { + prefix.0.into_owned() + } +} + +impl<'a> From> for crate::storage::StoredValue<'a> { + fn from(prefix: AddressPrefix<'a>) -> Self { + crate::storage::StoredValue::Address(Value(ValueImpl::AddressPrefix(prefix))) + } +} + +impl<'a> TryFrom> for AddressPrefix<'a> { + type Error = astria_eyre::eyre::Error; + + fn try_from(value: crate::storage::StoredValue<'a>) -> Result { + let crate::storage::StoredValue::Address(Value(ValueImpl::AddressPrefix(prefix))) = value + else { + bail!("address stored value type mismatch: expected address prefix, found {value:?}"); + }; + Ok(prefix) + } +} diff --git a/crates/astria-sequencer/src/app/action_handler.rs b/crates/astria-sequencer/src/app/action_handler.rs index a24788a7ea..1ef67ea4bb 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 7982c3df06..34a96202f2 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -1,5 +1,8 @@ +mod action_handler; #[cfg(feature = "benchmark")] mod benchmarks; +mod state_ext; +pub(crate) mod storage; #[cfg(any(test, feature = "benchmark"))] pub(crate) mod test_utils; #[cfg(test)] @@ -11,13 +14,11 @@ mod tests_breaking_changes; #[cfg(test)] mod tests_execute_transaction; -mod action_handler; use std::{ collections::VecDeque, sync::Arc, }; -pub(crate) use action_handler::ActionHandler; use astria_core::{ generated::protocol::transactions::v1alpha1 as raw, protocol::{ @@ -74,13 +75,19 @@ use tracing::{ instrument, }; +pub(crate) use self::{ + action_handler::ActionHandler, + state_ext::{ + StateReadExt, + StateWriteExt, + }, +}; use crate::{ accounts::{ component::AccountsComponent, StateWriteExt as _, }, address::StateWriteExt as _, - api_state_ext::StateWriteExt as _, assets::{ StateReadExt as _, StateWriteExt as _, @@ -99,6 +106,7 @@ use crate::{ StateWriteExt as _, }, component::Component as _, + grpc::StateWriteExt as _, ibc::component::IbcComponent, mempool::{ Mempool, @@ -113,10 +121,6 @@ use crate::{ }, }, sequence::component::SequenceComponent, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, transaction::InvalidNonce, }; @@ -231,27 +235,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()) + .wrap_err("failed to write base prefix to state")?; + state_tx + .put_ibc_compat_prefix(genesis_state.address_prefixes().ibc_compat().to_string()) + .wrap_err("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()) + .wrap_err("failed to write native asset to state")?; + state_tx + .put_ibc_asset(native_asset.clone()) .wrap_err("failed to commit native asset as ibc asset to state")?; state_tx - .put_chain_id_and_revision_number(chain_id.try_into().wrap_err("invalid chain ID")?); - state_tx.put_block_height(0); + .put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?) + .wrap_err("failed to write chain id to state")?; + state_tx + .put_block_height(0) + .wrap_err("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) + .wrap_err("failed to write allowed fee asset to state")?; } // call init_chain on all components AccountsComponent::init_chain(&mut state_tx, &genesis_state) .await - .wrap_err("failed to call init_chain on AccountsComponent")?; + .wrap_err("init_chain failed on AccountsComponent")?; AuthorityComponent::init_chain( &mut state_tx, &AuthorityComponentAppState { @@ -260,16 +275,16 @@ impl App { }, ) .await - .wrap_err("failed to call init_chain on AuthorityComponent")?; + .wrap_err("init_chain failed on AuthorityComponent")?; BridgeComponent::init_chain(&mut state_tx, &genesis_state) .await - .wrap_err("failed to call init_chain on BridgeComponent")?; + .wrap_err("init_chain failed on BridgeComponent")?; IbcComponent::init_chain(&mut state_tx, &genesis_state) .await - .wrap_err("failed to call init_chain on IbcComponent")?; + .wrap_err("init_chain failed on IbcComponent")?; SequenceComponent::init_chain(&mut state_tx, &genesis_state) .await - .wrap_err("failed to call init_chain on SequenceComponent")?; + .wrap_err("init_chain failed on SequenceComponent")?; state_tx.apply(); @@ -754,7 +769,7 @@ impl App { self.begin_block(&begin_block) .await - .wrap_err("failed to call begin_block")?; + .wrap_err("begin_block failed")?; Ok(()) } @@ -862,7 +877,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 deposits for this block from state's ephemeral cache and put them to storage. let mut state_tx = StateDelta::new(self.state.clone()); @@ -931,7 +946,9 @@ 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); + state + .put_storage_version_by_height(height, new_version) + .wrap_err("failed to put storage version by height")?; debug!( height, version = new_version, @@ -960,28 +977,30 @@ impl App { ) -> 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); + state_tx + .put_block_height(begin_block.header.height.into()) + .wrap_err("failed to put block height")?; + state_tx + .put_block_timestamp(begin_block.header.time) + .wrap_err("failed to put block timestamp")?; // 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 - .wrap_err("failed to call begin_block on AccountsComponent")?; + .wrap_err("begin_block failed on AccountsComponent")?; AuthorityComponent::begin_block(&mut arc_state_tx, begin_block) .await - .wrap_err("failed to call begin_block on AuthorityComponent")?; + .wrap_err("begin_block failed on AuthorityComponent")?; BridgeComponent::begin_block(&mut arc_state_tx, begin_block) .await - .wrap_err("failed to call begin_block on BridgeComponent")?; + .wrap_err("begin_block failed on BridgeComponent")?; IbcComponent::begin_block(&mut arc_state_tx, begin_block) .await - .wrap_err("failed to call begin_block on IbcComponent")?; + .wrap_err("begin_block failed on IbcComponent")?; SequenceComponent::begin_block(&mut arc_state_tx, begin_block) .await - .wrap_err("failed to call begin_block on SequenceComponent")?; + .wrap_err("begin_block failed on SequenceComponent")?; let state_tx = Arc::try_unwrap(arc_state_tx) .expect("components should not retain copies of shared state"); @@ -1025,7 +1044,7 @@ impl App { async fn end_block( &mut self, height: u64, - fee_recipient: [u8; 20], + fee_recipient: &[u8; 20], ) -> Result { let state_tx = StateDelta::new(self.state.clone()); let mut arc_state_tx = Arc::new(state_tx); @@ -1039,19 +1058,19 @@ impl App { // call end_block on all components AccountsComponent::end_block(&mut arc_state_tx, &end_block) .await - .wrap_err("failed to call end_block on AccountsComponent")?; + .wrap_err("end_block failed on AccountsComponent")?; AuthorityComponent::end_block(&mut arc_state_tx, &end_block) .await - .wrap_err("failed to call end_block on AuthorityComponent")?; + .wrap_err("end_block failed on AuthorityComponent")?; BridgeComponent::end_block(&mut arc_state_tx, &end_block) .await - .wrap_err("failed to call end_block on BridgeComponent")?; + .wrap_err("end_block failed on BridgeComponent")?; IbcComponent::end_block(&mut arc_state_tx, &end_block) .await - .wrap_err("failed to call end_block on IbcComponent")?; + .wrap_err("end_block failed on IbcComponent")?; SequenceComponent::end_block(&mut arc_state_tx, &end_block) .await - .wrap_err("failed to call end_block on SequenceComponent")?; + .wrap_err("end_block failed on SequenceComponent")?; let mut state_tx = Arc::try_unwrap(arc_state_tx) .expect("components should not retain copies of shared state"); @@ -1075,7 +1094,7 @@ impl App { for (asset, amount) in fees { state_tx - .increase_balance(fee_recipient, asset, amount) + .increase_balance(fee_recipient, &asset, amount) .await .wrap_err("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 e02ff29153..ce1e424533 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: 350 expression: app.app_hash.as_bytes() --- [ - 96, - 126, - 64, - 175, - 139, - 186, - 101, - 145, - 115, - 19, - 45, - 255, - 124, - 232, - 128, - 181, - 193, - 25, - 192, - 26, - 185, - 10, - 113, - 193, - 151, - 38, - 1, - 215, - 173, - 183, - 175, - 217 + 152, + 102, + 37, + 231, + 116, + 83, + 54, + 99, + 99, + 61, + 18, + 167, + 133, + 246, + 157, + 153, + 240, + 12, + 160, + 204, + 116, + 164, + 109, + 125, + 163, + 141, + 253, + 161, + 54, + 116, + 76, + 131 ] 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 33551b3453..991e89c41c 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,39 +1,39 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs -assertion_line: 157 +assertion_line: 159 expression: app.app_hash.as_bytes() --- [ - 7, - 224, - 115, - 113, - 195, - 128, - 219, - 248, - 198, - 108, - 251, - 204, - 202, - 182, + 185, + 5, + 77, + 15, + 122, + 4, + 105, + 34, + 166, + 43, + 138, + 143, + 70, + 189, + 121, + 106, 150, - 203, - 69, - 213, - 169, - 101, - 228, - 90, - 61, - 94, - 59, - 180, - 251, - 59, - 119, - 37, - 42, - 216 + 128, + 112, + 142, + 173, + 98, + 148, + 152, + 2, + 3, + 100, + 153, + 98, + 154, + 77, + 199 ] 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 f5f80e8998..30a896d96f 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: 78 expression: app.app_hash.as_bytes() --- [ - 7, - 34, - 84, - 104, - 180, - 74, - 207, - 31, - 198, + 216, 255, + 55, + 240, + 253, + 150, + 85, + 195, + 135, 107, - 249, - 25, - 37, - 103, - 202, - 199, + 242, + 140, + 90, + 79, + 175, + 169, + 15, + 89, 132, - 141, - 201, - 64, - 172, - 26, - 24, - 80, - 182, - 114, - 182, - 189, - 220, - 109, - 211 + 159, + 196, + 65, + 113, + 174, + 207, + 46, + 107, + 126, + 6, + 1, + 239, + 84 ] diff --git a/crates/astria-sequencer/src/state_ext.rs b/crates/astria-sequencer/src/app/state_ext.rs similarity index 75% rename from crates/astria-sequencer/src/state_ext.rs rename to crates/astria-sequencer/src/app/state_ext.rs index 1e5f4c69da..cd51110c84 100644 --- a/crates/astria-sequencer/src/state_ext.rs +++ b/crates/astria-sequencer/src/app/state_ext.rs @@ -2,7 +2,6 @@ use astria_eyre::{ anyhow_to_eyre, eyre::{ bail, - eyre, Result, WrapErr as _, }, @@ -15,7 +14,13 @@ use cnidarium::{ use tendermint::Time; use tracing::instrument; +use super::storage; +use crate::storage::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() @@ -26,18 +31,16 @@ 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 .map_err(anyhow_to_eyre) .wrap_err("failed to read raw chain_id from state")? else { bail!("chain id not found in state"); }; - - Ok(String::from_utf8(bytes) - .wrap_err("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)) + .wrap_err("invalid chain id bytes") } #[instrument(skip_all)] @@ -50,46 +53,39 @@ pub(crate) trait StateReadExt: StateRead { else { bail!("revision number not found in state"); }; - - let bytes = TryInto::<[u8; 8]>::try_into(bytes).map_err(|b| { - eyre!( - "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)) + .wrap_err("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 .map_err(anyhow_to_eyre) .wrap_err("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