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

Start the equivocation detection loop from the complex relayer #2507

Merged
merged 17 commits into from
Aug 30, 2023

Conversation

serban300
Copy link
Collaborator

Related to #2377

@serban300 serban300 self-assigned this Aug 30, 2023
Copy link
Contributor

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

LGTM. But have you considered adding a separate subcommand for the substrate-relay for starting ED loop? I don't think ED loop depends in any way on complex relay and vice versa. IMO complex relay is already overloaded with functionality (meaning its code configuration is too complex) and maybe it'll be easier to add a separate subcommand?

@@ -39,5 +39,7 @@ exec /home/user/substrate-relay relay-headers-and-messages millau-rialto-paracha
--rialto-parachain-transactions-mortality=64 \
--rialto-host rialto-node-alice \
--rialto-port 9944 \
--rialto-signer //Millau.HeadersAndMessagesRelay \
Copy link
Contributor

Choose a reason for hiding this comment

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

Spaces -> tabs please :)

@serban300
Copy link
Collaborator Author

LGTM. But have you considered adding a separate subcommand for the substrate-relay for starting ED loop? I don't think ED loop depends in any way on complex relay and vice versa. IMO complex relay is already overloaded with functionality (meaning its code configuration is too complex) and maybe it'll be easier to add a separate subcommand?

Ok, makes sense, will add a different command. I thought it would be safer to start the ED loop inside the complex relayer in order to not forget about it. But it's true that the complex relayer is overloaded with functionality.

@acatangiu
Copy link
Collaborator

acatangiu commented Aug 30, 2023

will add a different command

yes please 👍

(will review code a bit later)

Copy link
Contributor

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

💯


