chore(core, sequencer): move stateless checks to domain type parsing

crates/astria-bridge-contracts/Cargo.toml

@@ -7,7 +7,7 @@ rust-version = "1.81.0"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-astria-core = { path = "../astria-core", features = ["serde"] }
+astria-core = { path = "../astria-core" }
 
 ethers = { workspace = true }
 futures = { workspace = true }

crates/astria-bridge-contracts/src/lib.rs

@@ -15,7 +15,10 @@ use astria_core::{
     protocol::{
         memos,
         transaction::v1::{
-            action::Ics20Withdrawal,
+            action::{
+                Ics20Withdrawal,
+                Ics20WithdrawalWithBridgeAddress,
+            },
             Action,
         },
     },
@@ -454,32 +457,35 @@ where
                 .expect("must be set if this method is entered"),
         );
 
-        let memo = memo_to_json(&memos::v1::Ics20WithdrawalFromRollup {
+        let ics20_withdrawal_from_rollup = memos::v1::Ics20WithdrawalFromRollup {
             memo: event.memo.clone(),
             rollup_block_number,
             rollup_return_address: event.sender.encode_hex(),
             rollup_withdrawal_event_id,
-        })
-        .map_err(GetWithdrawalActionsError::encode_memo)?;
+        };
+
+        let memo = serde_json::to_string(&ics20_withdrawal_from_rollup)
+            .map_err(GetWithdrawalActionsError::serialize_memo)?;
 
         let amount = calculate_amount(&event, self.asset_withdrawal_divisor)
             .map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?;
 
-        let action = Ics20Withdrawal {
+        let action = Ics20Withdrawal::FromRollup(Box::new(Ics20WithdrawalWithBridgeAddress {
             denom,
             destination_chain_address: event.destination_chain_address,
             return_address: self.bridge_address,
             amount,
-            memo,
+            ics20_withdrawal_from_rollup,
             fee_asset: self.fee_asset.clone(),
             // note: this refers to the timeout on the destination chain, which we are unaware of.
             // thus, we set it to the maximum possible value.
             timeout_height: max_timeout_height(),
             timeout_time: timeout_in_5_min(),
             source_channel,
-            bridge_address: Some(self.bridge_address),
+            bridge_address: self.bridge_address,
             use_compat_address: self.use_compat_address,
-        };
+            memo,
+        }));
         Ok(Action::Ics20Withdrawal(action))
     }
 
@@ -547,10 +553,6 @@ impl GetWithdrawalActionsError {
         ))
     }
 
-    fn encode_memo(source: EncodeMemoError) -> Self {
-        Self(GetWithdrawalActionsErrorKind::EncodeMemo(source))
-    }
-
     fn get_logs(source: GetLogsError) -> Self {
         Self(GetWithdrawalActionsErrorKind::GetLogs(source))
     }
