Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Transfer asset via bridge pallet xcm with dynamic fees and back-pressure #2997

Open
wants to merge 31 commits into
base: bko-transfer-asset-via-bridge-pallet-xcm
Choose a base branch
from

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Aug 11, 2023

This PR extends asset transfer with pallet_xcm PR, adding dynamic fees and back-preassure mechanism.

You can review bridges subdirectory either here or in its dedicated PR in parity-bridges-common where it was originally developed.

TODO:

  • set/check static "base fee" in BridgeTable (according to ExportMessage calculation from BridgeHub)
  • set/verify default value for XcmBridgeHubRouterByteFee (actaully is TransactionByteFee) or setup governance call?
  • add possiblity to change XcmBridgeHubRouterByteFee via governance + test
  • set up benchmarks for router WeightInfo
  • add support/barrier for unpaid report_congestion + test
  • assert FeeManager charged fees in test when merged Introduce XcmFeesToAccount fee manager polkadot#7005
    • search code for // TODO:check-parameter: change and assert in tests when (https://github.com/paritytech/polkadot/pull/7005) merged

8f86ec78b7 ".git/.scripts/commands/fmt/fmt.sh"
ccf2f9483b Merge remote-tracking branch 'origin/polkadot-staging' into dynamic-fees-v1
f822ebc450 Dynamic fees v1: report congestion status to sending chain (#2318)
add9fb1d53 added/fixed some docs
569a80f233 Rename LocalXcmChannel to XcmChannelStatusProvider (#2319)
dc3618a4a5 Clippy
e7cab6ab49 (Suggestion) Ability to externalize configuration for `ExporterFor` (#2313)
c68467beff fmt
5d76f25311 use saturated_len where possible
7cc1470528 Update modules/xcm-bridge-hub-router/src/lib.rs
8d7a38a409 change log target for xcm bridge router pallet
773f93209f Revert "trigger CI"
48f1ba0323 trigger CI
b26aa98d1e fixing spellcheck, clippy and rustdoc
c467911a37 add new pallet to verify-pallets-build.sh
ed72ebe62b get rid of redundant storage value
522bbc7ec4 benchmarks for pallet-xcm-bridge-hub-router
958243564d extension_reject_call_when_dispatcher_is_inactive
38cd8f3df3 fix other tests in the bridge-runtime-common
b75e64fdf7 tests for new logic in the XcmBlobHaulerAdapter
4c741714cb tests for LocalXcmQueueMessageProcessor
d99420e14c tests for LocalXcmQueueSuspender
084f551bb6 new tests for logic changes in messages pallet
d9515f7317 use LocalXcmChannel in XcmBlobMessageDispatch
d9a0c2e468 improvements and tests for palle-xcm-bridge-router
c24301374a removed commented code
eea610a875 pallet-xcm-bridge-hub-router
1fdac85a14 forbid mesage delivery transactions when the channel between target bridge hub and target asset hub is suspended
3c98c245ac OnMessageDelviered callback
65787da038 LocalXcmQueueManager + more adapters
74b48e2cc3 impl backpressure in the XcmBlobHaulerAdapter

git-subtree-dir: bridges
git-subtree-split: 8f86ec78b7747ba32807e8691f022edb4ad3040d
…sfer-asset-via-bridge-pallet-xcm-with-dynamic-fees
Adjust test for paid or unpaid xcm matcher
@bkontur bkontur force-pushed the bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees branch 3 times, most recently from f7a5fbc to c725400 Compare August 14, 2023 10:59
@bkontur bkontur force-pushed the bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees branch 2 times, most recently from af0a0c3 to 62f2878 Compare August 14, 2023 15:18
@bkontur bkontur force-pushed the bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees branch from 62f2878 to d48d27e Compare August 14, 2023 15:30
@svyatonik svyatonik changed the title Transfer asset via bridge pallet xcm with dynamic fees and back-preassure Transfer asset via bridge pallet xcm with dynamic fees and back-pressure Aug 15, 2023
pub type XcmRouter = WithUniqueTopic<(
LocalXcmRouter,
// Router which wraps and sends xcm to BridgeHub to be delivered to the Polkadot GlobalConsensus
ToPolkadotXcmRouter,
Copy link
Contributor

Choose a reason for hiding this comment

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

@bkontur Just some random thought. Can you, please, confirm that noone can use the XCM pallet send (or something like that) to send the program with ExportMessage instruction from AH to sibling BH? Otherwise he could avoid paying the proper fee for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@svyatonik do you mean something like if any other sibling parachain send xcm::Transact(pallet_xcm::send(to_bridge_hub, ExportMessage(...))) ?

all AssetHubs have disabled pallet_xcm::send:

        // We want to disallow users sending (arbitrary) XCMs from this chain.
	type SendXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, ()>;
...
       type XcmExecuteFilter = Nothing;

but I think I got your good point,
if above stuff will be allowed, we would charge origin by XcmpQueue router fee and not by ToPolkadotXcmRouter fee, if XcmpQueue fee will be enough and lower than ToPolkadotXcmRouter, then yes we could have a problem

I think also there is a simple solution for this:
pallet_xcm::send_xcm adds DescendOrigin, so if we adjust xcm filter on BridgeHubs e.g. do not allow DescendOrigin or allow only MultiLocation {parents: 1, interior: X1(Parachain(para_id))} pattern it should be enough

Copy link
Contributor

Choose a reason for hiding this comment

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

I trust your judgement here :) If we do not allow sending arbitrary messages to sibling BH now, then probably it is OK - just wanted to be sure. For v2 we won't need it (I assume), because we will be accepting messages from non-system parachains and (I guess) we don't want to force them to use something that is not strictly necessary (like disallowing programs with DescendOrigin instruction). Because in any case, we'll either need to support fishermens, or impl something like https://github.com/paritytech/parity-bridges-common/issues/2324 for activating backpressure

@bkontur
Copy link
Contributor Author

bkontur commented Aug 16, 2023

bot bench cumulus-assets --subcommand=pallet --pallet=pallet_xcm_bridge_hub_router --runtime=asset-hub-kusama
bot bench cumulus-assets --subcommand=pallet --pallet=pallet_xcm_bridge_hub_router --runtime=asset-hub-polkadot

@command-bot
Copy link

command-bot bot commented Aug 16, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400837 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router. Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 34-c3c2d655-293d-4095-a2ec-8e927c2767d7 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Aug 16, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400838 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router. Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 35-d6b15980-47a9-45ac-bca9-2e8ae172ed52 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Aug 16, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400837 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400837/artifacts/download.

@command-bot
Copy link

command-bot bot commented Aug 16, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400838 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3400838/artifacts/download.

…llet-xcm' into bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees
@bkontur
Copy link
Contributor Author

bkontur commented Aug 16, 2023

bot rebase

@paritytech-processbot
Copy link

Error: Command 'Command { std: cd "/storage/repositories/cumulus" && "git" "merge" "origin/bko-transfer-asset-via-bridge-pallet-xcm" "--no-ff" "--no-edit", kill_on_drop: false }' failed with status Some(1); output: no output

…llet-xcm' into bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees
…llet-xcm' into bko-transfer-asset-via-bridge-pallet-xcm-with-dynamic-fees
@bkontur bkontur marked this pull request as ready for review August 17, 2023 08:13
@bkontur bkontur added the A0-please_review Pull request needs code review. label Aug 17, 2023
@bkontur
Copy link
Contributor Author

bkontur commented Aug 17, 2023

bot bench cumulus-assets --subcommand=pallet --pallet=pallet_xcm_bridge_hub_router --runtime=asset-hub-kusama
bot bench cumulus-assets --subcommand=pallet --pallet=pallet_xcm_bridge_hub_router --runtime=asset-hub-polkadot

@command-bot
Copy link

command-bot bot commented Aug 17, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406271 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router. Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 44-d8670105-e574-4ff2-a0fa-24b3944cdc66 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Aug 17, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406272 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router. Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 45-eb90cbbd-2544-4934-9478-8e8cd18af08a to cancel this command or bot cancel to cancel all commands in this pull request.

…=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router
@command-bot
Copy link

command-bot bot commented Aug 17, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406271 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406271/artifacts/download.

@command-bot
Copy link

command-bot bot commented Aug 17, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_bridge_hub_router has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406272 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3406272/artifacts/download.

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

looks good except couple of comments

parachains/common/src/xcm_config.rs Show resolved Hide resolved
@@ -769,6 +796,9 @@ construct_runtime!(
Multisig: pallet_multisig::{Pallet, Call, Storage, Event<T>} = 41,
Proxy: pallet_proxy::{Pallet, Call, Storage, Event<T>} = 42,

// Bridge utilities.
ToPolkadotXcmRouter: pallet_xcm_bridge_hub_router::<Instance1>::{Pallet, Storage, Call} = 43,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

could leave room for a couple more generic utilities;

how many bridging pallets do we expect in AH? there will probably be some Snowfork-specific ones too?

Suggested change
ToPolkadotXcmRouter: pallet_xcm_bridge_hub_router::<Instance1>::{Pallet, Storage, Call} = 43,
ToPolkadotXcmRouter: pallet_xcm_bridge_hub_router::<Instance1>::{Pallet, Storage, Call} = 45,

@paritytech-ci paritytech-ci requested review from a team August 18, 2023 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C7-critical PR touches the given topic and has a critical impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants