Skip to content

Commit

Permalink
feat(sequencer)!: add fee for Ics20Withdrawal action (#733)
Browse files Browse the repository at this point in the history
## Summary
add `fee_asset_id` to the `Ics20Withdrawal` action and charge for it,
`ICS20_WITHDRAWAL_FEE` is arbitrarily set to 24 right now.

## Background
don't want IBC transfers to be free as they can be submitted by anyone.

## Changes
- add `ICS20_WITHDRAWAL_FEE = 24`
- add `fee_asset_id` to the `Ics20Withdrawal` action 
- check for enough balance to pay for fee in `check_stateful`, deduct
fee from balance in `execute`

## Testing
hermes testing

## Breaking Changelist
- `Ics20Withdrawal` is no longer free but this doesn't really affect
anything currently deployed as we haven't used it.

## Related Issues

closes #718
  • Loading branch information
noot authored Feb 12, 2024
1 parent 768e01a commit ea4652b
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 9 deletions.
3 changes: 3 additions & 0 deletions crates/astria-core/src/generated/astria.sequencer.v1alpha1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,9 @@ pub struct Ics20Withdrawal {
/// the source channel used for the withdrawal.
#[prost(string, tag = "7")]
pub source_channel: ::prost::alloc::string::String,
/// the asset used to pay the transaction fee
#[prost(bytes = "vec", tag = "8")]
pub fee_asset_id: ::prost::alloc::vec::Vec<u8>,
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
Expand Down
18 changes: 18 additions & 0 deletions crates/astria-core/src/sequencer/v1alpha1/transaction/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,8 @@ pub struct Ics20Withdrawal {
timeout_time: u64,
// the source channel used for the withdrawal.
source_channel: ChannelId,
// the asset to use for fee payment.
fee_asset_id: asset::Id,
}

impl Ics20Withdrawal {
Expand Down Expand Up @@ -602,6 +604,11 @@ impl Ics20Withdrawal {
&self.source_channel
}

#[must_use]
pub fn fee_asset_id(&self) -> &asset::Id {
&self.fee_asset_id
}

#[must_use]
pub fn to_fungible_token_packet_data(&self) -> FungibleTokenPacketData {
FungibleTokenPacketData {
Expand All @@ -622,6 +629,7 @@ impl Ics20Withdrawal {
timeout_height: Some(self.timeout_height.into_raw()),
timeout_time: self.timeout_time,
source_channel: self.source_channel.to_string(),
fee_asset_id: self.fee_asset_id.as_bytes().to_vec(),
}
}

Expand All @@ -635,6 +643,7 @@ impl Ics20Withdrawal {
timeout_height: Some(self.timeout_height.into_raw()),
timeout_time: self.timeout_time,
source_channel: self.source_channel.to_string(),
fee_asset_id: self.fee_asset_id.as_bytes().to_vec(),
}
}

Expand Down Expand Up @@ -667,6 +676,8 @@ impl Ics20Withdrawal {
.source_channel
.parse()
.map_err(Ics20WithdrawalError::invalid_source_channel)?,
fee_asset_id: asset::Id::try_from_slice(&proto.fee_asset_id)
.map_err(Ics20WithdrawalError::invalid_fee_asset_id)?,
})
}
}
Expand Down Expand Up @@ -724,6 +735,11 @@ impl Ics20WithdrawalError {
fn invalid_source_channel(err: IdentifierError) -> Self {
Self(Ics20WithdrawalErrorKind::InvalidSourceChannel(err))
}

#[must_use]
fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self {
Self(Ics20WithdrawalErrorKind::InvalidFeeAssetId(err))
}
}

#[derive(Debug, thiserror::Error)]
Expand All @@ -736,4 +752,6 @@ enum Ics20WithdrawalErrorKind {
MissingTimeoutHeight,
#[error("`source_channel` field was invalid")]
InvalidSourceChannel(IdentifierError),
#[error("`fee_asset_id` field was invalid")]
InvalidFeeAssetId(asset::IncorrectAssetIdLength),
}
65 changes: 56 additions & 9 deletions crates/astria-sequencer/src/accounts/ics20_withdrawal.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::{
anyhow,
ensure,
Context as _,
Result,
Expand Down Expand Up @@ -28,6 +29,9 @@ use crate::{
transaction::action_handler::ActionHandler,
};

/// Fee charged for a `Ics20Withdrawal` action.
pub(crate) const ICS20_WITHDRAWAL_FEE: u128 = 24;

fn withdrawal_to_unchecked_ibc_packet(
withdrawal: &action::Ics20Withdrawal,
) -> IBCPacket<Unchecked> {
Expand Down Expand Up @@ -68,14 +72,42 @@ impl ActionHandler for action::Ics20Withdrawal {
.await
.context("packet failed send check")?;

let from_transfer_balance = state
.get_account_balance(from, self.denom().id())
let transfer_asset_id = self.denom().id();

let from_fee_balance = state
.get_account_balance(from, *self.fee_asset_id())
.await
.context("failed getting `from` account balance for transfer")?;
ensure!(
from_transfer_balance >= self.amount(),
"insufficient funds for transfer"
);
.context("failed getting `from` account balance for fee payment")?;

// if fee asset is same as transfer asset, ensure accounts has enough funds
// to cover both the fee and the amount transferred
if self.fee_asset_id() == &transfer_asset_id {
let payment_amount = self
.amount()
.checked_add(ICS20_WITHDRAWAL_FEE)
.ok_or(anyhow!("transfer amount plus fee overflowed"))?;

ensure!(
from_fee_balance >= payment_amount,
"insufficient funds for transfer and fee payment"
);
} else {
// otherwise, check the fee asset account has enough to cover the fees,
// and the transfer asset account has enough to cover the transfer
ensure!(
from_fee_balance >= ICS20_WITHDRAWAL_FEE,
"insufficient funds for fee payment"
);

let from_transfer_balance = state
.get_account_balance(from, transfer_asset_id)
.await
.context("failed to get account balance in transfer check")?;
ensure!(
from_transfer_balance >= self.amount(),
"insufficient funds for transfer"
);
}

Ok(())
}
Expand All @@ -87,15 +119,30 @@ impl ActionHandler for action::Ics20Withdrawal {
let from_transfer_balance = state
.get_account_balance(from, self.denom().id())
.await
.context("failed getting `from` account balance for transfer")?;
.context("failed getting `from` account balance for Ics20Withdrawal")?;

state
.put_account_balance(
from,
self.denom().id(),
from_transfer_balance
.checked_sub(self.amount())
.context("insufficient funds for transfer")?,
.context("insufficient funds for Ics20Withdrawal")?,
)
.context("failed to update sender balance")?;

let from_fee_balance = state
.get_account_balance(from, *self.fee_asset_id())
.await
.context("failed getting `from` account balance for Ics20Withdrawal fee payment")?;

state
.put_account_balance(
from,
*self.fee_asset_id(),
from_fee_balance
.checked_sub(ICS20_WITHDRAWAL_FEE)
.context("insufficient funds for Ics20Withdrawal fee")?,
)
.context("failed to update sender balance")?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ message Ics20Withdrawal {
uint64 timeout_time = 6;
// the source channel used for the withdrawal.
string source_channel = 7;
// the asset used to pay the transaction fee
bytes fee_asset_id = 8;
}

message IbcHeight {
Expand Down

0 comments on commit ea4652b

Please sign in to comment.