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(core, sequencer): prefix removal source non-refund ics20 packet #1162

Merged
merged 6 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ target/

# We don't want to commit the dependency directory of charts
charts/*/charts

crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,8 @@ fn event_to_ics20_withdrawal(
let sender = event.sender.to_fixed_bytes();
let denom = rollup_asset_denom.clone();

let (_, channel) = denom
.prefix()
.rsplit_once('/')
let channel = denom
.channel()
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of doing this ad-hoc I created a method for this.

.ok_or_eyre("denom must have a channel to be withdrawn via IBC")?;

let memo = Ics20WithdrawalFromRollupMemo {
Expand Down Expand Up @@ -189,12 +188,14 @@ fn calculate_packet_timeout_time(timeout_delta: Duration) -> eyre::Result<u64> {

#[cfg(test)]
mod tests {
use asset::default_native_asset;

use super::*;
use crate::withdrawer::ethereum::astria_withdrawer_interface::SequencerWithdrawalFilter;

#[test]
fn event_to_bridge_unlock() {
let denom = Denom::from("nria".to_string());
let denom = default_native_asset();
let event_with_meta = EventWithMetadata {
event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
sender: [0u8; 20].into(),
Expand Down Expand Up @@ -238,7 +239,7 @@ mod tests {

#[test]
fn event_to_bridge_unlock_divide_value() {
let denom = Denom::from("nria".to_string());
let denom = default_native_asset();
let event_with_meta = EventWithMetadata {
event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter {
sender: [0u8; 20].into(),
Expand Down Expand Up @@ -283,7 +284,7 @@ mod tests {

#[test]
fn event_to_ics20_withdrawal() {
let denom = Denom::from("transfer/channel-0/utia".to_string());
let denom = "transfer/channel-0/utia".parse::<Denom>().unwrap();
let destination_chain_address = "address".to_string();
let event_with_meta = EventWithMetadata {
event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Builder {
let contract_address = address_from_string(&ethereum_contract_address)
.wrap_err("failed to parse ethereum contract address")?;

if rollup_asset_denom.prefix().is_empty() {
if !rollup_asset_denom.is_prefixed() {
warn!(
"rollup asset denomination is not prefixed; Ics20Withdrawal actions will not be \
submitted"
Expand Down Expand Up @@ -419,6 +419,7 @@ fn address_from_string(s: &str) -> Result<ethers::types::Address> {

#[cfg(test)]
mod tests {
use asset::default_native_asset;
use astria_core::{
primitive::v1::{
Address,
Expand Down Expand Up @@ -547,8 +548,8 @@ mod tests {
block_number: receipt.block_number.unwrap(),
transaction_hash: receipt.transaction_hash,
};
let denom: Denom = Denom::from_base_denom("nria");
let bridge_address = crate::astria_address([1u8; 20]);
let denom = default_native_asset();
let expected_action =
event_to_action(expected_event, denom.id(), denom.clone(), 1, bridge_address).unwrap();
let Action::BridgeUnlock(expected_action) = expected_action else {
Expand Down Expand Up @@ -638,8 +639,8 @@ mod tests {
block_number: receipt.block_number.unwrap(),
transaction_hash: receipt.transaction_hash,
};
let denom = Denom::from("transfer/channel-0/utia".to_string());
let bridge_address = crate::astria_address([1u8; 20]);
let denom = "transfer/channel-0/utia".parse::<Denom>().unwrap();
let Action::Ics20Withdrawal(mut expected_action) =
event_to_action(expected_event, denom.id(), denom.clone(), 1, bridge_address).unwrap()
else {
Expand Down Expand Up @@ -761,7 +762,7 @@ mod tests {
block_number: receipt.block_number.unwrap(),
transaction_hash: receipt.transaction_hash,
};
let denom: Denom = Denom::from_base_denom("nria");
let denom = default_native_asset();
let bridge_address = crate::astria_address([1u8; 20]);
let expected_action =
event_to_action(expected_event, denom.id(), denom.clone(), 1, bridge_address).unwrap();
Expand Down Expand Up @@ -862,7 +863,7 @@ mod tests {
block_number: receipt.block_number.unwrap(),
transaction_hash: receipt.transaction_hash,
};
let denom = Denom::from("transfer/channel-0/utia".to_string());
let denom = "transfer/channel-0/utia".parse::<Denom>().unwrap();
let bridge_address = crate::astria_address([1u8; 20]);
let Action::Ics20Withdrawal(mut expected_action) =
event_to_action(expected_event, denom.id(), denom.clone(), 1, bridge_address).unwrap()
Expand Down
9 changes: 7 additions & 2 deletions crates/astria-bridge-withdrawer/src/withdrawer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use std::{
};

use astria_core::primitive::v1::{
asset,
asset::{
self,
Denom,
},
Address,
};
use astria_eyre::eyre::{
Expand Down Expand Up @@ -97,7 +100,9 @@ impl Service {
submitter_handle,
shutdown_token: shutdown_handle.token(),
state: state.clone(),
rollup_asset_denom: asset::Denom::from(rollup_asset_denomination),
rollup_asset_denom: rollup_asset_denomination
.parse::<Denom>()
.wrap_err("failed to parse ROLLUP_ASSET_DENOMINATION as Denom")?,
bridge_address: sequencer_bridge_address,
}
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use astria_core::{
primitive::v1::{
asset::{
self,
default_native_asset,
Denom,
},
Address,
Expand Down Expand Up @@ -91,7 +92,6 @@ const SEQUENCER_CHAIN_ID: &str = "test_sequencer-1000";
const DEFAULT_LAST_ROLLUP_HEIGHT: u64 = 1;
const DEFAULT_LAST_SEQUENCER_HEIGHT: u64 = 0;
const DEFAULT_SEQUENCER_NONCE: u32 = 0;
const DEFAULT_NATIVE_DEMON: &str = "nria";
const DEFAULT_IBC_DENOM: &str = "transfer/channel-0/utia";

static TELEMETRY: Lazy<()> = Lazy::new(|| {
Expand Down Expand Up @@ -151,7 +151,7 @@ impl TestSubmitter {
sequencer_chain_id: SEQUENCER_CHAIN_ID.to_string(),
sequencer_cometbft_endpoint,
state,
expected_fee_asset_id: Denom::from(DEFAULT_NATIVE_DEMON.to_string()).id(),
expected_fee_asset_id: default_native_asset().id(),
min_expected_fee_asset_balance: 1_000_000,
}
.build()
Expand Down Expand Up @@ -214,7 +214,7 @@ async fn register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGuar
}

async fn register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard {
let fee_asset_ids = vec![Denom::from(DEFAULT_NATIVE_DEMON.to_string()).id()];
let fee_asset_ids = vec![default_native_asset().id()];
register_allowed_fee_asset_ids_response(fee_asset_ids, cometbft_mock).await
}

Expand All @@ -223,7 +223,7 @@ async fn register_default_min_expected_fee_asset_balance_guard(
) -> MockGuard {
register_get_latest_balance(
vec![AssetBalance {
denom: Denom::from(DEFAULT_NATIVE_DEMON.to_string()),
denom: default_native_asset(),
balance: 1_000_000u128,
}],
cometbft_mock,
Expand Down Expand Up @@ -270,7 +270,7 @@ async fn register_sync_guards(cometbft_mock: &MockServer) -> HashMap<String, Moc
}

fn make_ics20_withdrawal_action() -> Action {
let denom = Denom::from(DEFAULT_IBC_DENOM.to_string());
let denom = DEFAULT_IBC_DENOM.parse::<Denom>().unwrap();
let destination_chain_address = "address".to_string();
let inner = Ics20Withdrawal {
denom: denom.clone(),
Expand Down Expand Up @@ -299,7 +299,7 @@ fn make_ics20_withdrawal_action() -> Action {
}

fn make_bridge_unlock_action() -> Action {
let denom = Denom::from(DEFAULT_NATIVE_DEMON.to_string());
let denom = default_native_asset();
let inner = BridgeUnlockAction {
to: Address::builder()
.array([0u8; 20])
Expand Down
26 changes: 11 additions & 15 deletions crates/astria-cli/src/commands/sequencer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use astria_core::{
crypto::SigningKey,
primitive::v1::asset,
primitive::v1::{
asset,
asset::default_native_asset,
},
protocol::transaction::v1alpha1::{
action::{
Action,
Expand Down Expand Up @@ -172,17 +175,15 @@ pub(crate) async fn get_block_height(args: &BlockHeightGetArgs) -> eyre::Result<
/// * If the http client cannot be created
/// * If the latest block height cannot be retrieved
pub(crate) async fn send_transfer(args: &TransferArgs) -> eyre::Result<()> {
use astria_core::primitive::v1::asset::default_native_asset_id;

let res = submit_transaction(
args.sequencer_url.as_str(),
args.sequencer_chain_id.clone(),
args.private_key.as_str(),
Action::Transfer(TransferAction {
to: args.to_address.0,
amount: args.amount,
asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset_id(),
asset_id: default_native_asset().id(),
fee_asset_id: default_native_asset().id(),
}),
)
.await
Expand Down Expand Up @@ -254,10 +255,7 @@ pub(crate) async fn ibc_relayer_remove(args: &IbcRelayerChangeArgs) -> eyre::Res
/// * If the http client cannot be created
/// * If the transaction failed to be included
pub(crate) async fn init_bridge_account(args: &InitBridgeAccountArgs) -> eyre::Result<()> {
use astria_core::primitive::v1::{
asset::default_native_asset_id,
RollupId,
};
use astria_core::primitive::v1::RollupId;

let rollup_id = RollupId::from_unhashed_bytes(args.rollup_name.as_bytes());
let res = submit_transaction(
Expand All @@ -266,8 +264,8 @@ pub(crate) async fn init_bridge_account(args: &InitBridgeAccountArgs) -> eyre::R
args.private_key.as_str(),
Action::InitBridgeAccount(InitBridgeAccountAction {
rollup_id,
asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset_id(),
asset_id: default_native_asset().id(),
fee_asset_id: default_native_asset().id(),
sudo_address: None,
withdrawer_address: None,
}),
Expand All @@ -293,17 +291,15 @@ pub(crate) async fn init_bridge_account(args: &InitBridgeAccountArgs) -> eyre::R
/// * If the http client cannot be created
/// * If the transaction failed to be included
pub(crate) async fn bridge_lock(args: &BridgeLockArgs) -> eyre::Result<()> {
use astria_core::primitive::v1::asset::default_native_asset_id;

let res = submit_transaction(
args.sequencer_url.as_str(),
args.sequencer_chain_id.clone(),
args.private_key.as_str(),
Action::BridgeLock(BridgeLockAction {
to: args.to_address.0,
asset_id: default_native_asset_id(),
asset_id: default_native_asset().id(),
amount: args.amount,
fee_asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset().id(),
destination_chain_address: args.destination_chain_address.clone(),
}),
)
Expand Down
4 changes: 2 additions & 2 deletions crates/astria-composer/src/collectors/geth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::time::Duration;

use astria_core::{
primitive::v1::{
asset::default_native_asset_id,
asset::default_native_asset,
RollupId,
},
protocol::transaction::v1alpha1::action::SequenceAction,
Expand Down Expand Up @@ -211,7 +211,7 @@ impl Geth {
let seq_action = SequenceAction {
rollup_id,
data,
fee_asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset().id(),
};

metrics::counter!(
Expand Down
4 changes: 2 additions & 2 deletions crates/astria-composer/src/collectors/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use astria_core::{
SubmitRollupTransactionResponse,
},
primitive::v1::{
asset::default_native_asset_id,
asset::default_native_asset,
RollupId,
},
protocol::transaction::v1alpha1::action::SequenceAction,
Expand Down Expand Up @@ -63,7 +63,7 @@ impl GrpcCollectorService for Grpc {
let sequence_action = SequenceAction {
rollup_id,
data: submit_rollup_tx_request.data,
fee_asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset().id(),
};

metrics::counter!(
Expand Down
Loading
Loading