Skip to content
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

Bridges lane id agnostic for backwards compatibility #5649

Merged
merged 47 commits into from
Sep 24, 2024

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Sep 9, 2024

This PR primarily fixes the issue with zombienet-bridges-0001-asset-transfer-works (see: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7404903).

The PR looks large, but most of the changes involve splitting LaneId into LegacyLaneId and HashedLaneId. All pallets now use LaneId as a generic parameter.

The actual bridging pallets are now backward compatible and work with actual substrate-relay v1.6.10, which does not even known anything about permissionless lanes or the new pallet changes.

Important

  • added migration for pallet_bridge_relayers and RewardsAccountParams change order of params, which generates different accounts

Deployment follow ups

  • fix monitoring for at_{}_relay_{}_reward_for_msgs_from_{}_on_lane_{}
  • check sovereign reward accounts - because of changed RewardsAccountParams
  • deploy another messages instances for permissionless lanes - on BHs or AHs?
    • return back open_and_close_bridge_works for another pallet-bridge-messages instance

@bkontur bkontur changed the title Bko bridges lane id agnostic Bridges lane id agnostic for backwards compatibility Sep 9, 2024
@bkontur bkontur force-pushed the bko-bridges-v2-permissionless-lanes branch from 72db319 to adf682e Compare September 9, 2024 13:50
@bkontur bkontur force-pushed the bko-bridges-lane-id-agnostic branch from 9b9c36a to c0c2d54 Compare September 9, 2024 13:50
@bkontur bkontur added R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges. A4-needs-backport Pull request must be backported to all maintained releases. labels Sep 19, 2024
@bkontur bkontur force-pushed the bko-bridges-lane-id-agnostic branch from c0c2d54 to 8f2ae83 Compare September 19, 2024 08:07
@bkontur bkontur changed the base branch from bko-bridges-v2-permissionless-lanes to master September 19, 2024 08:24
@bkontur bkontur mentioned this pull request Sep 19, 2024
2 tasks
@bkontur bkontur force-pushed the bko-bridges-lane-id-agnostic branch from fdd9948 to d5dc6f4 Compare September 19, 2024 20:30
@bkontur bkontur force-pushed the bko-bridges-lane-id-agnostic branch from d5dc6f4 to bebaba1 Compare September 20, 2024 07:58
@acatangiu acatangiu enabled auto-merge September 24, 2024 10:45
prdoc/pr_5649.prdoc Outdated Show resolved Hide resolved
Copy link
Contributor

@serban300 serban300 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 as far as I can understand. I just left a couple of comments, mostly very small.

I hope I didn't miss anything. But please, in the future let's avoid this kind of big PRs since they are very hard to review. Let's try to split them. For this one in particular I would have had at least 2-3 PRs:

  • the LaneID changes
  • instantiating the Relayer rewards pallet
  • any other changes
    And in each PR I would split the important changes from the non-important verbose ones (renamings, adding a generic parameter, etc)

bridges/primitives/messages/src/lane.rs Outdated Show resolved Hide resolved
bridges/relays/lib-substrate-relay/src/messages/mod.rs Outdated Show resolved Hide resolved
bridges/relays/lib-substrate-relay/src/messages/mod.rs Outdated Show resolved Hide resolved
bridges/modules/xcm-bridge-hub/src/exporter.rs Outdated Show resolved Hide resolved
bridges/modules/xcm-bridge-hub/src/lib.rs Show resolved Hide resolved
bridges/relays/lib-substrate-relay/src/cli/mod.rs Outdated Show resolved Hide resolved
@bkontur bkontur requested a review from serban300 September 24, 2024 13:02
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@acatangiu acatangiu added this pull request to the merge queue Sep 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 24, 2024
@bkontur bkontur enabled auto-merge September 24, 2024 20:28
@ggwpez ggwpez mentioned this pull request Sep 24, 2024
@bkontur bkontur added this pull request to the merge queue Sep 24, 2024
Merged via the queue into master with commit 710e74d Sep 24, 2024
219 checks passed
@bkontur bkontur deleted the bko-bridges-lane-id-agnostic branch September 24, 2024 22:17
@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2407:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-5649-to-stable2407
git worktree add --checkout .worktree/backport-5649-to-stable2407 backport-5649-to-stable2407
cd .worktree/backport-5649-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 710e74ddefdff1e36b77ba65abe54feb0ac15040
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2409:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-5649-to-stable2409
git worktree add --checkout .worktree/backport-5649-to-stable2409 backport-5649-to-stable2409
cd .worktree/backport-5649-to-stable2409
git reset --hard HEAD^
git cherry-pick -x 710e74ddefdff1e36b77ba65abe54feb0ac15040
git push --force-with-lease

bkontur added a commit that referenced this pull request Sep 24, 2024
This PR primarily fixes the issue with
`zombienet-bridges-0001-asset-transfer-works` (see:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7404903).

The PR looks large, but most of the changes involve splitting `LaneId`
into `LegacyLaneId` and `HashedLaneId`. All pallets now use `LaneId` as
a generic parameter.

The actual bridging pallets are now backward compatible and work with
actual **substrate-relay v1.6.10**, which does not even known anything
about permissionless lanes or the new pallet changes.

- [x] added migration for `pallet_bridge_relayers` and
`RewardsAccountParams` change order of params, which generates different
accounts

- [ ] fix monitoring for
`at_{}_relay_{}_reward_for_msgs_from_{}_on_lane_{}`
- [ ] check sovereign reward accounts - because of changed
`RewardsAccountParams`
- [ ] deploy another messages instances for permissionless lanes - on
BHs or AHs?
- [ ] return back `open_and_close_bridge_works` for another
`pallet-bridge-messages` instance

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
(cherry picked from commit 710e74d)
EgorPopelyaev pushed a commit that referenced this pull request Sep 25, 2024
Backport #5649 into `stable2409` from bkontur.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

---------

Co-authored-by: Branislav Kontur <bkontur@gmail.com>
pandres95 added a commit to pandres95/runtimes that referenced this pull request Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-needs-backport Pull request must be backported to all maintained releases. R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants