From f5432228090e794a917b6f0803f3a26dc1609dcc Mon Sep 17 00:00:00 2001 From: Ethan Oroshiba Date: Wed, 11 Sep 2024 10:14:41 -0500 Subject: [PATCH] feat(proto, core, sequencer)!: add traceability to rollup deposits (#1410) ## Summary Added `id_of_source_transaction` and `position_in_source_transaction` to `Deposit`. ## Background Previous deposit events had no information about the deposit source, so rollups could not trace if their deposits had landed without checking the balance. Additionally, this could result in deposit txs having the same hash in the EVM. ## Changes - Created primitive proto type `TransactionId` for identifying source transaction in deposits. - Added `id_of_source_transaction` and `position_in_source_transaction` to `Deposit`. - Included tx id in the ephemeral state via `current_source()` so that it can be accessed by actions. - Added accessors/mutators for deposit index to `StateReadExt` and `StateWriteExt` traits in `transaction`. - Updated `bridge_lock` and `ics20_transfer` actions to populate `Deposit` events with transaction id and index based on the state. ## Testing Added test to ensure `position_in_source_transaction` is incremented correctly. ## Breaking Changelist - App hash changed because of increasing block fees due to longer length of `Deposit`. - Changed SequencerBlock API `Deposit` event, having two new mandatory fields `source_transaction_id` and `source_action_index`. `source_transaction_id` uses a new primitive proto type `TransactionId`. ## Related Issues with https://github.com/astriaorg/astria-geth/pull/42, closes #1402 --------- Co-authored-by: Jordan Oroshiba Co-authored-by: Fraser Hutchison <190532+Fraser999@users.noreply.github.com> Co-authored-by: jesse snyder --- charts/evm-rollup/Chart.yaml | 2 +- charts/evm-rollup/values.yaml | 2 +- charts/evm-stack/Chart.lock | 6 +- charts/evm-stack/Chart.yaml | 4 +- .../src/bridge_withdrawer/startup.rs | 2 +- .../src/bridge_withdrawer/submitter/mod.rs | 4 +- crates/astria-composer/src/executor/mod.rs | 4 +- .../src/generated/astria.primitive.v1.rs | 18 ++++ .../generated/astria.primitive.v1.serde.rs | 91 ++++++++++++++++++ .../astria.sequencerblock.v1alpha1.rs | 9 ++ .../astria.sequencerblock.v1alpha1.serde.rs | 39 ++++++++ crates/astria-core/src/primitive/v1/mod.rs | 94 +++++++++++++++++++ .../src/protocol/transaction/v1alpha1/mod.rs | 9 +- ...alpha1__test__signed_transaction_hash.snap | 39 +------- .../src/sequencerblock/v1alpha1/block.rs | 35 +++++++ .../astria-sequencer/src/accounts/action.rs | 2 +- crates/astria-sequencer/src/api_state_ext.rs | 3 + ...ransaction_with_every_action_snapshot.snap | 64 ++++++------- ..._changes__app_finalize_block_snapshot.snap | 60 ++++++------ crates/astria-sequencer/src/app/tests_app.rs | 6 ++ .../src/app/tests_block_fees.rs | 5 +- .../src/app/tests_breaking_changes.rs | 3 + .../src/app/tests_execute_transaction.rs | 51 +++++++++- .../astria-sequencer/src/authority/action.rs | 11 ++- .../src/bridge/bridge_lock_action.rs | 35 +++---- .../src/bridge/bridge_sudo_change_action.rs | 15 ++- .../src/bridge/bridge_unlock_action.rs | 15 ++- .../src/bridge/init_bridge_account_action.rs | 2 +- crates/astria-sequencer/src/bridge/query.rs | 6 +- .../astria-sequencer/src/bridge/state_ext.rs | 46 ++++++--- .../astria-sequencer/src/fee_asset_change.rs | 2 +- .../src/ibc/ibc_relayer_change.rs | 2 +- .../src/ibc/ics20_transfer.rs | 56 +++++++++-- .../src/ibc/ics20_withdrawal.rs | 2 +- crates/astria-sequencer/src/mempool/mod.rs | 22 ++--- .../src/mempool/transactions_container.rs | 2 +- .../astria-sequencer/src/sequence/action.rs | 2 +- .../src/transaction/checks.rs | 21 +++-- .../astria-sequencer/src/transaction/mod.rs | 13 ++- .../src/transaction/state_ext.rs | 31 ++++-- .../astria/primitive/v1/types.proto | 9 ++ .../sequencerblock/v1alpha1/block.proto | 5 + 42 files changed, 640 insertions(+), 209 deletions(-) diff --git a/charts/evm-rollup/Chart.yaml b/charts/evm-rollup/Chart.yaml index 876e2eaee9..9f99bd19ea 100644 --- a/charts/evm-rollup/Chart.yaml +++ b/charts/evm-rollup/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.26.2 +version: 0.27.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 89659c9973..e4dab7a828 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -10,7 +10,7 @@ images: geth: repo: ghcr.io/astriaorg/astria-geth tag: 0.14.0 - devTag: latest + devTag: pr-42 # update to latest once geth changes(https://github.com/astriaorg/astria-geth/pull/42) merged overrideTag: "" conductor: repo: ghcr.io/astriaorg/conductor diff --git a/charts/evm-stack/Chart.lock b/charts/evm-stack/Chart.lock index e2e386c9d8..0dad49ab4f 100644 --- a/charts/evm-stack/Chart.lock +++ b/charts/evm-stack/Chart.lock @@ -4,7 +4,7 @@ dependencies: version: 0.3.6 - name: evm-rollup repository: file://../evm-rollup - version: 0.26.2 + version: 0.27.0 - name: composer repository: file://../composer version: 0.1.2 @@ -20,5 +20,5 @@ dependencies: - name: blockscout-stack repository: https://blockscout.github.io/helm-charts version: 1.6.2 -digest: sha256:d9eba8331d5b2adadb63118a62342f469a30387a7edb290047ff1bcd35d5d4f8 -generated: "2024-09-05T15:18:03.739771-04:00" +digest: sha256:e2209c2b6aa17c3163bcbd32eea4bdbb0df9b108ff353a0020ff951186bb1350 +generated: "2024-09-04T11:23:10.710044-05:00" diff --git a/charts/evm-stack/Chart.yaml b/charts/evm-stack/Chart.yaml index 517dbcc38d..b832ce6afb 100644 --- a/charts/evm-stack/Chart.yaml +++ b/charts/evm-stack/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.4.3 +version: 0.5.0 dependencies: - name: celestia-node @@ -23,7 +23,7 @@ dependencies: repository: "file://../celestia-node" condition: celestia-node.enabled - name: evm-rollup - version: 0.26.2 + version: 0.27.0 repository: "file://../evm-rollup" - name: composer version: 0.1.2 diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 1d5f353b92..9bbb7dc57d 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -480,7 +480,7 @@ fn rollup_height_from_signed_transaction( .expect("action is already checked to be either BridgeUnlock or Ics20Withdrawal"); info!( - last_batch.tx_hash = %telemetry::display::hex(&signed_transaction.sha256_of_proto_encoding()), + last_batch.transaction_id = %signed_transaction.id(), last_batch.rollup_height = last_batch_rollup_height, "extracted rollup height from last batch of withdrawals", ); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index 620bc55f00..e831043307 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -168,7 +168,7 @@ impl Submitter { // sign transaction let signed = unsigned.into_signed(signer.signing_key()); - debug!(tx_hash = %telemetry::display::hex(&signed.sha256_of_proto_encoding()), "signed transaction"); + debug!(transaction_id = %&signed.id(), "signed transaction"); // submit transaction and handle response let (check_tx, tx_response) = submit_tx( @@ -225,7 +225,7 @@ fn report_exit(reason: eyre::Result<&str>) { skip_all, fields( nonce = tx.nonce(), - transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), + transaction.id = %tx.id(), ), err )] diff --git a/crates/astria-composer/src/executor/mod.rs b/crates/astria-composer/src/executor/mod.rs index a873119d73..0f2533cd79 100644 --- a/crates/astria-composer/src/executor/mod.rs +++ b/crates/astria-composer/src/executor/mod.rs @@ -689,7 +689,7 @@ impl Future for SubmitFut { info!( nonce.actual = *this.nonce, bundle = %telemetry::display::json(&SizedBundleReport(this.bundle)), - transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), + transaction.id = %tx.id(), "submitting transaction to sequencer", ); SubmitState::WaitingForSend { @@ -767,7 +767,7 @@ impl Future for SubmitFut { info!( nonce.resubmission = *this.nonce, bundle = %telemetry::display::json(&SizedBundleReport(this.bundle)), - transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), + transaction.id = %tx.id(), "resubmitting transaction to sequencer with new nonce", ); SubmitState::WaitingForSend { diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index bc390ff4c7..f8b4736ad8 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -98,3 +98,21 @@ impl ::prost::Name for Address { ::prost::alloc::format!("astria.primitive.v1.{}", Self::NAME) } } +/// A `TransactionId` is a unique identifier for a transaction. +/// It contains the hash of the transaction, to be included in +/// rollup deposit events for source tracking. +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct TransactionId { + /// The hash of the transaction which the ID represents. + /// It must be a lower hex-encoded 32-byte hash. + #[prost(string, tag = "1")] + pub inner: ::prost::alloc::string::String, +} +impl ::prost::Name for TransactionId { + const NAME: &'static str = "TransactionId"; + const PACKAGE: &'static str = "astria.primitive.v1"; + fn full_name() -> ::prost::alloc::string::String { + ::prost::alloc::format!("astria.primitive.v1.{}", Self::NAME) + } +} diff --git a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs index 99febc360d..01dedf1d90 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs @@ -432,6 +432,97 @@ impl<'de> serde::Deserialize<'de> for RollupId { deserializer.deserialize_struct("astria.primitive.v1.RollupId", FIELDS, GeneratedVisitor) } } +impl serde::Serialize for TransactionId { + #[allow(deprecated)] + fn serialize(&self, serializer: S) -> std::result::Result + where + S: serde::Serializer, + { + use serde::ser::SerializeStruct; + let mut len = 0; + if !self.inner.is_empty() { + len += 1; + } + let mut struct_ser = serializer.serialize_struct("astria.primitive.v1.TransactionId", len)?; + if !self.inner.is_empty() { + struct_ser.serialize_field("inner", &self.inner)?; + } + struct_ser.end() + } +} +impl<'de> serde::Deserialize<'de> for TransactionId { + #[allow(deprecated)] + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + const FIELDS: &[&str] = &[ + "inner", + ]; + + #[allow(clippy::enum_variant_names)] + enum GeneratedField { + Inner, + } + impl<'de> serde::Deserialize<'de> for GeneratedField { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + struct GeneratedVisitor; + + impl<'de> serde::de::Visitor<'de> for GeneratedVisitor { + type Value = GeneratedField; + + fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(formatter, "expected one of: {:?}", &FIELDS) + } + + #[allow(unused_variables)] + fn visit_str(self, value: &str) -> std::result::Result + where + E: serde::de::Error, + { + match value { + "inner" => Ok(GeneratedField::Inner), + _ => Err(serde::de::Error::unknown_field(value, FIELDS)), + } + } + } + deserializer.deserialize_identifier(GeneratedVisitor) + } + } + struct GeneratedVisitor; + impl<'de> serde::de::Visitor<'de> for GeneratedVisitor { + type Value = TransactionId; + + fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str("struct astria.primitive.v1.TransactionId") + } + + fn visit_map(self, mut map_: V) -> std::result::Result + where + V: serde::de::MapAccess<'de>, + { + let mut inner__ = None; + while let Some(k) = map_.next_key()? { + match k { + GeneratedField::Inner => { + if inner__.is_some() { + return Err(serde::de::Error::duplicate_field("inner")); + } + inner__ = Some(map_.next_value()?); + } + } + } + Ok(TransactionId { + inner: inner__.unwrap_or_default(), + }) + } + } + deserializer.deserialize_struct("astria.primitive.v1.TransactionId", FIELDS, GeneratedVisitor) + } +} impl serde::Serialize for Uint128 { #[allow(deprecated)] fn serialize(&self, serializer: S) -> std::result::Result diff --git a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs index 725abeac05..56385a3ca7 100644 --- a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs @@ -126,6 +126,15 @@ pub struct Deposit { /// will receive the bridged funds #[prost(string, tag = "5")] pub destination_chain_address: ::prost::alloc::string::String, + /// the transaction ID of the source action for the deposit, consisting + /// of the transaction hash. + #[prost(message, optional, tag = "6")] + pub source_transaction_id: ::core::option::Option< + super::super::primitive::v1::TransactionId, + >, + /// index of the deposit's source action within its transaction + #[prost(uint64, tag = "7")] + pub source_action_index: u64, } impl ::prost::Name for Deposit { const NAME: &'static str = "Deposit"; diff --git a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs index f59a624817..2a7168de53 100644 --- a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs @@ -21,6 +21,12 @@ impl serde::Serialize for Deposit { if !self.destination_chain_address.is_empty() { len += 1; } + if self.source_transaction_id.is_some() { + len += 1; + } + if self.source_action_index != 0 { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.sequencerblock.v1alpha1.Deposit", len)?; if let Some(v) = self.bridge_address.as_ref() { struct_ser.serialize_field("bridgeAddress", v)?; @@ -37,6 +43,13 @@ impl serde::Serialize for Deposit { if !self.destination_chain_address.is_empty() { struct_ser.serialize_field("destinationChainAddress", &self.destination_chain_address)?; } + if let Some(v) = self.source_transaction_id.as_ref() { + struct_ser.serialize_field("sourceTransactionId", v)?; + } + if self.source_action_index != 0 { + #[allow(clippy::needless_borrow)] + struct_ser.serialize_field("sourceActionIndex", ToString::to_string(&self.source_action_index).as_str())?; + } struct_ser.end() } } @@ -55,6 +68,10 @@ impl<'de> serde::Deserialize<'de> for Deposit { "asset", "destination_chain_address", "destinationChainAddress", + "source_transaction_id", + "sourceTransactionId", + "source_action_index", + "sourceActionIndex", ]; #[allow(clippy::enum_variant_names)] @@ -64,6 +81,8 @@ impl<'de> serde::Deserialize<'de> for Deposit { Amount, Asset, DestinationChainAddress, + SourceTransactionId, + SourceActionIndex, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -90,6 +109,8 @@ impl<'de> serde::Deserialize<'de> for Deposit { "amount" => Ok(GeneratedField::Amount), "asset" => Ok(GeneratedField::Asset), "destinationChainAddress" | "destination_chain_address" => Ok(GeneratedField::DestinationChainAddress), + "sourceTransactionId" | "source_transaction_id" => Ok(GeneratedField::SourceTransactionId), + "sourceActionIndex" | "source_action_index" => Ok(GeneratedField::SourceActionIndex), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -114,6 +135,8 @@ impl<'de> serde::Deserialize<'de> for Deposit { let mut amount__ = None; let mut asset__ = None; let mut destination_chain_address__ = None; + let mut source_transaction_id__ = None; + let mut source_action_index__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::BridgeAddress => { @@ -146,6 +169,20 @@ impl<'de> serde::Deserialize<'de> for Deposit { } destination_chain_address__ = Some(map_.next_value()?); } + GeneratedField::SourceTransactionId => { + if source_transaction_id__.is_some() { + return Err(serde::de::Error::duplicate_field("sourceTransactionId")); + } + source_transaction_id__ = map_.next_value()?; + } + GeneratedField::SourceActionIndex => { + if source_action_index__.is_some() { + return Err(serde::de::Error::duplicate_field("sourceActionIndex")); + } + source_action_index__ = + Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) + ; + } } } Ok(Deposit { @@ -154,6 +191,8 @@ impl<'de> serde::Deserialize<'de> for Deposit { amount: amount__, asset: asset__.unwrap_or_default(), destination_chain_address: destination_chain_address__.unwrap_or_default(), + source_transaction_id: source_transaction_id__, + source_action_index: source_action_index__.unwrap_or_default(), }) } } diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index e9e5ce2d55..b544fa4241 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -25,6 +25,8 @@ pub const ADDRESS_LEN: usize = 20; pub const ROLLUP_ID_LEN: usize = 32; +pub const TRANSACTION_ID_LEN: usize = 32; + impl Protobuf for merkle::Proof { type Error = merkle::audit::InvalidProof; type Raw = raw::Proof; @@ -661,6 +663,98 @@ where tree } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr( + feature = "serde", + serde(try_from = "raw::TransactionId", into = "raw::TransactionId") +)] +pub struct TransactionId { + inner: [u8; TRANSACTION_ID_LEN], +} + +impl TransactionId { + /// Constructs a new `TransactionId` from a 32-byte array. + #[must_use] + pub const fn new(inner: [u8; TRANSACTION_ID_LEN]) -> Self { + Self { + inner, + } + } + + /// Returns the 32-byte transaction hash. + #[must_use] + pub fn get(self) -> [u8; TRANSACTION_ID_LEN] { + self.inner + } + + #[must_use] + pub fn to_raw(&self) -> raw::TransactionId { + raw::TransactionId { + inner: hex::encode(self.inner), + } + } + + #[must_use] + pub fn into_raw(self) -> raw::TransactionId { + raw::TransactionId { + inner: hex::encode(self.inner), + } + } + + /// Convert from a reference to raw protobuf type to a rust type for a transaction ID. + /// + /// # Errors + /// + /// Returns an error if the transaction ID buffer was not 32 bytes long or if it was not hex + /// encoded. + pub fn try_from_raw_ref(raw: &raw::TransactionId) -> Result { + use hex::FromHex as _; + + let inner = <[u8; TRANSACTION_ID_LEN]>::from_hex(&raw.inner).map_err(|err| { + TransactionIdError(TransactionIdErrorKind::HexDecode { + source: err, + }) + })?; + Ok(Self { + inner, + }) + } +} + +impl From for raw::TransactionId { + fn from(val: TransactionId) -> Self { + val.into_raw() + } +} + +impl TryFrom for TransactionId { + type Error = TransactionIdError; + + fn try_from(value: raw::TransactionId) -> Result { + Self::try_from_raw_ref(&value) + } +} + +impl std::fmt::Display for TransactionId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for byte in self.inner { + write!(f, "{byte:02x}")?; + } + Ok(()) + } +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct TransactionIdError(TransactionIdErrorKind); + +#[derive(Debug, thiserror::Error)] +enum TransactionIdErrorKind { + #[error("error decoding hex string `inner` to bytes")] + HexDecode { source: hex::FromHexError }, +} + #[cfg(test)] mod tests { use super::{ diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index c7569452f8..d534c9c548 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -14,6 +14,7 @@ use crate::{ }, primitive::v1::{ asset, + TransactionId, ADDRESS_LEN, }, Protobuf as _, @@ -88,18 +89,18 @@ impl SignedTransaction { self.verification_key.address_bytes() } - /// Returns the transaction hash. + /// Returns the transaction ID, containing the transaction hash. /// /// The transaction hash is calculated by protobuf-encoding the transaction /// and hashing the resulting bytes with sha256. #[must_use] - pub fn sha256_of_proto_encoding(&self) -> [u8; 32] { + pub fn id(&self) -> TransactionId { use sha2::{ Digest as _, Sha256, }; let bytes = self.to_raw().encode_to_vec(); - Sha256::digest(bytes).into() + TransactionId::new(Sha256::digest(bytes).into()) } #[must_use] @@ -609,7 +610,7 @@ mod test { transaction_bytes: unsigned.to_raw().encode_to_vec().into(), }; - insta::assert_json_snapshot!(tx.sha256_of_proto_encoding()); + insta::assert_json_snapshot!(tx.id()); } #[test] diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap index 2357850edd..e3ec0c3f1a 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap @@ -1,38 +1,7 @@ --- source: crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs -expression: tx.sha256_of_proto_encoding() +expression: tx.id() --- -[ - 69, - 121, - 100, - 127, - 170, - 148, - 126, - 89, - 26, - 55, - 220, - 69, - 192, - 9, - 204, - 228, - 198, - 50, - 224, - 128, - 204, - 76, - 243, - 148, - 32, - 203, - 177, - 19, - 14, - 106, - 67, - 114 -] +{ + "inner": "4579647faa947e591a37dc45c009cce4c632e080cc4cf39420cbb1130e6a4372" +} diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs index a6304c8baa..1be19902a1 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -29,6 +29,8 @@ use crate::{ AddressError, IncorrectRollupIdLength, RollupId, + TransactionId, + TransactionIdError, }, protocol::transaction::v1alpha1::{ action, @@ -1298,6 +1300,11 @@ pub struct Deposit { asset: asset::Denom, // the address on the destination chain (rollup) which to send the bridged funds to destination_chain_address: String, + // the transaction ID of the source action for the deposit, consisting + // of the transaction hash. + source_transaction_id: TransactionId, + // index of the deposit's source action within its transaction + source_action_index: u64, } impl From for crate::generated::sequencerblock::v1alpha1::Deposit { @@ -1314,6 +1321,8 @@ impl Deposit { amount: u128, asset: asset::Denom, destination_chain_address: String, + source_transaction_id: TransactionId, + source_action_index: u64, ) -> Self { Self { bridge_address, @@ -1321,6 +1330,8 @@ impl Deposit { amount, asset, destination_chain_address, + source_transaction_id, + source_action_index, } } @@ -1349,6 +1360,11 @@ impl Deposit { &self.destination_chain_address } + #[must_use] + pub fn source_action_index(&self) -> u64 { + self.source_action_index + } + #[must_use] pub fn into_raw(self) -> raw::Deposit { let Self { @@ -1357,6 +1373,8 @@ impl Deposit { amount, asset, destination_chain_address, + source_transaction_id, + source_action_index, } = self; raw::Deposit { bridge_address: Some(bridge_address.into_raw()), @@ -1364,6 +1382,8 @@ impl Deposit { amount: Some(amount.into()), asset: asset.to_string(), destination_chain_address, + source_transaction_id: Some(source_transaction_id.into_raw()), + source_action_index, } } @@ -1382,6 +1402,8 @@ impl Deposit { amount, asset, destination_chain_address, + source_transaction_id, + source_action_index, } = raw; let Some(bridge_address) = bridge_address else { return Err(DepositError::field_not_set("bridge_address")); @@ -1395,12 +1417,19 @@ impl Deposit { let rollup_id = RollupId::try_from_raw(&rollup_id).map_err(DepositError::incorrect_rollup_id_length)?; let asset = asset.parse().map_err(DepositError::incorrect_asset)?; + let Some(source_transaction_id) = source_transaction_id else { + return Err(DepositError::field_not_set("transaction_id")); + }; + let source_transaction_id = TransactionId::try_from_raw_ref(&source_transaction_id) + .map_err(DepositError::transaction_id_error)?; Ok(Self { bridge_address, rollup_id, amount, asset, destination_chain_address, + source_transaction_id, + source_action_index, }) } } @@ -1427,6 +1456,10 @@ impl DepositError { fn incorrect_asset(source: asset::ParseDenomError) -> Self { Self(DepositErrorKind::IncorrectAsset(source)) } + + fn transaction_id_error(source: TransactionIdError) -> Self { + Self(DepositErrorKind::TransactionIdError(source)) + } } #[derive(Debug, thiserror::Error)] @@ -1439,6 +1472,8 @@ enum DepositErrorKind { IncorrectRollupIdLength(#[source] IncorrectRollupIdLength), #[error("the `asset` field could not be parsed")] IncorrectAsset(#[source] asset::ParseDenomError), + #[error("field `source_transaction_id` was invalid")] + TransactionIdError(#[source] TransactionIdError), } /// A piece of data that is sent to a rollup execution node. diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index 3d665341b7..d722bdd2e7 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -36,7 +36,7 @@ impl ActionHandler for TransferAction { async fn check_and_execute(&self, state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); diff --git a/crates/astria-sequencer/src/api_state_ext.rs b/crates/astria-sequencer/src/api_state_ext.rs index 19bf277150..fd2c836308 100644 --- a/crates/astria-sequencer/src/api_state_ext.rs +++ b/crates/astria-sequencer/src/api_state_ext.rs @@ -384,6 +384,7 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { use astria_core::{ + primitive::v1::TransactionId, protocol::test_utils::ConfigureSequencerBlock, sequencerblock::v1alpha1::block::Deposit, }; @@ -412,6 +413,8 @@ mod test { amount, asset, destination_chain_address, + TransactionId::new([0; 32]), + 0, ); deposits.push(deposit); } 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 5ac3906f41..079d8f9769 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 @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 214, - 64, - 20, - 16, - 84, - 81, - 134, - 100, - 103, - 184, - 143, - 247, - 82, - 140, - 243, - 10, - 136, - 119, - 37, - 146, - 146, - 173, - 172, - 110, - 32, - 132, - 242, - 89, - 45, - 201, - 2, - 56 + 93, + 253, + 249, + 252, + 142, + 29, + 202, + 207, + 210, + 185, + 120, + 122, + 250, + 142, + 18, + 220, + 83, + 74, + 102, + 219, + 26, + 101, + 66, + 27, + 127, + 9, + 11, + 125, + 0, + 180, + 244, + 66 ] 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 f254b8eade..837462ddbd 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 @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 155, - 49, - 194, - 67, - 213, - 224, - 18, - 135, - 75, - 34, - 48, - 234, - 42, - 79, - 85, - 103, - 81, - 248, - 170, - 79, - 53, - 192, - 150, - 62, + 116, + 66, 168, + 111, + 249, + 60, + 151, 147, - 50, - 160, - 183, - 118, - 165, - 140 + 9, + 81, + 251, + 26, + 237, + 253, + 110, + 161, + 81, + 192, + 21, + 102, + 59, + 199, + 44, + 137, + 112, + 250, + 106, + 23, + 86, + 5, + 230, + 228 ] diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index 3974aa67eb..8e5b005b57 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -295,6 +295,7 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); @@ -334,6 +335,8 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { amount, nria().into(), "nootwashere".to_string(), + signed_tx.id(), + starting_index_of_action, ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); let commitments = generate_rollup_datas_commitment(&[signed_tx.clone()], deposits.clone()); @@ -385,6 +388,7 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let asset = nria().clone(); + let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); @@ -424,6 +428,8 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { amount, nria().into(), "nootwashere".to_string(), + signed_tx.id(), + starting_index_of_action, ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); let commitments = generate_rollup_datas_commitment(&[signed_tx.clone()], deposits.clone()); diff --git a/crates/astria-sequencer/src/app/tests_block_fees.rs b/crates/astria-sequencer/src/app/tests_block_fees.rs index 4801e661ea..29c2292c0e 100644 --- a/crates/astria-sequencer/src/app/tests_block_fees.rs +++ b/crates/astria-sequencer/src/app/tests_block_fees.rs @@ -225,6 +225,7 @@ async fn ensure_correct_block_fees_bridge_lock() { let bridge = get_bridge_signing_key(); let bridge_address = astria_address(&bridge.address_bytes()); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + let starting_index_of_action = 0; let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); @@ -259,7 +260,7 @@ async fn ensure_correct_block_fees_bridge_lock() { actions, }; let signed_tx = Arc::new(tx.into_signed(&alice)); - app.execute_transaction(signed_tx).await.unwrap(); + app.execute_transaction(signed_tx.clone()).await.unwrap(); let test_deposit = Deposit::new( bridge_address, @@ -267,6 +268,8 @@ async fn ensure_correct_block_fees_bridge_lock() { 1, nria().into(), rollup_id.to_string(), + signed_tx.id(), + starting_index_of_action, ); let total_block_fees: u128 = app diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 7db4f3acf6..473b4d5eaa 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -84,6 +84,7 @@ async fn app_finalize_block_snapshot() { let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); @@ -126,6 +127,8 @@ async fn app_finalize_block_snapshot() { amount, nria().into(), "nootwashere".to_string(), + signed_tx.id(), + starting_index_of_action, ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); let commitments = generate_rollup_datas_commitment(&[signed_tx.clone()], deposits.clone()); diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 2c60e06f83..7dc838435b 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -692,6 +692,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + let starting_index_of_action = 0; let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); @@ -729,7 +730,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() { .await .unwrap(); - app.execute_transaction(signed_tx).await.unwrap(); + app.execute_transaction(signed_tx.clone()).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); let expected_deposit = Deposit::new( @@ -738,6 +739,8 @@ async fn app_execute_transaction_bridge_lock_action_ok() { amount, nria().into(), "nootwashere".to_string(), + signed_tx.id(), + starting_index_of_action, ); let fee = transfer_fee @@ -1067,3 +1070,49 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { "bridge should've transferred out whole balance" ); } + +#[tokio::test] +async fn app_execute_transaction_action_index_correctly_increments() { + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let mut app = initialize_app(None, vec![]).await; + + let bridge_address = astria_address(&[99; 20]); + let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + let starting_index_of_action = 0; + + let mut state_tx = StateDelta::new(app.state.clone()); + state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); + state_tx + .put_bridge_account_ibc_asset(bridge_address, nria()) + .unwrap(); + app.apply(state_tx); + + let amount = 100; + let action = BridgeLockAction { + to: bridge_address, + amount, + asset: nria().into(), + fee_asset: nria().into(), + destination_chain_address: "nootwashere".to_string(), + }; + let tx = UnsignedTransaction { + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .build(), + actions: vec![action.clone().into(), action.into()], + }; + + let signed_tx = Arc::new(tx.into_signed(&alice)); + app.execute_transaction(signed_tx.clone()).await.unwrap(); + assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); + + let deposits = app.state.get_deposit_events(&rollup_id).await.unwrap(); + assert_eq!(deposits.len(), 2); + assert_eq!(deposits[0].source_action_index(), starting_index_of_action); + assert_eq!( + deposits[1].source_action_index(), + starting_index_of_action + 1 + ); +} diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 24e8ba7d20..1997c30f0a 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -34,7 +34,7 @@ impl ActionHandler for ValidatorUpdate { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); // ensure signer is the valid `sudo` key in state @@ -85,7 +85,7 @@ impl ActionHandler for SudoAddressChangeAction { /// as only that address can change the sudo address async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); state @@ -115,7 +115,7 @@ impl ActionHandler for FeeChangeAction { /// as only that address can change the fee async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); // ensure signer is the valid `sudo` key in state @@ -157,6 +157,7 @@ impl ActionHandler for FeeChangeAction { #[cfg(test)] mod test { + use astria_core::primitive::v1::TransactionId; use cnidarium::StateDelta; use super::*; @@ -178,8 +179,10 @@ mod test { let mut state = StateDelta::new(snapshot); let transfer_fee = 12; - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: [1; 20], + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_sudo_address([1; 20]).unwrap(); diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index 7c9afa822c..aaaa4c4f01 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -40,7 +40,7 @@ impl ActionHandler for BridgeLockAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); state @@ -72,12 +72,23 @@ impl ActionHandler for BridgeLockAction { .await .context("failed to get transfer base fee")?; + let transaction_id = state + .get_transaction_context() + .expect("current source should be set before executing action") + .transaction_id; + let source_action_index = state + .get_transaction_context() + .expect("current source should be set before executing action") + .source_action_index; + let deposit = Deposit::new( self.to, rollup_id, self.amount, self.asset.clone(), self.destination_chain_address.clone(), + transaction_id, + source_action_index, ); let byte_cost_multiplier = state @@ -103,20 +114,6 @@ impl ActionHandler for BridgeLockAction { // to the transfer-action logic. execute_transfer(&transfer_action, from, &mut state).await?; - let rollup_id = state - .get_bridge_account_rollup_id(self.to) - .await - .context("failed to get bridge account rollup id")? - .expect("recipient must be a bridge account; this is a bug in check_stateful"); - - let deposit = Deposit::new( - self.to, - rollup_id, - self.amount, - self.asset.clone(), - self.destination_chain_address.clone(), - ); - // the transfer fee is already deducted in `execute_transfer() above, // so we just deduct the bridge lock byte multiplier fee. // FIXME: similar to what is mentioned there: this should be reworked so that @@ -156,6 +153,7 @@ mod tests { use astria_core::primitive::v1::{ asset, RollupId, + TransactionId, }; use cnidarium::StateDelta; @@ -185,8 +183,11 @@ mod tests { let transfer_fee = 12; let from_address = astria_address(&[2; 20]); - state.put_current_source(TransactionContext { + let transaction_id = TransactionId::new([0; 32]); + state.put_transaction_context(TransactionContext { address_bytes: from_address.bytes(), + transaction_id, + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); @@ -227,6 +228,8 @@ mod tests { 100, asset.clone(), "someaddress".to_string(), + transaction_id, + 0, )) * 2; state .put_account_balance(from_address, &asset, 100 + expected_deposit_fee) diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs index 2dfd2cd16b..a7390df048 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -31,7 +31,7 @@ impl ActionHandler for BridgeSudoChangeAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); state @@ -102,7 +102,10 @@ impl ActionHandler for BridgeSudoChangeAction { #[cfg(test)] mod tests { - use astria_core::primitive::v1::asset; + use astria_core::primitive::v1::{ + asset, + TransactionId, + }; use cnidarium::StateDelta; use super::*; @@ -128,8 +131,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: [1; 20], + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); @@ -164,8 +169,10 @@ mod tests { let mut state = StateDelta::new(snapshot); let sudo_address = astria_address(&[98; 20]); - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: sudo_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); state.put_bridge_sudo_change_base_fee(10); diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index 88bab91a13..505671f2cb 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -47,7 +47,7 @@ impl ActionHandler for BridgeUnlockAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); state @@ -106,6 +106,7 @@ mod tests { primitive::v1::{ asset, RollupId, + TransactionId, }, protocol::transaction::v1alpha1::action::BridgeUnlockAction, }; @@ -138,8 +139,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: [1; 20], + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); @@ -175,8 +178,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: [1; 20], + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); @@ -215,8 +220,10 @@ mod tests { let mut state = StateDelta::new(snapshot); let bridge_address = astria_address(&[1; 20]); - state.put_current_source(TransactionContext { + state.put_transaction_context(TransactionContext { address_bytes: bridge_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, }); state.put_base_prefix(ASTRIA_PREFIX); diff --git a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs index bb1ae84d25..5a9c1575a1 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs @@ -37,7 +37,7 @@ impl ActionHandler for InitBridgeAccountAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); if let Some(withdrawer_address) = &self.withdrawer_address { diff --git a/crates/astria-sequencer/src/bridge/query.rs b/crates/astria-sequencer/src/bridge/query.rs index 9b7050f8ce..25f4fdf6f3 100644 --- a/crates/astria-sequencer/src/bridge/query.rs +++ b/crates/astria-sequencer/src/bridge/query.rs @@ -233,12 +233,12 @@ pub(crate) async fn bridge_account_last_tx_hash_request( }; let resp = match snapshot - .get_last_transaction_hash_for_bridge_account(&address) + .get_last_transaction_id_for_bridge_account(&address) .await { - Ok(Some(tx_hash)) => BridgeAccountLastTxHashResponse { + Ok(Some(tx_id)) => BridgeAccountLastTxHashResponse { height, - tx_hash: Some(tx_hash), + tx_hash: Some(tx_id.get()), }, Ok(None) => BridgeAccountLastTxHashResponse { height, diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index 5da4f9bf8a..846eb5f846 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -15,6 +15,7 @@ use astria_core::{ asset, Address, RollupId, + TransactionId, ADDRESS_LEN, }, sequencerblock::v1alpha1::block::Deposit, @@ -150,7 +151,7 @@ fn bridge_account_withdrawal_event_storage_key( ) } -fn last_transaction_hash_for_bridge_account_storage_key(address: &T) -> Vec { +fn last_transaction_id_for_bridge_account_storage_key(address: &T) -> Vec { format!( "{}/lasttx", BridgeAccountKey { @@ -356,24 +357,23 @@ pub(crate) trait StateReadExt: StateRead + address::StateReadExt { } #[instrument(skip_all)] - async fn get_last_transaction_hash_for_bridge_account( + async fn get_last_transaction_id_for_bridge_account( &self, address: &Address, - ) -> Result> { + ) -> Result> { let Some(tx_hash_bytes) = self - .nonverifiable_get_raw(&last_transaction_hash_for_bridge_account_storage_key( - address, - )) + .nonverifiable_get_raw(&last_transaction_id_for_bridge_account_storage_key(address)) .await .context("failed reading raw last transaction hash for bridge account from state")? else { return Ok(None); }; - let tx_hash = tx_hash_bytes + let tx_hash: [u8; 32] = tx_hash_bytes .try_into() .expect("all transaction hashes stored should be 32 bytes; this is a bug"); - Ok(Some(tx_hash)) + + Ok(Some(TransactionId::new(tx_hash))) } } @@ -538,14 +538,14 @@ pub(crate) trait StateWriteExt: StateWrite { } #[instrument(skip_all)] - fn put_last_transaction_hash_for_bridge_account( + fn put_last_transaction_id_for_bridge_account( &mut self, address: T, - tx_hash: &[u8; 32], + tx_id: &TransactionId, ) { self.nonverifiable_put_raw( - last_transaction_hash_for_bridge_account_storage_key(&address), - tx_hash.to_vec(), + last_transaction_id_for_bridge_account_storage_key(&address), + tx_id.get().to_vec(), ); } } @@ -559,6 +559,7 @@ mod test { asset, Address, RollupId, + TransactionId, }, sequencerblock::v1alpha1::block::Deposit, }; @@ -830,6 +831,7 @@ mod test { } #[tokio::test] + #[allow(clippy::too_many_lines)] // allow: it's a test async fn get_deposit_events() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); @@ -846,6 +848,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 0, ); let mut deposits = vec![deposit.clone()]; @@ -881,6 +885,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 1, ); deposits.append(&mut vec![deposit.clone()]); state @@ -915,6 +921,8 @@ mod test { amount, asset, destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 2, ); let deposits_1 = vec![deposit.clone()]; state @@ -958,6 +966,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 0, ); // write same rollup id twice @@ -980,6 +990,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 1, ); state .put_deposit_event(deposit) @@ -1029,6 +1041,8 @@ mod test { amount, asset, destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 0, ); let deposits = vec![deposit.clone()]; @@ -1084,6 +1098,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 0, ); // write to first @@ -1100,6 +1116,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 1, ); let deposits_1 = vec![deposit.clone()]; @@ -1176,6 +1194,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 0, ); // write to first @@ -1192,6 +1212,8 @@ mod test { amount, asset.clone(), destination_chain_address.to_string(), + TransactionId::new([0; 32]), + 1, ); state .put_deposit_event(deposit) diff --git a/crates/astria-sequencer/src/fee_asset_change.rs b/crates/astria-sequencer/src/fee_asset_change.rs index d496b6581d..e51cd3461f 100644 --- a/crates/astria-sequencer/src/fee_asset_change.rs +++ b/crates/astria-sequencer/src/fee_asset_change.rs @@ -26,7 +26,7 @@ impl ActionHandler for FeeAssetChangeAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); let authority_sudo_address = state diff --git a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs index e454e509ee..81f06b050d 100644 --- a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs +++ b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs @@ -25,7 +25,7 @@ impl ActionHandler for IbcRelayerChangeAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); match self { diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index 7372d284e5..8b294181ee 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -12,6 +12,7 @@ use std::borrow::Cow; use anyhow::{ + anyhow, bail, ensure, Context as _, @@ -75,6 +76,7 @@ use crate::{ }, ibc, ibc::StateReadExt as _, + transaction::StateReadExt as _, }; /// The maximum length of the encoded Ics20 `FungibleTokenPacketData` in bytes. @@ -475,11 +477,9 @@ async fn execute_ics20_transfer( .put_ibc_channel_balance( escrow_channel, &denom_trace, - escrow_balance - .checked_sub(packet_amount) - .ok_or(anyhow::anyhow!( - "insufficient balance in escrow account to transfer tokens" - ))?, + escrow_balance.checked_sub(packet_amount).ok_or(anyhow!( + "insufficient balance in escrow account to transfer tokens" + ))?, ) .context("failed to update escrow account balance in execute_ics20_transfer")?; @@ -698,12 +698,20 @@ async fn execute_deposit( "asset ID is not authorized for transfer to bridge account", ); + let transaction_context = state + .get_transaction_context() + .context("transaction source should be present in state when executing an action")?; + let transaction_id = transaction_context.transaction_id; + let index_of_action = transaction_context.source_action_index; + let deposit = Deposit::new( bridge_address, rollup_id, amount, denom.into(), destination_address, + transaction_id, + index_of_action, ); state .put_deposit_event(deposit) @@ -715,7 +723,10 @@ async fn execute_deposit( #[cfg(test)] mod test { - use astria_core::primitive::v1::RollupId; + use astria_core::primitive::v1::{ + RollupId, + TransactionId, + }; use cnidarium::StateDelta; use denom::TracePrefixed; @@ -731,6 +742,10 @@ mod test { ASTRIA_COMPAT_PREFIX, ASTRIA_PREFIX, }, + transaction::{ + StateWriteExt as _, + TransactionContext, + }, }; #[tokio::test] @@ -839,6 +854,12 @@ mod test { let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); + state_tx.put_transaction_context(TransactionContext { + address_bytes: bridge_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, + }); + state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx .put_bridge_account_ibc_asset(bridge_address, &denom) @@ -1078,6 +1099,12 @@ mod test { .parse::() .unwrap(); + state_tx.put_transaction_context(TransactionContext { + address_bytes: bridge_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, + }); + state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx .put_bridge_account_ibc_asset(bridge_address, &denom) @@ -1124,6 +1151,12 @@ mod test { let denom = "nootasset".parse::().unwrap(); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + state_tx.put_transaction_context(TransactionContext { + address_bytes: bridge_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, + }); + state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); state_tx .put_bridge_account_ibc_asset(bridge_address, &denom) @@ -1178,6 +1211,8 @@ mod test { 100, denom, destination_chain_address, + TransactionId::new([0; 32]), + 0, ); assert_eq!(deposit, &expected_deposit); } @@ -1216,6 +1251,13 @@ mod test { }; let packet_bytes = serde_json::to_vec(&packet).unwrap(); + let transaction_context = TransactionContext { + address_bytes: bridge_address.bytes(), + transaction_id: TransactionId::new([0; 32]), + source_action_index: 0, + }; + state_tx.put_transaction_context(transaction_context); + execute_ics20_transfer( &mut state_tx, &packet_bytes, @@ -1253,6 +1295,8 @@ mod test { * be converted from the compat bech32 to the * standard/non-compat bech32m version before emitting * the deposit event */ + TransactionId::new([0; 32]), + 0, ); assert_eq!(deposit, &expected_deposit); } diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index ee91e52eee..327194185b 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -177,7 +177,7 @@ impl ActionHandler for action::Ics20Withdrawal { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); diff --git a/crates/astria-sequencer/src/mempool/mod.rs b/crates/astria-sequencer/src/mempool/mod.rs index d8162f4952..11016f3da1 100644 --- a/crates/astria-sequencer/src/mempool/mod.rs +++ b/crates/astria-sequencer/src/mempool/mod.rs @@ -231,7 +231,7 @@ impl Mempool { signed_tx: Arc, reason: RemovalReason, ) { - let tx_hash = signed_tx.sha256_of_proto_encoding(); + let tx_hash = signed_tx.id().get(); let address = signed_tx.verification_key().address_bytes(); // Try to remove from pending. @@ -524,33 +524,23 @@ mod test { // assert that all were added to the cometbft removal cache // and the expected reasons were tracked assert!(matches!( - mempool - .check_removed_comet_bft(tx0.sha256_of_proto_encoding()) - .await, + mempool.check_removed_comet_bft(tx0.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool - .check_removed_comet_bft(tx1.sha256_of_proto_encoding()) - .await, + mempool.check_removed_comet_bft(tx1.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool - .check_removed_comet_bft(tx3.sha256_of_proto_encoding()) - .await, + mempool.check_removed_comet_bft(tx3.id().get()).await, Some(RemovalReason::LowerNonceInvalidated) )); assert!(matches!( - mempool - .check_removed_comet_bft(tx4.sha256_of_proto_encoding()) - .await, + mempool.check_removed_comet_bft(tx4.id().get()).await, Some(RemovalReason::FailedPrepareProposal(_)) )); assert!(matches!( - mempool - .check_removed_comet_bft(tx5.sha256_of_proto_encoding()) - .await, + mempool.check_removed_comet_bft(tx5.id().get()).await, Some(RemovalReason::LowerNonceInvalidated) )); } diff --git a/crates/astria-sequencer/src/mempool/transactions_container.rs b/crates/astria-sequencer/src/mempool/transactions_container.rs index 73fccd94fe..f9a2e8f0ec 100644 --- a/crates/astria-sequencer/src/mempool/transactions_container.rs +++ b/crates/astria-sequencer/src/mempool/transactions_container.rs @@ -38,7 +38,7 @@ pub(super) struct TimemarkedTransaction { impl TimemarkedTransaction { pub(super) fn new(signed_tx: Arc) -> Self { Self { - tx_hash: signed_tx.sha256_of_proto_encoding(), + tx_hash: signed_tx.id().get(), address: signed_tx.verification_key().address_bytes(), signed_tx, time_first_seen: Instant::now(), diff --git a/crates/astria-sequencer/src/sequence/action.rs b/crates/astria-sequencer/src/sequence/action.rs index 835386f65e..04b3d8eb67 100644 --- a/crates/astria-sequencer/src/sequence/action.rs +++ b/crates/astria-sequencer/src/sequence/action.rs @@ -37,7 +37,7 @@ impl ActionHandler for SequenceAction { async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state - .get_current_source() + .get_transaction_context() .expect("transaction source must be present in state when executing an action") .address_bytes(); diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index 47e3986bf9..76729f477d 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -8,6 +8,7 @@ use astria_core::{ primitive::v1::{ asset, RollupId, + TransactionId, }, protocol::transaction::v1alpha1::{ action::{ @@ -100,7 +101,7 @@ pub(crate) async fn get_fees_for_transaction( .context("failed to get bridge sudo change fee")?; let mut fees_by_asset = HashMap::new(); - for action in &tx.actions { + for (i, action) in tx.actions.iter().enumerate() { match action { Action::Transfer(act) => { transfer_update_fees(&act.fee_asset, &mut fees_by_asset, transfer_fee); @@ -119,12 +120,15 @@ pub(crate) async fn get_fees_for_transaction( .and_modify(|amt| *amt = amt.saturating_add(init_bridge_account_fee)) .or_insert(init_bridge_account_fee); } - Action::BridgeLock(act) => bridge_lock_update_fees( - act, - &mut fees_by_asset, - transfer_fee, - bridge_lock_byte_cost_multiplier, - ), + Action::BridgeLock(act) => { + bridge_lock_update_fees( + act, + &mut fees_by_asset, + transfer_fee, + bridge_lock_byte_cost_multiplier, + i as u64, + ); + } Action::BridgeUnlock(act) => { bridge_unlock_update_fees(&act.fee_asset, &mut fees_by_asset, transfer_fee); } @@ -261,6 +265,7 @@ fn bridge_lock_update_fees( fees_by_asset: &mut HashMap, transfer_fee: u128, bridge_lock_byte_cost_multiplier: u128, + tx_index_of_action: u64, ) { use astria_core::sequencerblock::v1alpha1::block::Deposit; @@ -272,6 +277,8 @@ fn bridge_lock_update_fees( act.amount, act.asset.clone(), act.destination_chain_address.clone(), + TransactionId::new([0; 32]), + tx_index_of_action, )) .saturating_mul(bridge_lock_byte_cost_multiplier), ); diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index 0d89e68452..fa429d5a16 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -152,7 +152,7 @@ impl ActionHandler for SignedTransaction { // Add the current signed transaction into the ephemeral state in case // downstream actions require access to it. // XXX: This must be deleted at the end of `check_stateful`. - state.put_current_source(self); + let mut transaction_context = state.put_transaction_context(self); // Transactions must match the chain id of the node. let chain_id = state.get_chain_id().await?; @@ -180,9 +180,9 @@ impl ActionHandler for SignedTransaction { .context("failed to check account rollup id")? .is_some() { - state.put_last_transaction_hash_for_bridge_account( + state.put_last_transaction_id_for_bridge_account( self, - &self.sha256_of_proto_encoding(), + &transaction_context.transaction_id, ); } @@ -198,7 +198,10 @@ impl ActionHandler for SignedTransaction { .context("failed updating `from` nonce")?; // FIXME: this should create one span per `check_and_execute` - for action in self.actions() { + for (i, action) in (0..).zip(self.actions().iter()) { + transaction_context.source_action_index = i; + state.put_transaction_context(transaction_context); + match action { Action::Transfer(act) => act .check_and_execute(&mut state) @@ -272,7 +275,7 @@ impl ActionHandler for SignedTransaction { } // XXX: Delete the current transaction data from the ephemeral state. - state.delete_current_source(); + state.delete_current_transaction_context(); Ok(()) } } diff --git a/crates/astria-sequencer/src/transaction/state_ext.rs b/crates/astria-sequencer/src/transaction/state_ext.rs index 4304505b76..b5650807cf 100644 --- a/crates/astria-sequencer/src/transaction/state_ext.rs +++ b/crates/astria-sequencer/src/transaction/state_ext.rs @@ -1,5 +1,8 @@ use astria_core::{ - primitive::v1::ADDRESS_LEN, + primitive::v1::{ + TransactionId, + ADDRESS_LEN, + }, protocol::transaction::v1alpha1::SignedTransaction, }; use cnidarium::{ @@ -7,13 +10,15 @@ use cnidarium::{ StateWrite, }; -fn current_source() -> &'static str { - "transaction/current_source" +fn transaction_context() -> &'static str { + "transaction/context" } -#[derive(Clone)] +#[derive(Clone, Copy)] pub(crate) struct TransactionContext { pub(crate) address_bytes: [u8; ADDRESS_LEN], + pub(crate) transaction_id: TransactionId, + pub(crate) source_action_index: u64, } impl TransactionContext { @@ -26,24 +31,30 @@ impl From<&SignedTransaction> for TransactionContext { fn from(value: &SignedTransaction) -> Self { Self { address_bytes: value.address_bytes(), + transaction_id: value.id(), + source_action_index: 0, } } } pub(crate) trait StateWriteExt: StateWrite { - fn put_current_source(&mut self, transaction: impl Into) { + fn put_transaction_context( + &mut self, + transaction: impl Into, + ) -> TransactionContext { let context: TransactionContext = transaction.into(); - self.object_put(current_source(), context); + self.object_put(transaction_context(), context); + context } - fn delete_current_source(&mut self) { - self.object_delete(current_source()); + fn delete_current_transaction_context(&mut self) { + self.object_delete(transaction_context()); } } pub(crate) trait StateReadExt: StateRead { - fn get_current_source(&self) -> Option { - self.object_get(current_source()) + fn get_transaction_context(&self) -> Option { + self.object_get(transaction_context()) } } diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index c317d315f4..0d22856965 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -55,3 +55,12 @@ message Address { reserved 1; reserved "inner"; } + +// A `TransactionId` is a unique identifier for a transaction. +// It contains the hash of the transaction, to be included in +// rollup deposit events for source tracking. +message TransactionId { + // The hash of the transaction which the ID represents. + // It must be a lower hex-encoded 32-byte hash. + string inner = 1; +} diff --git a/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto b/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto index 73cc05267b..bb53bba086 100644 --- a/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto +++ b/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto @@ -86,6 +86,11 @@ message Deposit { // the address on the destination chain which // will receive the bridged funds string destination_chain_address = 5; + // the transaction ID of the source action for the deposit, consisting + // of the transaction hash. + astria.primitive.v1.TransactionId source_transaction_id = 6; + // index of the deposit's source action within its transaction + uint64 source_action_index = 7; } // `FilteredSequencerBlock` is similar to `SequencerBlock` but with a subset