polkadot icon indicating copy to clipboard operation
polkadot copied to clipboard

Introduce XcmFeesToAccount fee manager

Open KiChjang opened this issue 2 years ago • 20 comments

This PR introduces a new XcmFeesToAccount struct which implements the FeeManager trait, and assigns this struct as the FeeManager in the XCM config for all runtimes.

The struct simply deposits all fees handled by the XCM executor to a specified account. In all runtimes, the specified account is configured as the treasury account.

XCM delivery fees are now being introduced (unless the root origin is sending a message to a system parachain on behalf of the originating chain).

Important note

After this change, instructions that create and send a new XCM (Query*, Report*, ExportMessage, InitiateReserveWithdraw, InitiateTeleport, DepositReserveAsset, TransferReserveAsset, LockAsset and RequestUnlock) will require the corresponding origin account in the origin register to pay for transport delivery fees, and the onward message will fail to be sent if the origin account does not have the required amount. This delivery fee is on top of what we already collect as tx fees and XCM BuyExecution fees!

Wallet UIs that want to expose the new delivery fee can do so using the formula delivery_fee_factor * (base_fee + encoded_msg_len * per_byte_fee), where the delivery fee factor can be obtained from the corresponding pallet based on which transport you are using (UMP, HRMP or bridges), the base fee is a constant, the encoded message length from the message itself and the per byte fee is the same as the configured per byte fee for txs.

cumulus companion: https://github.com/paritytech/cumulus/pull/2433

KiChjang avatar Apr 04 '23 15:04 KiChjang

bot fmt

KiChjang avatar Jun 22 '23 16:06 KiChjang

@KiChjang https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3055051 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-c62bad68-7375-4e23-a9af-cc36b44b7a74 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot[bot] avatar Jun 22 '23 16:06 command-bot[bot]

@KiChjang Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3055051 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3055051/artifacts/download.

command-bot[bot] avatar Jun 22 '23 16:06 command-bot[bot]

bot rebase

KiChjang avatar Jul 04 '23 10:07 KiChjang

Rebased

The CI pipeline was cancelled due to failure one of the required jobs. Job name: test-linux-stable Logs: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3322078

paritytech-cicd-pr avatar Aug 02 '23 23:08 paritytech-cicd-pr

@KiChjang what is the status of this PR? can we merge it?

acatangiu avatar Aug 07 '23 12:08 acatangiu

@KiChjang @joepetrowski I found one potential issue (with test and maybe with fix) with pallet_xcm::reserve_transfer_assets (probably this could affect all instructions in XcmExecutor which uses self.send), please check this PR: https://github.com/paritytech/polkadot/pull/7585

bkontur avatar Aug 07 '23 13:08 bkontur

@KiChjang what is the status of this PR? can we merge it?

It is ready whenever the cumulus companion is fixed.

KiChjang avatar Aug 07 '23 17:08 KiChjang

bot rebase

bkontur avatar Aug 08 '23 08:08 bkontur

Rebased

@KiChjang what is the status of this PR? can we merge it?

It is ready whenever the cumulus companion is fixed.

Seems like this PR breaks pallet_xcm::reserve_transfer_assets() without #7585 - @KiChjang @franciscoaguirre please review fix in #7585 so we can add it here before we can merge this.

acatangiu avatar Aug 08 '23 12:08 acatangiu

I made a cumulus companion to compile, but when I run tests in Cumulus, they fail - see expected NotHoldingFees

test tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets ... FAILED
test tests::teleport::teleport_native_assets_from_relay_to_assets_para ... FAILED
test tests::transact::transact_sudo_from_relay_to_assets_para ... ok

failures:

---- tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets stdout ----
EVENTS: [RuntimeEvent::System(Event::NewAccount { account: 70617261e8030000000000000000000000000000000000000000000000000000 (5Ec4AhPZ...) }), RuntimeEvent::Balances(Event::Endowed { account: 70617261e8030000000000000000000000000000000000000000000000000000 (5Ec4AhPZ...), free_balance: 10000000000000 }), RuntimeEvent::Balances(Event::Transfer { from: d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...), to: 70617261e8030000000000000000000000000000000000000000000000000000 (5Ec4AhPZ...), amount: 10000000000000 }), RuntimeEvent::XcmPallet(Event::Attempted { outcome: Incomplete(Weight { ref_time: 686043000, proof_size: 6196 }, NotHoldingFees) })]
thread 'tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets' panicked at '

Event RuntimeEvent::XcmPallet(pallet_xcm::Event::Attempted {
outcome: Outcome::Complete(weight) }) was never received', parachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/reserve_transfer.rs:49:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::teleport::teleport_native_assets_from_relay_to_assets_para stdout ----
EVENTS: [RuntimeEvent::Balances(Event::Withdraw { who: d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...), amount: 10000000000000 }), RuntimeEvent::Balances(Event::Deposit { who: 6d6f646c70792f78636d63680000000000000000000000000000000000000000 (5EYCAe5i...), amount: 10000000000000 }), RuntimeEvent::System(Event::NewAccount { account: 6d6f646c70792f78636d63680000000000000000000000000000000000000000 (5EYCAe5i...) }), RuntimeEvent::Balances(Event::Endowed { account: 6d6f646c70792f78636d63680000000000000000000000000000000000000000 (5EYCAe5i...), free_balance: 10000000000000 }), RuntimeEvent::XcmPallet(Event::Attempted { outcome: Incomplete(Weight { ref_time: 792675000, proof_size: 7585 }, NotHoldingFees) })]
thread 'tests::teleport::teleport_native_assets_from_relay_to_assets_para' panicked at '

Event RuntimeEvent::XcmPallet(pallet_xcm::Event::Attempted {
outcome: Outcome::Complete { .. } }) was never received', parachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/teleport.rs:49:9


failures:
    tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets
    tests::teleport::teleport_native_assets_from_relay_to_assets_para

so that fix https://github.com/paritytech/polkadot/pull/7585 is really needed

bkontur avatar Aug 08 '23 13:08 bkontur

with fix those tests fails on Balance asserts which needs to be corrected, because of these new xcm fees are charged from user account:

test tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets ... FAILED
test tests::teleport::teleport_native_assets_from_relay_to_assets_para ... FAILED
test tests::transact::transact_sudo_from_relay_to_assets_para ... ok

failures:

---- tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets stdout ----
thread 'tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets' panicked at 'assertion failed: `(left == right)`
  left: `30960000000000`,
 right: `30959632000000`', parachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/reserve_transfer.rs:79:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::teleport::teleport_native_assets_from_relay_to_assets_para stdout ----
thread 'tests::teleport::teleport_native_assets_from_relay_to_assets_para' panicked at 'assertion failed: `(left == right)`
  left: `30960000000000`,
 right: `30959632000000`', parachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/teleport.rs:76:5


failures:
    tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_assets
    tests::teleport::teleport_native_assets_from_relay_to_assets_para

test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 18.42s

bkontur avatar Aug 08 '23 14:08 bkontur

bot rebase

bkontur avatar Aug 13 '23 12:08 bkontur

Rebased

bot rebase

bkontur avatar Aug 15 '23 06:08 bkontur

Rebased

bot rebase

bkontur avatar Aug 16 '23 20:08 bkontur

Rebased