Snowbridge Ethereum Deneb fork preparation #3029

claravanstaden · 2024-01-23T11:15:24Z

Prepares for the Deneb hardfork on Sepolia testnet on 31 January (needs to be deployed to Rococo before then)
Removes beacon-minimal-spec flag for simpler config
Adds test comments

Co-authored-by: claravanstaden <Cats 4 life!>

* adds emulated test comments * pr comments * rename XCMs --------- Co-authored-by: claravanstaden <Cats 4 life!>

* ForkVersions for Deneb * Enable deneb from genesis

* Schedule dencun on sepolia * Update Cargo.lock

…nowbridge'

git-subtree-dir: bridges/snowbridge git-subtree-split: d65afb42f629a28eb1147fd6710f650c2928632b

# Conflicts: # Cargo.lock

* remove minimal spec * update test modules * fix fork versions in runtime * adds deneb * fix lodestar setup in nix, update fixtures with deneb fork --------- Co-authored-by: claravanstaden <Cats 4 life!>

53a5522d494 Remove minimal spec (paritytech#1119) git-subtree-dir: bridges/snowbridge git-subtree-split: 53a5522d494e38c5c3573c65e47bceda30572e45

…nowbridge-deneb-updates

# Conflicts: # Cargo.lock # cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/Cargo.toml

svyatonik

Don't want to block this PR, because most of changes are in the Snowfork pallets and are already reviewed. But need to ask a question before.

Looking at Eth hardfork times, won't it be hard for us to follow that schedule in deployment, given current deployment pipeline (fix in Snowfork repo -> update in polkadot-sdk -> wait for crates publish -> upgrade dependencies in the fellowship repo -> runtime upgrade)? Like Paris has been activated 9 days after the previous hardfork (Bellatrix). Do you have some safety switch to turn off bridge if we'll fail to deploy on time? Or we'll be trying to do that on time.

Also I remember some accidental hard forks that have happened because of differences in client implementations. Do you also have some solution for bridge fixes in such case (e.g.: force switch forks). Just curious

svyatonik · 2024-01-24T07:59:49Z

bridges/snowbridge/parachain/runtime/test-common/Cargo.toml

@@ -89,7 +89,7 @@ snowbridge-system-runtime-api = { path = "../../pallets/system/runtime-api", def
 [dev-dependencies]
 static_assertions = "1.1"
 bridge-hub-test-utils = { path = "../../../../../cumulus/parachains/runtimes/bridge-hubs/test-utils" }
-bridge-runtime-common = { path = "../../../../bin/runtime-common", features = ["integrity-test"] }
+bridge-runtime-common = { path = "../../../../../bridges/bin/runtime-common", features = ["integrity-test"] }


I think it was removed by purpose, so please revert :)

My bad, fixed in 57f9b19.

svyatonik · 2024-01-24T08:02:16Z

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs

@@ -203,7 +203,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("bridge-hub-rococo"),
 	impl_name: create_runtime_str!("bridge-hub-rococo"),
 	authoring_version: 1,
-	spec_version: 1_006_000,
+	spec_version: 1_007_000,


I have no idea - whether it should be bumped here, or during release?

Erm I'm also not sure. I wonder now if it shouldn't be a minor bump, like 1_006_001? @bkontur? 😅

I would not change spec_version here, last time for 1.6.0 tag/release, they did it in the release branch and when released they backport that to master, so I would better revert this

bkontur

I didn't really check your subtree stuff, at the first glance looks ok,
I saw you removed minimal/mainet features, is it possible to remove this:

fast-runtime = [
	"bridge-hub-rococo-runtime/fast-runtime",
]

and change to:

fast-runtime = [ ]

in the polkadot-parachain Cargo.toml?

claravanstaden · 2024-01-24T11:04:56Z

@svyatonik thanks for raising this, this is a valid point. A few things to note.

Looking at Eth hard-fork times, won't it be hard for us to follow that schedule in deployment, given current deployment pipeline (fix in Snowfork repo -> update in polkadot-sdk -> wait for crates publish -> upgrade dependencies in the fellowship repo -> runtime upgrade)? Like Paris has been activated 9 days after the previous hard-fork (Bellatrix).

After the Ethereum merge, there are 2 Ethereum chains, the execution chain (the "old" Ethereum chain as it was before the merge) and the consensus chain (the new Beacon chain). For the Merge, it required an upgrade on both chains. Paris was the hard-fork on the execution chain and Bellatrix was the hardfork on the consensus chain. So they were essentially the same upgrade, just on the two different chains. We do not interact with the execution chain for our Ethereum light client, so we would just have prepared for the Bellatrix upgrade. Never say never, but it highly unlikely for hard-fork to be days from each other.

Also I remember some accidental hard forks that have happened because of differences in client implementations. Do you also have some solution for bridge fixes in such case (e.g.: force switch forks). Just curious

Iirc those hard-forks were related to the execution chain, and it has not been required since the consensus chain was launched. Consensus client changes, so far, have been planned months in advance and usually have required code changes (like the changes in the PR, to accommodate changes in the ExecutionHeaderPayload struct). We could have merged these changes in months ago but we decided to hold off since we had other higher priorities.

This is usually how these hard-forks work:

Code changes announced here: https://github.com/ethereum/consensus-specs
We make the required code changes
Ethereum announces the expected epoch that the hard-fork will take place on

If we had to, we could configure the hard-fork versions via a governance call. I am not sure if we want to do that now though. We do run the risk that the time between the mainnet epoch announcement and fork is shorther than the Parity release train. cc @vgeddes

Do you have some safety switch to turn off bridge if we'll fail to deploy on time? Or we'll be trying to do that on time.

We can halt the light client if need be, but it would obviously be preferable not to have "downtime" where we can't process messages. If there should be an unexpected Ethereum hard-fork because of the bug, we could halt the bridge until we configured the new fork version (cc @yrong please correct me if I am wrong).

claravanstaden · 2024-01-24T11:06:10Z

I didn't really check your subtree stuff, at the first glance looks ok, I saw you removed minimal/mainet features, is it possible to remove this:
fast-runtime = [
	"bridge-hub-rococo-runtime/fast-runtime",
]
and change to:
fast-runtime = [ ]
in the polkadot-parachain Cargo.toml?

@bkontur it is unfortunately not possible because the fast-runtime feature needs to be propagated to the bridge hub runtime to toggle between the different fork versions for local testing and Rococo.

* reset nonces * use set storage instead of kill storage

* More integration tests * Fix tests * Register token with inbound fixture * Import snowbridge-pallet-inbound-queue-fixtures * Cleanup Cargo.toml * Update emulated test * Fix emulated test * Rename to register_token * Update emulated tests * Update tests * Disable create agent/channel by xcm * More checks * Fix test * Clean test case * Test fixture sent token to penpal * Update test

3c4fe56af66 Disable system calls from xcm & Refactoring emulated tests (#1115) 769ce69a10a Allow resetting of Nonces in Rococo (paritytech#1128) ec4df8e1fab PR comments for upstream (paritytech#1127) git-subtree-dir: bridges/snowbridge git-subtree-split: 3c4fe56af666ad94d7aa06976d5510d37f737dc2

paritytech-cicd-pr · 2024-01-29T12:42:54Z

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/5058459

…nowbridge-deneb-updates

bkontur · 2024-01-29T14:00:22Z

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs

+				)
+				.to_vec(), 0u64.encode()),
+				(snowbridge_pallet_inbound_queue::Nonce::<Runtime>::hashed_key_for::<ChannelId>(
+					channel_id_one,


Suggested change

channel_id_one,

channel_id_two,

@claravanstaden typo or intention?

I think this is intentional because if I change it, the test fails. @alistair-singh can provide more context here and perhaps add a comment why.

This test asserts that only channel_one nonces are reset and that channel_two nonces are left alone.

bkontur · 2024-01-29T14:01:37Z

cumulus/parachains/runtimes/test-utils/src/test_cases.rs

+		// encode `kill_storage` call
+		let kill_storage_call = runtime_call_encode(frame_system::Call::<Runtime>::set_storage {


@claravanstaden small nits, name kill_storage here is kind of misleading, because it does set_storage.
Also I think that this introduced set_storage_keys_by_governance_works is not necessary and its usage can be replaced with four calls of change_storage_constant_by_governance_works in-place.

Ok cool! Would it be OK if I create an internal ticket for this? Just because the Ethereum Deneb fork on Sepolia is tomorrow. Clean-up storage test

Originally I used kill_storage to reset nonces so this is leftover naming from that and needs to change.

The issue with change_storage_constant_by_governance_works is that it is bound to storage items which implement Get<StorageConstantType>. This limits its usage to storage constants that created in parameter_types! macro and only tests single keys. This will not work for pallets with a StorageMap. set_storage_keys_by_governance_works does this in a more general way by allowing two closures, one to initialize storage before modification, and one to assert storage after modification.

The tests that use change_storage_constant_by_governance_works could be re-implemented in terms of set_storage_keys_by_governance_works as it is more general, but that would affect 7 runtime tests which use it. Should we rather do that?

@alistair-singh yes, you're right, change_storage_constant_by_governance_works was dedicated more for parameter_types! { pub storage.. stuff. Or maybe change_storage_constant_by_governance_works can internally just call set_storage_keys_by_governance_works, anyway, if we could make it nicer, lets do it, but this is very low prio,

@claravanstaden we don't have access to your, it is related to the polkadot-sdk code directly, so it is also fine to create issue here.

…nowbridge-deneb-updates

claravanstaden · 2024-01-29T18:20:04Z

@ggwpez several runtime migration check CI tasks are failing with:

Other("Max message size for channel is too large. This means that the V5 migration can be front-run and an attacker could place a large message just right before the migration to make other messages un-decodable. Please either increase MaxPageSize or decrease the max_message_size for this channel.")

It is for the asset-hub-rococo, asset-hub-westend, bridge-hub-rococo, bridge-hub-westend and collectives-westend runtimes. (e.g. https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5065025)

Could you please point me in the right direction to correct this config?

Edit: I am thinking it is a misconfiguration between configs HeapSize and ServiceWeight, but I would like your input on what values would be sensible given that the issue seems to affect runtimes that we have not changed. It is worth noting, our Ethereum light client needs to be initialised with the first initial validator set, which is why we have increased this value in the Rococo runtime: https://github.com/Snowfork/polkadot-sdk/blob/snowbridge/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs#L359

ggwpez · 2024-01-29T20:45:07Z

Mea culpa: #3117. Its from the XCMP queue, not the MessageQueue.

claravanstaden · 2024-01-30T06:20:40Z

@ggwpez Thanks for the fix! 🙌🏻 Can this PR be merged or do we need to wait for #3117?

To deploy #3029 Co-authored-by: claravanstaden <Cats 4 life!>

claravanstaden and others added 18 commits January 23, 2024 12:56

cleanup test imports (#96)

4de9edf

Co-authored-by: claravanstaden <Cats 4 life!>

Adds emulated test comments (#97)

eef706e

* adds emulated test comments * pr comments * rename XCMs --------- Co-authored-by: claravanstaden <Cats 4 life!>

Dencun (#102)

2922db0

* ForkVersions for Deneb * Enable deneb from genesis

Schedule dencun on sepolia (#104)

0dff6f4

* Schedule dencun on sepolia * Update Cargo.lock

rm subtree

31b55c7

Merge commit '65b4312577e57d529c43269154b48a627d410d0e' as 'bridges/s…

a5b4fbb

…nowbridge'

Squashed 'bridges/snowbridge/' content from commit d65afb42f62

65b4312

git-subtree-dir: bridges/snowbridge git-subtree-split: d65afb42f629a28eb1147fd6710f650c2928632b

update subtree

3d3d1a7

adds prdoc

fd7c142

Merge branch 'master' into snowbridge-deneb-updates

071902b

# Conflicts: # Cargo.lock

updates lock file

9e2a925

Remove minimal spec (#100)

108e14c

* remove minimal spec * update test modules * fix fork versions in runtime * adds deneb * fix lodestar setup in nix, update fixtures with deneb fork --------- Co-authored-by: claravanstaden <Cats 4 life!>

Squashed 'bridges/snowbridge/' changes from d65afb42f62..53a5522d494

5eb62e1

53a5522d494 Remove minimal spec (paritytech#1119) git-subtree-dir: bridges/snowbridge git-subtree-split: 53a5522d494e38c5c3573c65e47bceda30572e45

updates subtree

20a5d6e

Merge branch 'master' into snowbridge-deneb-updates

d554861

fmt

cb6abc6

Merge remote-tracking branch 'origin/snowbridge-deneb-updates' into s…

82bba29

…nowbridge-deneb-updates

fix path

1cd207f

claravanstaden marked this pull request as ready for review January 23, 2024 12:12

paritytech-review-bot bot requested a review from a team January 23, 2024 12:12

claravanstaden added 3 commits January 23, 2024 14:13

update spec version

a72da8c

Merge branch 'master' into snowbridge-deneb-updates

d562823

# Conflicts: # Cargo.lock # cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/Cargo.toml

fix rust doc and lock file

969cfc0

svyatonik added the T15-bridges This PR/Issue is related to bridges. label Jan 24, 2024

svyatonik reviewed Jan 24, 2024

View reviewed changes

revert path update

57f9b19

paritytech-review-bot bot requested a review from a team January 24, 2024 09:02

bkontur approved these changes Jan 24, 2024

View reviewed changes

claravanstaden and others added 3 commits January 26, 2024 15:54

update subtree

ea2ca38

Merge branch 'master' into snowbridge-deneb-updates

e5cb3a7

update cargo.lock and fix markdown

d6f43a5

claravanstaden requested review from a team January 29, 2024 08:40

claravanstaden and others added 8 commits January 29, 2024 13:09

Merge branch 'master' into snowbridge-deneb-updates

a9c4a32

Reset Nonces in Rococo (#109)

808dd21

* reset nonces * use set storage instead of kill storage

updates subtree

34def83

fix clippy

55f4dd6

fix removing incorrect variable

dadc817

Merge branch 'master' into snowbridge-deneb-updates

557cfb8

claravanstaden added 2 commits January 29, 2024 14:43

fix removing incorrect variable

bec72f3

Merge remote-tracking branch 'origin/snowbridge-deneb-updates' into s…

8bb432a

…nowbridge-deneb-updates

bkontur reviewed Jan 29, 2024

View reviewed changes

claravanstaden and others added 4 commits January 29, 2024 19:31

fixing weird downgrades and missing no default features

dff0d17

Merge branch 'master' into snowbridge-deneb-updates

d13b08b

zepter

75ccedb

Merge remote-tracking branch 'origin/snowbridge-deneb-updates' into s…

1dedc2a

…nowbridge-deneb-updates

svyatonik added this pull request to the merge queue Jan 30, 2024

Merged via the queue into paritytech:master with commit 85191e9 Jan 30, 2024
119 of 124 checks passed

claravanstaden mentioned this pull request Jan 30, 2024

Bump Bridge Hub Rococo spec version #3135

Merged

github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2024

Bump Bridge Hub Rococo spec version (#3135)

f07316f

To deploy #3029 Co-authored-by: claravanstaden <Cats 4 life!>

github-actions bot mentioned this pull request Mar 13, 2024

Update polkadot-sdk from v1.3.0 to v1.7.2 moonbeam-foundation/moonbeam#2703

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snowbridge Ethereum Deneb fork preparation #3029

Snowbridge Ethereum Deneb fork preparation #3029

claravanstaden commented Jan 23, 2024

svyatonik left a comment

svyatonik Jan 24, 2024

claravanstaden Jan 24, 2024

svyatonik Jan 24, 2024

claravanstaden Jan 24, 2024

bkontur Jan 24, 2024

bkontur left a comment

claravanstaden commented Jan 24, 2024

claravanstaden commented Jan 24, 2024

paritytech-cicd-pr commented Jan 29, 2024

bkontur Jan 29, 2024

claravanstaden Jan 29, 2024

alistair-singh Jan 29, 2024 •

edited

Loading

bkontur Jan 29, 2024

claravanstaden Jan 29, 2024 •

edited

Loading

alistair-singh Jan 29, 2024 •

edited

Loading

bkontur Jan 30, 2024

claravanstaden commented Jan 29, 2024 •

edited

Loading

ggwpez commented Jan 29, 2024

claravanstaden commented Jan 30, 2024

		// encode `kill_storage` call
		let kill_storage_call = runtime_call_encode(frame_system::Call::<Runtime>::set_storage {

Snowbridge Ethereum Deneb fork preparation #3029

Snowbridge Ethereum Deneb fork preparation #3029

Conversation

claravanstaden commented Jan 23, 2024

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

bkontur left a comment

Choose a reason for hiding this comment

claravanstaden commented Jan 24, 2024

claravanstaden commented Jan 24, 2024

paritytech-cicd-pr commented Jan 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alistair-singh Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claravanstaden Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

alistair-singh Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claravanstaden commented Jan 29, 2024 • edited Loading

ggwpez commented Jan 29, 2024

claravanstaden commented Jan 30, 2024

alistair-singh Jan 29, 2024 •

edited

Loading

claravanstaden Jan 29, 2024 •

edited

Loading

alistair-singh Jan 29, 2024 •

edited

Loading

claravanstaden commented Jan 29, 2024 •

edited

Loading