Snowbridge: Synchronize from Snowfork repository #3761

claravanstaden · 2024-03-20T10:41:45Z

This PR includes the following 2 improvements:

Ethereum Client

Original Upstream PRs

Sync execution update on demand Snowfork/polkadot-sdk#123
Inline execution update into proof of inbound message & Remove execution header storage Snowfork/polkadot-sdk#125

Description

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: #2522 (comment)

Inbound Queue

Author: @yrong

Original Upstream PR

Ignore reward when funds unavailable Snowfork/polkadot-sdk#118

Description

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. ❤️

* pallet order * fmt * adds test for bridge order * revert unintended change --------- Co-authored-by: claravanstaden <Cats 4 life!>

* fix coverage * fix codecov config --------- Co-authored-by: claravanstaden <Cats 4 life!>

# Conflicts: # .github/workflows/check-labels.yml # .github/workflows/check-prdoc.yml # .github/workflows/check-workspace.yml # .github/workflows/gitspiegel-trigger.yml # .github/workflows/release-99_notif-published.yml # bridges/snowbridge/README.md # bridges/snowbridge/runtime/test-common/src/lib.rs # bridges/snowbridge/scripts/contribute-upstream.sh # cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs # cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/snowbridge.rs # polkadot/node/subsystem-bench/src/approval/mod.rs # polkadot/node/subsystem-bench/src/availability/mod.rs # polkadot/node/subsystem-bench/src/cli.rs # polkadot/node/subsystem-bench/src/core/environment.rs # polkadot/node/subsystem-bench/src/subsystem-bench.rs

* extract ethereum chain id * remove cargo.toml * fix chain id data type * fix missing chain id param * fix missing chain id param * fix coverage and missing param * fix CI * fix CI * revert scripts * revert coverage jobs * revert formatting --------- Co-authored-by: claravanstaden <Cats 4 life!>

* Ignore rewarding when funds unavailable * More tests * Fix taplo * Fix emulated test * Improve comments * Update bridges/snowbridge/pallets/inbound-queue/src/lib.rs Co-authored-by: Vincent Geddes <vincent@snowfork.com> * Fix format --------- Co-authored-by: Vincent Geddes <vincent@snowfork.com>

* test e2e scripts in GA * fix GA * test current dir * fix path * install moreutils * sponge * install go * export path * try something else * run nix * switch branch * nix develop * check setup * use nix setup * e2e stack script * e2e stack script * run command * check current dir * correct dir * correct dir * just cd * try something else * rn e2e script * adds submodules * ls * print vars * remove ls * cd instead of pushdir * try checking out differently * ls * ls * change checkout order * check what is in a dir * check what is in a dir * check something * check something * try something * try something * try e2e script again * remove ls * cleanup script * adds ssz gen * install js components * install js components * fix dir * adds output dir * create dir * try something else * bump * fix config paths * test output dir * adds wasm target * check wasm target * setup rustup in nix * missing quote * install rustup stable * adds e2e script in again * remove skip wasm flag * comment out things * start lodestar * print out log * print out log * print out lodestar * check geth version * bump * remove commented out code * update go files * adds smoketests * create dir * adjust scripts * only redirect start-services.sh output * only redirect start-services.sh output * switch dir * permissions * check script * allow service to be down * bump * last run, hopefully * fix tests * fixes * bump * fix * check lodestar * check lodestar * check lodestar * bump * bump * try again * try again * bump * bump * update go * update go * cleanup * check what is happening now * lodestar should run the background * run init.sh * check what happens with the download * change OS check * remove other steps to see what is happening * bump * run smoketests * run smoketests * cleanup jobs * extract ethereum chain id * remove cargo.toml * fix chain id data type * fix missing chain id param * fix missing chain id param * fix coverage and missing param * fix CI * fix CI * revert scripts * revert coverage jobs * revert formatting * check log * fix log * separate steps * cleanup job * remove running smoketests on a PR, only when merged * update branch * remove http endpoint from relayer * update nightly, revert unrelated changes * check rust version * chaneg branch for testing * adds beacon relay log * increase slot time * increase block time check * lower block check count * only run smoketests on merge to snowbridge --------- Co-authored-by: claravanstaden <Cats 4 life!>

* Sync execution update on demand * Fix updating LatestExecutionState * Revert change unrelated * Remove unused * Remove execution header from update & Remove LatestExecutionState

* adds finalized header gap check * fix comment * fmt * use constant for max gap * remove redundant config * fmt * fix tests * fix again * fmt * test works * revert change * Update bridges/snowbridge/pallets/ethereum-client/src/lib.rs Co-authored-by: Vincent Geddes <vincent@snowfork.com> * fmt --------- Co-authored-by: claravanstaden <Cats 4 life!> Co-authored-by: Vincent Geddes <vincent@snowfork.com>

