-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Snowbridge: Unpaid execution when bridging to Ethereum #8599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
acatangiu
merged 25 commits into
paritytech:master
from
yrong:unpaid-execution-to-ethereum
May 29, 2025
Merged
Changes from 10 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
832c864
Unpaid execution to Ethereum
yrong b70ad21
Add a custom SnowbridgeUnpaidRemoteExporter
yrong 14f37be
Add base_fee back
yrong 85343c0
Polish comment
yrong 069b814
Add prdoc
yrong ad0c944
Merge branch 'master' into unpaid-execution-to-ethereum
yrong b984fdb
Add delivery fees to UnpaidRemoteExporter and remove the custom one
yrong d0cadac
AllowExplicitUnpaidExecution from AH root location
yrong e136ea8
Apply suggestions from code review
bkontur 4f27913
Update Cargo.lock
bkontur 1413bb1
Update prdoc/pr_8599.prdoc
bkontur eb47f82
Merge branch 'master' into unpaid-execution-to-ethereum
bkontur 65c2f06
Merge branch 'master' into unpaid-execution-to-ethereum
yrong b48fecf
Snowbridge V1: unpaid execution (#20)
yrong 69dee36
Merge branch 'master' into unpaid-execution-to-ethereum
yrong c51cbcd
Update prdoc
yrong 4cf2753
Polish prdoc
yrong bc08753
Merge branch 'master' into unpaid-execution-to-ethereum
yrong 2a3c139
Fix prdoc
yrong ce0053b
Merge branch 'unpaid-execution-to-ethereum' of https://github.com/yro…
yrong d7bbaca
Merge branch 'master' into unpaid-execution-to-ethereum
bkontur 3542649
Fix breaking tests
yrong 155cb10
Merge branch 'master' into unpaid-execution-to-ethereum
acatangiu e1e6ee4
Update prdoc
yrong 7a16197
Merge branch 'unpaid-execution-to-ethereum' of https://github.com/yro…
yrong 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| title: Snowbridge: Unpaid execution when bridging to Ethereum | ||
| doc: | ||
| - audience: Runtime Dev | ||
| description: |- | ||
| Since all fees in Snowbridge V2 will be estimated on the fly and injected into the XCM, there is | ||
| no need to preconfigure a bridge fee using SovereignPaidRemoteExporter. Additionally, we want to | ||
| avoid maintaining Asset Hub’s sovereign account on Bridge Hub. | ||
| crates: | ||
| - name: snowbridge-runtime-common | ||
bkontur marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| bump: patch | ||
| - name: bridge-hub-westend-runtime | ||
| bump: patch | ||
| - name: asset-hub-westend-runtime | ||
| bump: patch | ||
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.
Since
UnpaidRemoteExporternow supports attaching a fee, how about applying it to the Snowbridge V1 route as well?That would eliminate the need to maintain a balance for AH’s sovereign account on BH.
@vgeddes @acatangiu WDYT?
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.
how long do you plan to support Snowbridge v1 for? might not be worth the change if its lifetime is limited anyway (lower risks of breaking something)
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.
As we discussed internally, it’s likely that we will run V1 along side with V2 for quite some time.
Btw, aside from changing this line
polkadot-sdk/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/mod.rs
Line 135 in e723cfa
There might be some cleanup needed, but considering that our integration tests cover most cases, I’m not too concerned about the breaking things.
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.
A seperate PR for this: yrong#20
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.
Looks good! Thanks Ron.
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.
@acatangiu @bkontur I've merged yrong#20. Please take another look at your convenience.