From 52961b69fe6ba142e52021fbad19c26df3869c22 Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 09:03:48 +0200
Subject: [PATCH 1/6] move withdrawl action construction to contracts crate

---
 crates/astria-bridge-contracts/Cargo.toml |   7 +
 crates/astria-bridge-contracts/src/lib.rs | 545 ++++++++++++++++++++++
 2 files changed, 552 insertions(+)

diff --git a/crates/astria-bridge-contracts/Cargo.toml b/crates/astria-bridge-contracts/Cargo.toml
index 72de607667..a96bba7088 100644
--- a/crates/astria-bridge-contracts/Cargo.toml
+++ b/crates/astria-bridge-contracts/Cargo.toml
@@ -6,4 +6,11 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+astria-core = { path = "../astria-core", features = ["serde"] }
+
 ethers = { workspace = true }
+futures = { workspace = true }
+ibc-types = { workspace = true }
+serde_json = { workspace = true }
+tendermint = { workspace = true }
+thiserror = { workspace = true }
diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs
index 7ca33aa159..0848c4230e 100644
--- a/crates/astria-bridge-contracts/src/lib.rs
+++ b/crates/astria-bridge-contracts/src/lib.rs
@@ -1,4 +1,549 @@
 #[rustfmt::skip]
 #[allow(clippy::pedantic)]
 mod generated;
+use std::{
+    borrow::Cow,
+    sync::Arc,
+};
+
+use astria_core::{
+    primitive::v1::{
+        asset::{
+            self,
+            TracePrefixed,
+        },
+        Address,
+        AddressError,
+    },
+    protocol::transaction::v1alpha1::{
+        action::Ics20Withdrawal,
+        Action,
+    },
+};
+use astria_withdrawer::{
+    Ics20WithdrawalFilter,
+    SequencerWithdrawalFilter,
+};
+use ethers::{
+    contract::EthEvent,
+    providers::Middleware,
+    types::{
+        Filter,
+        Log,
+        H256,
+    },
+};
 pub use generated::*;
