feat(sequencer)!: transaction categories on UnsignedTransaction

crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs

@@ -13,7 +13,6 @@ use astria_core::{
     },
     protocol::transaction::v1alpha1::{
         Action,
-        TransactionParams,
         UnsignedTransaction,
     },
 };
@@ -154,13 +153,12 @@ impl Submitter {
         .wrap_err("failed to get nonce from sequencer")?;
         debug!(nonce, "fetched latest nonce");
 
-        let unsigned = UnsignedTransaction {
-            actions,
-            params: TransactionParams::builder()
-                .nonce(nonce)
-                .chain_id(sequencer_chain_id)
-                .build(),
-        };
+        let unsigned = UnsignedTransaction::builder()
+            .actions(actions)
+            .nonce(nonce)
+            .chain_id(sequencer_chain_id)
+            .try_build()
+            .wrap_err("failed to build unsigned transaction")?;
 
         // sign transaction
         let signed = unsigned.into_signed(signer.signing_key());

crates/astria-cli/src/commands/bridge/submit.rs

@@ -7,7 +7,6 @@ use astria_core::{
     crypto::SigningKey,
     protocol::transaction::v1alpha1::{
         Action,
-        TransactionParams,
         UnsignedTransaction,
     },
 };
@@ -129,14 +128,13 @@ async fn submit_transaction(
         .await
         .wrap_err("failed to get nonce")?;
 
-    let tx = UnsignedTransaction {
-        params: TransactionParams::builder()
-            .nonce(nonce_res.nonce)
-            .chain_id(chain_id)
-            .build(),
-        actions,
-    }
-    .into_signed(signing_key);
+    let tx = UnsignedTransaction::builder()
+        .actions(actions)
+        .nonce(nonce_res.nonce)
+        .chain_id(chain_id)
+        .try_build()
+        .wrap_err("failed to build transaction from actions")?
+        .into_signed(signing_key);
     let res = client
         .submit_transaction_sync(tx)
         .await

crates/astria-cli/src/commands/sequencer.rs

@@ -16,7 +16,6 @@ use astria_core::{
             TransferAction,
             ValidatorUpdate,
         },
-        TransactionParams,
         UnsignedTransaction,
     },
 };
@@ -475,14 +474,13 @@ async fn submit_transaction(
         .await
         .wrap_err("failed to get nonce")?;
 
-    let tx = UnsignedTransaction {
-        params: TransactionParams::builder()
-            .nonce(nonce_res.nonce)
-            .chain_id(chain_id)
-            .build(),
-        actions: vec![action],
-    }
-    .into_signed(&sequencer_key);
+    let tx = UnsignedTransaction::builder()
+        .actions(vec![action])
+        .nonce(nonce_res.nonce)
+        .chain_id(chain_id)
+        .try_build()
+        .wrap_err("failed to build transaction from actions")?
+        .into_signed(&sequencer_key);
     let res = sequencer_client
         .submit_transaction_sync(tx)
         .await

crates/astria-composer/src/executor/bundle_factory/mod.rs

@@ -13,6 +13,7 @@ use astria_core::{
     protocol::transaction::v1alpha1::{
         action::SequenceAction,
         Action,
+        UnsignedTransaction,
     },
     Protobuf as _,
 };
@@ -74,6 +75,23 @@ impl SizedBundle {
         }
     }
 
+    /// Constructs an [`UnsignedTransaction`] from the actions contained in the bundle and `params`.
+    // Method is expected to never panic because only `SequenceActions` are added to the bundle,
+    // which should produce a valid variant of the `ActionGroup` type.
+    #[allow(clippy::panic)]
+    pub(super) fn to_unsigned_transaction(
+        &self,
+        nonce: u32,
+        chain_id: &str,
+    ) -> UnsignedTransaction {
+        UnsignedTransaction::builder()
+            .actions(self.buffer.clone())
+            .chain_id(chain_id)
+            .nonce(nonce)
+            .try_build()
+            .unwrap()
+    }
+
     /// Buffer `seq_action` into the bundle.
     /// # Errors
     /// - `seq_action` is beyond the max size allowed for the entire bundle
@@ -111,11 +129,6 @@ impl SizedBundle {
         self.curr_size
     }
 
