-
Notifications
You must be signed in to change notification settings - Fork 768
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
Add Rococo People <> Rococo Bulletin bridge support to Rococo Bridge Hub #2540
Add Rococo People <> Rococo Bulletin bridge support to Rococo Bridge Hub #2540
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't looked at the tests, but everything else looks good as is.
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs
Show resolved
Hide resolved
If I understand correctly, the question only affects the Bulletin chain runtime (which is deployed on both Polkadot and Rococo), and how it defines/sees the bridges (to PolkadotBH or RococoBH depending on where itself is deployed), right? IMO any of the 3 options is good. I expect that when Bulletin chain will be launched, it will also be moved to fellowship and we'll keep a Rococo copy here, and the problem "goes away". So in the meantime, I'm fine with whatever option is easiest for us :) |
…bridge_to_bulletin_config.rs Co-authored-by: Adrian Catangiu <[email protected]>
Ok, thanks. I'm leaving as is, because it is already deployed somewhere |
So far the `bridge-hub-test-utils` contained a tests set for testing BridgeHub runtime that is bridging with the remote **parachain**. But we have #2540 coming, which would add Rococo <> Bulletin chain bridge (where Bulletin = standalone chain that is using GRANDPA finality). Then it'll be expanded to Polkadot BH as well. So this PR adds the same set of tests to the `bridge-hub-test-utils`, but for the case when remote chain is the chain with GRANDPA finality. There's a lot of changes in this PR - I'll describe some: - I've added `BasicParachainRuntime` trait to decrease number of lines we need to add to `where` clause. Could revert, but imo it is useful; - `cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_data` is a submodule for generating test data for the test sets. `from_parachain.rs` is used in tests for the case when remote chain is a parachain, `from_grandpa_chain.rs` - for the bridges with remote GRANDPA chains. `mod.rs` has some code, shared by both types of tests; - `cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_data` is a submodule with all test cases. The `mod.rs` has tests, suitable for all cases. There's also `wth_parachain.rs` and `with_grandpa_chain.rs` with the same meaning as above; - I've merged the "core" code of two previous tests - `relayed_incoming_message_works` and `complex_relay_extrinsic_works` into one single `relayed_incoming_message_works` test. So now we are always constructing extrinsics and are dispatching them using executive module (meaning all signed extensions are also tested). New test set is used here: #2540. Once this PR is merged, I'll merge that other PR with master to remove duplicate changes. I'm also planning to cleanup generic constraints + remove some unnecessary assumptions about used chains in a follow-up PRs. But for now I think this PR has enough changes, so don't want to complicate it even more. --- Breaking changes for the code that have used those tests before: - the `construct_and_apply_extrinsic` callback now accepts the `RuntimeCall` instead of the `pallet_utility::Call`; - the `construct_and_apply_extrinsic` now may be called multiple times for the single test, so make sure the `frame_system::CheckNonce` is correctly constructed; - all previous tests have been moved from `bridge_hub_test_utils::test_cases` to `bridge_hub_test_utils::test_cases::from_parachain` module; - there are several changes in test arguments - please refer to https://github.com/paritytech/polkadot-sdk/compare/sv-tests-for-bridge-with-remote-grandpa-chain?expand=1#diff-79a28d4d3e1749050341c2424f00c4c139825b1a20937767f83e58b95166735c for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, lgtm, just few very nits which will be solved after rebase to actual master
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_common_config.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs
Outdated
Show resolved
Hide resolved
@@ -1004,6 +1063,41 @@ impl_runtime_apis! { | |||
} | |||
} | |||
|
|||
impl BridgeMessagesConfig<bridge_to_bulletin_config::WithRococoBulletinMessagesInstance> for Runtime { | |||
fn is_relayer_rewarded(_relayer: &Self::AccountId) -> bool { | |||
// we do not pay any rewards in this bridge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is_relayer_rewarded == true
=> "we do not pay any rewards" is little bit kind of confusing, but probably we can live with that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we can't test that the relayer is rewarded if we don't pay any rewards :) I could change a comment, if you have some better version :)
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs
Outdated
Show resolved
Hide resolved
// - BridgeWestendParachains | ||
// - BridgeWestendMessages | ||
// - BridgeRelayers | ||
// Bridge relayers pallet, used by several bridges here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the grouping here - this runtime is no longer shared by Rococo and Wococo, so imo it makes sense to group by bridged chain.
bot bench cumulus-bridge-hubs --runtime=bridge-hub-rococo --pallet=pallet_bridge_messages |
@svyatonik |
bot cancel 5-a34ea1ac-b42a-424e-ac12-0d58da204a4d |
@svyatonik Command |
bot bench cumulus-bridge-hubs --runtime=bridge-hub-rococo --pallet=pallet_bridge_messages |
@svyatonik https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4725738 was started for your command Comment |
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.0 to 4.1.1. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/releases">actions/checkout's releases</a>.</em></p> <blockquote> <h2>v4.1.1</h2> <h2>What's Changed</h2> <ul> <li>Update CODEOWNERS to Launch team by <a href="https://github.com/joshmgross"><code>@joshmgross</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1510">actions/checkout#1510</a></li> <li>Correct link to GitHub Docs by <a href="https://github.com/peterbe"><code>@peterbe</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1511">actions/checkout#1511</a></li> <li>Link to release page from what's new section by <a href="https://github.com/cory-miller"><code>@cory-miller</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1514">actions/checkout#1514</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/joshmgross"><code>@joshmgross</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1510">actions/checkout#1510</a></li> <li><a href="https://github.com/peterbe"><code>@peterbe</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1511">actions/checkout#1511</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v4.1.0...v4.1.1">https://github.com/actions/checkout/compare/v4.1.0...v4.1.1</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/checkout/commit/b4ffde65f46336ab88eb53be808477a3936bae11"><code>b4ffde6</code></a> Link to release page from what's new section (<a href="https://redirect.github.com/actions/checkout/issues/1514">#1514</a>)</li> <li><a href="https://github.com/actions/checkout/commit/8530928916aaef40f59e6f221989ccb31f5759e7"><code>8530928</code></a> Correct link to GitHub Docs (<a href="https://redirect.github.com/actions/checkout/issues/1511">#1511</a>)</li> <li><a href="https://github.com/actions/checkout/commit/7cdaf2fbc075e6f3b9ca94cfd6cec5adc8a75622"><code>7cdaf2f</code></a> Update CODEOWNERS to Launch team (<a href="https://redirect.github.com/actions/checkout/issues/1510">#1510</a>)</li> <li>See full diff in <a href="https://github.com/actions/checkout/compare/v4.1.0...b4ffde65f46336ab88eb53be808477a3936bae11">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/checkout&package-manager=github_actions&previous-version=4.1.0&new-version=4.1.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This tool makes it easy to run parachain consensus stress/performance testing on your development machine or in CI. ## Motivation The parachain consensus node implementation spans across many modules which we call subsystems. Each subsystem is responsible for a small part of logic of the parachain consensus pipeline, but in general the most load and performance issues are localized in just a few core subsystems like `availability-recovery`, `approval-voting` or `dispute-coordinator`. In the absence of such a tool, we would run large test nets to load/stress test these parts of the system. Setting up and making sense of the amount of data produced by such a large test is very expensive, hard to orchestrate and is a huge development time sink. ## PR contents - CLI tool - Data Availability Read test - reusable mockups and components needed so far - Documentation on how to get started ### Data Availability Read test An overseer is built with using a real `availability-recovery` susbsytem instance while dependent subsystems like `av-store`, `network-bridge` and `runtime-api` are mocked. The network bridge will emulate all the network peers and their answering to requests. The test is going to be run for a number of blocks. For each block it will generate send a “RecoverAvailableData” request for an arbitrary number of candidates. We wait for the subsystem to respond to all requests before moving to the next block. At the same time we collect the usual subsystem metrics and task CPU metrics and show some nice progress reports while running. ### Here is how the CLI looks like: ``` [2023-11-28T13:06:27Z INFO subsystem_bench::core::display] n_validators = 1000, n_cores = 20, pov_size = 5120 - 5120, error = 3, latency = Some(PeerLatency { min_latency: 1ms, max_latency: 100ms }) [2023-11-28T13:06:27Z INFO subsystem-bench::availability] Generating template candidate index=0 pov_size=5242880 [2023-11-28T13:06:27Z INFO subsystem-bench::availability] Created test environment. [2023-11-28T13:06:27Z INFO subsystem-bench::availability] Pre-generating 60 candidates. [2023-11-28T13:06:30Z INFO subsystem-bench::core] Initializing network emulation for 1000 peers. [2023-11-28T13:06:30Z INFO subsystem-bench::availability] Current block 1/3 [2023-11-28T13:06:30Z INFO substrate_prometheus_endpoint] 〽️ Prometheus exporter started at 127.0.0.1:9999 [2023-11-28T13:06:30Z INFO subsystem_bench::availability] 20 recoveries pending [2023-11-28T13:06:37Z INFO subsystem_bench::availability] Block time 6262ms [2023-11-28T13:06:37Z INFO subsystem-bench::availability] Sleeping till end of block (0ms) [2023-11-28T13:06:37Z INFO subsystem-bench::availability] Current block 2/3 [2023-11-28T13:06:37Z INFO subsystem_bench::availability] 20 recoveries pending [2023-11-28T13:06:43Z INFO subsystem_bench::availability] Block time 6369ms [2023-11-28T13:06:43Z INFO subsystem-bench::availability] Sleeping till end of block (0ms) [2023-11-28T13:06:43Z INFO subsystem-bench::availability] Current block 3/3 [2023-11-28T13:06:43Z INFO subsystem_bench::availability] 20 recoveries pending [2023-11-28T13:06:49Z INFO subsystem_bench::availability] Block time 6194ms [2023-11-28T13:06:49Z INFO subsystem-bench::availability] Sleeping till end of block (0ms) [2023-11-28T13:06:49Z INFO subsystem_bench::availability] All blocks processed in 18829ms [2023-11-28T13:06:49Z INFO subsystem_bench::availability] Throughput: 102400 KiB/block [2023-11-28T13:06:49Z INFO subsystem_bench::availability] Block time: 6276 ms [2023-11-28T13:06:49Z INFO subsystem_bench::availability] Total received from network: 415 MiB Total sent to network: 724 KiB Total subsystem CPU usage 24.00s CPU usage per block 8.00s Total test environment CPU usage 0.15s CPU usage per block 0.05s ``` ### Prometheus/Grafana stack in action <img width="1246" alt="Screenshot 2023-11-28 at 15 11 10" src="https://github.com/paritytech/polkadot-sdk/assets/54316454/eaa47422-4a5e-4a3a-aaef-14ca644c1574"> <img width="1246" alt="Screenshot 2023-11-28 at 15 12 01" src="https://github.com/paritytech/polkadot-sdk/assets/54316454/237329d6-1710-4c27-8f67-5fb11d7f66ea"> <img width="1246" alt="Screenshot 2023-11-28 at 15 12 38" src="https://github.com/paritytech/polkadot-sdk/assets/54316454/a07119e8-c9f1-4810-a1b3-f1b7b01cf357"> --------- Signed-off-by: Andrei Sandu <[email protected]>
This PR backports `transaction_version` bump from `1.5.0` release back to `master`
…=bridge-hub-rococo --runtime_dir=bridge-hubs --target_dir=cumulus --pallet=pallet_bridge_messages
@svyatonik Command |
…://github.com/paritytech/polkadot-sdk into add-with-bulletin-bridge-to-rococo-bridge-hub
The CI pipeline was cancelled due to failure one of the required jobs. |
This PR adds Rococo People <> Rococo Bulletin to the Rococo Bridge Hub code. There's a couple of things left to do here:
bridge-hub-test-utils
- will do in a separate PR;The reason why I'm opening it before this ^^^ is ready, is that I'd like to hear others opinion on how to deal with hacks with that bridge. Initially I was assuming that Rococo Bulletin will be the 1:1 copy of the Polkadot Bulletin (to avoid maintaining multiple runtimes/releases/...), so you can see many
PolkadotBulletin
mentions in this PR, even though we are going to bridge with the parallel chain (RococoBulletin
). That's because e.g. pallet names fromconstruct_runtime
are affecting runtime storage keys and bridges are using runtime storage proofs => it is important to use names that the Bulletin chain expects.But in the end, this hack won't work - we can't use Polkadot Bulletin runtime to bridge with Rococo Bridge Hub, because Polkadot Bulletin expects Polkadot Bridge hub to use
1002
parachain id and Rococo Bridge Hub seats on the1013
. This also affects storage keys using in bridging, so I had to add therococo
feature to the Bulletin chain. So now we can actually alter its runtime and adapt it for Rococo.So the question here is - what's better for us here
rococo
feature is used - e.g. use proper names there (WithPolkadotGrandpa
->WithRococoGrandpa
, ...)cc @acatangiu @bkontur @serban300
also cc @joepetrowski as the main "client" of this bridge
A couple words on how this bridge is different from the Rococo <> Westend bridge:
AllowExplicitUnpaidExecutionFrom<Equals<SiblingPeople>>
+ we are not paying any rewards to relayers (apart from compensating transaction costs).