+
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct BuildError(BuildErrorKind);
+
+impl BuildError {
+    fn bad_divisor(base_chain_asset_precision: u32) -> Self {
+        Self(BuildErrorKind::BadDivisor {
+            base_chain_asset_precision,
+        })
+    }
+
+    fn call_base_chain_asset_precision<T: Into<Box<dyn std::error::Error + 'static>>>(
+        source: T,
+    ) -> Self {
+        Self(BuildErrorKind::CallBaseChainAssetPrecision {
+            source: source.into(),
+        })
+    }
+
+    fn not_set(field: &'static str) -> Self {
+        Self(BuildErrorKind::NotSet {
+            field,
+        })
+    }
+
+    fn rollup_asset_without_channel() -> Self {
+        Self(BuildErrorKind::RollupAssetWithoutChannel)
+    }
+
+    fn parse_rollup_asset_source_channel(source: ibc_types::IdentifierError) -> Self {
+        Self(BuildErrorKind::ParseRollupAssetSourceChannel {
+            source,
+        })
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+enum BuildErrorKind {
+    #[error(
+        "failed calculating asset divisor. The base chain asset precision should be <= 18 as \
+         that's enforced by the contract, so the construction should work. Did the precision \
+         change? Precision returned by contract: `{base_chain_asset_precision}`"
+    )]
+    BadDivisor { base_chain_asset_precision: u32 },
+    #[error("required option `{field}` not set")]
+    NotSet { field: &'static str },
+    #[error("failed to call the `BASE_CHAIN_ASSET_PRECISION` of the provided contract")]
+    CallBaseChainAssetPrecision {
+        source: Box<dyn std::error::Error + 'static>,
+    },
+    #[error("rollup asset denom must have a channel to be withdrawn via IBC")]
+    RollupAssetWithoutChannel,
+    #[error("could not parse rollup asset channel as channel ID")]
+    ParseRollupAssetSourceChannel { source: ibc_types::IdentifierError },
+}
+
+pub struct NoProvider;
+pub struct WithProvider<P>(Arc<P>);
+
+pub struct GetWithdrawalActionsBuilder<TProvider = NoProvider> {
+    provider: TProvider,
+    contract_address: Option<ethers::types::Address>,
+    bridge_address: Option<Address>,
+    fee_asset: Option<asset::Denom>,
+    rollup_asset_denom: Option<asset::Denom>,
+}
+
+impl Default for GetWithdrawalActionsBuilder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl GetWithdrawalActionsBuilder {
+    pub fn new() -> Self {
+        Self {
+            provider: NoProvider,
+            contract_address: None,
+            bridge_address: None,
+            fee_asset: None,
+            rollup_asset_denom: None,
+        }
+    }
+}
+
+impl<P> GetWithdrawalActionsBuilder<P> {
+    pub fn provider<Q>(self, provider: Arc<Q>) -> GetWithdrawalActionsBuilder<WithProvider<Q>> {
+        let Self {
+            contract_address,
+            bridge_address,
+            fee_asset,
+            rollup_asset_denom,
+            ..
+        } = self;
+        GetWithdrawalActionsBuilder {
+            provider: WithProvider(provider),
+            contract_address,
+            bridge_address,
+            fee_asset,
+            rollup_asset_denom,
+        }
+    }
+
+    pub fn contract_address(self, contract_address: ethers::types::Address) -> Self {
+        Self {
+            contract_address: Some(contract_address),
+            ..self
+        }
+    }
+
+    pub fn bridge_address(self, bridge_address: Address) -> Self {
+        Self {
+            bridge_address: Some(bridge_address),
+            ..self
+        }
+    }
+
+    pub fn fee_asset(self, fee_asset: asset::Denom) -> Self {
+        Self {
+            fee_asset: Some(fee_asset),
+            ..self
+        }
+    }
+
+    pub fn rollup_asset_denom(self, rollup_asset_denom: asset::Denom) -> Self {
+        Self {
+            rollup_asset_denom: Some(rollup_asset_denom),
+            ..self
+        }
+    }
+}
+
+impl<P> GetWithdrawalActionsBuilder<WithProvider<P>>
+where
+    P: Middleware + 'static,
+    P::Error: std::error::Error + 'static,
+{
+    pub async fn build(self) -> Result<GetWithdrawalActions<P>, BuildError> {
+        let Self {
+            provider: WithProvider(provider),
+            contract_address,
+            bridge_address,
+            fee_asset,
+            rollup_asset_denom,
+        } = self;
+
+        let Some(contract_address) = contract_address else {
+            return Err(BuildError::not_set("contract_address"));
+        };
+        let Some(bridge_address) = bridge_address else {
+            return Err(BuildError::not_set("bridge_address"));
+        };
+        let Some(fee_asset) = fee_asset else {
+            return Err(BuildError::not_set("fee_asset"));
+        };
+        let Some(rollup_asset_denom) = rollup_asset_denom else {
+            return Err(BuildError::not_set("rollup_asset_denom"));
+        };
+
+        let rollup_asset_source_channel = rollup_asset_denom
+            .as_trace_prefixed()
+            .and_then(TracePrefixed::last_channel)
+            .ok_or(BuildError::rollup_asset_without_channel())?
+            .parse()
+            .map_err(BuildError::parse_rollup_asset_source_channel)?;
+
+        let contract =
+            i_astria_withdrawer::IAstriaWithdrawer::new(contract_address, provider.clone());
+
+        let base_chain_asset_precision = contract
+            .base_chain_asset_precision()
+            .call()
+            .await
+            .map_err(BuildError::call_base_chain_asset_precision)?;
+
+        let exponent = 18u32
+            .checked_sub(base_chain_asset_precision)
+            .ok_or_else(|| BuildError::bad_divisor(base_chain_asset_precision))?;
+
+        let asset_withdrawal_divisor = 10u128.pow(exponent);
+
+        Ok(GetWithdrawalActions {
+            provider,
+            contract_address,
+            asset_withdrawal_divisor,
+            bridge_address,
+            fee_asset,
+            rollup_asset_denom,
+            rollup_asset_source_channel,
+        })
+    }
+}
+
+pub struct GetWithdrawalActions<P> {
+    provider: Arc<P>,
+    contract_address: ethers::types::Address,
+    asset_withdrawal_divisor: u128,
+    bridge_address: Address,
+    fee_asset: asset::Denom,
+    rollup_asset_denom: asset::Denom,
+    rollup_asset_source_channel: ibc_types::core::channel::ChannelId,
+}
+
+impl<P> GetWithdrawalActions<P>
+where
+    P: Middleware,
+    P::Error: std::error::Error + 'static,
+{
+    pub async fn get_for_block_hash(
+        &self,
+        block_hash: H256,
+    ) -> Result<Vec<Action>, GetWithdrawalActionsError> {
+        let (ics20_logs, sequencer_logs) = futures::future::try_join(
+            get_logs::<Ics20WithdrawalFilter, _>(&self.provider, self.contract_address, block_hash),
+            get_logs::<SequencerWithdrawalFilter, _>(
+                &self.provider,
+                self.contract_address,
+                block_hash,
+            ),
+        )
+        .await
+        .map_err(GetWithdrawalActionsError::get_logs)?;
+
+        ics20_logs
+            .into_iter()
+            .map(|log| self.log_to_ics20_withdrawal_action(log))
+            .chain(
+                sequencer_logs
+                    .into_iter()
+                    .map(|log| self.log_to_sequencer_withdrawal_action(log)),
+            )
+            .collect()
+    }
+
+    fn log_to_ics20_withdrawal_action(
+        &self,
+        log: Log,
+    ) -> Result<Action, GetWithdrawalActionsError> {
+        let block_number = log
+            .block_number
+            .ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))?
+            .as_u64();
+
+        let transaction_hash = log
+            .transaction_hash
+            .ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))?
+            .into();
+
+        let event = decode_log::<Ics20WithdrawalFilter>(log)
+            .map_err(GetWithdrawalActionsError::decode_log)?;
+
+        let source_channel = self.rollup_asset_source_channel.clone();
+
+        let memo = serde_json::to_string(&astria_core::bridge::Ics20WithdrawalFromRollupMemo {
+            memo: event.memo.clone(),
+            block_number,
+            rollup_return_address: event.sender.to_string(),
+            transaction_hash,
+        })
+        .map_err(|source| {
+            GetWithdrawalActionsError::encode_memo("Ics20WithdrawalFromRollupMemo", source)
+        })?;
+
+        let amount = calculate_amount(&event, self.asset_withdrawal_divisor)
+            .map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?;
+
+        let action = Ics20Withdrawal {
+            denom: self.rollup_asset_denom.clone(),
+            destination_chain_address: event.destination_chain_address,
+            return_address: self.bridge_address,
+            amount,
+            memo,
+            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),
+        };
+        Ok(Action::Ics20Withdrawal(action))
+    }
+
+    fn log_to_sequencer_withdrawal_action(
+        &self,
+        log: Log,
+    ) -> Result<Action, GetWithdrawalActionsError> {
+        let block_number = log
+            .block_number
+            .ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))?
+            .as_u64();
+
+        let transaction_hash = log
+            .transaction_hash
+            .ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))?
+            .into();
+
+        let event = decode_log::<SequencerWithdrawalFilter>(log)
+            .map_err(GetWithdrawalActionsError::decode_log)?;
+
+        let memo = serde_json::to_string(&astria_core::bridge::UnlockMemo {
+            block_number,
+            transaction_hash,
+        })
+        .map_err(|err| GetWithdrawalActionsError::encode_memo("bridge::UnlockMemo", err))?;
+
+        let amount = calculate_amount(&event, self.asset_withdrawal_divisor)
+            .map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?;
+
+        let to = parse_destination_chain_as_address(&event)
+            .map_err(GetWithdrawalActionsError::destination_chain_as_address)?;
+
+        let action = astria_core::protocol::transaction::v1alpha1::action::BridgeUnlockAction {
+            to,
+            amount,
+            memo,
+            fee_asset: self.fee_asset.clone(),
+            bridge_address: Some(self.bridge_address),
+        };
+
+        Ok(Action::BridgeUnlock(action))
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct GetWithdrawalActionsError(GetWithdrawalActionsErrorKind);
+
+impl GetWithdrawalActionsError {
+    fn calculate_withdrawal_amount(source: CalculateWithdrawalAmountError) -> Self {
+        Self(GetWithdrawalActionsErrorKind::CalculateWithdrawalAmount(
+            source,
+        ))
+    }
+
+    fn decode_log(source: DecodeLogError) -> Self {
+        Self(GetWithdrawalActionsErrorKind::DecodeLog(source))
+    }
+
+    fn destination_chain_as_address(source: DestinationChainAsAddressError) -> Self {
+        Self(GetWithdrawalActionsErrorKind::DestinationChainAsAddress(
+            source,
+        ))
+    }
+
+    fn encode_memo(which: &'static str, source: serde_json::Error) -> Self {
+        Self(GetWithdrawalActionsErrorKind::EncodeMemo {
+            which,
+            source,
+        })
+    }
+
+    fn get_logs(source: GetLogsError) -> Self {
+        Self(GetWithdrawalActionsErrorKind::GetLogs(source))
+    }
+
+    // XXX: Somehow identify the log?
+    fn log_without_block_number(_log: &Log) -> Self {
+        Self(GetWithdrawalActionsErrorKind::LogWithoutBlockNumber)
+    }
+
+    // XXX: Somehow identify the log?
+    fn log_without_transaction_hash(_log: &Log) -> Self {
+        Self(GetWithdrawalActionsErrorKind::LogWithoutTransactionHash)
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+enum GetWithdrawalActionsErrorKind {
+    #[error(transparent)]
+    DecodeLog(DecodeLogError),
+    #[error(transparent)]
+    DestinationChainAsAddress(DestinationChainAsAddressError),
+    #[error("failed encoding memo `{which}`")]
+    EncodeMemo {
+        which: &'static str,
+        source: serde_json::Error,
+    },
+    #[error(transparent)]
+    GetLogs(GetLogsError),
+    #[error("log did not contain a block number")]
+    LogWithoutBlockNumber,
+    #[error("log did not contain a transaction hash")]
+    LogWithoutTransactionHash,
+    #[error(transparent)]
+    CalculateWithdrawalAmount(CalculateWithdrawalAmountError),
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error("failed decoding a log into an Astria bridge contract event `{event_name}`")]
+struct DecodeLogError {
+    event_name: Cow<'static, str>,
+    // use a trait object instead of the error to not force the middleware
+    // type parameter into the error.
+    source: Box<dyn std::error::Error>,
+}
+
+fn decode_log<T: EthEvent>(log: Log) -> Result<T, DecodeLogError> {
+    T::decode_log(&log.into()).map_err(|err| DecodeLogError {
+        event_name: T::name(),
+        source: err.into(),
+    })
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error("failed getting the eth logs for event `{event_name}`")]
+struct GetLogsError {
+    event_name: Cow<'static, str>,
+    // use a trait object instead of the error to not force the middleware
+    // type parameter into the error.
+    source: Box<dyn std::error::Error>,
+}
+
+async fn get_logs<T: EthEvent, M>(
+    provider: &M,
+    contract_address: ethers::types::Address,
+    block_hash: H256,
+) -> Result<Vec<Log>, GetLogsError>
+where
+    M: Middleware,
+    M::Error: std::error::Error + 'static,
+{
+    let event_sig = T::signature();
+    let filter = Filter::new()
+        .at_block_hash(block_hash)
+        .address(contract_address)
+        .topic0(event_sig);
+
+    provider
+        .get_logs(&filter)
+        .await
+        .map_err(|err| GetLogsError {
+            event_name: T::name(),
+            source: err.into(),
+        })
+}
+
+trait GetAmount {
+    fn get_amount(&self) -> u128;
+}
+
+impl GetAmount for Ics20WithdrawalFilter {
+    fn get_amount(&self) -> u128 {
+        self.amount.as_u128()
+    }
+}
+
+impl GetAmount for SequencerWithdrawalFilter {
+    fn get_amount(&self) -> u128 {
+        self.amount.as_u128()
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error(
+    "failed calculate amount to withdraw because mount in event could not be divided by the asset \
+     withdrawal divisor; amount: `{amount}`, divisor: `{divisor}`"
+)]
+struct CalculateWithdrawalAmountError {
+    amount: u128,
+    divisor: u128,
+}
+
+fn calculate_amount<T: GetAmount>(
+    event: &T,
+    asset_withdrawal_divisor: u128,
+) -> Result<u128, CalculateWithdrawalAmountError> {
+    event
+        .get_amount()
+        .checked_div(asset_withdrawal_divisor)
+        .ok_or_else(|| CalculateWithdrawalAmountError {
+            amount: event.get_amount(),
+            divisor: asset_withdrawal_divisor,
+        })
+}
+
+fn max_timeout_height() -> ibc_types::core::client::Height {
+    ibc_types::core::client::Height::new(u64::MAX, u64::MAX)
+        .expect("non-zero arguments should never fail")
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error("failed to parse destination chain address as Astria address for a bridge unlock")]
+struct DestinationChainAsAddressError {
+    #[from]
+    source: AddressError,
+}
+
+fn parse_destination_chain_as_address(
+    event: &SequencerWithdrawalFilter,
+) -> Result<Address, DestinationChainAsAddressError> {
+    event.destination_chain_address.parse().map_err(Into::into)
+}
+
+fn timeout_in_5_min() -> u64 {
+    use std::time::Duration;
+    tendermint::Time::now()
+        .checked_add(Duration::from_secs(300))
+        .expect("adding 5 minutes to the current time should never fail")
+        .unix_timestamp_nanos()
+        .try_into()
+        .expect("timestamp must be positive, so this conversion would only fail if negative")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::max_timeout_height;
+    #[test]
+    fn max_timeout_height_does_not_panic() {
+        max_timeout_height();
+    }
+}

From 94490d39d43b1b6fbdf5fc09e1d11d9001707597 Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 09:50:49 +0200
Subject: [PATCH 2/6] update CLI to use crate functionality

---
 Cargo.lock                                    |   6 +
 crates/astria-bridge-contracts/src/lib.rs     |  12 +-
 .../astria-cli/src/commands/bridge/collect.rs | 372 +++---------------
 3 files changed, 66 insertions(+), 324 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 94e5939f2e..71b83ee9c2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -488,7 +488,13 @@ dependencies = [
 name = "astria-bridge-contracts"
 version = "0.1.0"
 dependencies = [
+ "astria-core",
  "ethers",
+ "futures",
+ "ibc-types",
+ "serde_json",
+ "tendermint",
+ "thiserror",
 ]
 
 [[package]]
diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs
index 0848c4230e..40d8824131 100644
--- a/crates/astria-bridge-contracts/src/lib.rs
+++ b/crates/astria-bridge-contracts/src/lib.rs
@@ -46,7 +46,9 @@ impl BuildError {
         })
     }
 
-    fn call_base_chain_asset_precision<T: Into<Box<dyn std::error::Error + 'static>>>(
+    fn call_base_chain_asset_precision<
+        T: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
+    >(
         source: T,
     ) -> Self {
         Self(BuildErrorKind::CallBaseChainAssetPrecision {
@@ -83,7 +85,7 @@ enum BuildErrorKind {
     NotSet { field: &'static str },
     #[error("failed to call the `BASE_CHAIN_ASSET_PRECISION` of the provided contract")]
     CallBaseChainAssetPrecision {
-        source: Box<dyn std::error::Error + 'static>,
+        source: Box<dyn std::error::Error + Send + Sync + 'static>,
     },
     #[error("rollup asset denom must have a channel to be withdrawn via IBC")]
     RollupAssetWithoutChannel,
@@ -172,7 +174,7 @@ where
     P: Middleware + 'static,
     P::Error: std::error::Error + 'static,
 {
-    pub async fn build(self) -> Result<GetWithdrawalActions<P>, BuildError> {
+    pub async fn try_build(self) -> Result<GetWithdrawalActions<P>, BuildError> {
         let Self {
             provider: WithProvider(provider),
             contract_address,
@@ -429,7 +431,7 @@ struct DecodeLogError {
     event_name: Cow<'static, str>,
     // use a trait object instead of the error to not force the middleware
     // type parameter into the error.
-    source: Box<dyn std::error::Error>,
+    source: Box<dyn std::error::Error + Send + Sync + 'static>,
 }
 
 fn decode_log<T: EthEvent>(log: Log) -> Result<T, DecodeLogError> {
@@ -445,7 +447,7 @@ struct GetLogsError {
     event_name: Cow<'static, str>,
     // use a trait object instead of the error to not force the middleware
     // type parameter into the error.
-    source: Box<dyn std::error::Error>,
+    source: Box<dyn std::error::Error + Send + Sync + 'static>,
 }
 
 async fn get_logs<T: EthEvent, M>(
diff --git a/crates/astria-cli/src/commands/bridge/collect.rs b/crates/astria-cli/src/commands/bridge/collect.rs
index 46cf8dbcf2..479ce046d3 100644
--- a/crates/astria-cli/src/commands/bridge/collect.rs
+++ b/crates/astria-cli/src/commands/bridge/collect.rs
@@ -8,30 +8,18 @@ use std::{
     time::Duration,
 };
 
-use astria_bridge_contracts::i_astria_withdrawer::{
-    IAstriaWithdrawer,
-    Ics20WithdrawalFilter,
-    SequencerWithdrawalFilter,
+use astria_bridge_contracts::{
+    GetWithdrawalActions,
+    GetWithdrawalActionsBuilder,
 };
 use astria_core::{
-    bridge::{
-        self,
-        Ics20WithdrawalFromRollupMemo,
-    },
     primitive::v1::{
         asset::{
             self,
-            TracePrefixed,
         },
         Address,
     },
-    protocol::transaction::v1alpha1::{
-        action::{
-            BridgeUnlockAction,
-            Ics20Withdrawal,
-        },
-        Action,
-    },
+    protocol::transaction::v1alpha1::Action,
 };
 use clap::Args;
 use color_eyre::eyre::{
@@ -43,7 +31,6 @@ use color_eyre::eyre::{
     WrapErr as _,
 };
 use ethers::{
-    contract::EthEvent,
     core::types::Block,
     providers::{
         Middleware,
@@ -52,11 +39,7 @@ use ethers::{
         StreamExt as _,
         Ws,
     },
-    types::{
-        Filter,
-        Log,
-        H256,
-    },
+    types::H256,
 };
 use futures::stream::BoxStream;
 use tracing::{
@@ -118,10 +101,15 @@ impl WithdrawalEvents {
             .await
             .wrap_err("failed to connect to rollup")?;
 
-        let asset_withdrawal_divisor =
-            get_asset_withdrawal_divisor(contract_address, block_provider.clone())
-                .await
-                .wrap_err("failed determining asset withdrawal divisor")?;
+        let actions_fetcher = GetWithdrawalActionsBuilder::new()
+            .provider(block_provider.clone())
+            .contract_address(contract_address)
+            .fee_asset(fee_asset)
+            .rollup_asset_denom(rollup_asset_denom)
+            .bridge_address(bridge_address)
+            .try_build()
+            .await
+            .wrap_err("failed to initialize contract events to sequencer actions converter")?;
 
         let mut incoming_blocks =
             create_stream_of_blocks(&block_provider, from_rollup_height, to_rollup_height)
@@ -139,21 +127,20 @@ impl WithdrawalEvents {
 
                 block = incoming_blocks.next() => {
                     match block {
-                        Some(Ok(block)) =>
-                            if let Err(err) = actions_by_rollup_height.convert_and_insert(BlockToActions {
-                                block_provider: block_provider.clone(),
-                                contract_address,
+                        Some(Ok(block)) => {
+                            if let Err(e) = block_to_actions(
                                 block,
-                                fee_asset: fee_asset.clone(),
-                                rollup_asset_denom: rollup_asset_denom.clone(),
-                                bridge_address,
-                                asset_withdrawal_divisor,
-                             }).await {
-                                 error!(
-                                     err = AsRef::<dyn std::error::Error>::as_ref(&err),
-                                     "failed converting contract block to Sequencer actions and storing them; exiting stream");
-                                 break;
-                             }
+                                &mut actions_by_rollup_height,
+                                &actions_fetcher,
+                            ).await {
+                                error!(
+                                    error = AsRef::<dyn std::error::Error>::as_ref(&e),
+                                    "failed converting contract block to sequencer actions;
+                                    exiting stream",
+                                );
+                                break;
+                            }
+                        }
                         Some(Err(error)) => {
                             error!(
                                 error = AsRef::<dyn std::error::Error>::as_ref(&error),
@@ -176,6 +163,30 @@ impl WithdrawalEvents {
     }
 }
 
+async fn block_to_actions(
+    block: Block<H256>,
+    actions_by_rollup_height: &mut ActionsByRollupHeight,
+    actions_fetcher: &GetWithdrawalActions<Provider<Ws>>,
+) -> eyre::Result<()> {
+    let block_hash = block
+        .hash
+        .ok_or_eyre("block did not contain a hash; skipping")?;
+    let rollup_height = block
+        .number
+        .ok_or_eyre("block did not contain a rollup height; skipping")?
+        .as_u64();
+    let actions = actions_fetcher
+        .get_for_block_hash(block_hash)
+        .await
+        .wrap_err_with(|| {
+            format!(
+                "failed getting actions for block; block hash: `{block_hash}`, block height: \
+                 `{rollup_height}`"
+            )
+        })?;
+    actions_by_rollup_height.insert(rollup_height, actions)
+}
+
 #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)]
 #[serde(transparent)]
 pub(crate) struct ActionsByRollupHeight(BTreeMap<u64, Vec<Action>>);
@@ -190,13 +201,7 @@ impl ActionsByRollupHeight {
     }
 
     #[instrument(skip_all, err)]
-    async fn convert_and_insert(&mut self, block_to_actions: BlockToActions) -> eyre::Result<()> {
-        let rollup_height = block_to_actions
-            .block
-            .number
-            .ok_or_eyre("block was missing a number")?
-            .as_u64();
-        let actions = block_to_actions.run().await;
+    fn insert(&mut self, rollup_height: u64, actions: Vec<Action>) -> eyre::Result<()> {
         ensure!(
             self.0.insert(rollup_height, actions).is_none(),
             "already collected actions for block at rollup height `{rollup_height}`; no 2 blocks \
@@ -277,7 +282,7 @@ fn open_output<P: AsRef<Path>>(target: P) -> eyre::Result<Output> {
         .write(true)
         .create_new(true)
         .open(&target)
-        .wrap_err("failed to open specified fil}e for writing")?;
+        .wrap_err("failed to open specified file for writing")?;
     Ok(Output {
         handle,
         path: target.as_ref().to_path_buf(),
@@ -309,274 +314,3 @@ async fn connect_to_rollup(rollup_endpoint: &str) -> eyre::Result<Arc<Provider<W
         .wrap_err("failed connecting to rollup after several retries; giving up")?;
     Ok(Arc::new(provider))
 }
-
-#[instrument(skip_all, fields(%contract_address), err(Display))]
-async fn get_asset_withdrawal_divisor(
-    contract_address: ethers::types::Address,
-    provider: Arc<Provider<Ws>>,
-) -> eyre::Result<u128> {
-    let contract = IAstriaWithdrawer::new(contract_address, provider);
-
-    let base_chain_asset_precision = contract
-        .base_chain_asset_precision()
-        .call()
-        .await
-        .wrap_err("failed to get asset withdrawal decimals")?;
-
-    let exponent = 18u32.checked_sub(base_chain_asset_precision).ok_or_eyre(
-        "failed calculating asset divisor. The base chain asset precision should be <= 18 as \
-         that's enforced by the contract, so the construction should work. Did the precision \
-         change?",
-    )?;
-    Ok(10u128.pow(exponent))
-}
-
-fn packet_timeout_time() -> eyre::Result<u64> {
-    tendermint::Time::now()
-        .checked_add(Duration::from_secs(300))
-        .ok_or_eyre("adding 5 minutes to current time caused overflow")?
-        .unix_timestamp_nanos()
-        .try_into()
-        .wrap_err("failed to i128 nanoseconds to u64")
-}
-
-struct BlockToActions {
-    block_provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
-    block: Block<H256>,
-    fee_asset: asset::Denom,
-    rollup_asset_denom: asset::Denom,
-    bridge_address: Address,
-    asset_withdrawal_divisor: u128,
-}
-
-impl BlockToActions {
-    async fn run(self) -> Vec<Action> {
-        let mut actions = Vec::new();
-
-        let Some(block_hash) = self.block.hash else {
-            warn!("block hash missing; skipping");
-            return actions;
-        };
-
-        match get_log::<SequencerWithdrawalFilter>(
-            self.block_provider.clone(),
-            self.contract_address,
-            block_hash,
-        )
-        .await
-        {
-            Err(error) => warn!(
-                error = AsRef::<dyn std::error::Error>::as_ref(&error),
-                "encountered an error getting logs for sequencer withdrawal events",
-            ),
-            Ok(logs) => {
-                for log in logs {
-                    match self.log_to_sequencer_withdrawal_action(log) {
-                        Ok(action) => actions.push(action),
-                        Err(error) => {
-                            warn!(
-                                error = AsRef::<dyn std::error::Error>::as_ref(&error),
-                                "failed converting ethers contract log to sequencer withdrawal \
-                                 action; skipping"
-                            );
-                        }
-                    }
-                }
-            }
-        }
-        match get_log::<Ics20WithdrawalFilter>(
-            self.block_provider.clone(),
-            self.contract_address,
-            block_hash,
-        )
-        .await
-        {
-            Err(error) => warn!(
-                error = AsRef::<dyn std::error::Error>::as_ref(&error),
-                "encountered an error getting logs for ics20 withdrawal events",
-            ),
-            Ok(logs) => {
-                for log in logs {
-                    match self.log_to_ics20_withdrawal_action(log) {
-                        Ok(action) => actions.push(action),
-                        Err(error) => {
-                            warn!(
-                                error = AsRef::<dyn std::error::Error>::as_ref(&error),
-                                "failed converting ethers contract log to ics20 withdrawal \
-                                 action; skipping"
-                            );
-                        }
-                    }
-                }
-            }
-        }
-        actions
-    }
-
-    fn log_to_ics20_withdrawal_action(&self, log: Log) -> eyre::Result<Action> {
-        LogToIcs20WithdrawalAction {
-            log,
-            fee_asset: self.fee_asset.clone(),
-            rollup_asset_denom: self.rollup_asset_denom.clone(),
-            asset_withdrawal_divisor: self.asset_withdrawal_divisor,
-            bridge_address: self.bridge_address,
-        }
-        .try_convert()
-        .wrap_err("failed converting log to ics20 withdrawal action")
-    }
-
-    fn log_to_sequencer_withdrawal_action(&self, log: Log) -> eyre::Result<Action> {
-        LogToSequencerWithdrawalAction {
-            log,
-            bridge_address: self.bridge_address,
-            fee_asset: self.fee_asset.clone(),
-            asset_withdrawal_divisor: self.asset_withdrawal_divisor,
-        }
-        .try_into_action()
-        .wrap_err("failed converting log to sequencer withdrawal action")
-    }
-}
-
-fn action_inputs_from_log<T: EthEvent>(log: Log) -> eyre::Result<(T, u64, [u8; 32])> {
-    let block_number = log
-        .block_number
-        .ok_or_eyre("log did not contain block number")?
-        .as_u64();
-    let transaction_hash = log
-        .transaction_hash
-        .ok_or_eyre("log did not contain transaction hash")?
-        .into();
-
-    let event = T::decode_log(&log.into())
-        .wrap_err_with(|| format!("failed decoding contract log as `{}`", T::name()))?;
-    Ok((event, block_number, transaction_hash))
-}
-
-#[derive(Debug)]
-struct LogToIcs20WithdrawalAction {
-    log: Log,
-    fee_asset: asset::Denom,
-    rollup_asset_denom: asset::Denom,
-    asset_withdrawal_divisor: u128,
-    bridge_address: Address,
-}
-
-impl LogToIcs20WithdrawalAction {
-    fn try_convert(self) -> eyre::Result<Action> {
-        let Self {
-            log,
-            fee_asset,
-            rollup_asset_denom,
-            asset_withdrawal_divisor,
-            bridge_address,
-        } = self;
-
-        let (event, block_number, transaction_hash) =
-            action_inputs_from_log::<Ics20WithdrawalFilter>(log)
-                .wrap_err("failed getting required data from log")?;
-
-        let source_channel = rollup_asset_denom
-            .as_trace_prefixed()
-            .and_then(TracePrefixed::last_channel)
-            .ok_or_eyre("rollup asset denom must have a channel to be withdrawn via IBC")?
-            .parse()
-            .wrap_err("failed to parse channel from rollup asset denom")?;
-
-        let memo = Ics20WithdrawalFromRollupMemo {
-            memo: event.memo,
-            block_number,
-            rollup_return_address: event.sender.to_string(),
-            transaction_hash,
-        };
-
-        let action = Ics20Withdrawal {
-            denom: rollup_asset_denom,
-            destination_chain_address: event.destination_chain_address,
-            // note: this is actually a rollup address; we expect failed ics20 withdrawals to be
-            // returned to the rollup.
-            // this is only ok for now because addresses on the sequencer and the rollup are both 20
-            // bytes, but this won't work otherwise.
-            return_address: bridge_address,
-            amount: event
-                .amount
-                .as_u128()
-                .checked_div(asset_withdrawal_divisor)
-                .ok_or(eyre::eyre!(
-                    "failed to divide amount by asset withdrawal multiplier"
-                ))?,
-            memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?,
-            fee_asset,
-            // 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: ibc_types::core::client::Height::new(u64::MAX, u64::MAX)
-                .wrap_err("failed to generate timeout height")?,
-            timeout_time: packet_timeout_time()
-                .wrap_err("failed to calculate packet timeout time")?,
-            source_channel,
-            bridge_address: Some(bridge_address),
-        };
-        Ok(Action::Ics20Withdrawal(action))
-    }
-}
-
-#[derive(Debug)]
-struct LogToSequencerWithdrawalAction {
-    log: Log,
-    fee_asset: asset::Denom,
-    asset_withdrawal_divisor: u128,
-    bridge_address: Address,
-}
-
-impl LogToSequencerWithdrawalAction {
-    fn try_into_action(self) -> eyre::Result<Action> {
-        let Self {
-            log,
-            fee_asset,
-            asset_withdrawal_divisor,
-            bridge_address,
-        } = self;
-        let (event, block_number, transaction_hash) =
-            action_inputs_from_log::<SequencerWithdrawalFilter>(log)
-                .wrap_err("failed getting required data from log")?;
-
-        let memo = bridge::UnlockMemo {
-            block_number,
-            transaction_hash,
-        };
-
-        let action = BridgeUnlockAction {
-            to: event
-                .destination_chain_address
-                .parse()
-                .wrap_err("failed to parse destination chain address")?,
-            amount: event
-                .amount
-                .as_u128()
-                .checked_div(asset_withdrawal_divisor)
-                .ok_or_eyre("failed to divide amount by asset withdrawal multiplier")?,
-            memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?,
-            fee_asset,
-            bridge_address: Some(bridge_address),
-        };
-
-        Ok(Action::BridgeUnlock(action))
-    }
-}
-
-async fn get_log<T: EthEvent>(
-    provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
-    block_hash: H256,
-) -> eyre::Result<Vec<Log>> {
-    let event_sig = T::signature();
-    let filter = Filter::new()
-        .at_block_hash(block_hash)
-        .address(contract_address)
-        .topic0(event_sig);
-
-    provider
-        .get_logs(&filter)
-        .await
-        .wrap_err("failed to get sequencer withdrawal events")
-}

From b414c8faecf20e6cee620409a77d9fb99af791d6 Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 11:12:17 +0200
Subject: [PATCH 3/6] allow getting sequencer or ics20 or both

---
 charts/deploy.just                            |   2 +-
 crates/astria-bridge-contracts/src/lib.rs     | 146 +++++++++++++-----
 .../astria-cli/src/commands/bridge/collect.rs |  23 ++-
 3 files changed, 129 insertions(+), 42 deletions(-)

diff --git a/charts/deploy.just b/charts/deploy.just
index 520270f195..846bba7e29 100644
--- a/charts/deploy.just
+++ b/charts/deploy.just
@@ -388,7 +388,7 @@ run-smoke-cli:
     --contract-address {{evm_contract_address}} \
     --from-rollup-height 1 \
     --to-rollup-height $CURRENT_BLOCK \
-    --rollup-asset-denom nria \
+    --sequencer-asset-to-withdraw nria \
     --bridge-address {{sequencer_bridge_address}} \
     --output ./withdrawals.json
    astria-cli bridge submit-withdrawals \
diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs
index 40d8824131..d69f7ca357 100644
--- a/crates/astria-bridge-contracts/src/lib.rs
+++ b/crates/astria-bridge-contracts/src/lib.rs
@@ -56,18 +56,22 @@ impl BuildError {
         })
     }
 
+    pub fn no_withdraws_configured() -> Self {
+        Self(BuildErrorKind::NoWithdrawsConfigured)
+    }
+
     fn not_set(field: &'static str) -> Self {
         Self(BuildErrorKind::NotSet {
             field,
         })
     }
 
-    fn rollup_asset_without_channel() -> Self {
-        Self(BuildErrorKind::RollupAssetWithoutChannel)
+    fn ics20_asset_without_channel() -> Self {
+        Self(BuildErrorKind::Ics20AssetWithoutChannel)
     }
 
-    fn parse_rollup_asset_source_channel(source: ibc_types::IdentifierError) -> Self {
-        Self(BuildErrorKind::ParseRollupAssetSourceChannel {
+    fn parse_ics20_asset_source_channel(source: ibc_types::IdentifierError) -> Self {
+        Self(BuildErrorKind::ParseIcs20AssetSourceChannel {
             source,
         })
     }
@@ -83,14 +87,19 @@ enum BuildErrorKind {
     BadDivisor { base_chain_asset_precision: u32 },
     #[error("required option `{field}` not set")]
     NotSet { field: &'static str },
+    #[error(
+        "getting withdraws actions must be configured for one of sequencer or ics20 (or both); \
+         neither was set"
+    )]
+    NoWithdrawsConfigured,
     #[error("failed to call the `BASE_CHAIN_ASSET_PRECISION` of the provided contract")]
     CallBaseChainAssetPrecision {
         source: Box<dyn std::error::Error + Send + Sync + 'static>,
     },
-    #[error("rollup asset denom must have a channel to be withdrawn via IBC")]
-    RollupAssetWithoutChannel,
-    #[error("could not parse rollup asset channel as channel ID")]
-    ParseRollupAssetSourceChannel { source: ibc_types::IdentifierError },
+    #[error("ics20 asset must have a channel to be withdrawn via IBC")]
+    Ics20AssetWithoutChannel,
+    #[error("could not parse ics20 asset channel as channel ID")]
+    ParseIcs20AssetSourceChannel { source: ibc_types::IdentifierError },
 }
 
 pub struct NoProvider;
@@ -101,7 +110,8 @@ pub struct GetWithdrawalActionsBuilder<TProvider = NoProvider> {
     contract_address: Option<ethers::types::Address>,
     bridge_address: Option<Address>,
     fee_asset: Option<asset::Denom>,
-    rollup_asset_denom: Option<asset::Denom>,
+    sequencer_asset_to_withdraw: Option<asset::Denom>,
+    ics20_asset_to_withdraw: Option<asset::Denom>,
 }
 
 impl Default for GetWithdrawalActionsBuilder {
@@ -117,7 +127,8 @@ impl GetWithdrawalActionsBuilder {
             contract_address: None,
             bridge_address: None,
             fee_asset: None,
-            rollup_asset_denom: None,
+            sequencer_asset_to_withdraw: None,
+            ics20_asset_to_withdraw: None,
         }
     }
 }
@@ -128,7 +139,8 @@ impl<P> GetWithdrawalActionsBuilder<P> {
             contract_address,
             bridge_address,
             fee_asset,
-            rollup_asset_denom,
+            sequencer_asset_to_withdraw,
+            ics20_asset_to_withdraw,
             ..
         } = self;
         GetWithdrawalActionsBuilder {
@@ -136,7 +148,8 @@ impl<P> GetWithdrawalActionsBuilder<P> {
             contract_address,
             bridge_address,
             fee_asset,
-            rollup_asset_denom,
+            sequencer_asset_to_withdraw,
+            ics20_asset_to_withdraw,
         }
     }
 
@@ -161,9 +174,30 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
-    pub fn rollup_asset_denom(self, rollup_asset_denom: asset::Denom) -> Self {
+    pub fn sequencer_asset_to_withdraw(self, sequencer_asset_to_withdraw: asset::Denom) -> Self {
+        self.set_sequencer_asset_to_withdraw(Some(sequencer_asset_to_withdraw))
+    }
+
+    pub fn set_sequencer_asset_to_withdraw(
+        self,
+        sequencer_asset_to_withdraw: Option<asset::Denom>,
+    ) -> Self {
+        Self {
+            sequencer_asset_to_withdraw,
+            ..self
+        }
+    }
+
+    pub fn ics20_asset_to_withdraw(self, ics20_asset_to_withdraw: asset::Denom) -> Self {
+        self.set_ics20_asset_to_withdraw(Some(ics20_asset_to_withdraw))
+    }
+
+    pub fn set_ics20_asset_to_withdraw(
+        self,
+        ics20_asset_to_withdraw: Option<asset::Denom>,
+    ) -> Self {
         Self {
-            rollup_asset_denom: Some(rollup_asset_denom),
+            ics20_asset_to_withdraw,
             ..self
         }
     }
@@ -180,7 +214,8 @@ where
             contract_address,
             bridge_address,
             fee_asset,
-            rollup_asset_denom,
+            sequencer_asset_to_withdraw,
+            ics20_asset_to_withdraw,
         } = self;
 
         let Some(contract_address) = contract_address else {
@@ -192,16 +227,22 @@ where
         let Some(fee_asset) = fee_asset else {
             return Err(BuildError::not_set("fee_asset"));
         };
-        let Some(rollup_asset_denom) = rollup_asset_denom else {
-            return Err(BuildError::not_set("rollup_asset_denom"));
-        };
 
-        let rollup_asset_source_channel = rollup_asset_denom
-            .as_trace_prefixed()
-            .and_then(TracePrefixed::last_channel)
-            .ok_or(BuildError::rollup_asset_without_channel())?
-            .parse()
-            .map_err(BuildError::parse_rollup_asset_source_channel)?;
+        if sequencer_asset_to_withdraw.is_none() && ics20_asset_to_withdraw.is_none() {
+            return Err(BuildError::no_withdraws_configured());
+        }
+
+        let mut ics20_source_channel = None;
+        if let Some(ics20_asset_to_withdraw) = &ics20_asset_to_withdraw {
+            ics20_source_channel.replace(
+                ics20_asset_to_withdraw
+                    .as_trace_prefixed()
+                    .and_then(TracePrefixed::last_channel)
+                    .ok_or(BuildError::ics20_asset_without_channel())?
+                    .parse()
+                    .map_err(BuildError::parse_ics20_asset_source_channel)?,
+            );
+        };
 
         let contract =
             i_astria_withdrawer::IAstriaWithdrawer::new(contract_address, provider.clone());
@@ -224,8 +265,9 @@ where
             asset_withdrawal_divisor,
             bridge_address,
             fee_asset,
-            rollup_asset_denom,
-            rollup_asset_source_channel,
+            sequencer_asset_to_withdraw,
+            ics20_asset_to_withdraw,
+            ics20_source_channel,
         })
     }
 }
@@ -236,8 +278,9 @@ pub struct GetWithdrawalActions<P> {
     asset_withdrawal_divisor: u128,
     bridge_address: Address,
     fee_asset: asset::Denom,
-    rollup_asset_denom: asset::Denom,
-    rollup_asset_source_channel: ibc_types::core::channel::ChannelId,
+    sequencer_asset_to_withdraw: Option<asset::Denom>,
+    ics20_asset_to_withdraw: Option<asset::Denom>,
+    ics20_source_channel: Option<ibc_types::core::channel::ChannelId>,
 }
 
 impl<P> GetWithdrawalActions<P>
@@ -245,21 +288,43 @@ where
     P: Middleware,
     P::Error: std::error::Error + 'static,
 {
+    fn configured_for_sequencer_withdrawals(&self) -> bool {
+        self.sequencer_asset_to_withdraw.is_some()
+    }
+
+    fn configured_for_ics20_withdrawals(&self) -> bool {
+        self.ics20_asset_to_withdraw.is_some()
+    }
+
     pub async fn get_for_block_hash(
         &self,
         block_hash: H256,
     ) -> Result<Vec<Action>, GetWithdrawalActionsError> {
-        let (ics20_logs, sequencer_logs) = futures::future::try_join(
-            get_logs::<Ics20WithdrawalFilter, _>(&self.provider, self.contract_address, block_hash),
+        use futures::FutureExt as _;
+        let get_ics20_logs = if self.configured_for_ics20_withdrawals() {
+            get_logs::<Ics20WithdrawalFilter, _>(&self.provider, self.contract_address, block_hash)
+                .boxed()
+        } else {
+            futures::future::ready(Ok(vec![])).boxed()
+        };
+        let get_sequencer_logs = if self.configured_for_sequencer_withdrawals() {
             get_logs::<SequencerWithdrawalFilter, _>(
                 &self.provider,
                 self.contract_address,
                 block_hash,
-            ),
-        )
-        .await
-        .map_err(GetWithdrawalActionsError::get_logs)?;
-
+            )
+            .boxed()
+        } else {
+            futures::future::ready(Ok(vec![])).boxed()
+        };
+        let (ics20_logs, sequencer_logs) =
+            futures::future::try_join(get_ics20_logs, get_sequencer_logs)
+                .await
+                .map_err(GetWithdrawalActionsError::get_logs)?;
+
+        // XXX: The calls to `log_to_*_action` rely on only be called if `GetWithdrawalActions`
+        // is configured for either ics20 or sequencer withdrawals (or both). They would panic
+        // otherwise.
         ics20_logs
             .into_iter()
             .map(|log| self.log_to_ics20_withdrawal_action(log))
@@ -288,7 +353,14 @@ where
         let event = decode_log::<Ics20WithdrawalFilter>(log)
             .map_err(GetWithdrawalActionsError::decode_log)?;
 
-        let source_channel = self.rollup_asset_source_channel.clone();
+        let (denom, source_channel) = (
+            self.ics20_asset_to_withdraw
+                .clone()
+                .expect("must be set if this method is entered"),
+            self.ics20_source_channel
+                .clone()
+                .expect("must be set if this method is entered"),
+        );
 
         let memo = serde_json::to_string(&astria_core::bridge::Ics20WithdrawalFromRollupMemo {
             memo: event.memo.clone(),
@@ -304,7 +376,7 @@ where
             .map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?;
 
         let action = Ics20Withdrawal {
-            denom: self.rollup_asset_denom.clone(),
+            denom,
             destination_chain_address: event.destination_chain_address,
             return_address: self.bridge_address,
             amount,
diff --git a/crates/astria-cli/src/commands/bridge/collect.rs b/crates/astria-cli/src/commands/bridge/collect.rs
index 479ce046d3..8401287392 100644
--- a/crates/astria-cli/src/commands/bridge/collect.rs
+++ b/crates/astria-cli/src/commands/bridge/collect.rs
@@ -68,9 +68,12 @@ pub(crate) struct WithdrawalEvents {
     /// actions be submitted to the Sequencer).
     #[arg(long, default_value = "nria")]
     fee_asset: asset::Denom,
-    /// The asset denomination of the asset that's withdrawn from the bridge.
+    /// The sequencer asset withdrawn through the bridge.
     #[arg(long)]
-    rollup_asset_denom: asset::Denom,
+    sequencer_asset_to_withdraw: Option<asset::Denom>,
+    /// The is20 asset withdrawn through the bridge.
+    #[arg(long)]
+    ics20_asset_to_withdraw: Option<asset::Denom>,
     /// The bech32-encoded bridge address corresponding to the bridged rollup
     ///  asset on the sequencer. Should match the bridge address in the geth
     /// rollup's bridge configuration for that asset.
@@ -89,8 +92,9 @@ impl WithdrawalEvents {
             contract_address,
             from_rollup_height,
             to_rollup_height,
+            sequencer_asset_to_withdraw,
+            ics20_asset_to_withdraw,
             fee_asset,
-            rollup_asset_denom,
             bridge_address,
             output,
         } = self;
@@ -105,7 +109,8 @@ impl WithdrawalEvents {
             .provider(block_provider.clone())
             .contract_address(contract_address)
             .fee_asset(fee_asset)
-            .rollup_asset_denom(rollup_asset_denom)
+            .set_ics20_asset_to_withdraw(ics20_asset_to_withdraw)
+            .set_sequencer_asset_to_withdraw(sequencer_asset_to_withdraw)
             .bridge_address(bridge_address)
             .try_build()
             .await
@@ -157,6 +162,16 @@ impl WithdrawalEvents {
             }
         }
 
+        info!(
+            "collected a total of {} actions across {} rollup heights; writing to file",
+            actions_by_rollup_height
+                .0
+                .values()
+                .map(Vec::len)
+                .sum::<usize>(),
+            actions_by_rollup_height.0.len(),
+        );
+
         actions_by_rollup_height
             .write_to_output(output)
             .wrap_err("failed to write actions to file")

From 72e88e21a6006d482f55a090bca26587d3245e2d Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 12:02:00 +0200
Subject: [PATCH 4/6] fix bridge withdrawer

---
 .../src/bridge_withdrawer/ethereum/convert.rs | 326 -------
 .../src/bridge_withdrawer/ethereum/mod.rs     |   4 -
 .../bridge_withdrawer/ethereum/test_utils.rs  | 205 ----
 .../src/bridge_withdrawer/ethereum/watcher.rs | 876 ++----------------
 4 files changed, 58 insertions(+), 1353 deletions(-)
 delete mode 100644 crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs
 delete mode 100644 crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs

diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs
deleted file mode 100644
index dab2828183..0000000000
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs
+++ /dev/null
@@ -1,326 +0,0 @@
-use std::time::Duration;
-
-use astria_bridge_contracts::i_astria_withdrawer::{
-    Ics20WithdrawalFilter,
-    SequencerWithdrawalFilter,
-};
-use astria_core::{
-    bridge::{
-        self,
-        Ics20WithdrawalFromRollupMemo,
-    },
-    primitive::v1::{
-        asset::{
-            self,
-            denom::TracePrefixed,
-        },
-        Address,
-    },
-    protocol::transaction::v1alpha1::{
-        action::{
-            BridgeUnlockAction,
-            Ics20Withdrawal,
-        },
-        Action,
-    },
-};
-use astria_eyre::eyre::{
-    self,
-    OptionExt,
-    WrapErr as _,
-};
-use ethers::types::{
-    TxHash,
-    U64,
-};
-use ibc_types::core::client::Height as IbcHeight;
-
-#[derive(Debug, PartialEq, Eq)]
-pub(crate) enum WithdrawalEvent {
-    Sequencer(SequencerWithdrawalFilter),
-    Ics20(Ics20WithdrawalFilter),
-}
-
-#[derive(Debug, PartialEq, Eq)]
-pub(crate) struct EventWithMetadata {
-    pub(crate) event: WithdrawalEvent,
-    /// The block in which the log was emitted
-    pub(crate) block_number: U64,
-    /// The transaction hash in which the log was emitted
-    pub(crate) transaction_hash: TxHash,
-}
-
-pub(crate) fn event_to_action(
-    event_with_metadata: EventWithMetadata,
-    fee_asset: asset::Denom,
-    rollup_asset_denom: asset::Denom,
-    asset_withdrawal_divisor: u128,
-    bridge_address: Address,
-) -> eyre::Result<Action> {
-    let action = match event_with_metadata.event {
-        WithdrawalEvent::Sequencer(event) => event_to_bridge_unlock(
-            &event,
-            event_with_metadata.block_number,
-            event_with_metadata.transaction_hash,
-            fee_asset,
-            asset_withdrawal_divisor,
-            bridge_address,
-        )
-        .wrap_err("failed to convert sequencer withdrawal event to action")?,
-        WithdrawalEvent::Ics20(event) => event_to_ics20_withdrawal(
-            event,
-            event_with_metadata.block_number,
-            event_with_metadata.transaction_hash,
-            fee_asset,
-            rollup_asset_denom,
-            asset_withdrawal_divisor,
-            bridge_address,
-        )
-        .wrap_err("failed to convert ics20 withdrawal event to action")?,
-    };
-    Ok(action)
-}
-
-fn event_to_bridge_unlock(
-    event: &SequencerWithdrawalFilter,
-    block_number: U64,
-    transaction_hash: TxHash,
-    fee_asset: asset::Denom,
-    asset_withdrawal_divisor: u128,
-    bridge_address: Address,
-) -> eyre::Result<Action> {
-    let memo = bridge::UnlockMemo {
-        // XXX: The documentation mentions that the ethers U64 type will panic if it cannot be
-        // converted to u64. However, this is part of a catch-all documentation that does not apply
-        // to U64.
-        block_number: block_number.as_u64(),
-        transaction_hash: transaction_hash.into(),
-    };
-    let action = BridgeUnlockAction {
-        to: event
-            .destination_chain_address
-            .parse()
-            .wrap_err("failed to parse destination chain address")?,
-        amount: event
-            .amount
-            .as_u128()
-            .checked_div(asset_withdrawal_divisor)
-            .ok_or(eyre::eyre!(
-                "failed to divide amount by asset withdrawal multiplier"
-            ))?,
-        memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?,
-        fee_asset,
-        bridge_address: Some(bridge_address),
-    };
-
-    Ok(Action::BridgeUnlock(action))
-}
-
-// FIXME: Get this to work for now, but replace this with a builder.
-#[allow(clippy::too_many_arguments)]
-fn event_to_ics20_withdrawal(
-    event: Ics20WithdrawalFilter,
-    block_number: U64,
-    transaction_hash: TxHash,
-    fee_asset: asset::Denom,
-    rollup_asset_denom: asset::Denom,
-    asset_withdrawal_divisor: u128,
-    bridge_address: Address,
-) -> eyre::Result<Action> {
-    // TODO: make this configurable
-    const ICS20_WITHDRAWAL_TIMEOUT: Duration = Duration::from_secs(300);
-
-    let denom = rollup_asset_denom.clone();
-
-    let channel = denom
-        .as_trace_prefixed()
-        .and_then(TracePrefixed::last_channel)
-        .ok_or_eyre("denom must have a channel to be withdrawn via IBC")?;
-
-    let memo = Ics20WithdrawalFromRollupMemo {
-        memo: event.memo,
-        block_number: block_number.as_u64(),
-        rollup_return_address: event.sender.to_string(),
-        transaction_hash: transaction_hash.into(),
-    };
-
-    let action = Ics20Withdrawal {
-        denom: rollup_asset_denom,
-        destination_chain_address: event.destination_chain_address,
-        return_address: bridge_address,
-        amount: event
-            .amount
-            .as_u128()
-            .checked_div(asset_withdrawal_divisor)
-            .ok_or(eyre::eyre!(
-                "failed to divide amount by asset withdrawal multiplier"
-            ))?,
-        memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?,
-        fee_asset,
-        // 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: IbcHeight::new(u64::MAX, u64::MAX)
-            .wrap_err("failed to generate timeout height")?,
-        timeout_time: calculate_packet_timeout_time(ICS20_WITHDRAWAL_TIMEOUT)
-            .wrap_err("failed to calculate packet timeout time")?,
-        source_channel: channel
-            .parse()
-            .wrap_err("failed to parse channel from denom")?,
-        bridge_address: Some(bridge_address),
-    };
-    Ok(Action::Ics20Withdrawal(action))
-}
-
-fn calculate_packet_timeout_time(timeout_delta: Duration) -> eyre::Result<u64> {
-    tendermint::Time::now()
-        .checked_add(timeout_delta)
-        .ok_or_eyre("time must not overflow from now plus 10 minutes")?
-        .unix_timestamp_nanos()
-        .try_into()
-        .wrap_err("failed to convert packet timeout i128 to u64")
-}
-
-#[cfg(test)]
-mod tests {
-    use astria_bridge_contracts::i_astria_withdrawer::SequencerWithdrawalFilter;
-
-    use super::*;
-
-    fn default_native_asset() -> asset::Denom {
-        "nria".parse().unwrap()
-    }
-
-    #[test]
-    fn event_to_bridge_unlock() {
-        let denom = default_native_asset();
-        let event_with_meta = EventWithMetadata {
-            event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
-                sender: [0u8; 20].into(),
-                amount: 99.into(),
-                destination_chain_address: crate::astria_address([1u8; 20]).to_string(),
-            }),
-            block_number: 1.into(),
-            transaction_hash: [2u8; 32].into(),
-        };
-        let bridge_address = crate::astria_address([99u8; 20]);
-        let action = event_to_action(
-            event_with_meta,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap();
-        let Action::BridgeUnlock(action) = action else {
-            panic!("expected BridgeUnlock action, got {action:?}");
-        };
-
-        let expected_action = BridgeUnlockAction {
-            to: crate::astria_address([1u8; 20]),
-            amount: 99,
-            memo: serde_json::to_string(&bridge::UnlockMemo {
-                block_number: 1,
-                transaction_hash: [2u8; 32],
-            })
-            .unwrap(),
-            fee_asset: denom,
-            bridge_address: Some(bridge_address),
-        };
-
-        assert_eq!(action, expected_action);
-    }
-
-    #[test]
-    fn event_to_bridge_unlock_divide_value() {
-        let denom = default_native_asset();
-        let event_with_meta = EventWithMetadata {
-            event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
-                sender: [0u8; 20].into(),
-                amount: 990.into(),
-                destination_chain_address: crate::astria_address([1u8; 20]).to_string(),
-            }),
-            block_number: 1.into(),
-            transaction_hash: [2u8; 32].into(),
-        };
-        let divisor = 10;
-        let bridge_address = crate::astria_address([99u8; 20]);
-        let action = event_to_action(
-            event_with_meta,
-            denom.clone(),
-            denom.clone(),
-            divisor,
-            bridge_address,
-        )
-        .unwrap();
-        let Action::BridgeUnlock(action) = action else {
-            panic!("expected BridgeUnlock action, got {action:?}");
-        };
-
-        let expected_action = BridgeUnlockAction {
-            to: crate::astria_address([1u8; 20]),
-            amount: 99,
-            memo: serde_json::to_string(&bridge::UnlockMemo {
-                block_number: 1,
-                transaction_hash: [2u8; 32],
-            })
-            .unwrap(),
-            fee_asset: denom,
-            bridge_address: Some(bridge_address),
-        };
-
-        assert_eq!(action, expected_action);
-    }
-
-    #[test]
-    fn event_to_ics20_withdrawal() {
-        let denom = "transfer/channel-0/utia".parse::<asset::Denom>().unwrap();
-        let destination_chain_address = crate::astria_address([1u8; 20]).to_string();
-        let event_with_meta = EventWithMetadata {
-            event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter {
-                sender: [0u8; 20].into(),
-                amount: 99.into(),
-                destination_chain_address: destination_chain_address.clone(),
-                memo: "hello".to_string(),
-            }),
-            block_number: 1.into(),
-            transaction_hash: [2u8; 32].into(),
-        };
-
-        let bridge_address = crate::astria_address([99u8; 20]);
-        let action = event_to_action(
-            event_with_meta,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap();
-        let Action::Ics20Withdrawal(mut action) = action else {
-            panic!("expected Ics20Withdrawal action, got {action:?}");
-        };
-
-        // TODO: instead of zeroing this, we should pass in the latest block time to the function
-        // and generate the timeout time from that.
-        action.timeout_time = 0; // zero this for testing
-
-        let expected_action = Ics20Withdrawal {
-            denom: denom.clone(),
-            destination_chain_address,
-            return_address: bridge_address,
-            amount: 99,
-            memo: serde_json::to_string(&Ics20WithdrawalFromRollupMemo {
-                memo: "hello".to_string(),
-                block_number: 1u64,
-                rollup_return_address: ethers::types::Address::from([0u8; 20]).to_string(),
-                transaction_hash: [2u8; 32],
-            })
-            .unwrap(),
-            fee_asset: denom,
-            timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(),
-            timeout_time: 0, // zero this for testing
-            source_channel: "channel-0".parse().unwrap(),
-            bridge_address: Some(bridge_address),
-        };
-        assert_eq!(action, expected_action);
-    }
-}
diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs
index 8216a66bc7..1d889b863e 100644
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs
+++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs
@@ -1,5 +1 @@
-pub(crate) mod convert;
 pub(crate) mod watcher;
-
-#[cfg(test)]
-mod test_utils;
diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs
deleted file mode 100644
index e63fb5b4cd..0000000000
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs
+++ /dev/null
@@ -1,205 +0,0 @@
-use std::{
-    sync::Arc,
-    time::Duration,
-};
-
-use astria_bridge_contracts::{
-    astria_bridgeable_erc20::{
-        ASTRIABRIDGEABLEERC20_ABI,
-        ASTRIABRIDGEABLEERC20_BYTECODE,
-    },
-    astria_withdrawer::{
-        ASTRIAWITHDRAWER_ABI,
-        ASTRIAWITHDRAWER_BYTECODE,
-    },
-};
-use ethers::{
-    abi::Tokenizable,
-    core::utils::Anvil,
-    prelude::*,
-    utils::AnvilInstance,
-};
-
-#[allow(clippy::struct_field_names)]
-pub(crate) struct ConfigureAstriaWithdrawerDeployer {
-    pub(crate) base_chain_asset_precision: u32,
-    pub(crate) base_chain_bridge_address: astria_core::primitive::v1::Address,
-    pub(crate) base_chain_asset_denomination: String,
-}
-
-impl Default for ConfigureAstriaWithdrawerDeployer {
-    fn default() -> Self {
-        Self {
-            base_chain_asset_precision: 18,
-            base_chain_bridge_address: crate::astria_address([0u8; 20]),
-            base_chain_asset_denomination: "test-denom".to_string(),
-        }
-    }
-}
-
-impl ConfigureAstriaWithdrawerDeployer {
-    pub(crate) async fn deploy(self) -> (Address, Arc<Provider<Ws>>, LocalWallet, AnvilInstance) {
-        let Self {
-            base_chain_asset_precision,
-            base_chain_bridge_address,
-            base_chain_asset_denomination,
-        } = self;
-
-        deploy_astria_withdrawer(
-            base_chain_asset_precision.into(),
-            base_chain_bridge_address,
-            base_chain_asset_denomination,
-        )
-        .await
-    }
-}
-
-/// Starts a local anvil instance and deploys the `AstriaWithdrawer` contract to it.
-///
-/// Returns the contract address, provider, wallet, and anvil instance.
-///
-/// # Panics
-///
-/// - if the provider fails to connect to the anvil instance
-/// - if the contract fails to deploy
-pub(crate) async fn deploy_astria_withdrawer(
-    base_chain_asset_precision: U256,
-    base_chain_bridge_address: astria_core::primitive::v1::Address,
-    base_chain_asset_denomination: String,
-) -> (Address, Arc<Provider<Ws>>, LocalWallet, AnvilInstance) {
-    // setup anvil and signing wallet
-    let anvil = Anvil::new().spawn();
-    let wallet: LocalWallet = anvil.keys()[0].clone().into();
-    let provider = Arc::new(
-        Provider::<Ws>::connect(anvil.ws_endpoint())
-            .await
-            .unwrap()
-            .interval(Duration::from_millis(10u64)),
-    );
-    let signer = SignerMiddleware::new(
-        provider.clone(),
-        wallet.clone().with_chain_id(anvil.chain_id()),
-    );
-
-    let abi = ASTRIAWITHDRAWER_ABI.clone();
-    let bytecode = ASTRIAWITHDRAWER_BYTECODE.to_vec();
-
-    let args = vec![
-        base_chain_asset_precision.into_token(),
-        base_chain_bridge_address.to_string().into_token(),
-        base_chain_asset_denomination.into_token(),
-    ];
-
-    let factory = ContractFactory::new(abi.clone(), bytecode.into(), signer.into());
-    let contract = factory.deploy_tokens(args).unwrap().send().await.unwrap();
-    let contract_address = contract.address();
-
-    (
-        contract_address,
-        provider,
-        wallet.with_chain_id(anvil.chain_id()),
-        anvil,
-    )
-}
-
-pub(crate) struct ConfigureAstriaBridgeableERC20Deployer {
-    pub(crate) bridge_address: Address,
-    pub(crate) base_chain_asset_precision: u32,
-    pub(crate) base_chain_bridge_address: astria_core::primitive::v1::Address,
-    pub(crate) base_chain_asset_denomination: String,
-    pub(crate) name: String,
-    pub(crate) symbol: String,
-}
-
-impl Default for ConfigureAstriaBridgeableERC20Deployer {
-    fn default() -> Self {
-        Self {
-            bridge_address: Address::zero(),
-            base_chain_asset_precision: 18,
-            base_chain_bridge_address: crate::astria_address([0u8; 20]),
-            base_chain_asset_denomination: "testdenom".to_string(),
-            name: "test-token".to_string(),
-            symbol: "TT".to_string(),
-        }
-    }
-}
-
-impl ConfigureAstriaBridgeableERC20Deployer {
-    pub(crate) async fn deploy(self) -> (Address, Arc<Provider<Ws>>, LocalWallet, AnvilInstance) {
-        let Self {
-            bridge_address,
-            base_chain_asset_precision,
-            base_chain_bridge_address,
-            base_chain_asset_denomination,
-            name,
-            symbol,
-        } = self;
-
-        deploy_astria_bridgeable_erc20(
-            bridge_address,
-            base_chain_asset_precision.into(),
-            base_chain_bridge_address,
-            base_chain_asset_denomination,
-            name,
-            symbol,
-        )
-        .await
-    }
-}
-
-/// Starts a local anvil instance and deploys the `AstriaBridgeableERC20` contract to it.
-///
-/// Returns the contract address, provider, wallet, and anvil instance.
-///
-/// # Panics
-///
-/// - if the provider fails to connect to the anvil instance
-/// - if the contract fails to deploy
-pub(crate) async fn deploy_astria_bridgeable_erc20(
-    mut bridge_address: Address,
-    base_chain_asset_precision: ethers::abi::Uint,
-    base_chain_bridge_address: astria_core::primitive::v1::Address,
-    base_chain_asset_denomination: String,
-    name: String,
-    symbol: String,
-) -> (Address, Arc<Provider<Ws>>, LocalWallet, AnvilInstance) {
-    // setup anvil and signing wallet
-    let anvil = Anvil::new().spawn();
-    let wallet: LocalWallet = anvil.keys()[0].clone().into();
-    let provider = Arc::new(
-        Provider::<Ws>::connect(anvil.ws_endpoint())
-            .await
-            .unwrap()
-            .interval(Duration::from_millis(10u64)),
-    );
-    let signer = SignerMiddleware::new(
-        provider.clone(),
-        wallet.clone().with_chain_id(anvil.chain_id()),
-    );
-
-    let abi = ASTRIABRIDGEABLEERC20_ABI.clone();
-    let bytecode = ASTRIABRIDGEABLEERC20_BYTECODE.to_vec();
-
-    let factory = ContractFactory::new(abi.clone(), bytecode.into(), signer.into());
-
-    if bridge_address == Address::zero() {
-        bridge_address = wallet.address();
-    }
-    let args = vec![
-        bridge_address.into_token(),
-        base_chain_asset_precision.into_token(),
-        base_chain_bridge_address.to_string().into_token(),
-        base_chain_asset_denomination.into_token(),
-        name.into_token(),
-        symbol.into_token(),
-    ];
-    let contract = factory.deploy_tokens(args).unwrap().send().await.unwrap();
-    let contract_address = contract.address();
-
-    (
-        contract_address,
-        provider,
-        wallet.with_chain_id(anvil.chain_id()),
-        anvil,
-    )
-}
diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
index 4e24918000..0e00392cb7 100644
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
+++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
@@ -3,33 +3,25 @@ use std::{
     time::Duration,
 };
 
-use astria_bridge_contracts::i_astria_withdrawer::{
-    IAstriaWithdrawer,
-    Ics20WithdrawalFilter,
-    SequencerWithdrawalFilter,
+use astria_bridge_contracts::{
+    GetWithdrawalActions,
+    GetWithdrawalActionsBuilder,
 };
-use astria_core::{
-    primitive::v1::{
-        asset::{
-            self,
-            denom,
-            Denom,
-        },
-        Address,
-    },
-    protocol::transaction::v1alpha1::Action,
+use astria_core::primitive::v1::{
+    asset,
+    Address,
 };
 use astria_eyre::{
     eyre::{
         self,
         bail,
         eyre,
+        OptionExt as _,
         WrapErr as _,
     },
     Result,
 };
 use ethers::{
-    contract::EthEvent as _,
     core::types::Block,
     providers::{
         Middleware,
@@ -38,29 +30,18 @@ use ethers::{
         StreamExt as _,
         Ws,
     },
-    types::{
-        Filter,
-        Log,
-        H256,
-    },
+    types::H256,
     utils::hex,
 };
 use tokio::select;
 use tokio_util::sync::CancellationToken;
 use tracing::{
-    debug,
     info,
-    trace,
     warn,
 };
 
 use crate::bridge_withdrawer::{
     batch::Batch,
-    ethereum::convert::{
-        event_to_action,
-        EventWithMetadata,
-        WithdrawalEvent,
-    },
     startup,
     state::State,
     submitter,
@@ -72,7 +53,7 @@ pub(crate) struct Builder {
     pub(crate) ethereum_contract_address: String,
     pub(crate) ethereum_rpc_endpoint: String,
     pub(crate) state: Arc<State>,
-    pub(crate) rollup_asset_denom: Denom,
+    pub(crate) rollup_asset_denom: asset::Denom,
     pub(crate) bridge_address: Address,
     pub(crate) submitter_handle: submitter::Handle,
 }
@@ -93,16 +74,6 @@ impl Builder {
         let contract_address = address_from_string(&ethereum_contract_address)
             .wrap_err("failed to parse ethereum contract address")?;
 
-        if rollup_asset_denom
-            .as_trace_prefixed()
-            .map_or(false, denom::TracePrefixed::trace_is_empty)
-        {
-            warn!(
-                "rollup asset denomination is not prefixed; Ics20Withdrawal actions will not be \
-                 submitted"
-            );
-        }
-
         Ok(Watcher {
             contract_address,
             ethereum_rpc_endpoint: ethereum_rpc_endpoint.to_string(),
@@ -123,42 +94,32 @@ pub(crate) struct Watcher {
     submitter_handle: submitter::Handle,
     contract_address: ethers::types::Address,
     ethereum_rpc_endpoint: String,
-    rollup_asset_denom: Denom,
+    rollup_asset_denom: asset::Denom,
     bridge_address: Address,
     state: Arc<State>,
 }
 
 impl Watcher {
     pub(crate) async fn run(mut self) -> Result<()> {
-        let (provider, contract, fee_asset, asset_withdrawal_divisor, next_rollup_block_height) =
-            self.startup()
-                .await
-                .wrap_err("watcher failed to start up")?;
+        let (provider, action_fetcher, next_rollup_block_height) = self
+            .startup()
+            .await
+            .wrap_err("watcher failed to start up")?;
 
         let Self {
-            rollup_asset_denom,
-            bridge_address,
             state,
             shutdown_token,
             submitter_handle,
             ..
         } = self;
 
-        let converter = EventToActionConvertConfig {
-            fee_asset,
-            rollup_asset_denom,
-            bridge_address,
-            asset_withdrawal_divisor,
-        };
-
         state.set_watcher_ready();
 
         tokio::select! {
             res = watch_for_blocks(
                 provider,
-                contract.address(),
+                action_fetcher,
                 next_rollup_block_height,
-                converter,
                 submitter_handle,
                 shutdown_token.clone(),
             ) => {
@@ -183,13 +144,7 @@ impl Watcher {
     /// - If the asset withdrawal decimals cannot be fetched.
     async fn startup(
         &mut self,
-    ) -> eyre::Result<(
-        Arc<Provider<Ws>>,
-        IAstriaWithdrawer<Provider<Ws>>,
-        asset::Denom,
-        u128,
-        u64,
-    )> {
+    ) -> eyre::Result<(Arc<Provider<Ws>>, GetWithdrawalActions<Provider<Ws>>, u64)> {
         let startup::Info {
             fee_asset,
             starting_rollup_height,
@@ -233,39 +188,27 @@ impl Watcher {
         .with_config(retry_config)
         .await
         .wrap_err("failed connecting to rollup after several retries; giving up")?;
-        let provider = Arc::new(provider);
-
-        // get contract handle
-        let contract = IAstriaWithdrawer::new(self.contract_address, provider.clone());
 
-        // get asset withdrawal decimals
-        let base_chain_asset_precision = contract
-            .base_chain_asset_precision()
-            .call()
+        let provider = Arc::new(provider);
+        let action_fetcher = GetWithdrawalActionsBuilder::new()
+            .provider(provider.clone())
+            .fee_asset(fee_asset)
+            .contract_address(self.contract_address)
+            .bridge_address(self.bridge_address)
+            .sequencer_asset_to_withdraw(self.rollup_asset_denom.clone())
+            .try_build()
             .await
-            .wrap_err("failed to get asset withdrawal decimals")?;
-        let asset_withdrawal_divisor =
-            10u128.pow(18u32.checked_sub(base_chain_asset_precision).expect(
-                "base_chain_asset_precision must be <= 18, as the contract constructor enforces \
-                 this",
-            ));
+            .wrap_err("failed to construct contract event to sequencer action fetcher")?;
 
         self.state.set_watcher_ready();
 
-        Ok((
-            provider.clone(),
-            contract,
-            fee_asset,
-            asset_withdrawal_divisor,
-            starting_rollup_height,
-        ))
+        Ok((provider.clone(), action_fetcher, starting_rollup_height))
     }
 }
 
 async fn sync_from_next_rollup_block_height(
     provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
-    converter: &EventToActionConvertConfig,
+    action_fetcher: &GetWithdrawalActions<Provider<Ws>>,
     submitter_handle: &submitter::Handle,
     next_rollup_block_height_to_check: u64,
     current_rollup_block_height: u64,
@@ -283,15 +226,9 @@ async fn sync_from_next_rollup_block_height(
             bail!("block with number {i} missing");
         };
 
-        get_and_send_events_at_block(
-            provider.clone(),
-            contract_address,
-            block,
-            converter,
-            submitter_handle,
-        )
-        .await
-        .wrap_err("failed to get and send events at block")?;
+        get_and_send_events_at_block(action_fetcher, block, submitter_handle)
+            .await
+            .wrap_err("failed to get and send events at block")?;
     }
 
     info!("synced from {next_rollup_block_height_to_check} to {current_rollup_block_height}");
@@ -300,9 +237,8 @@ async fn sync_from_next_rollup_block_height(
 
 async fn watch_for_blocks(
     provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
+    action_fetcher: GetWithdrawalActions<Provider<Ws>>,
     next_rollup_block_height: u64,
-    converter: EventToActionConvertConfig,
     submitter_handle: submitter::Handle,
     shutdown_token: CancellationToken,
 ) -> Result<()> {
@@ -325,8 +261,7 @@ async fn watch_for_blocks(
     // (inclusive).
     sync_from_next_rollup_block_height(
         provider.clone(),
-        contract_address,
-        &converter,
+        &action_fetcher,
         &submitter_handle,
         next_rollup_block_height,
         current_rollup_block_height.as_u64(),
@@ -343,10 +278,8 @@ async fn watch_for_blocks(
             block = block_rx.next() => {
                 if let Some(block) = block {
                     get_and_send_events_at_block(
-                        provider.clone(),
-                        contract_address,
+                        &action_fetcher,
                         block,
-                        &converter,
                         &submitter_handle,
                     )
                     .await
@@ -360,151 +293,43 @@ async fn watch_for_blocks(
 }
 
 async fn get_and_send_events_at_block(
-    provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
+    actions_fetcher: &GetWithdrawalActions<Provider<Ws>>,
     block: Block<H256>,
-    converter: &EventToActionConvertConfig,
     submitter_handle: &submitter::Handle,
 ) -> Result<()> {
-    let Some(block_hash) = block.hash else {
-        bail!("block hash missing; skipping")
-    };
-
-    let Some(block_number) = block.number else {
-        bail!("block number missing; skipping")
-    };
-
-    let sequencer_withdrawal_events =
-        get_sequencer_withdrawal_events(provider.clone(), contract_address, block_hash)
-            .await
-            .wrap_err("failed to get sequencer withdrawal events")?;
-    let ics20_withdrawal_events =
-        get_ics20_withdrawal_events(provider.clone(), contract_address, block_hash)
-            .await
-            .wrap_err("failed to get ics20 withdrawal events")?;
-    let events = vec![sequencer_withdrawal_events, ics20_withdrawal_events]
-        .into_iter()
-        .flatten();
-    let mut batch = Batch {
-        actions: Vec::new(),
-        rollup_height: block_number.as_u64(),
-    };
-    for (event, log) in events {
-        let Some(transaction_hash) = log.transaction_hash else {
-            warn!("transaction hash missing; skipping");
-            continue;
-        };
-
-        let event_with_metadata = EventWithMetadata {
-            event,
-            block_number,
-            transaction_hash,
-        };
-        let action = converter
-            .convert(event_with_metadata)
-            .wrap_err("failed to convert event to action")?;
-        batch.actions.push(action);
-    }
-
-    if batch.actions.is_empty() {
-        trace!("no actions to send at block {block_number}");
+    let block_hash = block.hash.ok_or_eyre("block did not contain a hash")?;
+    let rollup_height = block
+        .number
+        .ok_or_eyre("block did not contain a rollup height")?
+        .as_u64();
+    let actions = actions_fetcher
+        .get_for_block_hash(block_hash)
+        .await
+        .wrap_err_with(|| {
+            format!(
+                "failed getting actions for block; block hash: `{block_hash}`, block height: \
+                 `{rollup_height}`"
+            )
+        })?;
+
+    if actions.is_empty() {
+        info!(
+            "no withdrawal actions found for block `{block_hash}` at rollup height \
+             `{rollup_height}; skipping"
+        );
     } else {
-        let actions_len = batch.actions.len();
         submitter_handle
-            .send_batch(batch)
+            .send_batch(Batch {
+                actions,
+                rollup_height,
+            })
             .await
             .wrap_err("failed to send batched events; receiver dropped?")?;
-        debug!(
-            "sent batch with {} actions at block {block_number}",
-            actions_len
-        );
     }
 
     Ok(())
 }
 
-async fn get_sequencer_withdrawal_events(
-    provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
-    block_hash: H256,
-) -> Result<Vec<(WithdrawalEvent, Log)>> {
-    let sequencer_withdrawal_event_sig = SequencerWithdrawalFilter::signature();
-    let sequencer_withdrawal_filter = Filter::new()
-        .at_block_hash(block_hash)
-        .address(contract_address)
-        .topic0(sequencer_withdrawal_event_sig);
-
-    let logs = provider
-        .get_logs(&sequencer_withdrawal_filter)
-        .await
-        .wrap_err("failed to get sequencer withdrawal events")?;
-
-    let events = logs
-        .into_iter()
-        .map(|log| {
-            let raw_log = ethers::abi::RawLog {
-                topics: log.topics.clone(),
-                data: log.data.to_vec(),
-            };
-            let event = SequencerWithdrawalFilter::decode_log(&raw_log)?;
-            Ok((WithdrawalEvent::Sequencer(event), log))
-        })
-        .collect::<Result<Vec<_>>>()?;
-
-    Ok(events)
-}
-
-async fn get_ics20_withdrawal_events(
-    provider: Arc<Provider<Ws>>,
-    contract_address: ethers::types::Address,
-    block_hash: H256,
-) -> Result<Vec<(WithdrawalEvent, Log)>> {
-    let ics20_withdrawal_event_sig = Ics20WithdrawalFilter::signature();
-    let ics20_withdrawal_filter = Filter::new()
-        .at_block_hash(block_hash)
-        .address(contract_address)
-        .topic0(ics20_withdrawal_event_sig);
-
-    let logs = provider
-        .get_logs(&ics20_withdrawal_filter)
-        .await
-        .wrap_err("failed to get ics20 withdrawal events")?;
-
-    let events = logs
-        .into_iter()
-        .map(|log| {
-            let raw_log = ethers::abi::RawLog {
-                topics: log.topics.clone(),
-                data: log.data.to_vec(),
-            };
-            let event = Ics20WithdrawalFilter::decode_log(&raw_log)?;
-            Ok((WithdrawalEvent::Ics20(event), log))
-        })
-        .collect::<Result<Vec<_>>>()?;
-
-    Ok(events)
-}
-
-#[derive(Clone)]
-struct EventToActionConvertConfig {
-    fee_asset: Denom,
-    rollup_asset_denom: Denom,
-    bridge_address: Address,
-    asset_withdrawal_divisor: u128,
-}
-
-impl EventToActionConvertConfig {
-    fn convert(&self, event: EventWithMetadata) -> Result<Action> {
-        event_to_action(
-            event,
-            self.fee_asset.clone(),
-            self.rollup_asset_denom.clone(),
-            self.asset_withdrawal_divisor,
-            self.bridge_address,
-        )
-    }
-}
-
 // converts an ethereum address string to an `ethers::types::Address`.
 // the input string may be prefixed with "0x" or not.
 fn address_from_string(s: &str) -> Result<ethers::types::Address> {
@@ -518,588 +343,3 @@ fn address_from_string(s: &str) -> Result<ethers::types::Address> {
     })?;
     Ok(address.into())
 }
-
-#[cfg(test)]
-mod tests {
-    use astria_bridge_contracts::{
-        astria_bridgeable_erc20::AstriaBridgeableERC20,
-        astria_withdrawer::AstriaWithdrawer,
-        i_astria_withdrawer::{
-            Ics20WithdrawalFilter,
-            SequencerWithdrawalFilter,
-        },
-    };
-    use astria_core::{
-        primitive::v1::{
-            asset,
-            Address,
-        },
-        protocol::transaction::v1alpha1::Action,
-    };
-    use ethers::{
-        prelude::SignerMiddleware,
-        providers::Middleware,
-        signers::Signer as _,
-        types::{
-            TransactionReceipt,
-            U256,
-        },
-        utils::hex,
-    };
-    use tokio::sync::mpsc::{
-        self,
-        error::TryRecvError,
-    };
-
-    use super::*;
-    use crate::bridge_withdrawer::ethereum::{
-        convert::EventWithMetadata,
-        test_utils::{
-            ConfigureAstriaBridgeableERC20Deployer,
-            ConfigureAstriaWithdrawerDeployer,
-        },
-    };
-
-    fn default_native_asset() -> asset::Denom {
-        "nria".parse().unwrap()
-    }
-
-    #[test]
-    fn address_from_string_prefix() {
-        let address = address_from_string("0x1234567890123456789012345678901234567890").unwrap();
-        let bytes: [u8; 20] = hex::decode("1234567890123456789012345678901234567890")
-            .unwrap()
-            .try_into()
-            .unwrap();
-        assert_eq!(address, ethers::types::Address::from(bytes));
-    }
-
-    #[test]
-    fn address_from_string_no_prefix() {
-        let address = address_from_string("1234567890123456789012345678901234567890").unwrap();
-        let bytes: [u8; 20] = hex::decode("1234567890123456789012345678901234567890")
-            .unwrap()
-            .try_into()
-            .unwrap();
-        assert_eq!(address, ethers::types::Address::from(bytes));
-    }
-
-    async fn send_sequencer_withdraw_transaction<M: Middleware>(
-        contract: &AstriaWithdrawer<M>,
-        value: U256,
-        recipient: Address,
-    ) -> TransactionReceipt {
-        let tx = contract
-            .withdraw_to_sequencer(recipient.to_string())
-            .value(value);
-        let receipt = tx
-            .send()
-            .await
-            .expect("failed to submit transaction")
-            .await
-            .expect("failed to await pending transaction")
-            .expect("no receipt found");
-
-        assert!(
-            receipt.status == Some(ethers::types::U64::from(1)),
-            "`withdraw` transaction failed: {receipt:?}",
-        );
-
-        receipt
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn astria_withdrawer_invalid_value_fails() {
-        let (contract_address, provider, wallet, _anvil) = ConfigureAstriaWithdrawerDeployer {
-            base_chain_asset_precision: 15,
-            ..Default::default()
-        }
-        .deploy()
-        .await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaWithdrawer::new(contract_address, signer.clone());
-
-        let value: U256 = 999.into(); // 10^3 - 1
-        let recipient = crate::astria_address([1u8; 20]);
-        let tx = contract
-            .withdraw_to_sequencer(recipient.to_string())
-            .value(value);
-        tx.send()
-            .await
-            .expect_err("`withdraw` transaction should have failed due to value < 10^3");
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn watcher_can_watch_sequencer_withdrawals_astria_withdrawer() {
-        let (contract_address, provider, wallet, anvil) =
-            ConfigureAstriaWithdrawerDeployer::default().deploy().await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaWithdrawer::new(contract_address, signer.clone());
-
-        let value = 1_000_000_000.into();
-        let recipient = crate::astria_address([1u8; 20]);
-        let bridge_address = crate::astria_address([1u8; 20]);
-        let denom = "nria".parse::<Denom>().unwrap();
-
-        let state = Arc::new(State::new());
-        let startup_handle = startup::InfoHandle::new(state.subscribe());
-        state.set_startup_info(startup::Info {
-            starting_rollup_height: 1,
-            fee_asset: denom.clone(),
-            chain_id: "astria".to_string(),
-        });
-        let (batch_tx, mut batch_rx) = mpsc::channel(100);
-        let submitter_handle = submitter::Handle::new(batch_tx);
-
-        let watcher = Builder {
-            ethereum_contract_address: hex::encode(contract_address),
-            ethereum_rpc_endpoint: anvil.ws_endpoint(),
-            startup_handle,
-            submitter_handle,
-            shutdown_token: CancellationToken::new(),
-            state: Arc::new(State::new()),
-            rollup_asset_denom: denom.clone(),
-            bridge_address,
-        }
-        .build()
-        .unwrap();
-
-        tokio::task::spawn(watcher.run());
-        let receipt = send_sequencer_withdraw_transaction(&contract, value, recipient).await;
-        let expected_event = EventWithMetadata {
-            event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
-                sender: wallet.address(),
-                destination_chain_address: recipient.to_string(),
-                amount: value,
-            }),
-            block_number: receipt.block_number.unwrap(),
-            transaction_hash: receipt.transaction_hash,
-        };
-        let expected_action =
-            event_to_action(expected_event, denom.clone(), denom, 1, bridge_address).unwrap();
-        let Action::BridgeUnlock(expected_action) = expected_action else {
-            panic!("expected action to be BridgeUnlock, got {expected_action:?}");
-        };
-
-        let batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-        let Action::BridgeUnlock(action) = &batch.actions[0] else {
-            panic!(
-                "expected action to be BridgeUnlock, got {:?}",
-                batch.actions[0]
-            );
-        };
-        assert_eq!(action, &expected_action);
-        assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty);
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn watcher_can_watch_sequencer_withdrawals_astria_withdrawer_sync_from_next_rollup_height()
-     {
-        let (contract_address, provider, wallet, anvil) =
-            ConfigureAstriaWithdrawerDeployer::default().deploy().await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaWithdrawer::new(contract_address, signer.clone());
-
-        let value = 1_000_000_000.into();
-        let recipient = crate::astria_address([1u8; 20]);
-        let bridge_address = crate::astria_address([1u8; 20]);
-        let denom = default_native_asset();
-
-        // send tx before watcher starts
-        let receipt = send_sequencer_withdraw_transaction(&contract, value, recipient).await;
-
-        let expected_event = EventWithMetadata {
-            event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
-                sender: wallet.address(),
-                destination_chain_address: recipient.to_string(),
-                amount: value,
-            }),
-            block_number: receipt.block_number.unwrap(),
-            transaction_hash: receipt.transaction_hash,
-        };
-        let expected_action = event_to_action(
-            expected_event,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap();
-        let Action::BridgeUnlock(expected_action) = expected_action else {
-            panic!("expected action to be BridgeUnlock, got {expected_action:?}");
-        };
-
-        let state = Arc::new(State::new());
-        let startup_handle = startup::InfoHandle::new(state.subscribe());
-        state.set_startup_info(startup::Info {
-            starting_rollup_height: 1,
-            fee_asset: denom.clone(),
-            chain_id: "astria".to_string(),
-        });
-        let (batch_tx, mut batch_rx) = mpsc::channel(100);
-
-        let watcher = Builder {
-            ethereum_contract_address: hex::encode(contract_address),
-            ethereum_rpc_endpoint: anvil.ws_endpoint(),
-            startup_handle,
-            shutdown_token: CancellationToken::new(),
-            state: Arc::new(State::new()),
-            rollup_asset_denom: denom.clone(),
-            bridge_address,
-            submitter_handle: submitter::Handle::new(batch_tx),
-        }
-        .build()
-        .unwrap();
-
-        tokio::task::spawn(watcher.run());
-
-        // send another tx to trigger a new block
-        send_sequencer_withdraw_transaction(&contract, value, recipient).await;
-
-        let batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-        let Action::BridgeUnlock(action) = &batch.actions[0] else {
-            panic!(
-                "expected action to be BridgeUnlock, got {:?}",
-                batch.actions[0]
-            );
-        };
-        assert_eq!(action, &expected_action);
-
-        // should receive a second batch containing the second tx
-        let batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-    }
-
-    async fn send_ics20_withdraw_transaction<M: Middleware>(
-        contract: &AstriaWithdrawer<M>,
-        value: U256,
-        recipient: String,
-    ) -> TransactionReceipt {
-        let tx = contract
-            .withdraw_to_ibc_chain(recipient, "nootwashere".to_string())
-            .value(value);
-        let receipt = tx
-            .send()
-            .await
-            .expect("failed to submit transaction")
-            .await
-            .expect("failed to await pending transaction")
-            .expect("no receipt found");
-
-        assert!(
-            receipt.status == Some(ethers::types::U64::from(1)),
-            "`withdraw` transaction failed: {receipt:?}",
-        );
-
-        receipt
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn watcher_can_watch_ics20_withdrawals_astria_withdrawer() {
-        let (contract_address, provider, wallet, anvil) =
-            ConfigureAstriaWithdrawerDeployer::default().deploy().await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaWithdrawer::new(contract_address, signer.clone());
-
-        let value = 1_000_000_000.into();
-        let recipient = "somebech32address".to_string();
-
-        let bridge_address = crate::astria_address([1u8; 20]);
-        let denom = "transfer/channel-0/utia".parse::<Denom>().unwrap();
-
-        let state = Arc::new(State::new());
-        let startup_handle = startup::InfoHandle::new(state.subscribe());
-        state.set_startup_info(startup::Info {
-            starting_rollup_height: 1,
-            fee_asset: denom.clone(),
-            chain_id: "astria".to_string(),
-        });
-        let (batch_tx, mut batch_rx) = mpsc::channel(100);
-
-        let watcher = Builder {
-            ethereum_contract_address: hex::encode(contract_address),
-            ethereum_rpc_endpoint: anvil.ws_endpoint(),
-            startup_handle,
-            shutdown_token: CancellationToken::new(),
-            state: Arc::new(State::new()),
-            rollup_asset_denom: denom.clone(),
-            bridge_address,
-            submitter_handle: submitter::Handle::new(batch_tx),
-        }
-        .build()
-        .unwrap();
-
-        tokio::task::spawn(watcher.run());
-
-        let receipt = send_ics20_withdraw_transaction(&contract, value, recipient.clone()).await;
-        let expected_event = EventWithMetadata {
-            event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter {
-                sender: wallet.address(),
-                destination_chain_address: recipient.clone(),
-                amount: value,
-                memo: "nootwashere".to_string(),
-            }),
-            block_number: receipt.block_number.unwrap(),
-            transaction_hash: receipt.transaction_hash,
-        };
-
-        let Action::Ics20Withdrawal(mut expected_action) = event_to_action(
-            expected_event,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap() else {
-            panic!("expected action to be Ics20Withdrawal");
-        };
-        expected_action.timeout_time = 0; // zero this for testing
-
-        let mut batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-        let Action::Ics20Withdrawal(ref mut action) = batch.actions[0] else {
-            panic!(
-                "expected action to be Ics20Withdrawal, got {:?}",
-                batch.actions[0]
-            );
-        };
-        action.timeout_time = 0; // zero this for testing
-        assert_eq!(action, &expected_action);
-        assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty);
-    }
-
-    async fn mint_tokens<M: Middleware>(
-        contract: &AstriaBridgeableERC20<M>,
-        amount: U256,
-        recipient: ethers::types::Address,
-    ) -> TransactionReceipt {
-        let mint_tx = contract.mint(recipient, amount);
-        let receipt = mint_tx
-            .send()
-            .await
-            .expect("failed to submit mint transaction")
-            .await
-            .expect("failed to await pending mint transaction")
-            .expect("no mint receipt found");
-
-        assert!(
-            receipt.status == Some(ethers::types::U64::from(1)),
-            "`mint` transaction failed: {receipt:?}",
-        );
-
-        receipt
-    }
-
-    async fn send_sequencer_withdraw_transaction_erc20<M: Middleware>(
-        contract: &AstriaBridgeableERC20<M>,
-        value: U256,
-        recipient: Address,
-    ) -> TransactionReceipt {
-        let tx = contract.withdraw_to_sequencer(value, recipient.to_string());
-        let receipt = tx
-            .send()
-            .await
-            .expect("failed to submit transaction")
-            .await
-            .expect("failed to await pending transaction")
-            .expect("no receipt found");
-
-        assert!(
-            receipt.status == Some(ethers::types::U64::from(1)),
-            "`withdraw` transaction failed: {receipt:?}",
-        );
-
-        receipt
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn watcher_can_watch_sequencer_withdrawals_astria_bridgeable_erc20() {
-        let (contract_address, provider, wallet, anvil) = ConfigureAstriaBridgeableERC20Deployer {
-            base_chain_asset_precision: 18,
-            ..Default::default()
-        }
-        .deploy()
-        .await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaBridgeableERC20::new(contract_address, signer.clone());
-
-        // mint some tokens to the wallet
-        mint_tokens(&contract, 2_000_000_000.into(), wallet.address()).await;
-
-        let value = 1_000_000_000.into();
-        let recipient = crate::astria_address([1u8; 20]);
-        let bridge_address = crate::astria_address([1u8; 20]);
-        let denom = default_native_asset();
-
-        let state = Arc::new(State::new());
-        let startup_handle = startup::InfoHandle::new(state.subscribe());
-        state.set_startup_info(startup::Info {
-            starting_rollup_height: 1,
-            fee_asset: denom.clone(),
-            chain_id: "astria".to_string(),
-        });
-        let (batch_tx, mut batch_rx) = mpsc::channel(100);
-
-        let watcher = Builder {
-            ethereum_contract_address: hex::encode(contract_address),
-            ethereum_rpc_endpoint: anvil.ws_endpoint(),
-            startup_handle,
-            shutdown_token: CancellationToken::new(),
-            state: Arc::new(State::new()),
-            rollup_asset_denom: denom.clone(),
-            bridge_address,
-            submitter_handle: submitter::Handle::new(batch_tx),
-        }
-        .build()
-        .unwrap();
-
-        tokio::task::spawn(watcher.run());
-
-        let receipt = send_sequencer_withdraw_transaction_erc20(&contract, value, recipient).await;
-        let expected_event = EventWithMetadata {
-            event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
-                sender: wallet.address(),
-                destination_chain_address: recipient.to_string(),
-                amount: value,
-            }),
-            block_number: receipt.block_number.unwrap(),
-            transaction_hash: receipt.transaction_hash,
-        };
-        let expected_action = event_to_action(
-            expected_event,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap();
-        let Action::BridgeUnlock(expected_action) = expected_action else {
-            panic!("expected action to be BridgeUnlock, got {expected_action:?}");
-        };
-
-        let batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-        let Action::BridgeUnlock(action) = &batch.actions[0] else {
-            panic!(
-                "expected action to be BridgeUnlock, got {:?}",
-                batch.actions[0]
-            );
-        };
-        assert_eq!(action, &expected_action);
-        assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty);
-    }
-
-    async fn send_ics20_withdraw_transaction_astria_bridgeable_erc20<M: Middleware>(
-        contract: &AstriaBridgeableERC20<M>,
-        value: U256,
-        recipient: String,
-    ) -> TransactionReceipt {
-        let tx = contract.withdraw_to_ibc_chain(value, recipient, "nootwashere".to_string());
-        let receipt = tx
-            .send()
-            .await
-            .expect("failed to submit transaction")
-            .await
-            .expect("failed to await pending transaction")
-            .expect("no receipt found");
-
-        assert!(
-            receipt.status == Some(ethers::types::U64::from(1)),
-            "`withdraw` transaction failed: {receipt:?}",
-        );
-
-        receipt
-    }
-
-    #[tokio::test]
-    #[ignore = "requires foundry to be installed"]
-    async fn watcher_can_watch_ics20_withdrawals_astria_bridgeable_erc20() {
-        let (contract_address, provider, wallet, anvil) = ConfigureAstriaBridgeableERC20Deployer {
-            base_chain_asset_precision: 18,
-            ..Default::default()
-        }
-        .deploy()
-        .await;
-        let signer = Arc::new(SignerMiddleware::new(provider, wallet.clone()));
-        let contract = AstriaBridgeableERC20::new(contract_address, signer.clone());
-
-        // mint some tokens to the wallet
-        mint_tokens(&contract, 2_000_000_000.into(), wallet.address()).await;
-
-        let value = 1_000_000_000.into();
-        let recipient = "somebech32address".to_string();
-        let bridge_address = crate::astria_address([1u8; 20]);
-        let denom = "transfer/channel-0/utia".parse::<Denom>().unwrap();
-
-        let state = Arc::new(State::new());
-        let startup_handle = startup::InfoHandle::new(state.subscribe());
-        state.set_startup_info(startup::Info {
-            starting_rollup_height: 1,
-            fee_asset: denom.clone(),
-            chain_id: "astria".to_string(),
-        });
-        let (batch_tx, mut batch_rx) = mpsc::channel(100);
-
-        let watcher = Builder {
-            ethereum_contract_address: hex::encode(contract_address),
-            ethereum_rpc_endpoint: anvil.ws_endpoint(),
-            startup_handle,
-            shutdown_token: CancellationToken::new(),
-            state: Arc::new(State::new()),
-            rollup_asset_denom: denom.clone(),
-            bridge_address,
-            submitter_handle: submitter::Handle::new(batch_tx),
-        }
-        .build()
-        .unwrap();
-
-        tokio::task::spawn(watcher.run());
-
-        let receipt = send_ics20_withdraw_transaction_astria_bridgeable_erc20(
-            &contract,
-            value,
-            recipient.clone(),
-        )
-        .await;
-        let expected_event = EventWithMetadata {
-            event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter {
-                sender: wallet.address(),
-                destination_chain_address: recipient.clone(),
-                amount: value,
-                memo: "nootwashere".to_string(),
-            }),
-            block_number: receipt.block_number.unwrap(),
-            transaction_hash: receipt.transaction_hash,
-        };
-        let Action::Ics20Withdrawal(mut expected_action) = event_to_action(
-            expected_event,
-            denom.clone(),
-            denom.clone(),
-            1,
-            bridge_address,
-        )
-        .unwrap() else {
-            panic!("expected action to be Ics20Withdrawal");
-        };
-        expected_action.timeout_time = 0; // zero this for testing
-
-        let mut batch = batch_rx.recv().await.unwrap();
-        assert_eq!(batch.actions.len(), 1);
-        let Action::Ics20Withdrawal(ref mut action) = batch.actions[0] else {
-            panic!(
-                "expected action to be Ics20Withdrawal, got {:?}",
-                batch.actions[0]
-            );
-        };
-        action.timeout_time = 0; // zero this for testing
-        assert_eq!(action, &expected_action);
-        assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty);
-    }
-}

From 60386e65a7a26bb895c622e27110de1208cdb8b5 Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 12:37:26 +0200
Subject: [PATCH 5/6] unbreak ics20 withdrawals

---
 crates/astria-bridge-contracts/src/lib.rs     | 19 ++++++++----------
 .../src/bridge_withdrawer/ethereum/watcher.rs | 20 ++++++++++++++++---
 .../src/bridge_withdrawer/mod.rs              |  5 +----
 crates/astria-bridge-withdrawer/src/config.rs |  2 +-
 4 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs
index d69f7ca357..16c22658d7 100644
--- a/crates/astria-bridge-contracts/src/lib.rs
+++ b/crates/astria-bridge-contracts/src/lib.rs
@@ -8,10 +8,7 @@ use std::{
 
 use astria_core::{
     primitive::v1::{
-        asset::{
-            self,
-            TracePrefixed,
-        },
+        asset,
         Address,
         AddressError,
     },
@@ -111,7 +108,7 @@ pub struct GetWithdrawalActionsBuilder<TProvider = NoProvider> {
     bridge_address: Option<Address>,
     fee_asset: Option<asset::Denom>,
     sequencer_asset_to_withdraw: Option<asset::Denom>,
-    ics20_asset_to_withdraw: Option<asset::Denom>,
+    ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
 }
 
 impl Default for GetWithdrawalActionsBuilder {
@@ -188,13 +185,13 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
-    pub fn ics20_asset_to_withdraw(self, ics20_asset_to_withdraw: asset::Denom) -> Self {
+    pub fn ics20_asset_to_withdraw(self, ics20_asset_to_withdraw: asset::TracePrefixed) -> Self {
         self.set_ics20_asset_to_withdraw(Some(ics20_asset_to_withdraw))
     }
 
     pub fn set_ics20_asset_to_withdraw(
         self,
-        ics20_asset_to_withdraw: Option<asset::Denom>,
+        ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
     ) -> Self {
         Self {
             ics20_asset_to_withdraw,
@@ -236,8 +233,7 @@ where
         if let Some(ics20_asset_to_withdraw) = &ics20_asset_to_withdraw {
             ics20_source_channel.replace(
                 ics20_asset_to_withdraw
-                    .as_trace_prefixed()
-                    .and_then(TracePrefixed::last_channel)
+                    .last_channel()
                     .ok_or(BuildError::ics20_asset_without_channel())?
                     .parse()
                     .map_err(BuildError::parse_ics20_asset_source_channel)?,
@@ -279,7 +275,7 @@ pub struct GetWithdrawalActions<P> {
     bridge_address: Address,
     fee_asset: asset::Denom,
     sequencer_asset_to_withdraw: Option<asset::Denom>,
-    ics20_asset_to_withdraw: Option<asset::Denom>,
+    ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
     ics20_source_channel: Option<ibc_types::core::channel::ChannelId>,
 }
 
@@ -356,7 +352,8 @@ where
         let (denom, source_channel) = (
             self.ics20_asset_to_withdraw
                 .clone()
-                .expect("must be set if this method is entered"),
+                .expect("must be set if this method is entered")
+                .into(),
             self.ics20_source_channel
                 .clone()
                 .expect("must be set if this method is entered"),
diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
index 0e00392cb7..87cbccad24 100644
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
+++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs
@@ -53,7 +53,7 @@ pub(crate) struct Builder {
     pub(crate) ethereum_contract_address: String,
     pub(crate) ethereum_rpc_endpoint: String,
     pub(crate) state: Arc<State>,
-    pub(crate) rollup_asset_denom: asset::Denom,
+    pub(crate) rollup_asset_denom: asset::TracePrefixed,
     pub(crate) bridge_address: Address,
     pub(crate) submitter_handle: submitter::Handle,
 }
@@ -94,7 +94,7 @@ pub(crate) struct Watcher {
     submitter_handle: submitter::Handle,
     contract_address: ethers::types::Address,
     ethereum_rpc_endpoint: String,
-    rollup_asset_denom: asset::Denom,
+    rollup_asset_denom: asset::TracePrefixed,
     bridge_address: Address,
     state: Arc<State>,
 }
@@ -190,12 +190,26 @@ impl Watcher {
         .wrap_err("failed connecting to rollup after several retries; giving up")?;
 
         let provider = Arc::new(provider);
+        let ics20_asset_to_withdraw = if self.rollup_asset_denom.last_channel().is_some() {
+            info!(
+                rollup_asset_denom = %self.rollup_asset_denom,
+                "configured rollup asset contains an ics20 channel; ics20 withdrawals will be emitted"
+            );
+            Some(self.rollup_asset_denom.clone())
+        } else {
+            info!(
+                rollup_asset_denom = %self.rollup_asset_denom,
+                "configured rollup asset does not contain an ics20 channel; ics20 withdrawals will not be emitted"
+            );
+            None
+        };
         let action_fetcher = GetWithdrawalActionsBuilder::new()
             .provider(provider.clone())
             .fee_asset(fee_asset)
             .contract_address(self.contract_address)
             .bridge_address(self.bridge_address)
-            .sequencer_asset_to_withdraw(self.rollup_asset_denom.clone())
+            .sequencer_asset_to_withdraw(self.rollup_asset_denom.clone().into())
+            .set_ics20_asset_to_withdraw(ics20_asset_to_withdraw)
             .try_build()
             .await
             .wrap_err("failed to construct contract event to sequencer action fetcher")?;
diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs
index 216ec60770..6f58679c08 100644
--- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs
+++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs
@@ -7,7 +7,6 @@ use std::{
     time::Duration,
 };
 
-use astria_core::primitive::v1::asset::Denom;
 use astria_eyre::eyre::{
     self,
     WrapErr as _,
@@ -119,9 +118,7 @@ impl BridgeWithdrawer {
             startup_handle,
             shutdown_token: shutdown_handle.token(),
             state: state.clone(),
-            rollup_asset_denom: rollup_asset_denomination
-                .parse::<Denom>()
-                .wrap_err("failed to parse ROLLUP_ASSET_DENOMINATION as Denom")?,
+            rollup_asset_denom: rollup_asset_denomination,
             bridge_address: sequencer_bridge_address,
             submitter_handle,
         }
diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs
index 7a2f33f754..85ac347d6c 100644
--- a/crates/astria-bridge-withdrawer/src/config.rs
+++ b/crates/astria-bridge-withdrawer/src/config.rs
@@ -19,7 +19,7 @@ pub struct Config {
     // The fee asset denomination to use for the bridge account's transactions.
     pub fee_asset_denomination: asset::Denom,
     // The asset denomination being withdrawn from the rollup.
-    pub rollup_asset_denomination: String,
+    pub rollup_asset_denomination: asset::denom::TracePrefixed,
     // The bridge address corresponding to the bridged rollup asset on the sequencer.
     pub sequencer_bridge_address: String,
     // The address of the AstriaWithdrawer contract on the evm rollup.

From ea3b3cde493c274c379f28ce956aefe19b1f4114 Mon Sep 17 00:00:00 2001
From: Richard Janis Goldschmidt <github@aberrat.io>
Date: Mon, 15 Jul 2024 12:47:27 +0200
Subject: [PATCH 6/6] clippy

---
 crates/astria-bridge-contracts/src/lib.rs     | 34 +++++++++++++++++++
 .../astria-cli/src/commands/bridge/collect.rs |  2 +-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs
index 16c22658d7..7266c93eb8 100644
--- a/crates/astria-bridge-contracts/src/lib.rs
+++ b/crates/astria-bridge-contracts/src/lib.rs
@@ -37,12 +37,14 @@ pub use generated::*;
 pub struct BuildError(BuildErrorKind);
 
 impl BuildError {
+    #[must_use]
     fn bad_divisor(base_chain_asset_precision: u32) -> Self {
         Self(BuildErrorKind::BadDivisor {
             base_chain_asset_precision,
         })
     }
 
+    #[must_use]
     fn call_base_chain_asset_precision<
         T: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
     >(
@@ -53,20 +55,24 @@ impl BuildError {
         })
     }
 
+    #[must_use]
     pub fn no_withdraws_configured() -> Self {
         Self(BuildErrorKind::NoWithdrawsConfigured)
     }
 
+    #[must_use]
     fn not_set(field: &'static str) -> Self {
         Self(BuildErrorKind::NotSet {
             field,
         })
     }
 
+    #[must_use]
     fn ics20_asset_without_channel() -> Self {
         Self(BuildErrorKind::Ics20AssetWithoutChannel)
     }
 
+    #[must_use]
     fn parse_ics20_asset_source_channel(source: ibc_types::IdentifierError) -> Self {
         Self(BuildErrorKind::ParseIcs20AssetSourceChannel {
             source,
@@ -118,6 +124,7 @@ impl Default for GetWithdrawalActionsBuilder {
 }
 
 impl GetWithdrawalActionsBuilder {
+    #[must_use]
     pub fn new() -> Self {
         Self {
             provider: NoProvider,
@@ -131,6 +138,7 @@ impl GetWithdrawalActionsBuilder {
 }
 
 impl<P> GetWithdrawalActionsBuilder<P> {
+    #[must_use]
     pub fn provider<Q>(self, provider: Arc<Q>) -> GetWithdrawalActionsBuilder<WithProvider<Q>> {
         let Self {
             contract_address,
@@ -150,6 +158,7 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
+    #[must_use]
     pub fn contract_address(self, contract_address: ethers::types::Address) -> Self {
         Self {
             contract_address: Some(contract_address),
@@ -157,6 +166,7 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
+    #[must_use]
     pub fn bridge_address(self, bridge_address: Address) -> Self {
         Self {
             bridge_address: Some(bridge_address),
@@ -164,6 +174,7 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
+    #[must_use]
     pub fn fee_asset(self, fee_asset: asset::Denom) -> Self {
         Self {
             fee_asset: Some(fee_asset),
@@ -171,10 +182,12 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
+    #[must_use]
     pub fn sequencer_asset_to_withdraw(self, sequencer_asset_to_withdraw: asset::Denom) -> Self {
         self.set_sequencer_asset_to_withdraw(Some(sequencer_asset_to_withdraw))
     }
 
+    #[must_use]
     pub fn set_sequencer_asset_to_withdraw(
         self,
         sequencer_asset_to_withdraw: Option<asset::Denom>,
@@ -185,10 +198,12 @@ impl<P> GetWithdrawalActionsBuilder<P> {
         }
     }
 
+    #[must_use]
     pub fn ics20_asset_to_withdraw(self, ics20_asset_to_withdraw: asset::TracePrefixed) -> Self {
         self.set_ics20_asset_to_withdraw(Some(ics20_asset_to_withdraw))
     }
 
+    #[must_use]
     pub fn set_ics20_asset_to_withdraw(
         self,
         ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
@@ -205,6 +220,19 @@ where
     P: Middleware + 'static,
     P::Error: std::error::Error + 'static,
 {
+    /// Constructs a [`GetWithdrawalActions`] fetcher.
+    ///
+    /// # Errors
+    /// Returns an error in one of these cases:
+    /// + `contract_address` is not set
+    /// + `bridge_address` is not set
+    /// + `fee_asset` is not set
+    /// + neither `source_asset_to_withdraw` nor `ics20_asset_to_withdraw` are set
+    /// + `ics20_asset_to_withdraw` is set, but does not contain a ics20 channel
+    /// + the `BASE_CHAIN_ASSET_PRECISION` call on the provided `contract_address` cannot be
+    ///   executed
+    /// + the base chain asset precision retrieved from the contract at `contract_address` is
+    ///   greater than 18 (this is currently hardcoded in the smart contract).
     pub async fn try_build(self) -> Result<GetWithdrawalActions<P>, BuildError> {
         let Self {
             provider: WithProvider(provider),
@@ -292,6 +320,12 @@ where
         self.ics20_asset_to_withdraw.is_some()
     }
 
+    /// Gets all withdrawal events for `block_hash` and converts them to astria sequencer actions.
+    ///
+    /// # Errors
+    /// Returns an error in one of the following cases:
+    /// + fetching logs for either ics20 or sequencer withdrawal events fails
+    /// + converting either event to Sequencer actions fails due to the events being malformed.
     pub async fn get_for_block_hash(
         &self,
         block_hash: H256,
diff --git a/crates/astria-cli/src/commands/bridge/collect.rs b/crates/astria-cli/src/commands/bridge/collect.rs
index 8401287392..317b9d574f 100644
--- a/crates/astria-cli/src/commands/bridge/collect.rs
+++ b/crates/astria-cli/src/commands/bridge/collect.rs
@@ -73,7 +73,7 @@ pub(crate) struct WithdrawalEvents {
     sequencer_asset_to_withdraw: Option<asset::Denom>,
     /// The is20 asset withdrawn through the bridge.
     #[arg(long)]
-    ics20_asset_to_withdraw: Option<asset::Denom>,
+    ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
     /// The bech32-encoded bridge address corresponding to the bridged rollup
     ///  asset on the sequencer. Should match the bridge address in the geth
     /// rollup's bridge configuration for that asset.