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

feat(relayer): Introduce KillSwitch relayer #200

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ByteNacked
Copy link
Member

Resolves #148 .

@reviewer-or-team

@ByteNacked ByteNacked self-assigned this Nov 12, 2024
@ByteNacked ByteNacked marked this pull request as draft November 12, 2024 07:20
@ByteNacked ByteNacked marked this pull request as ready for review November 19, 2024 06:39
relayer/src/kill_switch.rs Outdated Show resolved Hide resolved
relayer/src/main.rs Outdated Show resolved Hide resolved
relayer/src/main.rs Outdated Show resolved Hide resolved
relayer/src/relay_merkle_roots.rs Outdated Show resolved Hide resolved
relayer/src/kill_switch.rs Outdated Show resolved Hide resolved
@ByteNacked
Copy link
Member Author

ByteNacked commented Dec 1, 2024

Update: block finality proof history now stored in an embedded database.

use gear_rpc_client::GearApi;

impl_metered_service! {
pub(crate) struct Metrics {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to duplicate metrics if they match. Anyway the names should be relayer-specific

Copy link
Member

Choose a reason for hiding this comment

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

To not depend on Metrics in sync_authority_set_id you can query latest_observed_gear_era and latest_proven_era before calling it

)
}

pub(crate) async fn submit_proof_to_ethereum(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub(crate) async fn submit_proof_to_ethereum(
pub(crate) async fn submit_merkle_root_to_ethereum(


let mut metrics = MetricsBuilder::new();

let proof_storage: Box<dyn ProofStorage> =
Copy link
Member

Choose a reason for hiding this comment

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

maybe move to a separate fn create_proof_storage? To reuse in the merkle root relayer

Comment on lines +363 to +364
// TODO: metrics
let _metrics = metrics;
Copy link
Member

Choose a reason for hiding this comment

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

Don't leave TODOs - implement it or open an issue

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, forgot about that

Comment on lines +366 to +375
let authority_set_hash = hex::decode(&args.genesis_config_args.authority_set_hash)
.expect("Incorrect format for authority set hash: hex-encoded hash is expected");
let authority_set_hash = authority_set_hash
.try_into()
.expect("Incorrect format for authority set hash: wrong length");

let genesis_config = GenesisConfig {
authority_set_id: args.genesis_config_args.authority_set_id,
authority_set_hash,
};
Copy link
Member

Choose a reason for hiding this comment

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

This also can be reused with merkle root relayer

genesis_config: GenesisConfig,
proof_storage: Box<dyn ProofStorage>,
from_eth_block: Option<u64>,
block_finality_storage_path: impl AsRef<Path>,
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to create sled::DB outside

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? For what purpose?

proof_storage: Box<dyn ProofStorage>,

// Next eth block to process.
from_eth_block: Option<u64>,
Copy link
Member

Choose a reason for hiding this comment

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

it looks more like latest_eth_block

// [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
// ```
// Some if we don't have the proof for block #3 or #4, we can try to get the proof for block #5.
let res = self.block_finality_storage.get_gt(key_bytes)?;
Copy link
Member

Choose a reason for hiding this comment

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

Actually submitting some other block will not lead to a bridge halt, so I think None should be returned

Copy link
Member

Choose a reason for hiding this comment

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

And, ofc, this error should be reported somehow at the main_loop as it means bridge's total failure

Copy link
Member

Choose a reason for hiding this comment

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

I'd add more metrics here such as:

  • latest block (to see if relayer will be not in sync with ethereum network)
  • amount of merkle roots discovered (to see if we're listening live contract)
  • merkle root mismatch counter (that will display amount of merkle root mismatches found)
  • latest stored finality proof (to see if relayer is not in sync with gear network)

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.

Killswitch relayer
2 participants