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 952153b7b7..a768873c5e 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/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()); diff --git a/crates/astria-cli/src/commands/bridge/submit.rs b/crates/astria-cli/src/commands/bridge/submit.rs index c332a5a6d1..cd0085b54c 100644 --- a/crates/astria-cli/src/commands/bridge/submit.rs +++ b/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 diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index c8f19bdb65..95c274f534 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/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 diff --git a/crates/astria-composer/src/executor/bundle_factory/mod.rs b/crates/astria-composer/src/executor/bundle_factory/mod.rs index 58a3e5a9b2..f4fb47eaaa 100644 --- a/crates/astria-composer/src/executor/bundle_factory/mod.rs +++ b/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,27 @@ impl SizedBundle { } } + /// Constructs an [`UnsignedTransaction`] from the actions contained in the bundle and `params`. + /// # Panics + /// Method is expected to never panic because only `SequenceActions` are added to the bundle, + /// which should produce a valid variant of the `ActionGroup` type. + 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() + .expect( + "method is expected to never panic because only `SequenceActions` are added to \ + the bundle, which should produce a valid variant of the `ActionGroup` type; this \ + is checked by `tests::transaction_construction_should_not_panic", + ) + } + /// Buffer `seq_action` into the bundle. /// # Errors /// - `seq_action` is beyond the max size allowed for the entire bundle @@ -111,11 +133,6 @@ impl SizedBundle { self.curr_size } - /// Consume self and return the underlying buffer of actions. - pub(super) fn into_actions(self) -> Vec { - self.buffer - } - /// Returns the number of sequence actions in the bundle. pub(super) fn actions_count(&self) -> usize { self.buffer.len() diff --git a/crates/astria-composer/src/executor/bundle_factory/tests.rs b/crates/astria-composer/src/executor/bundle_factory/tests.rs index 4dab9b056a..762934782e 100644 --- a/crates/astria-composer/src/executor/bundle_factory/tests.rs +++ b/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); @@ -240,7 +240,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); @@ -265,7 +265,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); @@ -300,7 +300,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); @@ -310,7 +310,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); @@ -337,4 +337,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); + } } diff --git a/crates/astria-composer/src/executor/mod.rs b/crates/astria-composer/src/executor/mod.rs index 5e88d5fbba..26260cbfaa 100644 --- a/crates/astria-composer/src/executor/mod.rs +++ b/crates/astria-composer/src/executor/mod.rs @@ -17,8 +17,6 @@ use astria_core::{ transaction::v1alpha1::{ action::SequenceAction, SignedTransaction, - TransactionParams, - UnsignedTransaction, }, }, }; @@ -670,7 +668,6 @@ impl Future for SubmitFut { type Output = eyre::Result; // FIXME (https://github.com/astriaorg/astria/issues/1572): This function is too long and should be refactored. - #[expect(clippy::too_many_lines, reason = "this may warrant a refactor")] fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { const INVALID_NONCE: Code = Code::Err(AbciErrorCode::INVALID_NONCE.value()); loop { @@ -678,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)), @@ -756,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)), diff --git a/crates/astria-core/src/protocol/test_utils.rs b/crates/astria-core/src/protocol/test_utils.rs index bc39e94edf..c1edc8e92f 100644 --- a/crates/astria-core/src/protocol/test_utils.rs +++ b/crates/astria-core/src/protocol/test_utils.rs @@ -7,11 +7,7 @@ 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, @@ -19,6 +15,7 @@ use crate::{ derive_merkle_tree_from_rollup_txs, RollupId, }, + protocol::transaction::v1alpha1::UnsignedTransaction, sequencerblock::v1alpha1::{ block::Deposit, SequencerBlock, @@ -107,16 +104,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> = HashMap::new(); for deposit in deposits { if let Some(entry) = deposits_map.get_mut(&deposit.rollup_id) { diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index c6d8caf716..5a4ef5d88b 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/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 for Action { @@ -263,6 +272,33 @@ impl TryFrom 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", + } + } +} + #[expect( clippy::module_name_repetitions, reason = "for parity with the Protobuf spec" diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/mod.rs new file mode 100644 index 0000000000..e39971d956 --- /dev/null +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/mod.rs @@ -0,0 +1,199 @@ +#[cfg(test)] +mod tests; + +use std::fmt::{ + self, + Debug, +}; + +use penumbra_ibc::IbcRelay; + +use super::{ + action::{ + ActionName, + BridgeLockAction, + BridgeSudoChangeAction, + BridgeUnlockAction, + FeeAssetChangeAction, + FeeChangeAction, + IbcRelayerChangeAction, + IbcSudoChangeAction, + Ics20Withdrawal, + InitBridgeAccountAction, + SequenceAction, + SudoAddressChangeAction, + TransferAction, + ValidatorUpdate, + }, + Action, +}; + +trait BelongsToGroup { + const GROUP: ActionGroup; +} + +macro_rules! impl_belong_to_group { + ($(($act:ty, $group:expr)),*$(,)?) => { + $( + impl BelongsToGroup for $act { + const GROUP: ActionGroup = $group; + } + )* + } +} + +impl_belong_to_group!( + (SequenceAction, ActionGroup::BundleableGeneral), + (TransferAction, ActionGroup::BundleableGeneral), + (ValidatorUpdate, ActionGroup::BundleableGeneral), + (SudoAddressChangeAction, ActionGroup::UnbundleableSudo), + (IbcRelayerChangeAction, ActionGroup::BundleableSudo), + (Ics20Withdrawal, ActionGroup::BundleableGeneral), + (InitBridgeAccountAction, ActionGroup::UnbundleableGeneral), + (BridgeLockAction, ActionGroup::BundleableGeneral), + (BridgeUnlockAction, ActionGroup::BundleableGeneral), + (BridgeSudoChangeAction, ActionGroup::UnbundleableGeneral), + (FeeChangeAction, ActionGroup::BundleableSudo), + (FeeAssetChangeAction, ActionGroup::BundleableSudo), + (IbcRelay, ActionGroup::BundleableGeneral), + (IbcSudoChangeAction, ActionGroup::UnbundleableSudo), +); + +impl Action { + const fn group(&self) -> ActionGroup { + match self { + Action::Sequence(_) => SequenceAction::GROUP, + Action::Transfer(_) => TransferAction::GROUP, + Action::ValidatorUpdate(_) => ValidatorUpdate::GROUP, + Action::SudoAddressChange(_) => SudoAddressChangeAction::GROUP, + Action::IbcRelayerChange(_) => IbcRelayerChangeAction::GROUP, + Action::Ics20Withdrawal(_) => Ics20Withdrawal::GROUP, + Action::InitBridgeAccount(_) => InitBridgeAccountAction::GROUP, + Action::BridgeLock(_) => BridgeLockAction::GROUP, + Action::BridgeUnlock(_) => BridgeUnlockAction::GROUP, + Action::BridgeSudoChange(_) => BridgeSudoChangeAction::GROUP, + Action::FeeChange(_) => FeeChangeAction::GROUP, + Action::FeeAssetChange(_) => FeeAssetChangeAction::GROUP, + Action::Ibc(_) => IbcRelay::GROUP, + Action::IbcSudoChange(_) => IbcSudoChangeAction::GROUP, + } + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(super) enum ActionGroup { + BundleableGeneral, + UnbundleableGeneral, + BundleableSudo, + UnbundleableSudo, +} + +impl ActionGroup { + pub(super) fn is_bundleable(self) -> bool { + matches!( + self, + ActionGroup::BundleableGeneral | ActionGroup::BundleableSudo + ) + } + + pub(super) fn is_bundleable_sudo(self) -> bool { + matches!(self, ActionGroup::BundleableSudo) + } +} + +impl fmt::Display for ActionGroup { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ActionGroup::BundleableGeneral => write!(f, "bundleable general"), + ActionGroup::UnbundleableGeneral => write!(f, "unbundleable general"), + ActionGroup::BundleableSudo => write!(f, "bundleable sudo"), + ActionGroup::UnbundleableSudo => write!(f, "unbundleable sudo"), + } + } +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct Error(ErrorKind); + +impl Error { + fn mixed( + original_group: ActionGroup, + additional_group: ActionGroup, + action: &'static str, + ) -> Self { + Self(ErrorKind::Mixed { + original_group, + additional_group, + action, + }) + } + + fn not_bundleable(group: ActionGroup) -> Self { + Self(ErrorKind::NotBundleable { + group, + }) + } +} + +#[derive(Debug, thiserror::Error)] +enum ErrorKind { + #[error( + "input contains mixed `ActionGroup` types. original group: {original_group}, additional \ + group: {additional_group}, triggering action: {action}" + )] + Mixed { + original_group: ActionGroup, + additional_group: ActionGroup, + action: &'static str, + }, + #[error("attempted to create bundle with non bundleable `ActionGroup` type: {group}")] + NotBundleable { group: ActionGroup }, +} + +#[derive(Clone, Debug, Default)] +pub(super) struct Actions { + inner: Vec, +} + +impl Actions { + pub(super) fn actions(&self) -> &[Action] { + &self.inner + } + + #[must_use] + pub(super) fn into_actions(self) -> Vec { + self.inner + } + + pub(super) fn group(&self) -> Option { + self.inner.first().map(super::action::Action::group) + } + + pub(super) fn try_from_list_of_actions(actions: Vec) -> Result { + let mut actions_iter = actions.iter(); + let group = match actions_iter.next() { + Some(action) => action.group(), + None => { + // empty `actions` + return Ok(Self::default()); + } + }; + + // assert size constraints on non-bundleable action groups + if actions.len() > 1 && !group.is_bundleable() { + return Err(Error::not_bundleable(group)); + } + + // assert the rest of the actions have the same group as the first + for action in actions_iter { + if action.group() != group { + return Err(Error::mixed(group, action.group(), action.name())); + } + } + + Ok(Self { + inner: actions, + }) + } +} diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/tests.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/tests.rs new file mode 100644 index 0000000000..6aa2afb353 --- /dev/null +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action_group/tests.rs @@ -0,0 +1,241 @@ +use ibc_types::core::client::Height; + +use crate::{ + crypto::VerificationKey, + primitive::v1::{ + asset::Denom, + Address, + RollupId, + }, + protocol::transaction::v1alpha1::{ + action::{ + Action, + BridgeLockAction, + BridgeSudoChangeAction, + BridgeUnlockAction, + FeeAssetChangeAction, + FeeChange, + FeeChangeAction, + IbcRelayerChangeAction, + IbcSudoChangeAction, + Ics20Withdrawal, + InitBridgeAccountAction, + SequenceAction, + SudoAddressChangeAction, + TransferAction, + ValidatorUpdate, + }, + action_group::{ + ActionGroup, + Actions, + ErrorKind, + }, + }, +}; +const ASTRIA_ADDRESS_PREFIX: &str = "astria"; + +#[test] +fn try_from_list_of_actions_bundleable_general() { + let address: Address<_> = Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + + let asset: Denom = "nria".parse().unwrap(); + let actions = vec![ + Action::Sequence(SequenceAction { + rollup_id: RollupId::from([8; 32]), + data: vec![].into(), + fee_asset: asset.clone(), + }), + Action::Transfer(TransferAction { + to: address, + amount: 100, + asset: asset.clone(), + fee_asset: asset.clone(), + }), + Action::BridgeLock(BridgeLockAction { + to: address, + amount: 100, + asset: asset.clone(), + fee_asset: asset.clone(), + destination_chain_address: String::new(), + }), + Action::BridgeUnlock(BridgeUnlockAction { + to: address, + amount: 100, + fee_asset: asset.clone(), + bridge_address: address, + memo: String::new(), + rollup_block_number: 0, + rollup_withdrawal_event_id: String::new(), + }), + Action::ValidatorUpdate(ValidatorUpdate { + power: 100, + verification_key: VerificationKey::try_from([0; 32]).unwrap(), + }), + Action::Ics20Withdrawal(Ics20Withdrawal { + denom: asset.clone(), + destination_chain_address: String::new(), + return_address: address, + amount: 1_000_000u128, + memo: String::new(), + fee_asset: asset.clone(), + timeout_height: Height::new(1, 1).unwrap(), + timeout_time: 0, + source_channel: "channel-0".parse().unwrap(), + bridge_address: Some(address), + use_compat_address: false, + }), + ]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::BundleableGeneral) + )); +} + +#[test] +fn from_list_of_actions_bundleable_sudo() { + let address: Address<_> = Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + + let asset: Denom = "nria".parse().unwrap(); + let actions = vec![ + Action::FeeChange(FeeChangeAction { + fee_change: FeeChange::TransferBaseFee, + new_value: 100, + }), + Action::FeeAssetChange(FeeAssetChangeAction::Addition(asset)), + Action::IbcRelayerChange(IbcRelayerChangeAction::Addition(address)), + ]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::BundleableSudo) + )); +} + +#[test] +fn from_list_of_actions_unbundleable_sudo() { + let address: Address<_> = Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + + let actions = vec![Action::SudoAddressChange(SudoAddressChangeAction { + new_address: address, + })]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::UnbundleableSudo) + )); + + let actions = vec![Action::IbcSudoChange(IbcSudoChangeAction { + new_address: address, + })]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::UnbundleableSudo) + )); + + let actions = vec![ + Action::SudoAddressChange(SudoAddressChangeAction { + new_address: address, + }), + Action::SudoAddressChange(SudoAddressChangeAction { + new_address: address, + }), + ]; + + let error_kind = Actions::try_from_list_of_actions(actions).unwrap_err().0; + assert!( + matches!(error_kind, ErrorKind::NotBundleable { .. }), + "expected ErrorKind::NotBundleable, got {error_kind:?}" + ); +} + +#[test] +fn from_list_of_actions_unbundleable_general() { + let address: Address<_> = Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + + let asset: Denom = "nria".parse().unwrap(); + + let init_bridge_account_action = InitBridgeAccountAction { + rollup_id: RollupId::from([8; 32]), + asset: asset.clone(), + fee_asset: asset.clone(), + sudo_address: Some(address), + withdrawer_address: Some(address), + }; + + let sudo_bridge_address_change_action = BridgeSudoChangeAction { + new_sudo_address: Some(address), + bridge_address: address, + new_withdrawer_address: Some(address), + fee_asset: asset.clone(), + }; + + let actions = vec![init_bridge_account_action.clone().into()]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::UnbundleableGeneral) + )); + + let actions = vec![sudo_bridge_address_change_action.clone().into()]; + + assert!(matches!( + Actions::try_from_list_of_actions(actions).unwrap().group(), + Some(ActionGroup::UnbundleableGeneral) + )); + + let actions = vec![ + init_bridge_account_action.into(), + sudo_bridge_address_change_action.into(), + ]; + + let error_kind = Actions::try_from_list_of_actions(actions).unwrap_err().0; + assert!( + matches!(error_kind, ErrorKind::NotBundleable { .. }), + "expected ErrorKind::NotBundleable, got {error_kind:?}" + ); +} + +#[test] +fn from_list_of_actions_mixed() { + let address: Address<_> = Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + + let asset: Denom = "nria".parse().unwrap(); + let actions = vec![ + Action::Sequence(SequenceAction { + rollup_id: RollupId::from([8; 32]), + data: vec![].into(), + fee_asset: asset.clone(), + }), + Action::SudoAddressChange(SudoAddressChangeAction { + new_address: address, + }), + ]; + + let error_kind = Actions::try_from_list_of_actions(actions).unwrap_err().0; + assert!( + matches!(error_kind, ErrorKind::Mixed { .. }), + "expected ErrorKind::Mixed, got {error_kind:?}" + ); +} diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index d7d926a6c1..67f86add44 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -1,3 +1,4 @@ +use action_group::Actions; use bytes::Bytes; use prost::{ Message as _, @@ -21,6 +22,7 @@ use crate::{ }; pub mod action; +pub mod action_group; pub use action::Action; #[derive(Debug, thiserror::Error)] @@ -197,7 +199,16 @@ impl SignedTransaction { #[must_use] pub fn actions(&self) -> &[Action] { - &self.transaction.actions + self.transaction.actions.actions() + } + + #[must_use] + pub fn is_bundleable_sudo_action_group(&self) -> bool { + if let Some(group) = self.transaction.actions.group() { + group.is_bundleable_sudo() + } else { + false + } } #[must_use] @@ -227,11 +238,26 @@ impl SignedTransaction { #[derive(Clone, Debug)] pub struct UnsignedTransaction { - pub actions: Vec, - pub params: TransactionParams, + actions: Actions, + params: TransactionParams, } impl UnsignedTransaction { + #[must_use] + pub fn builder() -> UnsignedTransactionBuilder { + UnsignedTransactionBuilder::new() + } + + #[must_use] + pub fn into_actions(self) -> Vec { + self.actions.into_actions() + } + + #[must_use] + pub fn actions(&self) -> &[Action] { + self.actions.actions() + } + #[must_use] pub fn nonce(&self) -> u32 { self.params.nonce @@ -260,7 +286,11 @@ impl UnsignedTransaction { actions, params, } = self; - let actions = actions.into_iter().map(Action::into_raw).collect(); + let actions = actions + .into_actions() + .into_iter() + .map(Action::into_raw) + .collect(); raw::UnsignedTransaction { actions, params: Some(params.into_raw()), @@ -281,7 +311,7 @@ impl UnsignedTransaction { actions, params, } = self; - let actions = actions.iter().map(Action::to_raw).collect(); + let actions = actions.actions().iter().map(Action::to_raw).collect(); let params = params.clone().into_raw(); raw::UnsignedTransaction { actions, @@ -315,10 +345,12 @@ impl UnsignedTransaction { .collect::>() .map_err(UnsignedTransactionError::action)?; - Ok(Self { - actions, - params, - }) + UnsignedTransaction::builder() + .actions(actions) + .chain_id(params.chain_id) + .nonce(params.nonce) + .try_build() + .map_err(UnsignedTransactionError::action_group) } /// Attempt to convert from a protobuf [`pbjson_types::Any`]. @@ -360,6 +392,10 @@ impl UnsignedTransactionError { fn decode_any(inner: prost::DecodeError) -> Self { Self(UnsignedTransactionErrorKind::DecodeAny(inner)) } + + fn action_group(inner: action_group::Error) -> Self { + Self(UnsignedTransactionErrorKind::ActionGroup(inner)) + } } #[derive(Debug, thiserror::Error)] @@ -379,59 +415,69 @@ enum UnsignedTransactionErrorKind { raw::UnsignedTransaction::type_url() )] DecodeAny(#[source] prost::DecodeError), + #[error("`actions` field does not form a valid group of actions")] + ActionGroup(#[source] action_group::Error), } -pub struct TransactionParamsBuilder> { +#[derive(Default)] +pub struct UnsignedTransactionBuilder { nonce: u32, - chain_id: TChainId, + chain_id: String, + actions: Vec, } -impl TransactionParamsBuilder { - fn new() -> Self { +impl UnsignedTransactionBuilder { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + #[must_use] + pub fn actions(self, actions: Vec) -> Self { Self { - nonce: 0, - chain_id: "".into(), + actions, + ..self } } -} -impl TransactionParamsBuilder { - #[must_use = "the transaction params builder must be built to be useful"] - pub fn chain_id<'a, T: Into>>( - self, - chain_id: T, - ) -> TransactionParamsBuilder> { - TransactionParamsBuilder { + #[must_use] + pub fn chain_id>(self, chain_id: T) -> UnsignedTransactionBuilder { + UnsignedTransactionBuilder { chain_id: chain_id.into(), nonce: self.nonce, + actions: self.actions, } } - #[must_use = "the transaction params builder must be built to be useful"] + #[must_use] pub fn nonce(self, nonce: u32) -> Self { Self { nonce, ..self } } -} -impl<'a> TransactionParamsBuilder> { - /// Constructs a [`TransactionParams`] from the configured builder. + /// Constructs a [`UnsignedTransaction`] from the configured builder. /// /// # Errors + /// Returns an error if the actions do not make a valid `ActionGroup`. + /// /// Returns an error if the set chain ID does not contain a chain name that can be turned into /// a bech32 human readable prefix (everything before the first dash i.e. `-`). - #[must_use] - pub fn build(self) -> TransactionParams { + pub fn try_build(self) -> Result { let Self { nonce, chain_id, + actions, } = self; - TransactionParams { - nonce, - chain_id: chain_id.into(), - } + let actions = Actions::try_from_list_of_actions(actions)?; + Ok(UnsignedTransaction { + actions, + params: TransactionParams { + nonce, + chain_id, + }, + }) } } @@ -442,11 +488,6 @@ pub struct TransactionParams { } impl TransactionParams { - #[must_use = "the transaction params builder must be built to be useful"] - pub fn builder() -> TransactionParamsBuilder { - TransactionParamsBuilder::new() - } - #[must_use] pub fn into_raw(self) -> raw::TransactionParams { let Self { @@ -461,16 +502,17 @@ impl TransactionParams { } /// Convert from a raw protobuf [`raw::UnsignedTransaction`]. - /// - /// # Errors - /// See [`TransactionParamsBuilder::try_build`] for errors returned by this method. #[must_use] pub fn from_raw(proto: raw::TransactionParams) -> Self { let raw::TransactionParams { nonce, chain_id, } = proto; - Self::builder().nonce(nonce).chain_id(chain_id).build() + + Self { + nonce, + chain_id, + } } } @@ -555,10 +597,7 @@ enum TransactionFeeResponseErrorKind { mod tests { use super::*; use crate::{ - primitive::v1::{ - asset, - Address, - }, + primitive::v1::Address, protocol::transaction::v1alpha1::action::TransferAction, }; const ASTRIA_ADDRESS_PREFIX: &str = "astria"; @@ -592,14 +631,12 @@ mod tests { fee_asset: asset(), }; - let params = TransactionParams::from_raw(raw::TransactionParams { - nonce: 1, - chain_id: "test-1".to_string(), - }); - let unsigned = UnsignedTransaction { - actions: vec![transfer.into()], - params, - }; + let unsigned = UnsignedTransaction::builder() + .actions(vec![transfer.into()]) + .chain_id("test-1".to_string()) + .nonce(1) + .try_build() + .unwrap(); let tx = SignedTransaction { signature, @@ -629,16 +666,14 @@ mod tests { fee_asset: asset(), }; - let params = TransactionParams::from_raw(raw::TransactionParams { - nonce: 1, - chain_id: "test-1".to_string(), - }); - let unsigned = UnsignedTransaction { - actions: vec![transfer.into()], - params, - }; + let unsigned_tx = UnsignedTransaction::builder() + .actions(vec![transfer.into()]) + .chain_id("test-1".to_string()) + .nonce(1) + .try_build() + .unwrap(); - let signed_tx = unsigned.into_signed(&signing_key); + let signed_tx = unsigned_tx.into_signed(&signing_key); let raw = signed_tx.to_raw(); // `try_from_raw` verifies the signature diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs index 8dee1fdd80..6df6428e0e 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -737,7 +737,7 @@ impl SequencerBlock { .map_err(SequencerBlockError::signed_transaction_protobuf_decode)?; let signed_tx = SignedTransaction::try_from_raw(raw_tx) .map_err(SequencerBlockError::raw_signed_transaction_conversion)?; - for action in signed_tx.into_unsigned().actions { + for action in signed_tx.into_unsigned().into_actions() { // XXX: The fee asset is dropped. We shjould explain why that's ok. if let action::Action::Sequence(action::SequenceAction { rollup_id, diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index 5bfbc89f00..c23f14b1d1 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -7,7 +7,6 @@ use astria_core::{ protocol::transaction::v1alpha1::{ action::TransferAction, SignedTransaction, - TransactionParams, UnsignedTransaction, }, }; @@ -157,14 +156,13 @@ fn create_signed_transaction() -> SignedTransaction { } .into(), ]; - UnsignedTransaction { - params: TransactionParams::builder() - .nonce(1) - .chain_id("test") - .build(), - actions, - } - .into_signed(&alice_key) + UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .nonce(1) + .try_build() + .unwrap() + .into_signed(&alice_key) } #[tokio::test] diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 921f0fe9ac..7982c3df06 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -530,7 +530,7 @@ impl App { // check if tx's sequence data will fit into sequence block let tx_sequence_data_bytes = tx .unsigned_transaction() - .actions + .actions() .iter() .filter_map(Action::as_sequence) .fold(0usize, |acc, seq| acc.saturating_add(seq.data.len())); @@ -650,7 +650,7 @@ impl App { // check if tx's sequence data will fit into sequence block let tx_sequence_data_bytes = tx .unsigned_transaction() - .actions + .actions() .iter() .filter_map(Action::as_sequence) .fold(0usize, |acc, seq| acc.saturating_add(seq.data.len())); @@ -1012,10 +1012,11 @@ impl App { // flag mempool for cleaning if we ran a fee change action self.recost_mempool = self.recost_mempool - || signed_tx - .actions() - .iter() - .any(|action| matches!(action, Action::FeeAssetChange(_) | Action::FeeChange(_))); + || signed_tx.is_bundleable_sudo_action_group() + && signed_tx + .actions() + .iter() + .any(|act| act.is_fee_asset_change() || act.is_fee_change()); Ok(state_tx.apply().1) } 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 52c47f2f6a..e02ff29153 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap @@ -1,39 +1,38 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs -assertion_line: 308 expression: app.app_hash.as_bytes() --- [ - 67, - 124, - 63, - 240, - 228, - 207, - 78, - 64, - 191, - 89, - 84, - 121, - 150, - 21, - 207, - 248, - 173, - 132, - 77, + 96, 126, - 148, - 252, - 239, - 104, - 130, - 55, - 201, - 32, - 57, - 167, + 64, + 175, + 139, + 186, + 101, + 145, + 115, + 19, + 45, + 255, + 124, + 232, + 128, + 181, + 193, + 25, + 192, + 26, + 185, + 10, + 113, + 193, + 151, + 38, + 1, 215, - 228 + 173, + 183, + 175, + 217 ] diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index be68c81fc3..508c6003c6 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -24,7 +24,6 @@ use astria_core::{ ValidatorUpdate, }, SignedTransaction, - TransactionParams, UnsignedTransaction, }, }, @@ -263,20 +262,19 @@ pub(crate) fn mock_tx( signer: &SigningKey, rollup_name: &str, ) -> Arc { - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(nonce) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(rollup_name.as_bytes()), data: Bytes::from_static(&[0x99]), fee_asset: denom_0(), } .into(), - ], - }; + ]) + .chain_id("test") + .nonce(nonce) + .try_build() + .unwrap(); Arc::new(tx.into_signed(signer)) } diff --git a/crates/astria-sequencer/src/app/tests_app/mempool.rs b/crates/astria-sequencer/src/app/tests_app/mempool.rs index ec325f6e28..d90f969a0b 100644 --- a/crates/astria-sequencer/src/app/tests_app/mempool.rs +++ b/crates/astria-sequencer/src/app/tests_app/mempool.rs @@ -9,7 +9,6 @@ use astria_core::{ FeeChangeAction, TransferAction, }, - TransactionParams, UnsignedTransaction, }, }, @@ -47,23 +46,20 @@ async fn trigger_cleaning() { let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; app.prepare_commit(storage.clone()).await.unwrap(); app.commit(storage.clone()).await; - let sudo = get_judy_signing_key(); // create tx which will cause mempool cleaning flag to be set - let tx_trigger = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_trigger = UnsignedTransaction::builder() + .actions(vec![ FeeChangeAction { fee_change: FeeChange::TransferBaseFee, new_value: 10, } .into(), - ], - } - .into_signed(&sudo); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_judy_signing_key()); app.mempool .insert( @@ -146,24 +142,20 @@ async fn do_not_trigger_cleaning() { app.prepare_commit(storage.clone()).await.unwrap(); app.commit(storage.clone()).await; - let alice = get_alice_signing_key(); - // create tx which will fail execution and not trigger flag // (wrong sudo signer) - let tx_fail = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_fail = UnsignedTransaction::builder() + .actions(vec![ FeeChangeAction { fee_change: FeeChange::TransferBaseFee, new_value: 10, } .into(), - ], - } - .into_signed(&alice); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_alice_signing_key()); app.mempool .insert( @@ -223,12 +215,8 @@ async fn maintenance_recosting_promotes() { // create tx which will not be included in block due to // having insufficient funds (transaction will be recosted to enable) - let tx_fail_recost_funds = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_fail_recost_funds = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: astria_address_from_hex_string(CAROL_ADDRESS), amount: 1u128, @@ -236,9 +224,11 @@ async fn maintenance_recosting_promotes() { fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&get_bob_signing_key()); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_bob_signing_key()); let mut bob_funds = HashMap::new(); bob_funds.insert(nria().into(), 11); @@ -255,20 +245,18 @@ async fn maintenance_recosting_promotes() { .unwrap(); // create tx which will enable recost tx to pass - let tx_recost = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_recost = UnsignedTransaction::builder() + .actions(vec![ FeeChangeAction { fee_change: FeeChange::TransferBaseFee, new_value: 10, // originally 12 } .into(), - ], - } - .into_signed(&get_judy_signing_key()); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_judy_signing_key()); let mut judy_funds = HashMap::new(); judy_funds.insert(nria().into(), 0); @@ -396,12 +384,8 @@ async fn maintenance_funds_added_promotes() { // create tx that will not be included in block due to // having no funds (will be sent transfer to then enable) - let tx_fail_transfer_funds = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_fail_transfer_funds = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: astria_address_from_hex_string(BOB_ADDRESS), amount: 10u128, @@ -409,9 +393,11 @@ async fn maintenance_funds_added_promotes() { fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&get_carol_signing_key()); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_carol_signing_key()); let mut carol_funds = HashMap::new(); carol_funds.insert(nria().into(), 0); @@ -428,12 +414,8 @@ async fn maintenance_funds_added_promotes() { .unwrap(); // create tx which will enable no funds to pass - let tx_fund = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_fund = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: astria_address_from_hex_string(CAROL_ADDRESS), amount: 22u128, @@ -441,9 +423,11 @@ async fn maintenance_funds_added_promotes() { fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&get_alice_signing_key()); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&get_alice_signing_key()); let mut alice_funds = HashMap::new(); alice_funds.insert(nria().into(), 100); diff --git a/crates/astria-sequencer/src/app/tests_app/mod.rs b/crates/astria-sequencer/src/app/tests_app/mod.rs index f62c655bf9..7967e5a0d8 100644 --- a/crates/astria-sequencer/src/app/tests_app/mod.rs +++ b/crates/astria-sequencer/src/app/tests_app/mod.rs @@ -16,7 +16,6 @@ use astria_core::{ SequenceAction, TransferAction, }, - TransactionParams, UnsignedTransaction, }, }, @@ -229,12 +228,8 @@ async fn app_transfer_block_fees_to_sudo() { // transfer funds from Alice to Bob; use native token for fee payment let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let amount = 333_333; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob_address, amount, @@ -242,8 +237,10 @@ async fn app_transfer_block_fees_to_sudo() { fee_asset: nria().into(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); @@ -336,13 +333,12 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { data: Bytes::from_static(b"hello world"), fee_asset: nria().into(), }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![lock_action.into(), sequence_action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![lock_action.into(), sequence_action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); @@ -427,13 +423,12 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { data: Bytes::from_static(b"hello world"), fee_asset: nria().into(), }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![lock_action.into(), sequence_action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![lock_action.into(), sequence_action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); @@ -564,36 +559,34 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { // create txs which will cause cometBFT overflow let alice = get_alice_signing_key(); - let tx_pass = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_pass = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: Bytes::copy_from_slice(&[1u8; 100_000]), fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&alice); - let tx_overflow = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(1) - .chain_id("test") - .build(), - actions: vec![ + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&alice); + + let tx_overflow = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: Bytes::copy_from_slice(&[1u8; 100_000]), fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&alice); + ]) + .chain_id("test") + .nonce(1) + .try_build() + .unwrap() + .into_signed(&alice); app.mempool .insert( @@ -656,36 +649,33 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { // create txs which will cause sequencer overflow (max is currently 256_000 bytes) let alice = get_alice_signing_key(); - let tx_pass = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_pass = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: Bytes::copy_from_slice(&[1u8; 200_000]), fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&alice); - let tx_overflow = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(1) - .chain_id("test") - .build(), - actions: vec![ + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&alice); + let tx_overflow = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: Bytes::copy_from_slice(&[1u8; 100_000]), fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&alice); + ]) + .nonce(1) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&alice); app.mempool .insert( diff --git a/crates/astria-sequencer/src/app/tests_block_fees.rs b/crates/astria-sequencer/src/app/tests_block_fees.rs index 63f6b92f3c..14efea59be 100644 --- a/crates/astria-sequencer/src/app/tests_block_fees.rs +++ b/crates/astria-sequencer/src/app/tests_block_fees.rs @@ -10,7 +10,6 @@ use astria_core::{ SequenceAction, TransferAction, }, - TransactionParams, UnsignedTransaction, }, sequencerblock::v1alpha1::block::Deposit, @@ -53,12 +52,8 @@ async fn transaction_execution_records_fee_event() { let alice = get_alice_signing_key(); let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let value = 333_333; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob_address, amount: value, @@ -66,9 +61,10 @@ async fn transaction_execution_records_fee_event() { fee_asset: nria().into(), } .into(), - ], - }; - + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let events = app.execute_transaction(signed_tx).await.unwrap(); @@ -114,13 +110,11 @@ async fn ensure_correct_block_fees_transfer() { .into(), ]; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions, - }; + let tx = UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -155,13 +149,11 @@ async fn ensure_correct_block_fees_sequence() { .into(), ]; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions, - }; + let tx = UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -198,13 +190,11 @@ async fn ensure_correct_block_fees_init_bridge_acct() { .into(), ]; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions, - }; + let tx = UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -252,13 +242,11 @@ async fn ensure_correct_block_fees_bridge_lock() { .into(), ]; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions, - }; + let tx = UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx.clone()).await.unwrap(); @@ -314,13 +302,11 @@ async fn ensure_correct_block_fees_bridge_sudo_change() { .into(), ]; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions, - }; + let tx = UnsignedTransaction::builder() + .actions(actions) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 4fd6225973..ef578114ad 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -30,7 +30,6 @@ use astria_core::{ ValidatorUpdate, }, Action, - TransactionParams, UnsignedTransaction, }, }, @@ -112,13 +111,12 @@ async fn app_finalize_block_snapshot() { data: Bytes::from_static(b"hello world"), fee_asset: nria().into(), }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![lock_action.into(), sequence_action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![lock_action.into(), sequence_action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); @@ -202,12 +200,8 @@ async fn app_execute_transaction_with_every_action_snapshot() { let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx_bundleable_general = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob_address, amount: 333_333, @@ -222,33 +216,67 @@ async fn app_execute_transaction_with_every_action_snapshot() { } .into(), Action::ValidatorUpdate(update.clone()), + ]) + .chain_id("test") + .try_build() + .unwrap(); + + let tx_bundleable_sudo = UnsignedTransaction::builder() + .actions(vec![ IbcRelayerChangeAction::Addition(bob_address).into(), IbcRelayerChangeAction::Addition(carol_address).into(), IbcRelayerChangeAction::Removal(bob_address).into(), - // TODO: should fee assets be stored in state? FeeAssetChangeAction::Addition("test-0".parse().unwrap()).into(), FeeAssetChangeAction::Addition("test-1".parse().unwrap()).into(), FeeAssetChangeAction::Removal("test-0".parse().unwrap()).into(), + ]) + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(); + + let tx_sudo_ibc = UnsignedTransaction::builder() + .actions(vec![ IbcSudoChangeAction { new_address: bob_address, } .into(), + ]) + .nonce(2) + .chain_id("test") + .try_build() + .unwrap(); + + let tx_sudo = UnsignedTransaction::builder() + .actions(vec![ SudoAddressChangeAction { new_address: bob_address, } .into(), - ], - }; + ]) + .nonce(3) + .chain_id("test") + .try_build() + .unwrap(); - let signed_tx = Arc::new(tx.into_signed(&alice)); - app.execute_transaction(signed_tx).await.unwrap(); + let signed_tx_general_bundleable = Arc::new(tx_bundleable_general.into_signed(&alice)); + app.execute_transaction(signed_tx_general_bundleable) + .await + .unwrap(); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let signed_tx_sudo_bundleable = Arc::new(tx_bundleable_sudo.into_signed(&alice)); + app.execute_transaction(signed_tx_sudo_bundleable) + .await + .unwrap(); + + let signed_tx_sudo_ibc = Arc::new(tx_sudo_ibc.into_signed(&alice)); + app.execute_transaction(signed_tx_sudo_ibc).await.unwrap(); + + let signed_tx_sudo = Arc::new(tx_sudo.into_signed(&alice)); + app.execute_transaction(signed_tx_sudo).await.unwrap(); + + let tx = UnsignedTransaction::builder() + .actions(vec![ InitBridgeAccountAction { rollup_id, asset: nria().into(), @@ -257,17 +285,15 @@ async fn app_execute_transaction_with_every_action_snapshot() { withdrawer_address: None, } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&bridge)); app.execute_transaction(signed_tx).await.unwrap(); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .chain_id("test") - .nonce(1) - .build(), - actions: vec![ + let tx_bridge_bundleable = UnsignedTransaction::builder() + .actions(vec![ BridgeLockAction { to: bridge_address, amount: 100, @@ -286,6 +312,17 @@ async fn app_execute_transaction_with_every_action_snapshot() { rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), } .into(), + ]) + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(); + + let signed_tx = Arc::new(tx_bridge_bundleable.into_signed(&bridge)); + app.execute_transaction(signed_tx).await.unwrap(); + + let tx_bridge = UnsignedTransaction::builder() + .actions(vec![ BridgeSudoChangeAction { bridge_address, new_sudo_address: Some(bob_address), @@ -293,10 +330,13 @@ async fn app_execute_transaction_with_every_action_snapshot() { fee_asset: nria().into(), } .into(), - ], - }; + ]) + .nonce(2) + .chain_id("test") + .try_build() + .unwrap(); - let signed_tx = Arc::new(tx.into_signed(&bridge)); + let signed_tx = Arc::new(tx_bridge.into_signed(&bridge)); app.execute_transaction(signed_tx).await.unwrap(); let sudo_address = app.state.get_sudo_address().await.unwrap(); diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 30a6290ecf..6dd088face 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -20,7 +20,6 @@ use astria_core::{ ValidatorUpdate, }, Action, - TransactionParams, UnsignedTransaction, }, }, @@ -104,12 +103,8 @@ async fn app_execute_transaction_transfer() { let alice_address = astria_address(&alice.address_bytes()); let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let value = 333_333; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob_address, amount: value, @@ -117,8 +112,10 @@ async fn app_execute_transaction_transfer() { fee_asset: crate::test_utils::nria().into(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -161,12 +158,8 @@ async fn app_execute_transaction_transfer_not_native_token() { // transfer funds from Alice to Bob; use native token for fee payment let bob_address = astria_address_from_hex_string(BOB_ADDRESS); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob_address, amount: value, @@ -174,8 +167,10 @@ async fn app_execute_transaction_transfer_not_native_token() { fee_asset: nria().into(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -226,12 +221,8 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { let bob = astria_address_from_hex_string(BOB_ADDRESS); // 0-value transfer; only fee is deducted from sender - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: bob, amount: 0, @@ -239,8 +230,10 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { fee_asset: nria().into(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&keypair)); let res = app @@ -267,20 +260,18 @@ async fn app_execute_transaction_sequence() { let data = Bytes::from_static(b"hello world"); let fee = calculate_fee_from_state(&data, &app.state).await.unwrap(); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, fee_asset: nria().into(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -302,20 +293,18 @@ async fn app_execute_transaction_invalid_fee_asset() { let alice = get_alice_signing_key(); let data = Bytes::from_static(b"hello world"); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, fee_asset: test_asset(), } .into(), - ], - }; + ]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); @@ -333,13 +322,11 @@ async fn app_execute_transaction_validator_update() { verification_key: crate::test_utils::verification_key(1), }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::ValidatorUpdate(update.clone())], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![Action::ValidatorUpdate(update.clone())]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -360,13 +347,13 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![IbcRelayerChangeAction::Addition(alice_address).into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![Action::IbcRelayerChange( + IbcRelayerChangeAction::Addition(alice_address), + )]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -388,14 +375,11 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { .unwrap(); let mut app = initialize_app(Some(genesis_state), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], - }; - + let tx = UnsignedTransaction::builder() + .actions(vec![IbcRelayerChangeAction::Removal(alice_address).into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); @@ -418,13 +402,11 @@ async fn app_execute_transaction_ibc_relayer_change_invalid() { .unwrap(); let mut app = initialize_app(Some(genesis_state), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![IbcRelayerChangeAction::Removal(alice_address).into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); @@ -438,16 +420,13 @@ async fn app_execute_transaction_sudo_address_change() { let mut app = initialize_app(Some(genesis_state()), vec![]).await; let new_address = astria_address_from_hex_string(BOB_ADDRESS); - - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::SudoAddressChange(SudoAddressChangeAction { + let tx = UnsignedTransaction::builder() + .actions(vec![Action::SudoAddressChange(SudoAddressChangeAction { new_address, - })], - }; + })]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -476,16 +455,13 @@ async fn app_execute_transaction_sudo_address_change_error() { .try_into() .unwrap(); let mut app = initialize_app(Some(genesis_state), vec![]).await; - - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::SudoAddressChange(SudoAddressChangeAction { + let tx = UnsignedTransaction::builder() + .actions(vec![Action::SudoAddressChange(SudoAddressChangeAction { new_address: alice_address, - })], - }; + })]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let res = app @@ -506,15 +482,13 @@ async fn app_execute_transaction_fee_asset_change_addition() { let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Addition( - test_asset(), - ))], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![Action::FeeAssetChange( + FeeAssetChangeAction::Addition(test_asset()), + )]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -539,15 +513,13 @@ async fn app_execute_transaction_fee_asset_change_removal() { .unwrap(); let mut app = initialize_app(Some(genesis_state), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( + let tx = UnsignedTransaction::builder() + .actions(vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( test_asset(), - ))], - }; + ))]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -564,15 +536,13 @@ async fn app_execute_transaction_fee_asset_change_invalid() { let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( + let tx = UnsignedTransaction::builder() + .actions(vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( nria().into(), - ))], - }; + ))]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let res = app @@ -605,13 +575,12 @@ async fn app_execute_transaction_init_bridge_account_ok() { sudo_address: None, withdrawer_address: None, }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); @@ -661,14 +630,11 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( sudo_address: None, withdrawer_address: None, }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - - actions: vec![action.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -680,13 +646,12 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( sudo_address: None, withdrawer_address: None, }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); @@ -717,13 +682,11 @@ async fn app_execute_transaction_bridge_lock_action_ok() { 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.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); @@ -797,13 +760,11 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { 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.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); @@ -818,20 +779,20 @@ async fn app_execute_transaction_invalid_nonce() { // create tx with invalid nonce 1 let data = Bytes::from_static(b"hello world"); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(1) - .chain_id("test") - .build(), - actions: vec![ + + let tx = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, fee_asset: nria().into(), } .into(), - ], - }; + ]) + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let response = app.execute_transaction(signed_tx).await; @@ -865,21 +826,18 @@ async fn app_execute_transaction_invalid_chain_id() { // create tx with invalid nonce 1 let data = Bytes::from_static(b"hello world"); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("wrong-chain") - .build(), - actions: vec![ + let tx = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, fee_asset: nria().into(), } .into(), - ], - }; - + ]) + .chain_id("wrong-chain") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let response = app.execute_transaction(signed_tx).await; @@ -922,12 +880,8 @@ async fn app_stateful_check_fails_insufficient_total_balance() { .unwrap(); // transfer just enough to cover single sequence fee with data - let signed_tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let signed_tx = UnsignedTransaction::builder() + .actions(vec![ TransferAction { to: keypair_address, amount: fee, @@ -935,20 +889,16 @@ async fn app_stateful_check_fails_insufficient_total_balance() { fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&alice); - - // make transfer + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&alice); app.execute_transaction(Arc::new(signed_tx)).await.unwrap(); // build double transfer exceeding balance - let signed_tx_fail = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let signed_tx_fail = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: data.clone(), @@ -961,10 +911,11 @@ async fn app_stateful_check_fails_insufficient_total_balance() { fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&keypair); - + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&keypair); // try double, see fails stateful check let res = signed_tx_fail .check_and_execute(Arc::get_mut(&mut app.state).unwrap()) @@ -975,21 +926,19 @@ async fn app_stateful_check_fails_insufficient_total_balance() { assert!(res.contains("insufficient funds for asset")); // build single transfer to see passes - let signed_tx_pass = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + let signed_tx_pass = UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, fee_asset: nria().into(), } .into(), - ], - } - .into_signed(&keypair); + ]) + .chain_id("test") + .try_build() + .unwrap() + .into_signed(&keypair); signed_tx_pass .check_and_execute(Arc::get_mut(&mut app.state).unwrap()) @@ -1034,13 +983,11 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { 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.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); @@ -1058,13 +1005,11 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { rollup_withdrawal_event_id: "id-from-rollup".to_string(), }; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![action.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&bridge)); app.execute_transaction(signed_tx) @@ -1105,13 +1050,12 @@ async fn app_execute_transaction_action_index_correctly_increments() { 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 tx = UnsignedTransaction::builder() + .actions(vec![action.clone().into(), action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx.clone()).await.unwrap(); @@ -1142,23 +1086,19 @@ async fn transaction_execution_records_deposit_event() { state_tx .put_bridge_account_ibc_asset(bob_address, nria()) .unwrap(); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ - BridgeLockAction { - to: bob_address, - amount: 1, - asset: nria().into(), - fee_asset: nria().into(), - destination_chain_address: "test_chain_address".to_string(), - } - .into(), - ], - }; + let action = BridgeLockAction { + to: bob_address, + amount: 1, + asset: nria().into(), + fee_asset: nria().into(), + destination_chain_address: "test_chain_address".to_string(), + }; + let tx = UnsignedTransaction::builder() + .actions(vec![action.into()]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let expected_deposit = Deposit { @@ -1189,15 +1129,13 @@ async fn app_execute_transaction_ibc_sudo_change() { let new_address = astria_address_from_hex_string(BOB_ADDRESS); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::IbcSudoChange(IbcSudoChangeAction { + let tx = UnsignedTransaction::builder() + .actions(vec![Action::IbcSudoChange(IbcSudoChangeAction { new_address, - })], - }; + })]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); @@ -1226,15 +1164,13 @@ async fn app_execute_transaction_ibc_sudo_change_error() { .unwrap(); let mut app = initialize_app(Some(genesis_state), vec![]).await; - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![Action::IbcSudoChange(IbcSudoChangeAction { + let tx = UnsignedTransaction::builder() + .actions(vec![Action::IbcSudoChange(IbcSudoChangeAction { new_address: alice_address, - })], - }; + })]) + .chain_id("test") + .try_build() + .unwrap(); let signed_tx = Arc::new(tx.into_signed(&alice)); let res = app diff --git a/crates/astria-sequencer/src/benchmark_utils.rs b/crates/astria-sequencer/src/benchmark_utils.rs index cbf9faafca..9cc901065f 100644 --- a/crates/astria-sequencer/src/benchmark_utils.rs +++ b/crates/astria-sequencer/src/benchmark_utils.rs @@ -22,7 +22,6 @@ use astria_core::{ TransferAction, }, SignedTransaction, - TransactionParams, UnsignedTransaction, }, }; @@ -86,21 +85,18 @@ fn sequence_actions() -> Vec> { let (nonce, chain_id) = nonces_and_chain_ids .entry(verification_key) .or_insert_with(|| (0_u32, format!("chain-{}", signing_key.verification_key()))); - let params = TransactionParams::builder() - .nonce(*nonce) - .chain_id(chain_id.as_str()) - .build(); - *nonce = (*nonce).wrapping_add(1); let sequence_action = SequenceAction { rollup_id: RollupId::new([1; 32]), data: vec![2; 1000].into(), fee_asset: Denom::IbcPrefixed(IbcPrefixed::new([3; 32])), }; - let tx = UnsignedTransaction { - actions: vec![Action::Sequence(sequence_action)], - params, - } - .into_signed(signing_key); + let tx = UnsignedTransaction::builder() + .actions(vec![Action::Sequence(sequence_action)]) + .nonce(*nonce) + .chain_id(chain_id.as_str()) + .try_build() + .expect("failed to build transaction from actions") + .into_signed(signing_key); Arc::new(tx) }) .take(SEQUENCE_ACTION_TX_COUNT) @@ -119,17 +115,17 @@ fn transfers() -> Vec> { }); (0..TRANSFERS_TX_COUNT) .map(|nonce| { - let params = TransactionParams::builder() + let tx = UnsignedTransaction::builder() + .actions( + std::iter::repeat(action.clone()) + .take(TRANSFERS_PER_TX) + .collect(), + ) .nonce(u32::try_from(nonce).unwrap()) .chain_id("test") - .build(); - let tx = UnsignedTransaction { - actions: std::iter::repeat(action.clone()) - .take(TRANSFERS_PER_TX) - .collect(), - params, - } - .into_signed(sender); + .try_build() + .expect("failed to build transaction from actions") + .into_signed(sender); Arc::new(tx) }) .collect() diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index bf902e0bdb..627cb1998f 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -98,7 +98,6 @@ mod tests { SequenceAction, TransferAction, }, - TransactionParams, UnsignedTransaction, }, }; @@ -121,13 +120,12 @@ mod tests { }; let signing_key = SigningKey::new(OsRng); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test-chain-1") - .build(), - actions: vec![sequence_action.clone().into(), transfer_action.into()], - }; + + let tx = UnsignedTransaction::builder() + .actions(vec![sequence_action.clone().into(), transfer_action.into()]) + .chain_id("test-chain-1") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&signing_key); let txs = vec![signed_tx]; @@ -137,13 +135,11 @@ mod tests { } = generate_rollup_datas_commitment(&txs, HashMap::new()); let signing_key = SigningKey::new(OsRng); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test-chain-1") - .build(), - actions: vec![sequence_action.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![sequence_action.into()]) + .chain_id("test-chain-1") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&signing_key); let txs = vec![signed_tx]; @@ -175,13 +171,11 @@ mod tests { }; let signing_key = SigningKey::new(OsRng); - let tx = UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test-chain-1") - .build(), - actions: vec![sequence_action.into(), transfer_action.into()], - }; + let tx = UnsignedTransaction::builder() + .actions(vec![sequence_action.clone().into(), transfer_action.into()]) + .chain_id("test-chain-1") + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&signing_key); let txs = vec![signed_tx]; diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 001d7aceae..441e1f2268 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -211,7 +211,6 @@ mod tests { primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::SequenceAction, - TransactionParams, UnsignedTransaction, }, }; @@ -237,20 +236,18 @@ mod tests { }; fn make_unsigned_tx() -> UnsignedTransaction { - UnsignedTransaction { - params: TransactionParams::builder() - .nonce(0) - .chain_id("test") - .build(), - actions: vec![ + UnsignedTransaction::builder() + .actions(vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: Bytes::from_static(b"hello world"), fee_asset: crate::test_utils::nria().into(), } .into(), - ], - } + ]) + .chain_id("test") + .try_build() + .unwrap() } fn new_prepare_proposal_request() -> request::PrepareProposal { diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index b594b18c40..b4eedff56a 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -91,7 +91,7 @@ pub(crate) async fn get_fees_for_transaction( .wrap_err("failed to get bridge sudo change fee")?; let mut fees_by_asset = HashMap::new(); - for (i, action) in tx.actions.iter().enumerate() { + 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); @@ -316,12 +316,9 @@ mod tests { RollupId, ADDRESS_LEN, }, - protocol::transaction::v1alpha1::{ - action::{ - SequenceAction, - TransferAction, - }, - TransactionParams, + protocol::transaction::v1alpha1::action::{ + SequenceAction, + TransferAction, }, }; use bytes::Bytes; @@ -404,14 +401,11 @@ mod tests { }), ]; - let params = TransactionParams::builder() - .nonce(0) + let tx = UnsignedTransaction::builder() + .actions(actions) .chain_id("test-chain-id") - .build(); - let tx = UnsignedTransaction { - actions, - params, - }; + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); check_balance_for_total_fees_and_transfers(&signed_tx, &state_tx) @@ -470,14 +464,11 @@ mod tests { }), ]; - let params = TransactionParams::builder() - .nonce(0) + let tx = UnsignedTransaction::builder() + .actions(actions) .chain_id("test-chain-id") - .build(); - let tx = UnsignedTransaction { - actions, - params, - }; + .try_build() + .unwrap(); let signed_tx = tx.into_signed(&alice); let err = check_balance_for_total_fees_and_transfers(&signed_tx, &state_tx)