You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
relax XcmFeeToAccount trait bound on AccountId (paritytech#4959)
Fixesparitytech#4960
Configuring `FeeManager` enforces the boundary `Into<[u8; 32]>` for the
`AccountId` type.
Here is how it works currently:
Configuration:
```rust
type FeeManager = XcmFeeManagerFromComponents<
IsChildSystemParachain<primitives::Id>,
XcmFeeToAccount<Self::AssetTransactor, AccountId, TreasuryAccount>,
>;
```
`XcmToFeeAccount` struct:
```rust
/// A `HandleFee` implementation that simply deposits the fees into a specific on-chain
/// `ReceiverAccount`.
///
/// It reuses the `AssetTransactor` configured on the XCM executor to deposit fee assets. If
/// the `AssetTransactor` returns an error while calling `deposit_asset`, then a warning will be
/// logged and the fee burned.
pub struct XcmFeeToAccount<AssetTransactor, AccountId, ReceiverAccount>(
PhantomData<(AssetTransactor, AccountId, ReceiverAccount)>,
);
impl<
AssetTransactor: TransactAsset,
AccountId: Clone + Into<[u8; 32]>,
ReceiverAccount: Get<AccountId>,
> HandleFee for XcmFeeToAccount<AssetTransactor, AccountId, ReceiverAccount>
{
fn handle_fee(fee: Assets, context: Option<&XcmContext>, _reason: FeeReason) -> Assets {
deposit_or_burn_fee::<AssetTransactor, _>(fee, context, ReceiverAccount::get());
Assets::new()
}
}
```
`deposit_or_burn_fee()` function:
```rust
/// Try to deposit the given fee in the specified account.
/// Burns the fee in case of a failure.
pub fn deposit_or_burn_fee<AssetTransactor: TransactAsset, AccountId: Clone + Into<[u8; 32]>>(
fee: Assets,
context: Option<&XcmContext>,
receiver: AccountId,
) {
let dest = AccountId32 { network: None, id: receiver.into() }.into();
for asset in fee.into_inner() {
if let Err(e) = AssetTransactor::deposit_asset(&asset, &dest, context) {
log::trace!(
target: "xcm::fees",
"`AssetTransactor::deposit_asset` returned error: {:?}. Burning fee: {:?}. \
They might be burned.",
e, asset,
);
}
}
}
```
---
In order to use **another** `AccountId` type (for example, 20 byte
addresses for compatibility with Ethereum or Bitcoin), one has to
duplicate the code as the following (roughly changing every `32` to
`20`):
```rust
/// A `HandleFee` implementation that simply deposits the fees into a specific on-chain
/// `ReceiverAccount`.
///
/// It reuses the `AssetTransactor` configured on the XCM executor to deposit fee assets. If
/// the `AssetTransactor` returns an error while calling `deposit_asset`, then a warning will be
/// logged and the fee burned.
pub struct XcmFeeToAccount<AssetTransactor, AccountId, ReceiverAccount>(
PhantomData<(AssetTransactor, AccountId, ReceiverAccount)>,
);
impl<
AssetTransactor: TransactAsset,
AccountId: Clone + Into<[u8; 20]>,
ReceiverAccount: Get<AccountId>,
> HandleFee for XcmFeeToAccount<AssetTransactor, AccountId, ReceiverAccount>
{
fn handle_fee(fee: XcmAssets, context: Option<&XcmContext>, _reason: FeeReason) -> XcmAssets {
deposit_or_burn_fee::<AssetTransactor, _>(fee, context, ReceiverAccount::get());
XcmAssets::new()
}
}
pub fn deposit_or_burn_fee<AssetTransactor: TransactAsset, AccountId: Clone + Into<[u8; 20]>>(
fee: XcmAssets,
context: Option<&XcmContext>,
receiver: AccountId,
) {
let dest = AccountKey20 { network: None, key: receiver.into() }.into();
for asset in fee.into_inner() {
if let Err(e) = AssetTransactor::deposit_asset(&asset, &dest, context) {
log::trace!(
target: "xcm::fees",
"`AssetTransactor::deposit_asset` returned error: {:?}. Burning fee: {:?}. \
They might be burned.",
e, asset,
);
}
}
}
```
---
This results in code duplication, which can be avoided simply by
relaxing the trait enforced by `XcmFeeToAccount`.
In this PR, I propose to introduce a new trait called `IntoLocation` to
be able to express both `Into<[u8; 32]>` and `Into<[u8; 20]>` should be
accepted (and every other `AccountId` type as long as they implement
this trait).
Currently, `deposit_or_burn_fee()` function converts the `receiver:
AccountId` to a location. I think converting an account to `Location`
should not be the responsibility of `deposit_or_burn_fee()` function.
This trait also decouples the conversion of `AccountId` to `Location`,
from `deposit_or_burn_fee()` function. And exposes `IntoLocation` trait.
Thus, allowing everyone to come up with their `AccountId` type and make
it compatible for configuring `FeeManager`.
---
Note 1: if there is a better file/location to put `IntoLocation`, I'm
all ears
Note 2: making `deposit_or_burn_fee` or `XcmToFeeAccount` generic was
not possible from what I understood, due to Rust currently do not
support a way to express the generic should implement either `trait A`
or `trait B` (since the compiler cannot guarantee they won't overlap).
In this case, they are `Into<[u8; 32]>` and `Into<[u8; 20]>`.
See [this](rust-lang/rust#20400) and
[this](rust-lang/rfcs#1672 (comment)).
Note 3: I should also submit a PR to `frontier` that implements
`IntoLocation` for `AccountId20` if this PR gets accepted.
### Summary
this new trait:
- decouples the conversion of `AccountId` to `Location`, from
`deposit_or_burn_fee()` function
- makes `XcmFeeToAccount` accept every possible `AccountId` type as long
as they they implement `IntoLocation`
- backwards compatible
- keeps the API simple and clean while making it less restrictive
@franciscoaguirre and @gupnik are already aware of the issue, so tagging
them here for visibility.
---------
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: command-bot <>
0 commit comments