@@ -569,6 +571,10 @@ impl GetWithdrawalActionsError {
     fn log_without_log_index(_log: &Log) -> Self {
         Self(GetWithdrawalActionsErrorKind::LogWithoutLogIndex)
     }
+
+    fn serialize_memo(source: serde_json::Error) -> Self {
+        Self(GetWithdrawalActionsErrorKind::SerializeMemo(source))
+    }
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -578,8 +584,6 @@ enum GetWithdrawalActionsErrorKind {
     #[error(transparent)]
     DestinationChainAsAddress(DestinationChainAsAddressError),
     #[error(transparent)]
-    EncodeMemo(EncodeMemoError),
-    #[error(transparent)]
     GetLogs(GetLogsError),
     #[error("log did not contain a block number")]
     LogWithoutBlockNumber,
@@ -589,6 +593,8 @@ enum GetWithdrawalActionsErrorKind {
     LogWithoutLogIndex,
     #[error(transparent)]
     CalculateWithdrawalAmount(CalculateWithdrawalAmountError),
+    #[error("failed to serialize ics20 withdrawal rollup information as memo")]
+    SerializeMemo(serde_json::Error),
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -691,20 +697,6 @@ struct DestinationChainAsAddressError {
     source: AddressError,
 }
 
-#[derive(Debug, thiserror::Error)]
-#[error("failed encoding memo `{proto_message}` as JSON")]
-struct EncodeMemoError {
-    proto_message: String,
-    source: serde_json::Error,
-}
-
-fn memo_to_json<T: prost::Name + serde::Serialize>(memo: &T) -> Result<String, EncodeMemoError> {
-    serde_json::to_string(memo).map_err(|source| EncodeMemoError {
-        proto_message: T::full_name(),
-        source,
-    })
-}
-
 fn parse_destination_chain_as_address(
     event: &SequencerWithdrawalFilter,
 ) -> Result<Address, DestinationChainAsAddressError> {

crates/astria-bridge-withdrawer/Cargo.toml

@@ -34,11 +34,7 @@ astria-bridge-contracts = { path = "../astria-bridge-contracts", features = [
   "tracing",
 ] }
 astria-build-info = { path = "../astria-build-info", features = ["runtime"] }
-astria-core = { path = "../astria-core", features = [
-  "serde",
-  "server",
-  "client",
-] }
+astria-core = { path = "../astria-core", features = ["server", "client"] }
 astria-eyre = { path = "../astria-eyre" }
 config = { package = "astria-config", path = "../astria-config" }
 sequencer-client = { package = "astria-sequencer-client", path = "../astria-sequencer-client", features = [

crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs

@@ -12,7 +12,6 @@ use astria_core::{
     protocol::{
         asset::v1::AllowedFeeAssetsResponse,
         bridge::v1::BridgeAccountLastTxHashResponse,
-        memos,
         transaction::v1::Action,
     },
     Protobuf as _,
@@ -450,11 +449,12 @@ fn rollup_height_from_signed_transaction(signed_transaction: &Transaction) -> ey
 
     let last_batch_rollup_height = match withdrawal_action {
         Action::BridgeUnlock(action) => Some(action.rollup_block_number),
-        Action::Ics20Withdrawal(action) => {
-            let memo: memos::v1::Ics20WithdrawalFromRollup = serde_json::from_str(&action.memo)
-                .wrap_err("failed to parse memo from last transaction by the bridge account")?;
-            Some(memo.rollup_block_number)
-        }
+        Action::Ics20Withdrawal(action) => Some(
+            action
+                .ics20_withdrawal_from_rollup()
+                .ok_or_eyre("ics20 withdrawal action did not contain rollup information")?
+                .rollup_block_number,
+        ),
         _ => None,
     }
     .expect("action is already checked to be either BridgeUnlock or Ics20Withdrawal");

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

@@ -157,7 +157,7 @@ impl Submitter {
             .iter()
             .map(|action| match action {
                 Action::BridgeUnlock(withdraw) => withdraw.amount,
-                Action::Ics20Withdrawal(withdraw) => withdraw.amount,
+                Action::Ics20Withdrawal(withdraw) => withdraw.amount(),
                 _ => 0,
             })
             .sum();

crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs

@@ -24,6 +24,7 @@ use astria_core::{
             action::{
                 BridgeUnlock,
                 Ics20Withdrawal,
+                Ics20WithdrawalWithBridgeAddress,
             },
             Action,
         },
@@ -403,29 +404,16 @@ struct SubsetOfIcs20Withdrawal {
 
 impl From<Ics20Withdrawal> for SubsetOfIcs20Withdrawal {
     fn from(value: Ics20Withdrawal) -> Self {
-        let Ics20Withdrawal {
-            amount,
-            denom,
-            destination_chain_address,
-            return_address,
-            timeout_height,
-            timeout_time: _timeout_time,
-            source_channel,
-            fee_asset,
-            memo,
-            bridge_address,
-            use_compat_address: _use_compat_address,
-        } = value;
         Self {
-            amount,
-            denom,
-            destination_chain_address,
-            return_address,
-            timeout_height,
-            source_channel,
-            fee_asset,
-            memo,
-            bridge_address,
+            amount: value.amount(),
+            denom: value.denom().clone(),
+            destination_chain_address: value.destination_chain_address().to_string(),
+            return_address: *value.return_address(),
+            timeout_height: *value.timeout_height(),
+            source_channel: value.source_channel().clone(),
+            fee_asset: value.fee_asset().clone(),
+            memo: value.memo().to_string(),
+            bridge_address: value.bridge_address().copied(),
         }
     }
 }
@@ -456,25 +444,29 @@ pub fn make_native_ics20_withdrawal_action(receipt: &TransactionReceipt) -> Acti
     let rollup_transaction_hash = receipt.transaction_hash.encode_hex();
     let event_index = receipt.logs[0].log_index.unwrap().encode_hex();
 
-    let inner = Ics20Withdrawal {
+    let ics20_withdrawal_from_rollup = Ics20WithdrawalFromRollup {
+        memo: "nootwashere".to_string(),
+        rollup_return_address: receipt.from.encode_hex(),
+        rollup_block_number: receipt.block_number.unwrap().as_u64(),
+        rollup_withdrawal_event_id: format!("{rollup_transaction_hash}.{event_index}"),
+    };
+
+    let memo = serde_json::to_string(&ics20_withdrawal_from_rollup).unwrap();
+
+    let inner = Ics20Withdrawal::FromRollup(Box::new(Ics20WithdrawalWithBridgeAddress {
         denom: denom.clone(),
         destination_chain_address: default_sequencer_address().to_string(),
         return_address: default_bridge_address(),
         amount: 1_000_000u128,
-        memo: serde_json::to_string(&Ics20WithdrawalFromRollup {
-            memo: "nootwashere".to_string(),
-            rollup_return_address: receipt.from.encode_hex(),
-            rollup_block_number: receipt.block_number.unwrap().as_u64(),
-            rollup_withdrawal_event_id: format!("{rollup_transaction_hash}.{event_index}"),
-        })
-        .unwrap(),
+        ics20_withdrawal_from_rollup,
         fee_asset: denom,
         timeout_height,
         timeout_time,
         source_channel: "channel-0".parse().unwrap(),
-        bridge_address: Some(default_bridge_address()),
+        bridge_address: default_bridge_address(),
         use_compat_address: false,
-    };
+        memo,
+    }));
 
     Action::Ics20Withdrawal(inner)
 }
@@ -507,25 +499,29 @@ pub fn make_erc20_ics20_withdrawal_action(receipt: &TransactionReceipt) -> Actio
     // use the second event because the erc20 transfer also emits an event
     let event_index = receipt.logs[1].log_index.unwrap().encode_hex();
 
-    let inner = Ics20Withdrawal {
+    let ics20_withdrawal_from_rollup = Ics20WithdrawalFromRollup {
+        memo: "nootwashere".to_string(),
+        rollup_return_address: receipt.from.encode_hex(),
+        rollup_block_number: receipt.block_number.unwrap().as_u64(),
+        rollup_withdrawal_event_id: format!("{rollup_transaction_hash}.{event_index}"),
+    };
+
+    let memo = serde_json::to_string(&ics20_withdrawal_from_rollup).unwrap();
+
+    let inner = Ics20Withdrawal::FromRollup(Box::new(Ics20WithdrawalWithBridgeAddress {
         denom: denom.clone(),
         destination_chain_address: default_sequencer_address().to_string(),
         return_address: default_bridge_address(),
         amount: 1_000_000u128,
-        memo: serde_json::to_string(&Ics20WithdrawalFromRollup {
-            memo: "nootwashere".to_string(),
-            rollup_return_address: receipt.from.encode_hex(),
-            rollup_block_number: receipt.block_number.unwrap().as_u64(),
-            rollup_withdrawal_event_id: format!("{rollup_transaction_hash}.{event_index}"),
-        })
-        .unwrap(),
+        ics20_withdrawal_from_rollup,
         fee_asset: denom,
         timeout_height,
         timeout_time,
         source_channel: "channel-0".parse().unwrap(),
-        bridge_address: Some(default_bridge_address()),
+        bridge_address: default_bridge_address(),
         use_compat_address: false,
-    };
+        memo,
+    }));
 
     Action::Ics20Withdrawal(inner)
 }

crates/astria-cli/Cargo.toml

@@ -23,7 +23,7 @@ tracing-subscriber = "0.3.18"
 astria-bridge-contracts = { path = "../astria-bridge-contracts", features = [
   "tracing",
 ] }
-astria-core = { path = "../astria-core", features = ["serde"] }
+astria-core = { path = "../astria-core" }
 
 clap = { workspace = true, features = ["derive", "env"] }
 ethers = { workspace = true, features = ["ws"] }

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

@@ -1,4 +1,5 @@
 use astria_core::{
+    generated::protocol::transaction::v1::IbcHeight,
     primitive::v1::{
         asset,
         Address,
@@ -7,15 +8,13 @@ use astria_core::{
         action::Ics20Withdrawal,
         Action,
     },
+    Protobuf as _,
 };
 use color_eyre::eyre::{
     self,
     WrapErr as _,
 };
-use ibc_types::core::{
-    channel::ChannelId,
-    client::Height,
-};
+use ibc_types::core::channel::ChannelId;
 use tracing::info;
 
 use crate::utils::{
@@ -97,22 +96,27 @@ impl Command {
             self.sequencer_chain_id.clone(),
             &self.prefix,
             self.private_key.as_str(),
-            Action::Ics20Withdrawal(Ics20Withdrawal {
-                amount: self.amount,
-                denom: self.asset,
-                destination_chain_address: self.destination_chain_address,
-                return_address: self.return_address.unwrap_or(from_address),
-                timeout_height: Height {
-                    revision_number: u64::MAX,
-                    revision_height: u64::MAX,
-                },
-                timeout_time: now_plus_5_minutes(),
-                source_channel: ChannelId(self.source_channel),
-                fee_asset: self.fee_asset,
-                memo: self.memo.unwrap_or_default(),
-                bridge_address: self.bridge_address,
-                use_compat_address: self.compat,
-            }),
+            Action::Ics20Withdrawal(
+                Ics20Withdrawal::try_from_raw(
+                    astria_core::generated::protocol::transaction::v1::Ics20Withdrawal {
+                        amount: Some(self.amount.into()),
+                        denom: self.asset.to_string(),
+                        destination_chain_address: self.destination_chain_address,
+                        return_address: Some(self.return_address.unwrap_or(from_address).to_raw()),
+                        timeout_height: Some(IbcHeight {
+                            revision_number: u64::MAX,
+                            revision_height: u64::MAX,
+                        }),
+                        timeout_time: now_plus_5_minutes(),
+                        source_channel: ChannelId(self.source_channel).to_string(),
+                        fee_asset: self.fee_asset.to_string(),
+                        memo: self.memo.unwrap_or_default(),
+                        bridge_address: self.bridge_address.map(|act| act.to_raw()),
+                        use_compat_address: self.compat,
+                    },
+                )
+                .wrap_err("failed to create ics20 withdrawal action")?,
+            ),
         )
         .await
         .wrap_err("failed to perform ics20 withdrawal")?;