-    /// Consume self and return the underlying buffer of actions.
-    pub(super) fn into_actions(self) -> Vec<Action> {
-        self.buffer
-    }
-
     /// Returns the number of sequence actions in the bundle.
     pub(super) fn actions_count(&self) -> usize {
         self.buffer.len()

crates/astria-composer/src/executor/bundle_factory/tests.rs

@@ -73,7 +73,7 @@ mod sized_bundle {
         assert!(bundle.buffer.is_empty());
 
         // assert that the flushed bundle has just the sequence action pushed earlier
-        let actions = flushed_bundle.into_actions();
+        let actions = flushed_bundle.buffer;
         assert_eq!(actions.len(), 1);
         let actual_seq_action = actions[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action.rollup_id);
@@ -137,7 +137,7 @@ mod bundle_factory {
         assert_eq!(bundle_factory.finished.len(), 1);
         // assert `pop_finished()` will return `seq_action0`
         let next_actions = bundle_factory.next_finished();
-        let actions = next_actions.unwrap().pop().into_actions();
+        let actions = next_actions.unwrap().pop().buffer;
         let actual_seq_action = actions[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action0.rollup_id);
         assert_eq!(actual_seq_action.data, seq_action0.data);
@@ -236,7 +236,7 @@ mod bundle_factory {
         // assert that the finished queue is empty (curr wasn't flushed)
         assert_eq!(bundle_factory.finished.len(), 0);
         // assert `pop_now()` returns `seq_action`
-        let actions = bundle_factory.pop_now().into_actions();
+        let actions = bundle_factory.pop_now().buffer;
         let actual_seq_action = actions[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action.rollup_id);
         assert_eq!(actual_seq_action.data, seq_action.data);
@@ -261,7 +261,7 @@ mod bundle_factory {
         // assert that the bundle factory has one bundle in the finished queue
         assert_eq!(bundle_factory.finished.len(), 1);
         // assert `pop_now()` will return `seq_action0`
-        let actions = bundle_factory.pop_now().into_actions();
+        let actions = bundle_factory.pop_now().buffer;
         let actual_seq_action = actions[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action0.rollup_id);
         assert_eq!(actual_seq_action.data, seq_action0.data);
@@ -296,7 +296,7 @@ mod bundle_factory {
         assert_eq!(bundle_factory.finished.len(), 1);
 
         // assert `pop_now()` will return `seq_action0` on the first call
-        let actions_finished = bundle_factory.pop_now().into_actions();
+        let actions_finished = bundle_factory.pop_now().buffer;
         assert_eq!(actions_finished.len(), 1);
         let actual_seq_action = actions_finished[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action0.rollup_id);
@@ -306,7 +306,7 @@ mod bundle_factory {
         assert_eq!(bundle_factory.finished.len(), 0);
 
         // assert `pop_now()` will return `seq_action1` on the second call (i.e. from curr)
-        let actions_curr = bundle_factory.pop_now().into_actions();
+        let actions_curr = bundle_factory.pop_now().buffer;
         assert_eq!(actions_curr.len(), 1);
         let actual_seq_action = actions_curr[0].as_sequence().unwrap();
         assert_eq!(actual_seq_action.rollup_id, seq_action1.rollup_id);
@@ -333,4 +333,26 @@ mod bundle_factory {
         assert_eq!(bundle_factory.finished.len(), 0);
         assert!(!bundle_factory.is_full());
     }
+
+    #[test]
+    fn transaction_construction_does_not_panic() {
+        let mut bundle_factory = BundleFactory::new(1000, 10);
+
+        bundle_factory
+            .try_push(sequence_action_with_n_bytes(50))
+            .unwrap();
+        bundle_factory
+            .try_push(sequence_action_with_n_bytes(50))
+            .unwrap();
+        bundle_factory
+            .try_push(sequence_action_with_n_bytes(50))
+            .unwrap();
+
+        let bundle = bundle_factory.pop_now();
+
+        // construction of multiple sequence actions should not panic
+        let unsigned_tx = bundle.to_unsigned_transaction(0, "astria-testnet-1");
+
+        assert_eq!(unsigned_tx.actions().len(), 3);
+    }
 }

crates/astria-composer/src/executor/mod.rs

@@ -17,8 +17,6 @@ use astria_core::{
         transaction::v1alpha1::{
             action::SequenceAction,
             SignedTransaction,
-            TransactionParams,
-            UnsignedTransaction,
         },
     },
 };
@@ -677,15 +675,10 @@ impl Future for SubmitFut {
 
             let new_state = match this.state.project() {
                 SubmitStateProj::NotStarted => {
-                    let params = TransactionParams::builder()
-                        .nonce(*this.nonce)
-                        .chain_id(&*this.chain_id)
-                        .build();
-                    let tx = UnsignedTransaction {
-                        actions: this.bundle.clone().into_actions(),
-                        params,
-                    }
-                    .into_signed(this.signing_key);
+                    let tx = this
+                        .bundle
+                        .to_unsigned_transaction(*this.nonce, &*this.chain_id)
+                        .into_signed(this.signing_key);
                     info!(
                         nonce.actual = *this.nonce,
                         bundle = %telemetry::display::json(&SizedBundleReport(this.bundle)),
@@ -755,15 +748,10 @@ impl Future for SubmitFut {
                 } => match ready!(fut.poll(cx)) {
                     Ok(nonce) => {
                         *this.nonce = nonce;
-                        let params = TransactionParams::builder()
-                            .nonce(*this.nonce)
-                            .chain_id(&*this.chain_id)
-                            .build();
-                        let tx = UnsignedTransaction {
-                            actions: this.bundle.clone().into_actions(),
-                            params,
-                        }
-                        .into_signed(this.signing_key);
+                        let tx = this
+                            .bundle
+                            .to_unsigned_transaction(*this.nonce, &*this.chain_id)
+                            .into_signed(this.signing_key);
                         info!(
                             nonce.resubmission = *this.nonce,
                             bundle = %telemetry::display::json(&SizedBundleReport(this.bundle)),

crates/astria-core/src/protocol/test_utils.rs

@@ -7,18 +7,15 @@ use prost::Message as _;
 
 use super::{
     group_sequence_actions_in_signed_transaction_transactions_by_rollup_id,
-    transaction::v1alpha1::{
-        action::SequenceAction,
-        TransactionParams,
-        UnsignedTransaction,
-    },
+    transaction::v1alpha1::action::SequenceAction,
 };
 use crate::{
     crypto::SigningKey,
     primitive::v1::{
         derive_merkle_tree_from_rollup_txs,
         RollupId,
     },
+    protocol::transaction::v1alpha1::UnsignedTransaction,
     sequencerblock::v1alpha1::{
         block::Deposit,
         SequencerBlock,
@@ -104,16 +101,17 @@ impl ConfigureSequencerBlock {
         let txs = if actions.is_empty() {
             vec![]
         } else {
-            let unsigned_transaction = UnsignedTransaction {
-                actions,
-                params: TransactionParams::builder()
-                    .nonce(1)
-                    .chain_id(chain_id.clone())
-                    .build(),
-            };
+            let unsigned_transaction = UnsignedTransaction::builder()
+                .actions(actions)
+                .chain_id(chain_id.clone())
+                .nonce(1)
+                .try_build()
+                .expect(
+                    "should be able to build unsigned transaction since only sequence actions are \
+                     contained",
+                );
             vec![unsigned_transaction.into_signed(&signing_key)]
         };
-
         let mut deposits_map: HashMap<RollupId, Vec<Deposit>> = HashMap::new();
         for deposit in deposits {
             if let Some(entry) = deposits_map.get_mut(&deposit.rollup_id) {

crates/astria-core/src/protocol/transaction/v1alpha1/action.rs

@@ -153,6 +153,7 @@ impl Protobuf for Action {
     }
 }
 
+// TODO: add unit tests for these methods (https://github.com/astriaorg/astria/issues/1593)
 impl Action {
     #[must_use]
     pub fn as_sequence(&self) -> Option<&SequenceAction> {
@@ -169,6 +170,14 @@ impl Action {
         };
         Some(transfer_action)
     }
+
+    pub fn is_fee_asset_change(&self) -> bool {
+        matches!(self, Self::FeeAssetChange(_))
+    }
+
+    pub fn is_fee_change(&self) -> bool {
+        matches!(self, Self::FeeChange(_))
+    }
 }
 
 impl From<SequenceAction> for Action {
@@ -263,6 +272,33 @@ impl TryFrom<raw::Action> for Action {
     }
 }
 
+// TODO: replace this trait with a Protobuf:FullName implementation.
+// Issue tracked in #1567
+pub(super) trait ActionName {
+    fn name(&self) -> &'static str;
+}
+
+impl ActionName for Action {
+    fn name(&self) -> &'static str {
+        match self {
+            Action::Sequence(_) => "Sequence",
+            Action::Transfer(_) => "Transfer",
+            Action::ValidatorUpdate(_) => "ValidatorUpdate",
+            Action::SudoAddressChange(_) => "SudoAddressChange",
+            Action::Ibc(_) => "Ibc",
+            Action::IbcSudoChange(_) => "IbcSudoChange",
+            Action::Ics20Withdrawal(_) => "Ics20Withdrawal",
+            Action::IbcRelayerChange(_) => "IbcRelayerChange",
+            Action::FeeAssetChange(_) => "FeeAssetChange",
+            Action::InitBridgeAccount(_) => "InitBridgeAccount",
+            Action::BridgeLock(_) => "BridgeLock",
+            Action::BridgeUnlock(_) => "BridgeUnlock",
+            Action::BridgeSudoChange(_) => "BridgeSudoChange",
+            Action::FeeChange(_) => "FeeChange",
+        }
+    }
+}
+
 #[allow(clippy::module_name_repetitions)]
 #[derive(Debug, thiserror::Error)]
 #[error(transparent)]