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

Snowbridge: Synchronize from Snowfork repository (#3761) #4829

Conversation

claravanstaden
Copy link
Contributor

@claravanstaden claravanstaden commented Jun 19, 2024

This is a cherry-pick from master of #3761, excluding PR Snowfork#118

Expected patches for (1.7.0):

  • snowbridge-pallet-ethereum-client
  • snowbridge-pallet-ethereum-client-fixtures
  • snowbridge-pallet-inbound-queue
  • snowbridge-pallet-inbound-queue-fixtures
  • snowbridge-beacon-primitives
  • snowbridge-core
  • snowbridge-runtime-test-common
  • asset-hub-rococo-runtime
  • bridge-hub-rococo-runtime
  • bridge-hub-rococo-integration-tests

This PR includes the following 2 improvements:

Author: @yrong
- #123
- #125

The Ethereum client syncs beacon headers as they are finalized, and
imports every execution header. When a message is received, it is
verified against the import execution header. This is unnecessary, since
the execution header can be sent with the message as proof. The recent
Deneb Ethereum upgrade made it easier to locate the relevant beacon
header from an execution header, and so this improvement was made
possible. This resolves a concern @svyatonik had in our initial Rococo
PR:
paritytech#2522 (comment)

Author: @yrong
- #118

When the AH sovereign account (who pays relayer rewards) is depleted,
the inbound message will not fail. The relayer just will not receive
rewards.

Both these changes were done by @yrong, many thanks. ❤️

---------

Co-authored-by: claravanstaden <Cats 4 life!>
Co-authored-by: Ron <yrong1997@gmail.com>
Co-authored-by: Vincent Geddes <vincent@snowfork.com>
Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>
@acatangiu
Copy link
Contributor

What kind of testing has been done on this backport?

@claravanstaden
Copy link
Contributor Author

claravanstaden commented Jun 24, 2024

  • Local E2E tests (register token + send token passes, finalized headers import)
  • cargo test
  • cargo test all-features (note: passes but complains about pallet_im_online WeightInfo not being used, unrelated)
  • clippy passes
  • BH benchmarks pass

@claravanstaden claravanstaden marked this pull request as ready for review June 25, 2024 10:45
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 25, 2024 10:45
@claravanstaden
Copy link
Contributor Author

@yrong may you do a final review here before Parity reviews, please? :)

- audience: Runtime Dev
description: |
This PR improves the beacon client to send the execution header along with the message as proof and removes the verification and storing of all execution headers.
If the AH sovereign account is depleted and relayer rewards cannot be paid, the message should still be processed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not, removed in 1877257. :) Good catch

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6541631

@acatangiu acatangiu merged commit 6dba07c into paritytech:release-crates-io-v1.7.0 Jun 25, 2024
16 of 127 checks passed
@claravanstaden claravanstaden deleted the snowbridge-headers-on-demand branch June 25, 2024 16:53
@acatangiu
Copy link
Contributor

@Morganamilo the PR description has been updated to contain full set of crates needing patch update publish

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants