Bridges: Making relayer compatible with runtime upgrades #4256
Conversation
This file contains 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
…pallet_bridge_grandpa + tests to ensure GRANDPA relayer compatibility
…f filters to generate signed extension identifier + renamed refund extensions to "ComplexRefund" - we will change it in upcoming upgrade
… + tests to ensure parachains relayer compatibility
… tests to ensure messages relayer compatibility
… are upgraded to metadata v15, we will be able to fetch that using regural state_metadata call (given that we will be able to parse metadata from rust code)
…w versioning scheme
…yer versions (see paritytech/parity-bridges-common#2891 for request)
…re running an older version
…rsion into bridge version
svyatonik
changed the title
… generated; - change RelayerVersion comparison in relayer - only `manual` field matters; - fixed merge damage and updated versions
|
Latest commit introduces some important changes:
|
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 3, 2024
Due to recent bump of Rococo/Westend versions + the fact that paritytech/parity-bridges-common#2894 has finally reached this repo, tests now fail, because we've started checking all client versions (even source) unless we specify `--source-version-mode Auto` in CLI arguments. This looks like an overkill, but all those version checks will be fixed by #4256, so now it makes sense just to add this CLI option. We also need to propagate it to running relays eventually.
serban300
reviewed
May 6, 2024
| @@ -403,9 +403,9 @@ impl<C: Chain> Client<C> { | |||
| } | |||
|
|
|||
| /// Return runtime version. | |||
| pub async fn runtime_version(&self) -> Result<RuntimeVersion> { | |||
| pub async fn runtime_version(&self, at: HeaderIdOf<C>) -> Result<RuntimeVersion> { | |||
There was a problem hiding this comment.
Looks like we only need the hash here and also looks like sometimes we want to query the runtime version at the latest block. So could we do at: Option<C::Hash> ?
There was a problem hiding this comment.
Changed to HashOf. All places where the latest block is used (version guards) shall be removed once this update is applied, so no Option<_>
| //! version every time one of chains is upgraded. Recently we have "eased" our | ||
| //! requirements and now only watch for bridge hub versions (2 chains). | ||
| //! | ||
| //! What we are trying to solve with that: |
There was a problem hiding this comment.
Overall seems quite complex to me. I think my personal preference would be to keep only the manual version for simplicity. I'm not totally sure. I'll spend some more time thinking about it. Also I'm curious what others think as well.
svyatonik
added a commit
that referenced
this pull request
May 8, 2024
Due to recent bump of Rococo/Westend versions + the fact that paritytech/parity-bridges-common#2894 has finally reached this repo, tests now fail, because we've started checking all client versions (even source) unless we specify `--source-version-mode Auto` in CLI arguments. This looks like an overkill, but all those version checks will be fixed by #4256, so now it makes sense just to add this CLI option. We also need to propagate it to running relays eventually.
|
I think this RFC with transaction/signed extension version: polkadot-fellows/RFCs#99 could also simplify/avoid lots of these code probably |
TarekkMA
pushed a commit
to moonbeam-foundation/polkadot-sdk
that referenced
this pull request
Aug 2, 2024
Due to recent bump of Rococo/Westend versions + the fact that paritytech/parity-bridges-common#2894 has finally reached this repo, tests now fail, because we've started checking all client versions (even source) unless we specify `--source-version-mode Auto` in CLI arguments. This looks like an overkill, but all those version checks will be fixed by paritytech#4256, so now it makes sense just to add this CLI option. We also need to propagate it to running relays eventually.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See paritytech/parity-bridges-common#2956 for details
Relayer PR: paritytech/parity-bridges-common#2969
Would be better to have paritytech/parity-bridges-common#2947 implemented before
Main changes here:
RelayerVersionstruct with two fields:autoisH256, computed with a tests. When it changes, it means that existing relayer is no longer compatible with new runtime.manualshould be bumped in that case.manualcan also be bumped if there are breaking changes, not covered by the tests;RelayerVersionas a constant in a pallet configuration;RelayerVersionas a constant in a pipeline configuration;offchain.manual <= onchain.manualand keep working otherwise.