Skip to content

Commit

Permalink
refactor(core): parse ics20 denoms as ibc or trace prefixed variants (#…
Browse files Browse the repository at this point in the history
…1181)

## Summary
Rewrites the ics20 parsing logic to distinguish between `ibc/` and
`<path>` prefixed denoms.

## Background
While #1162 was an improvement
of what we had before, it was still problematic in that the parsing
logic did not distinguish between denom of the form
`[port/channel...]/base` and `ibc/hash`, so that a lot of the logic in
sequencer was done ad-hoc. This patch actually distinguishes between
both forms, allowing sequencer to enforce which type of denom it stores
in its database, and which type it processes further.

## Changes
- Rewrite `astria_core::primitive::v1::Denom` as an enum with variants
`TracePrefixed` and `IbcPrefixed`

## Testing
Adapt and expands unit tests, update sequencer tests.
  • Loading branch information
SuperFluffy authored Jun 17, 2024
1 parent 0180cbf commit 616dd9a
Show file tree
Hide file tree
Showing 26 changed files with 962 additions and 627 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ use std::time::Duration;
use astria_core::{
bridge::Ics20WithdrawalFromRollupMemo,
primitive::v1::{
asset,
asset::Denom,
asset::{
self,
denom::TracePrefixed,
Denom,
},
Address,
ASTRIA_ADDRESS_PREFIX,
},
Expand Down Expand Up @@ -132,7 +135,8 @@ fn event_to_ics20_withdrawal(
let denom = rollup_asset_denom.clone();

let channel = denom
.channel()
.as_trace_prefixed()
.and_then(TracePrefixed::last_channel)
.ok_or_eyre("denom must have a channel to be withdrawn via IBC")?;

let memo = Ics20WithdrawalFromRollupMemo {
Expand Down
14 changes: 10 additions & 4 deletions crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ use std::{
};

use astria_core::primitive::v1::{
asset,
asset::Denom,
asset::{
self,
denom,
Denom,
},
Address,
};
use astria_eyre::{
Expand Down Expand Up @@ -79,7 +82,10 @@ impl Builder {
let contract_address = address_from_string(&ethereum_contract_address)
.wrap_err("failed to parse ethereum contract address")?;

if !rollup_asset_denom.is_prefixed() {
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"
Expand Down Expand Up @@ -877,7 +883,7 @@ mod tests {
let submitter_handle = submitter::Handle::new(startup_rx, batch_tx);
startup_tx
.send(SequencerStartupInfo {
fee_asset_id: asset::Id::from_denom("transfer/channel-0/utia"),
fee_asset_id: asset::Id::from_str_unchecked("transfer/channel-0/utia"),
next_batch_rollup_height: 0,
})
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-bridge-withdrawer/src/withdrawer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl Service {
sequencer_chain_id,
sequencer_key_path,
state: state.clone(),
expected_fee_asset_id: asset::Id::from_denom(&fee_asset_denomination),
expected_fee_asset_id: asset::Id::from_str_unchecked(&fee_asset_denomination),
min_expected_fee_asset_balance: u128::from(min_expected_fee_asset_balance),
metrics,
}
Expand Down
12 changes: 6 additions & 6 deletions crates/astria-cli/src/commands/sequencer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,9 @@ pub(crate) async fn fee_asset_add(args: &FeeAssetChangeArgs) -> eyre::Result<()>
args.sequencer_url.as_str(),
args.sequencer_chain_id.clone(),
args.private_key.as_str(),
Action::FeeAssetChange(FeeAssetChangeAction::Addition(asset::Id::from_denom(
&args.asset,
))),
Action::FeeAssetChange(FeeAssetChangeAction::Addition(
asset::Id::from_str_unchecked(&args.asset),
)),
)
.await
.wrap_err("failed to submit FeeAssetChangeAction::Addition transaction")?;
Expand All @@ -353,9 +353,9 @@ pub(crate) async fn fee_asset_remove(args: &FeeAssetChangeArgs) -> eyre::Result<
args.sequencer_url.as_str(),
args.sequencer_chain_id.clone(),
args.private_key.as_str(),
Action::FeeAssetChange(FeeAssetChangeAction::Removal(asset::Id::from_denom(
&args.asset,
))),
Action::FeeAssetChange(FeeAssetChangeAction::Removal(
asset::Id::from_str_unchecked(&args.asset),
)),
)
.await
.wrap_err("failed to submit FeeAssetChangeAction::Removal transaction")?;
Expand Down
1 change: 1 addition & 0 deletions crates/astria-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ celestia-tendermint = { workspace = true }
ed25519-consensus = { version = "2.1.0", default-features = false, features = [
"std",
] }
hex = { workspace = true }
ibc-types = { workspace = true }
indexmap = { workspace = true }
pbjson-types = { workspace = true }
Expand Down
Loading

0 comments on commit 616dd9a

Please sign in to comment.