This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Introduce XcmFeesToAccount fee manager #7005
Open
KiChjang
wants to merge
38
commits into
master
Choose a base branch
from
kckyeung/xcm-fee-manager
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
272c31d
Introduce XcmFeesToAccount fee manager
KiChjang 13f5944
Fixes
KiChjang 3d0cfde
Fixes
KiChjang c28700d
Put system parachain IDs into consts
KiChjang d5b760b
Fixes
KiChjang a2d3604
Remove XcmFeesToAccount config for Westend
KiChjang 7e99987
Include Encointer as a system parachain
KiChjang 04d4410
Emit warning when deposit_asset fails
KiChjang 5d6e2c2
Add comment on what happens when deposit_asset errors
KiChjang 7e4cc0a
Fixes
KiChjang 17c0f5c
Move SystemParachains to constants
KiChjang 12b816e
Fixes
KiChjang d22046f
Fixes
KiChjang 6c9f3f9
cargo fmt
KiChjang 424c903
Added BridgeHubs constants (#7053)
bkontur 7b140f0
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
KiChjang a852957
Add SystemParachain type to Westmint
KiChjang e1b6af0
Fixes
KiChjang 20b4f66
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
KiChjang 09828da
Fixes
KiChjang 0e06f73
Waive fee handling during benchmarking
KiChjang 6a4fd76
Fixes
KiChjang 6eccb05
Fixes
KiChjang 2df8cbf
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
KiChjang de0e397
Rename to ASSET_HUB_ID
KiChjang 8bb8fb9
Rename all asset parachains to AssetHub
KiChjang 3ea877f
Merge branch 'master' into kckyeung/xcm-fee-manager
KiChjang 4843e58
Fix typo
KiChjang 7c3bb1a
".git/.scripts/commands/fmt/fmt.sh"
50a008e
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
76a4cd0
Merge branch 'master' into kckyeung/xcm-fee-manager
KiChjang 7d61d2e
Fixes
KiChjang 18945f8
Fixes
KiChjang 6f4a8bb
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
KiChjang 4fa5016
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
88b9582
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
2c7d14c
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
57b3a6d
Merge remote-tracking branch 'origin/master' into kckyeung/xcm-fee-ma…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this change
() -> XcmFeesToAccount
Is there any change or consequence for non-system parachains or end users?
I mean, is there anything which will start failing after this is upgraded on-live?
Something like: non-system parachain should drip their sovereign account on relay chain?
If so, maybe should be part of release notes or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be -- with or without a
FeeManager
, we've still been collecting fees, the only difference here is that previously we were just burning them after collecting them, but with this change, we simply deposit them to the treasury account.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actual
impl FeeManager for ()
here behaves as everything is waived now,so, iiuc
charge_fees
andtakes_fee
they never doAssetTransactor::withdraw_asset
, but now withXcmFeesToAccount
they will doAssetTransactor::withdraw_asset
,so I think this is the change, that origin needs to have some balance > ED + fees, but I dont know how much this is a really issue. I can imaging just some scenarios/calls from some non-system parachain that does not have sovereign account with balance on relay chain, so after this change they will need to drip some DOTs/KSMs to pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, so I just took a look at where we collect fees in the XCM executor: it looks like any instruction where it creates and sends an XCM back to the origin would require the origin to pay for fees, and this includes the
Query*
instructions, theReport*
instructions, theExportMessage
instruction and most of the cross-chain asset transfer instructions (e.g.InitiateTeleport
).This isn't a huge deal, I think what we do need to do is to display delivery fees in the UI clearly so that users are informed about how much they need to pay, otherwise their XCM would fail to be sent.