Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bridge-withdrawer): check if base denom is usdc to determine when to use compat address #1671

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions crates/astria-bridge-contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{
use astria_core::{
primitive::v1::{
asset,
asset::Denom,
Address,
AddressError,
},
Expand Down Expand Up @@ -36,6 +37,7 @@ use ethers::{
},
};
pub use generated::*;
use ibc_types::core::channel::ChannelId;

#[derive(Debug, thiserror::Error)]
#[error(transparent)]
Expand Down Expand Up @@ -263,6 +265,7 @@ where
}

let mut ics20_source_channel = None;
let mut use_compat_address = false;
if let Some(ics20_asset_to_withdraw) = &ics20_asset_to_withdraw {
ics20_source_channel.replace(
ics20_asset_to_withdraw
Expand All @@ -271,6 +274,13 @@ where
.parse()
.map_err(BuildError::parse_ics20_asset_source_channel)?,
);

// USDC from Noble requires using the bech32 compat address as the sender/return
// address.
//
// since we're always unwrapping the asset (sending back to the originating chain), we
// can check that the base denom is USDC, and if it is, we know we're sending to Noble.
use_compat_address = ics20_asset_to_withdraw.base_denom() == "usdc";
Copy link
Member

@SuperFluffy SuperFluffy Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of hardcoding this here, I'd prefer to add a method fn use_compat_address(self, use_compat_address: bool) -> Self on the builder and let the user of this crate decide if they want compat or not.

};

let contract =
Expand All @@ -297,6 +307,7 @@ where
sequencer_asset_to_withdraw,
ics20_asset_to_withdraw,
ics20_source_channel,
use_compat_address,
})
}
}
Expand All @@ -310,6 +321,7 @@ pub struct GetWithdrawalActions<P> {
sequencer_asset_to_withdraw: Option<asset::Denom>,
ics20_asset_to_withdraw: Option<asset::TracePrefixed>,
ics20_source_channel: Option<ibc_types::core::channel::ChannelId>,
use_compat_address: bool,
}

impl<P> GetWithdrawalActions<P>
Expand Down Expand Up @@ -394,7 +406,7 @@ where
let event = decode_log::<Ics20WithdrawalFilter>(log)
.map_err(GetWithdrawalActionsError::decode_log)?;

let (denom, source_channel) = (
let (denom, source_channel): (Denom, ChannelId) = (
self.ics20_asset_to_withdraw
.clone()
.expect("must be set if this method is entered")
Expand Down Expand Up @@ -428,9 +440,7 @@ where
timeout_time: timeout_in_5_min(),
source_channel,
bridge_address: Some(self.bridge_address),
// FIXME: this needs a way to determine when to use compat address
// https://github.com/astriaorg/astria/issues/1424
use_compat_address: false,
use_compat_address: self.use_compat_address,
};
Ok(Action::Ics20Withdrawal(action))
}
Expand Down
Loading