diff --git a/charts/evm-rollup/Chart.yaml b/charts/evm-rollup/Chart.yaml index 876e2eaee..9f99bd19e 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 89659c997..e4dab7a82 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 e2e386c9d..0dad49ab4 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 517dbcc38..b832ce6af 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 1d5f353b9..9bbb7dc57 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 620bc55f0..e83104330 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 a873119d7..0f2533cd7 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 bc390ff4c..f8b4736ad 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 99febc360..01dedf1d9 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 725abeac0..56385a3ca 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 f59a62481..2a7168de5 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 e9e5ce2d5..b544fa424 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 c7569452f..d534c9c54 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 2357850ed..e3ec0c3f1 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 a6304c8ba..1be19902a 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 3d665341b..d722bdd2e 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 19bf27715..fd2c83630 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 5ac3906f4..079d8f976 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 f254b8ead..837462ddb 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 3974aa67e..8e5b005b5 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 4801e661e..29c2292c0 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 7db4f3acf..473b4d5ea 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 2c60e06f8..7dc838435 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 24e8ba7d2..1997c30f0 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 7c9afa822..aaaa4c4f0 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 2dfd2cd16..a7390df04 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 88bab91a1..505671f2c 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 bb1ae84d2..5a9c1575a 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 9b7050f8c..25f4fdf6f 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 5da4f9bf8..846eb5f84 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 d496b6581..e51cd3461 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 e454e509e..81f06b050 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 7372d284e..8b294181e 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 ee91e52ee..327194185 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 d8162f495..11016f3da 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 73fccd94f..f9a2e8f0e 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 835386f65..04b3d8eb6 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 47e3986bf..76729f477 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 0d89e6845..fa429d5a1 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 4304505b7..b5650807c 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 c317d315f..0d2285696 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 73cc05267..bb53bba08 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