…ion header storage (#125) * Remove execution header storage & submit inbound message with execution update * Fix ci breaking * More refactoring * Update beacon test fixtures * Format code * Initialize storage with finalized checkpoint for benchmark * Update weights include verify the execution proof * Fix breaking test * Remove fixture not in use * Add detail for InvalidExecutionProof * Initialize checkpoint for tests on demand * Revert error as NotBootstrapped for submit_update_with_missing_bootstrap * Add verify_execution_header tests back * Add error test back * Cleanup templates * Update bridges/snowbridge/primitives/beacon/src/types.rs Co-authored-by: Vincent Geddes <vincent@snowfork.com> * Narrowed inputs * Remove fields irrelevant from proof * Update bridges/snowbridge/primitives/core/src/inbound.rs Co-authored-by: Vincent Geddes <vincent@snowfork.com> * Update bridges/snowbridge/pallets/ethereum-client/src/impls.rs Co-authored-by: Vincent Geddes <vincent@snowfork.com> * Polish --------- Co-authored-by: Vincent Geddes <vincent@snowfork.com>

* fix smoketests ci * remove merge from upstream * first commit before pulling from master --------- Co-authored-by: claravanstaden <Cats 4 life!>

… beacon-client-improvements

* Add fee multiplier * fmt * fmt * Update test mock * Swap order for parameters to calculate_fee --------- Co-authored-by: claravanstaden <Cats 4 life!>

svyatonik

Have been mostly checking direct changes to pallet code - all looks great, thank you!

svyatonik · 2024-04-02T06:33:21Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -191,25 +174,6 @@ pub mod pallet {
 	pub(super) type NextSyncCommittee<T: Config> =
 		StorageValue<_, SyncCommitteePrepared, ValueQuery>;

-	/// Latest imported execution header


A migration for removal those values will be added later, right?

Yes, definitely. As soon as Multi-block Migrations are ready on BH.

maybe this could be useful: #3828

Looks great! Will that feature be OK even if all the storage items cannot be deleted in one block?

bridges/snowbridge/pallets/inbound-queue/src/lib.rs

Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>

bkontur · 2024-04-02T08:21:47Z

bridges/snowbridge/pallets/ethereum-client/src/impls.rs

@@ -53,18 +36,11 @@ impl<T: Config> Verifier for Pallet<T> {
 		if !receipt.contains_log(&event_log) {
 			log::error!(
 				target: "ethereum-client",
-				"💫 Event log not found in receipt for transaction at index {} in block {}",
-				proof.tx_index, proof.block_hash,
+				"💫 Event log not found in receipt for transaction",


Why did you remove log params? I think this log became pretty non-relevant and useless, because you also remove input log with block_hash Verifying message with block hash.
How will you investigate Event log not found in receipt for transaction without knowing block_hash or tx?

The reasoning behind this is on the original PR, over here: Snowfork#125 (comment)

A side-note, we don't generally have access to the parachain logs either, so we try and solve issues without having to look at the logs.

ok, got it, and what about adding there some "identifier" from proof.execution_proof or proof.execution_proof.execution_header, does it make sense?

I don't know if it would be useful. We have the same info in the relayer and that is more accessible to us.

bkontur · 2024-04-02T08:30:40Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -191,25 +174,6 @@ pub mod pallet {
 	pub(super) type NextSyncCommittee<T: Config> =
 		StorageValue<_, SyncCommitteePrepared, ValueQuery>;

-	/// Latest imported execution header


maybe this could be useful: #3828

prdoc/pr_3761.prdoc

… beacon-client-improvements

@yrong

This PR includes the following 2 improvements: ## Ethereum Client Author: @yrong ### Original Upstream PRs - Snowfork#123 - Snowfork#125 ### Description 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: #2522 (comment) ## Inbound Queue Author: @yrong ### Original Upstream PR - Snowfork#118 ### Description 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>

@yrong

This PR includes the following 2 improvements: ## Ethereum Client Author: @yrong ### Original Upstream PRs - Snowfork#123 - Snowfork#125 ### Description 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) ## Inbound Queue Author: @yrong ### Original Upstream PR - Snowfork#118 ### Description 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>

@yrong

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>

This is a cherry-pick from master of #3761, excluding PR Snowfork#118 Expected patches for (1.7.0): - name: snowbridge-pallet-ethereum-client - name: snowbridge-pallet-inbound-queue - name: snowbridge-beacon-primitives - name: snowbridge-core - name: snowbridge-runtime-test-common --------- Co-authored-by: Ron <yrong1997@gmail.com> Co-authored-by: Vincent Geddes <vincent@snowfork.com> Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>

Upgrades Snowbridge to have a much more cost effective Ethereum light client. In the current light client in production, all Ethereum execution headers are stored, whether they contain a transfer message or not. This PR upgrades Snowbridge to not store execution headers at all, but provide the execution header proof alongside a message. Ethereum beacon headers are still stored, to track finalized headers and do ancestry proofs. More details on the change on the original PR: paritytech/polkadot-sdk#3761 --------- Co-authored-by: Branislav Kontur <bkontur@gmail.com>

claravanstaden and others added 20 commits February 8, 2024 13:37

adds snowbridge

2536e78

Test pallet order (#112)

bc607a0

* pallet order * fmt * adds test for bridge order * revert unintended change --------- Co-authored-by: claravanstaden <Cats 4 life!>

Fix coverage (#115)

4cede80

* fix coverage * fix codecov config --------- Co-authored-by: claravanstaden <Cats 4 life!>

Increase MaxExecutionHeadersToKeep (#120)

628b251

Sync execution update on demand (#123)

5bac4ee

* Sync execution update on demand * Fix updating LatestExecutionState * Revert change unrelated * Remove unused * Remove execution header from update & Remove LatestExecutionState

Fix Smoketests CI (#126)

cbcc449

* fix smoketests ci * remove merge from upstream * first commit before pulling from master --------- Co-authored-by: claravanstaden <Cats 4 life!>

cleanup branch

ae80de0

merge

b6fee8b

remove fmt file

e353ec0

Merge branch 'master' into beacon-client-improvements

20ca52e

unused param

98d8f2c

Merge remote-tracking branch 'origin/beacon-client-improvements' into…

0892540

… beacon-client-improvements

prdoc

0432944

Merge branch 'master' into beacon-client-improvements

ad9c398

claravanstaden marked this pull request as ready for review March 21, 2024 08:47

paritytech-review-bot bot requested a review from a team March 21, 2024 08:48

claravanstaden and others added 7 commits March 21, 2024 11:01

remove rustdoc describing removed extrinsic

db1b4c1

Merge branch 'master' into beacon-client-improvements

2730776

taplo

71443be

Merge remote-tracking branch 'origin/beacon-client-improvements' into…

5f2e751

… beacon-client-improvements

Add a linear fee multiplier to ensure safety margins (#127)

5da7df7

* Add fee multiplier * fmt * fmt * Update test mock * Swap order for parameters to calculate_fee --------- Co-authored-by: claravanstaden <Cats 4 life!>

change bitwise or to logical or

a43b54e

Merge branch 'master' into beacon-client-improvements

0fbceec

claravanstaden mentioned this pull request Mar 22, 2024

Snowbridge Beacon header age check #3727

Merged

yrong approved these changes Mar 30, 2024

View reviewed changes

Merge branch 'master' into beacon-client-improvements

2ae3cab

alistair-singh approved these changes Apr 1, 2024

View reviewed changes

Merge branch 'master' into beacon-client-improvements

b81f262

svyatonik approved these changes Apr 2, 2024

View reviewed changes

claravanstaden and others added 2 commits April 2, 2024 09:41

Update bridges/snowbridge/pallets/inbound-queue/src/lib.rs

fcd5f71

Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>

Merge branch 'master' into beacon-client-improvements

7c0a097

bkontur approved these changes Apr 2, 2024

View reviewed changes

bkontur reviewed Apr 2, 2024

View reviewed changes

prdoc/pr_3761.prdoc Outdated Show resolved Hide resolved

bkontur added the T15-bridges This PR/Issue is related to bridges. label Apr 2, 2024

claravanstaden and others added 5 commits April 2, 2024 11:15

update prdoc

ef97c0b

Merge remote-tracking branch 'origin/beacon-client-improvements' into…

87de172

… beacon-client-improvements

Merge branch 'master' into beacon-client-improvements

3aad93a

fix prdoc

304f890

Merge remote-tracking branch 'origin/beacon-client-improvements' into…

0cfd048

… beacon-client-improvements

bkontur added this pull request to the merge queue Apr 2, 2024

Merged via the queue into paritytech:master with commit 5d9826c Apr 2, 2024
135 of 136 checks passed

claravanstaden deleted the beacon-client-improvements branch May 24, 2024 19:22

claravanstaden mentioned this pull request Jun 19, 2024

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

Merged

claravanstaden mentioned this pull request Jul 1, 2024

Snowbridge Headers on Demand polkadot-fellows/runtimes#365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snowbridge: Synchronize from Snowfork repository #3761

Snowbridge: Synchronize from Snowfork repository #3761

claravanstaden commented Mar 20, 2024 •

edited

Loading

svyatonik left a comment

svyatonik Apr 2, 2024

claravanstaden Apr 2, 2024

bkontur Apr 2, 2024

claravanstaden Apr 2, 2024

bkontur Apr 2, 2024

claravanstaden Apr 2, 2024

bkontur Apr 2, 2024

claravanstaden Apr 2, 2024

bkontur Apr 2, 2024

Snowbridge: Synchronize from Snowfork repository #3761

Snowbridge: Synchronize from Snowfork repository #3761

Conversation

claravanstaden commented Mar 20, 2024 • edited Loading

Ethereum Client

Original Upstream PRs

Description

Inbound Queue

Original Upstream PR

Description

svyatonik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claravanstaden commented Mar 20, 2024 •

edited

Loading