#[allow(dead_code, unused_imports, non_camel_case_types)]
#[allow(clippy::all)]
pub mod api {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we were to move relays to polkadot-sdk repo, we'd no longer need these huge generated files, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

and we'd have better control on the runtime API version used too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we'll be able to use the actual runtimes directly from the repo.

Comment on lines +3 to +4
- detect-millau-to-rialto-equivocations:9616
- detect-rialto-to-millau-equivocations:9616
Copy link
Collaborator

Choose a reason for hiding this comment

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

💯


/// Helper trait to define some extra methods on bridge hubs signed extension (and
/// overcome Rust limitations).
pub trait BridgeHubSignedExtension {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once you merge this, please also update polkadot-sdk and fix breaking changes like this one while still fresh in our minds 😄 . (used here)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I will. I have to merge it to polkadot-staging first. And then will try to update the bridges repo in polkadot-sdk.

@@ -3,5 +3,8 @@
cd tools/runtime-codegen
cargo run --bin runtime-codegen -- --from-node-url "wss://rococo-bridge-hub-rpc.polkadot.io:443" > ../../relays/client-bridge-hub-rococo/src/codegen_runtime.rs
cargo run --bin runtime-codegen -- --from-node-url "http://localhost:20433" > ../../relays/client-rialto-parachain/src/codegen_runtime.rs
cargo run --bin runtime-codegen -- --from-node-url "wss://rococo-rpc.polkadot.io:443" > ../../relays/client-rococo/src/codegen_runtime.rs
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we're fine to use Rococo runtime api for Wococo too.. but taking mental note to look at this if in the future things start failing on Wococo all of a sudden 😆

@serban300 serban300 merged commit 3e9592a into paritytech:master Aug 30, 2023
serban300 added a commit to serban300/parity-bridges-common that referenced this pull request Aug 30, 2023
…ytech#2507)

* Impl SubstrateEquivocationDetectionPipeline for Millau

* Impl SubstrateEquivocationDetectionPipeline for Rialto

* Make BridgeHubSignedExtension more generic

* Define generate_report_equivocation_call_builder!() macro

* Impl SubstrateEquivocationDetectionPipeline for Rococo

* Impl SubstrateEquivocationDetectionPipeline for Wococo

* Impl SubstrateEquivocationDetectionPipeline for Kusama

* Impl SubstrateEquivocationDetectionPipeline for Polkadot

* Complex relayer simplification

- remove `signer` and `transactions_mortality` and add `tx_params`
- change the order of some fields

* Add equivocation detection CLI traits

* Complex relayer: start equivocation detectin loop

* Update runtimes regeneration script

* Equivocation loop: Fix infinite loop

* Revert "Complex relayer: start equivocation detectin loop"

This reverts commit b518ef8.

* Add subcommand for starting the equivocation detection loop

* Fixes

* Initialize empty metrics for the equivocations detector loop
serban300 added a commit that referenced this pull request Aug 31, 2023
#2512)

* Impl SubstrateEquivocationDetectionPipeline for Millau

* Impl SubstrateEquivocationDetectionPipeline for Rialto

* Make BridgeHubSignedExtension more generic

* Define generate_report_equivocation_call_builder!() macro

* Impl SubstrateEquivocationDetectionPipeline for Rococo

* Impl SubstrateEquivocationDetectionPipeline for Wococo

* Impl SubstrateEquivocationDetectionPipeline for Kusama

* Impl SubstrateEquivocationDetectionPipeline for Polkadot

* Complex relayer simplification

- remove `signer` and `transactions_mortality` and add `tx_params`
- change the order of some fields

* Add equivocation detection CLI traits

* Complex relayer: start equivocation detectin loop

* Update runtimes regeneration script

* Equivocation loop: Fix infinite loop

* Revert "Complex relayer: start equivocation detectin loop"

This reverts commit b518ef8.

* Add subcommand for starting the equivocation detection loop

* Fixes

* Initialize empty metrics for the equivocations detector loop
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-kusama-bridge/2971/5

serban300 added a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
…ytech#2507) (paritytech#2512)

* Impl SubstrateEquivocationDetectionPipeline for Millau

* Impl SubstrateEquivocationDetectionPipeline for Rialto

* Make BridgeHubSignedExtension more generic

* Define generate_report_equivocation_call_builder!() macro

* Impl SubstrateEquivocationDetectionPipeline for Rococo

* Impl SubstrateEquivocationDetectionPipeline for Wococo

* Impl SubstrateEquivocationDetectionPipeline for Kusama

* Impl SubstrateEquivocationDetectionPipeline for Polkadot

* Complex relayer simplification

- remove `signer` and `transactions_mortality` and add `tx_params`
- change the order of some fields

* Add equivocation detection CLI traits

* Complex relayer: start equivocation detectin loop

* Update runtimes regeneration script

* Equivocation loop: Fix infinite loop

* Revert "Complex relayer: start equivocation detectin loop"

This reverts commit b518ef8.

* Add subcommand for starting the equivocation detection loop

* Fixes

* Initialize empty metrics for the equivocations detector loop
serban300 added a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
…ytech#2507) (paritytech#2512)

* Impl SubstrateEquivocationDetectionPipeline for Millau

* Impl SubstrateEquivocationDetectionPipeline for Rialto

* Make BridgeHubSignedExtension more generic

* Define generate_report_equivocation_call_builder!() macro

* Impl SubstrateEquivocationDetectionPipeline for Rococo

* Impl SubstrateEquivocationDetectionPipeline for Wococo

* Impl SubstrateEquivocationDetectionPipeline for Kusama

* Impl SubstrateEquivocationDetectionPipeline for Polkadot

* Complex relayer simplification

- remove `signer` and `transactions_mortality` and add `tx_params`
- change the order of some fields

* Add equivocation detection CLI traits

* Complex relayer: start equivocation detectin loop

* Update runtimes regeneration script

* Equivocation loop: Fix infinite loop

* Revert "Complex relayer: start equivocation detectin loop"

This reverts commit b518ef8.

* Add subcommand for starting the equivocation detection loop

* Fixes

* Initialize empty metrics for the equivocations detector loop
bkontur pushed a commit that referenced this pull request May 7, 2024
* Impl SubstrateEquivocationDetectionPipeline for Millau

* Impl SubstrateEquivocationDetectionPipeline for Rialto

* Make BridgeHubSignedExtension more generic

* Define generate_report_equivocation_call_builder!() macro

* Impl SubstrateEquivocationDetectionPipeline for Rococo

* Impl SubstrateEquivocationDetectionPipeline for Wococo

* Impl SubstrateEquivocationDetectionPipeline for Kusama

* Impl SubstrateEquivocationDetectionPipeline for Polkadot

* Complex relayer simplification

- remove `signer` and `transactions_mortality` and add `tx_params`
- change the order of some fields

* Add equivocation detection CLI traits

* Complex relayer: start equivocation detectin loop

* Update runtimes regeneration script

* Equivocation loop: Fix infinite loop

* Revert "Complex relayer: start equivocation detectin loop"

This reverts commit b518ef8.

* Add subcommand for starting the equivocation detection loop

* Fixes

* Initialize empty metrics for the equivocations detector loop
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.

4 participants