From 6ee0f6f223e3e4dce439776005ba2756eaf7da8e Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 12 Jul 2024 21:04:39 +0200 Subject: [PATCH 1/2] fix(core, bridge, sequencer)!: separate rollup and sequencer return addresses --- .../src/bridge_withdrawer/ethereum/convert.rs | 25 ++++------------- .../src/bridge_withdrawer/ethereum/watcher.rs | 28 ++----------------- .../src/bridge_withdrawer/mod.rs | 5 +--- .../src/bridge_withdrawer/submitter/tests.rs | 3 +- crates/astria-bridge-withdrawer/src/lib.rs | 7 ++--- crates/astria-core/src/bridge.rs | 15 ++++++---- crates/astria-core/src/serde.rs | 20 +++++++++++++ ...ge__test__bridge_unlock_memo_snapshot.snap | 4 +-- ..._withdrawal_from_rollup_memo_snapshot.snap | 9 +++--- .../src/ibc/ics20_transfer.rs | 5 ++-- 10 files changed, 52 insertions(+), 69 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs index 8dd50f85c8..6bd4cea978 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs @@ -56,7 +56,6 @@ pub(crate) fn event_to_action( rollup_asset_denom: asset::Denom, asset_withdrawal_divisor: u128, bridge_address: Address, - sequencer_address_prefix: &str, ) -> eyre::Result { let action = match event_with_metadata.event { WithdrawalEvent::Sequencer(event) => event_to_bridge_unlock( @@ -76,7 +75,6 @@ pub(crate) fn event_to_action( rollup_asset_denom, asset_withdrawal_divisor, bridge_address, - sequencer_address_prefix, ) .wrap_err("failed to convert ics20 withdrawal event to action")?, }; @@ -128,12 +126,10 @@ fn event_to_ics20_withdrawal( rollup_asset_denom: asset::Denom, asset_withdrawal_divisor: u128, bridge_address: Address, - sequencer_address_prefix: &str, ) -> eyre::Result { // TODO: make this configurable const ICS20_WITHDRAWAL_TIMEOUT: Duration = Duration::from_secs(300); - let sender = event.sender.to_fixed_bytes(); let denom = rollup_asset_denom.clone(); let channel = denom @@ -143,23 +139,16 @@ fn event_to_ics20_withdrawal( let memo = Ics20WithdrawalFromRollupMemo { memo: event.memo, - bridge_address, block_number: block_number.as_u64(), + rollup_return_address: event.sender.to_string(), + sequencer_bridge_address: bridge_address, transaction_hash: transaction_hash.into(), }; 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: Address::builder() - .array(sender) - .prefix(sequencer_address_prefix) - .try_build() - .wrap_err("failed to construct return address")?, + return_address: bridge_address, amount: event .amount .as_u128() @@ -221,7 +210,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap(); let Action::BridgeUnlock(action) = action else { @@ -263,7 +251,6 @@ mod tests { denom.clone(), divisor, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap(); let Action::BridgeUnlock(action) = action else { @@ -307,7 +294,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap(); let Action::Ics20Withdrawal(mut action) = action else { @@ -321,12 +307,13 @@ mod tests { let expected_action = Ics20Withdrawal { denom: denom.clone(), destination_chain_address, - return_address: crate::astria_address([0u8; 20]), + return_address: bridge_address, amount: 99, memo: serde_json::to_string(&Ics20WithdrawalFromRollupMemo { memo: "hello".to_string(), - bridge_address, block_number: 1u64, + rollup_return_address: ethers::types::Address::from([0u8; 20]).to_string(), + sequencer_bridge_address: bridge_address, transaction_hash: [2u8; 32], }) .unwrap(), 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 adb7de7457..4e24918000 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -75,7 +75,6 @@ pub(crate) struct Builder { pub(crate) rollup_asset_denom: Denom, pub(crate) bridge_address: Address, pub(crate) submitter_handle: submitter::Handle, - pub(crate) sequencer_address_prefix: String, } impl Builder { @@ -89,7 +88,6 @@ impl Builder { rollup_asset_denom, bridge_address, submitter_handle, - sequencer_address_prefix, } = self; let contract_address = address_from_string(ðereum_contract_address) @@ -114,7 +112,6 @@ impl Builder { shutdown_token: shutdown_token.clone(), startup_handle, submitter_handle, - sequencer_address_prefix, }) } } @@ -129,7 +126,6 @@ pub(crate) struct Watcher { rollup_asset_denom: Denom, bridge_address: Address, state: Arc, - sequencer_address_prefix: String, } impl Watcher { @@ -145,7 +141,6 @@ impl Watcher { state, shutdown_token, submitter_handle, - sequencer_address_prefix, .. } = self; @@ -154,7 +149,6 @@ impl Watcher { rollup_asset_denom, bridge_address, asset_withdrawal_divisor, - sequencer_address_prefix, }; state.set_watcher_ready(); @@ -497,7 +491,6 @@ struct EventToActionConvertConfig { rollup_asset_denom: Denom, bridge_address: Address, asset_withdrawal_divisor: u128, - sequencer_address_prefix: String, } impl EventToActionConvertConfig { @@ -508,7 +501,6 @@ impl EventToActionConvertConfig { self.rollup_asset_denom.clone(), self.asset_withdrawal_divisor, self.bridge_address, - &self.sequencer_address_prefix, ) } } @@ -670,7 +662,6 @@ mod tests { state: Arc::new(State::new()), rollup_asset_denom: denom.clone(), bridge_address, - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } .build() .unwrap(); @@ -686,15 +677,8 @@ mod tests { 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, - crate::ASTRIA_ADDRESS_PREFIX, - ) - .unwrap(); + 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:?}"); }; @@ -743,7 +727,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap(); let Action::BridgeUnlock(expected_action) = expected_action else { @@ -768,7 +751,6 @@ mod tests { rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } .build() .unwrap(); @@ -849,7 +831,6 @@ mod tests { rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } .build() .unwrap(); @@ -874,7 +855,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap() else { panic!("expected action to be Ics20Withdrawal"); @@ -976,7 +956,6 @@ mod tests { rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } .build() .unwrap(); @@ -999,7 +978,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap(); let Action::BridgeUnlock(expected_action) = expected_action else { @@ -1078,7 +1056,6 @@ mod tests { rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } .build() .unwrap(); @@ -1107,7 +1084,6 @@ mod tests { denom.clone(), 1, bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, ) .unwrap() else { panic!("expected action to be Ics20Withdrawal"); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index d655f9c645..216ec60770 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -124,7 +124,6 @@ impl BridgeWithdrawer { .wrap_err("failed to parse ROLLUP_ASSET_DENOMINATION as Denom")?, bridge_address: sequencer_bridge_address, submitter_handle, - sequencer_address_prefix: sequencer_address_prefix.clone(), } .build() .wrap_err("failed to build ethereum watcher")?; @@ -410,8 +409,6 @@ pub(crate) fn flatten_result(res: Result, JoinError>) -> eyre } #[cfg(test)] -pub(crate) const ASTRIA_ADDRESS_PREFIX: &str = "astria"; - /// Constructs an [`Address`] prefixed by `"astria"`. #[cfg(test)] pub(crate) fn astria_address( @@ -419,7 +416,7 @@ pub(crate) fn astria_address( ) -> astria_core::primitive::v1::Address { astria_core::primitive::v1::Address::builder() .array(array) - .prefix(ASTRIA_ADDRESS_PREFIX) + .prefix("astria") .try_build() .unwrap() } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index b0e681897f..4270acae9b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -189,8 +189,9 @@ fn make_ics20_withdrawal_action() -> Action { amount: 99, memo: serde_json::to_string(&Ics20WithdrawalFromRollupMemo { memo: "hello".to_string(), - bridge_address: crate::astria_address([0u8; 20]), block_number: DEFAULT_LAST_ROLLUP_HEIGHT, + rollup_return_address: ethers::types::Address::from([0u8; 20]).to_string(), + sequencer_bridge_address: crate::astria_address([0u8; 20]), transaction_hash: [2u8; 32], }) .unwrap(), diff --git a/crates/astria-bridge-withdrawer/src/lib.rs b/crates/astria-bridge-withdrawer/src/lib.rs index 623892b393..8ce4dae86e 100644 --- a/crates/astria-bridge-withdrawer/src/lib.rs +++ b/crates/astria-bridge-withdrawer/src/lib.rs @@ -4,11 +4,8 @@ mod build_info; pub(crate) mod config; pub(crate) mod metrics; -pub use bridge_withdrawer::BridgeWithdrawer; #[cfg(test)] -pub(crate) use bridge_withdrawer::{ - astria_address, - ASTRIA_ADDRESS_PREFIX, -}; +pub(crate) use bridge_withdrawer::astria_address; +pub use bridge_withdrawer::BridgeWithdrawer; pub use build_info::BUILD_INFO; pub use config::Config; diff --git a/crates/astria-core/src/bridge.rs b/crates/astria-core/src/bridge.rs index a8eedce914..9ce88dc486 100644 --- a/crates/astria-core/src/bridge.rs +++ b/crates/astria-core/src/bridge.rs @@ -4,7 +4,8 @@ use crate::primitive::v1::Address; #[cfg_attr( feature = "serde", derive(serde::Serialize), - derive(serde::Deserialize) + derive(serde::Deserialize), + serde(rename_all = "camelCase", deny_unknown_fields) )] pub struct UnlockMemo { pub block_number: u64, @@ -24,12 +25,15 @@ pub struct UnlockMemo { #[cfg_attr( feature = "serde", derive(serde::Serialize), - derive(serde::Deserialize) + derive(serde::Deserialize), + serde(rename_all = "camelCase", deny_unknown_fields) )] pub struct Ics20WithdrawalFromRollupMemo { pub memo: String, - pub bridge_address: Address, pub block_number: u64, + pub rollup_return_address: String, + #[cfg_attr(feature = "serde", serde(with = "crate::serde::address_string"))] + pub sequencer_bridge_address: Address, #[cfg_attr( feature = "serde", serde( @@ -72,12 +76,13 @@ mod test { fn ics20_withdrawal_from_rollup_memo_snapshot() { let memo = Ics20WithdrawalFromRollupMemo { memo: "hello".to_string(), - bridge_address: Address::builder() + block_number: 1, + rollup_return_address: "rollup-defined".to_string(), + sequencer_bridge_address: Address::builder() .array([99; 20]) .prefix("astria") .try_build() .unwrap(), - block_number: 1, transaction_hash: [88; 32], }; diff --git a/crates/astria-core/src/serde.rs b/crates/astria-core/src/serde.rs index fae04f8e00..266a9c87f9 100644 --- a/crates/astria-core/src/serde.rs +++ b/crates/astria-core/src/serde.rs @@ -21,3 +21,23 @@ where let bytes = Base64Standard::deserialize(deserializer)?; T::try_from(bytes).map_err(|_| serde::de::Error::custom("invalid array length")) } + +pub(crate) mod address_string { + use serde::{ + Deserialize as _, + Deserializer, + Serializer, + }; + + use crate::primitive::v1::Address; + + pub(crate) fn serialize(address: &Address, ser: S) -> Result { + ser.collect_str(address) + } + + pub(crate) fn deserialize<'de, D: Deserializer<'de>>(deser: D) -> Result { + use serde::de::Error as _; + let s = std::borrow::Cow::<'_, str>::deserialize(deser)?; + s.trim().parse().map_err(D::Error::custom) + } +} diff --git a/crates/astria-core/src/snapshots/astria_core__bridge__test__bridge_unlock_memo_snapshot.snap b/crates/astria-core/src/snapshots/astria_core__bridge__test__bridge_unlock_memo_snapshot.snap index f0f7700ccb..611bd40d7d 100644 --- a/crates/astria-core/src/snapshots/astria_core__bridge__test__bridge_unlock_memo_snapshot.snap +++ b/crates/astria-core/src/snapshots/astria_core__bridge__test__bridge_unlock_memo_snapshot.snap @@ -3,6 +3,6 @@ source: crates/astria-core/src/bridge.rs expression: memo --- { - "block_number": 42, - "transaction_hash": "WFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFg=" + "blockNumber": 42, + "transactionHash": "WFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFg=" } diff --git a/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap b/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap index 6de0a3120b..2d342e2d71 100644 --- a/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap +++ b/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap @@ -4,9 +4,8 @@ expression: memo --- { "memo": "hello", - "bridge_address": { - "bech32m": "astria1vd3kxcmrvd3kxcmrvd3kxcmrvd3kxcmrj6p6kl" - }, - "block_number": 1, - "transaction_hash": "WFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFg=" + "blockNumber": 1, + "rollupReturnAddress": "rollup-defined", + "sequencerBridgeAddress": "astria1vd3kxcmrvd3kxcmrvd3kxcmrvd3kxcmrj6p6kl", + "transactionHash": "WFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFg=" } diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index 197fe214f3..d2c2bca330 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -418,7 +418,7 @@ async fn execute_ics20_transfer( maybe_memo.expect("memo is valid as it was checked by is_ok()"); execute_rollup_withdrawal_refund( state, - &memo.bridge_address, + &memo.sequencer_bridge_address, &denom_trace, packet_amount, recipient, @@ -1046,9 +1046,10 @@ mod test { amount: "100".to_string(), receiver: "other-chain-address".to_string(), memo: serde_json::to_string(&Ics20WithdrawalFromRollupMemo { - bridge_address, memo: String::new(), block_number: 1, + rollup_return_address: "rollup-defined".to_string(), + sequencer_bridge_address: bridge_address, transaction_hash: [1u8; 32], }) .unwrap(), From a404983e0385abd018731f240ed722d184259301 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 12 Jul 2024 21:25:42 +0200 Subject: [PATCH 2/2] use packet sender as bridge address for refunds of rollup initialized ics20 withdrawals --- .../src/bridge_withdrawer/ethereum/convert.rs | 2 - .../src/bridge_withdrawer/submitter/tests.rs | 1 - crates/astria-core/src/bridge.rs | 9 ----- crates/astria-core/src/serde.rs | 20 ---------- ..._withdrawal_from_rollup_memo_snapshot.snap | 1 - .../src/ibc/ics20_transfer.rs | 39 ++++++++++--------- 6 files changed, 20 insertions(+), 52 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs index 6bd4cea978..dab2828183 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs @@ -141,7 +141,6 @@ fn event_to_ics20_withdrawal( memo: event.memo, block_number: block_number.as_u64(), rollup_return_address: event.sender.to_string(), - sequencer_bridge_address: bridge_address, transaction_hash: transaction_hash.into(), }; @@ -313,7 +312,6 @@ mod tests { memo: "hello".to_string(), block_number: 1u64, rollup_return_address: ethers::types::Address::from([0u8; 20]).to_string(), - sequencer_bridge_address: bridge_address, transaction_hash: [2u8; 32], }) .unwrap(), diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 4270acae9b..1c4e693ed5 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -191,7 +191,6 @@ fn make_ics20_withdrawal_action() -> Action { memo: "hello".to_string(), block_number: DEFAULT_LAST_ROLLUP_HEIGHT, rollup_return_address: ethers::types::Address::from([0u8; 20]).to_string(), - sequencer_bridge_address: crate::astria_address([0u8; 20]), transaction_hash: [2u8; 32], }) .unwrap(), diff --git a/crates/astria-core/src/bridge.rs b/crates/astria-core/src/bridge.rs index 9ce88dc486..c8cb1fb570 100644 --- a/crates/astria-core/src/bridge.rs +++ b/crates/astria-core/src/bridge.rs @@ -1,5 +1,3 @@ -use crate::primitive::v1::Address; - #[derive(Clone, Debug)] #[cfg_attr( feature = "serde", @@ -32,8 +30,6 @@ pub struct Ics20WithdrawalFromRollupMemo { pub memo: String, pub block_number: u64, pub rollup_return_address: String, - #[cfg_attr(feature = "serde", serde(with = "crate::serde::address_string"))] - pub sequencer_bridge_address: Address, #[cfg_attr( feature = "serde", serde( @@ -78,11 +74,6 @@ mod test { memo: "hello".to_string(), block_number: 1, rollup_return_address: "rollup-defined".to_string(), - sequencer_bridge_address: Address::builder() - .array([99; 20]) - .prefix("astria") - .try_build() - .unwrap(), transaction_hash: [88; 32], }; diff --git a/crates/astria-core/src/serde.rs b/crates/astria-core/src/serde.rs index 266a9c87f9..fae04f8e00 100644 --- a/crates/astria-core/src/serde.rs +++ b/crates/astria-core/src/serde.rs @@ -21,23 +21,3 @@ where let bytes = Base64Standard::deserialize(deserializer)?; T::try_from(bytes).map_err(|_| serde::de::Error::custom("invalid array length")) } - -pub(crate) mod address_string { - use serde::{ - Deserialize as _, - Deserializer, - Serializer, - }; - - use crate::primitive::v1::Address; - - pub(crate) fn serialize(address: &Address, ser: S) -> Result { - ser.collect_str(address) - } - - pub(crate) fn deserialize<'de, D: Deserializer<'de>>(deser: D) -> Result { - use serde::de::Error as _; - let s = std::borrow::Cow::<'_, str>::deserialize(deser)?; - s.trim().parse().map_err(D::Error::custom) - } -} diff --git a/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap b/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap index 2d342e2d71..614cb85239 100644 --- a/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap +++ b/crates/astria-core/src/snapshots/astria_core__bridge__test__ics20_withdrawal_from_rollup_memo_snapshot.snap @@ -6,6 +6,5 @@ expression: memo "memo": "hello", "blockNumber": 1, "rollupReturnAddress": "rollup-defined", - "sequencerBridgeAddress": "astria1vd3kxcmrvd3kxcmrvd3kxcmrvd3kxcmrj6p6kl", "transactionHash": "WFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFg=" } diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index d2c2bca330..0d60274bde 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -389,7 +389,7 @@ async fn execute_ics20_transfer( .parse() .context("failed to parse packet data amount to u128")?; let recipient = if is_refund { - packet_data.sender + packet_data.sender.clone() } else { packet_data.receiver }; @@ -412,13 +412,15 @@ async fn execute_ics20_transfer( // // in this case, we lock the tokens back in the bridge account and // emit a `Deposit` event to send the tokens back to the rollup. - let maybe_memo = serde_json::from_slice(packet_data.memo.as_bytes()); - if is_refund && maybe_memo.is_ok() { - let memo: Ics20WithdrawalFromRollupMemo = - maybe_memo.expect("memo is valid as it was checked by is_ok()"); + if is_refund && serde_json::from_str::(&packet_data.memo).is_ok() + { + let bridge_account = packet_data.sender.parse().context( + "sender not an Astria Address: for refunds of ics20 withdrawals that came from a \ + rollup, the sender must be a valid Astria Address (usually the bridge account)", + )?; execute_rollup_withdrawal_refund( state, - &memo.sequencer_bridge_address, + bridge_account, &denom_trace, packet_amount, recipient, @@ -448,7 +450,7 @@ async fn execute_ics20_transfer( // execute relevant state changes if it is execute_ics20_transfer_bridge_lock( state, - &recipient, + recipient, &trace_with_dest, packet_amount, packet_data.memo.clone(), @@ -525,7 +527,7 @@ async fn execute_ics20_transfer( /// and locks the tokens back in the specified bridge account. async fn execute_rollup_withdrawal_refund( state: &mut S, - bridge_address: &Address, + bridge_address: Address, denom: &denom::TracePrefixed, amount: u128, destination_address: String, @@ -533,7 +535,7 @@ async fn execute_rollup_withdrawal_refund( execute_deposit(state, bridge_address, denom, amount, destination_address).await?; state - .increase_balance(*bridge_address, denom, amount) + .increase_balance(bridge_address, denom, amount) .await .context( "failed to update bridge account account balance in execute_rollup_withdrawal_refund", @@ -548,7 +550,7 @@ async fn execute_rollup_withdrawal_refund( /// this function is a no-op. async fn execute_ics20_transfer_bridge_lock( state: &mut S, - recipient: &Address, + recipient: Address, denom: &denom::TracePrefixed, amount: u128, memo: String, @@ -557,7 +559,7 @@ async fn execute_ics20_transfer_bridge_lock( // check if the recipient is a bridge account; if so, // ensure that the packet memo field (`destination_address`) is set. let is_bridge_lock = state - .get_bridge_account_rollup_id(recipient) + .get_bridge_account_rollup_id(&recipient) .await .context("failed to get bridge account rollup ID from state")? .is_some(); @@ -597,7 +599,7 @@ async fn execute_ics20_transfer_bridge_lock( async fn execute_deposit( state: &mut S, - bridge_address: &Address, + bridge_address: Address, denom: &denom::TracePrefixed, amount: u128, destination_address: String, @@ -606,7 +608,7 @@ async fn execute_deposit( // ensure that the asset ID being transferred // to it is allowed. let Some(rollup_id) = state - .get_bridge_account_rollup_id(bridge_address) + .get_bridge_account_rollup_id(&bridge_address) .await .context("failed to get bridge account rollup ID from state")? else { @@ -614,7 +616,7 @@ async fn execute_deposit( }; let allowed_asset = state - .get_bridge_account_ibc_asset(bridge_address) + .get_bridge_account_ibc_asset(&bridge_address) .await .context("failed to get bridge account asset ID")?; ensure!( @@ -623,7 +625,7 @@ async fn execute_deposit( ); let deposit = Deposit::new( - *bridge_address, + bridge_address, rollup_id, amount, denom.into(), @@ -1003,7 +1005,7 @@ mod test { let destination_address = "destinationaddress".to_string(); execute_rollup_withdrawal_refund( &mut state_tx, - &bridge_address, + bridge_address, &denom, amount, destination_address, @@ -1030,8 +1032,8 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let destination_chain_address = "destinationchainaddress".to_string(); let bridge_address = crate::address::base_prefixed([99u8; 20]); + let destination_chain_address = bridge_address.to_string(); let denom = "nootasset".parse::().unwrap(); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); @@ -1042,14 +1044,13 @@ mod test { let packet = FungibleTokenPacketData { denom: denom.to_string(), - sender: destination_chain_address.clone(), + sender: bridge_address.to_string(), amount: "100".to_string(), receiver: "other-chain-address".to_string(), memo: serde_json::to_string(&Ics20WithdrawalFromRollupMemo { memo: String::new(), block_number: 1, rollup_return_address: "rollup-defined".to_string(), - sequencer_bridge_address: bridge_address, transaction_hash: [1u8; 32], }) .unwrap(),