-
Notifications
You must be signed in to change notification settings - Fork 130
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
Improved ExportXcm::validate
implementation for BridgeHubs - step 1
#2727
Conversation
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.
Overall - LGTM. But this PR actually doesn't solve the #2452? I mean - this xcm exporter works on the source BH and we also need to check that the lane id is correct at the target BH.
ah, yes, you're right, I was focused on that sender validation/export stuff, |
## Summary This PR aligns Rococo/Westend bridge with latest Bridges repo development: - paritytech/parity-bridges-common#2727 - paritytech/parity-bridges-common#2728 - paritytech/parity-bridges-common#2729 Part of: paritytech/parity-bridges-common#2452
* separate constants for average and worst case relay headers (#2728) * separate constants for average and worst case relay headers * fix compilation * Select header that will be fully refunded in on-demand batch finality relay (#2729) * select header that will be fully refunded for submission in on-demand **batch** finality relay * added the only possible test * spelling * nl * updated comment * backport some nits from #2727 * NotApplicable instead of Unroutable * Grafana update stuff (#2733) --------- Co-authored-by: Branislav Kontur <[email protected]>
…paritytech#2727) * Improved `ExportXcm::validate` implementation for BridgeHubs * spellcheck * Fix try-runtime * Fix try-runtime
…paritytech#2727) * Improved `ExportXcm::validate` implementation for BridgeHubs * spellcheck * Fix try-runtime * Fix try-runtime
* separate constants for average and worst case relay headers (#2728) * separate constants for average and worst case relay headers * fix compilation * Select header that will be fully refunded in on-demand batch finality relay (#2729) * select header that will be fully refunded for submission in on-demand **batch** finality relay * added the only possible test * spelling * nl * updated comment * backport some nits from #2727 * NotApplicable instead of Unroutable * Grafana update stuff (#2733) --------- Co-authored-by: Branislav Kontur <[email protected]>
Motivation
At first, this PR addresses #2452. Currently, on BridgeHubs V1, we don't check the sender to verify if it is allowed to use a dedicated lane. As a result, anyone opening an HRMP channel to the local BridgeHub will be able to send messages to the dedicated lane for AssetHubs.
Secondly, there is an issue raised by Jakob (
#361
) regarding the splitting/improvement ofExportXcm::validate
for the messages pallet. As we discussed here on Matrix - we need a wrapper/adapter for HaulBlobExporter, as Slava proposedEnsureMessagesPalletActive
.Solution
I reviewed Slava's master implementation for dynamic bridges, and he has already addressed almost both issues with
pallet-xcm-bridge-hub
. Therefore, I believe creating any temporaryEnsureMessagesPalletActive
is not necessary.Instead, I backported
pallet-xcm-bridge-hub
with justExportXcm
capabilities and adapted it withXcmBlobHauler
elements to minimize the impact on Bridges V1. We will still proceed with the hard-coded lane for AssetHubs.You can find the
polkadot-sdk
copy of this PR for the Rococo/Westend bridge here: paritytech/polkadot-sdk#2602This PR also brings
polkadot-staging
closer to themaster
, making backports and migrations to V2 easier.Next step
So, if this is the way to go, we should focus on adding the
validate_message
feature to bothpallet-xcm-bridge-hub
andpallet-bridge-messages
, which will resolve Jacob's issue, as a follow up PR.Part of: #2452