From 9f559b28ab28c18e8bad192fa6a1239444da61ec Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 3 Nov 2021 15:38:04 +0530 Subject: [PATCH 01/36] Add config mode --- relayer/src/config.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 2ce0912bbe..8bb827c784 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -115,6 +115,7 @@ pub mod default { pub struct Config { #[serde(default)] pub global: GlobalConfig, + pub mode: ModeConfig, #[serde(default)] pub rest: RestConfig, #[serde(default)] @@ -179,6 +180,46 @@ impl Default for Strategy { } } +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct ModeConfig { + pub clients: Clients, + pub connections: Connections, + pub channels: Channels, + pub packets: Packets, +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct Clients { + pub enabled: bool, + #[serde(default)] + pub refresh: bool, + #[serde(default)] + pub misbehaviour: bool, +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct Connections { + pub enabled: bool, +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct Channels { + pub enabled: bool, +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct Packets { + pub enabled: bool, + #[serde(default = "default::clear_packets_interval")] + pub clear_interval: u64, + #[serde(default)] + pub clear_on_start: bool, + #[serde(default)] + pub filter: bool, + #[serde(default)] + pub tx_confirmation: bool, +} + /// Log levels are wrappers over [`tracing_core::Level`]. /// /// [`tracing_core::Level`]: https://docs.rs/tracing-core/0.1.17/tracing_core/struct.Level.html From 403d280217a0853c9c4dffd9d0ce39baaf95ce0e Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 3 Nov 2021 16:00:23 +0530 Subject: [PATCH 02/36] Remove config strategy --- ci/simple_config.toml | 1 - config.toml | 6 ------ relayer/src/config.rs | 20 ++----------------- .../config/fixtures/relayer_conf_example.toml | 1 - 4 files changed, 2 insertions(+), 26 deletions(-) diff --git a/ci/simple_config.toml b/ci/simple_config.toml index fd4736f55f..2a076755f6 100644 --- a/ci/simple_config.toml +++ b/ci/simple_config.toml @@ -1,5 +1,4 @@ [global] -strategy = 'all' log_level = 'trace' filter = false clear_packets_interval = 100 diff --git a/config.toml b/config.toml index 13ddfd859e..4bc049d4c2 100644 --- a/config.toml +++ b/config.toml @@ -1,12 +1,6 @@ # The global section has parameters that apply globally to the relayer operation. [global] -# Specify the strategy to be used by the relayer. Default: 'packets' -# Two options are currently supported: -# - 'all': Relay packets and perform channel and connection handshakes. -# - 'packets': Relay packets only. -strategy = 'packets' - # Enable or disable the filtering mechanism. Default: 'false' # Valid options are 'true', 'false'. # Currently Hermes supports two filters: diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 8bb827c784..4991e38f36 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -157,7 +157,8 @@ impl Config { } pub fn handshake_enabled(&self) -> bool { - self.global.strategy == Strategy::HandshakeAndPackets + // FIXME(hu55a1n1) + unimplemented!() } pub fn chains_map(&self) -> HashMap<&ChainId, &ChainConfig> { @@ -165,21 +166,6 @@ impl Config { } } -#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] -pub enum Strategy { - #[serde(rename = "packets")] - Packets, - - #[serde(rename = "all")] - HandshakeAndPackets, -} - -impl Default for Strategy { - fn default() -> Self { - Self::Packets - } -} - #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct ModeConfig { pub clients: Clients, @@ -254,7 +240,6 @@ impl fmt::Display for LogLevel { #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(default, deny_unknown_fields)] pub struct GlobalConfig { - pub strategy: Strategy, pub log_level: LogLevel, #[serde(default = "default::filter")] pub filter: bool, @@ -267,7 +252,6 @@ pub struct GlobalConfig { impl Default for GlobalConfig { fn default() -> Self { Self { - strategy: Strategy::default(), log_level: LogLevel::default(), filter: default::filter(), clear_packets_interval: default::clear_packets_interval(), diff --git a/relayer/tests/config/fixtures/relayer_conf_example.toml b/relayer/tests/config/fixtures/relayer_conf_example.toml index 44d001bcc5..b07289291e 100644 --- a/relayer/tests/config/fixtures/relayer_conf_example.toml +++ b/relayer/tests/config/fixtures/relayer_conf_example.toml @@ -1,5 +1,4 @@ [global] -strategy = 'packets' log_level = 'error' [[chains]] From 12875765cfb2dae997536e0c8744411096093492 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 3 Nov 2021 16:09:09 +0530 Subject: [PATCH 03/36] Remove config global clear_packets_interval option --- ci/simple_config.toml | 1 - config.toml | 6 ------ relayer/src/config.rs | 3 --- relayer/src/worker.rs | 2 +- 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/ci/simple_config.toml b/ci/simple_config.toml index 2a076755f6..5f82c5b6a3 100644 --- a/ci/simple_config.toml +++ b/ci/simple_config.toml @@ -1,7 +1,6 @@ [global] log_level = 'trace' filter = false -clear_packets_interval = 100 [telemetry] enabled = false diff --git a/config.toml b/config.toml index 4bc049d4c2..82fba076d6 100644 --- a/config.toml +++ b/config.toml @@ -16,12 +16,6 @@ filter = false # Valid options are 'error', 'warn', 'info', 'debug', 'trace'. log_level = 'info' -# Parametrize the periodic packet clearing feature. -# Interval (in number of blocks) at which pending packets -# should be eagerly cleared. A value of '0' will disable -# periodic packet clearing. Default: 100 -clear_packets_interval = 100 - # Toggle the transaction confirmation mechanism. # The tx confirmation mechanism periodically queries the `/tx_search` RPC # endpoint to check that previously-submitted transactions diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 4991e38f36..4e8197d1cb 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -243,8 +243,6 @@ pub struct GlobalConfig { pub log_level: LogLevel, #[serde(default = "default::filter")] pub filter: bool, - #[serde(default = "default::clear_packets_interval")] - pub clear_packets_interval: u64, #[serde(default = "default::tx_confirmation")] pub tx_confirmation: bool, } @@ -254,7 +252,6 @@ impl Default for GlobalConfig { Self { log_level: LogLevel::default(), filter: default::filter(), - clear_packets_interval: default::clear_packets_interval(), tx_confirmation: default::tx_confirmation(), } } diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index b9ed310ec9..9f594d2aad 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -107,7 +107,7 @@ impl Worker Date: Wed, 3 Nov 2021 16:54:38 +0530 Subject: [PATCH 04/36] Skip misbehaviour based on client mode in config --- relayer/src/worker.rs | 6 +++--- relayer/src/worker/client.rs | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index 9f594d2aad..a9e26f2360 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -113,18 +113,18 @@ impl Worker) { + fn run(self, config: &Config, msg_tx: Sender) { let id = self.id(); let object = self.object(); let name = format!("{}#{}", object.short_name(), id); let result = match self { - Self::Client(_, w) => w.run(), + Self::Client(_, w) => w.run(config), Self::Connection(_, w) => w.run(), Self::Channel(_, w) => w.run(), Self::Packet(_, w) => w.run(), diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index a98a1cd9ec..2f3f500b54 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -8,6 +8,7 @@ use ibc::{core::ics02_client::events::UpdateClient, events::IbcEvent}; use crate::{ chain::handle::{ChainHandle, ChainHandlePair}, + config::Config, foreign_client::{ForeignClient, ForeignClientErrorDetail, MisbehaviourResults}, object::Client, telemetry, @@ -36,7 +37,7 @@ impl ClientWorker { } /// Run the event loop for events associated with a [`Client`]. - pub fn run(self) -> Result<(), RunError> { + pub fn run(self, config: &Config) -> Result<(), RunError> { let mut client = ForeignClient::restore( self.client.dst_client_id.clone(), self.chains.b.clone(), @@ -49,7 +50,8 @@ impl ClientWorker { ); // initial check for evidence of misbehaviour for all updates - let skip_misbehaviour = self.detect_misbehaviour(&client, None); + let skip_misbehaviour = + !config.mode.clients.misbehaviour && self.detect_misbehaviour(&client, None); // remember the time of the last refresh so we backoff let mut last_refresh = Instant::now() - Duration::from_secs(61); From 26e72cffac208d62c0ce310f2d86fe1133c54d84 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 3 Nov 2021 19:13:48 +0530 Subject: [PATCH 05/36] Clear packets on start based on config mode --- relayer/src/worker.rs | 19 +++++++++++++------ relayer/src/worker/client.rs | 9 +++++---- relayer/src/worker/packet.rs | 24 ++++++++++++++++++------ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index a9e26f2360..90e8389cdc 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -91,9 +91,15 @@ impl Worker { - Self::Client(id, ClientWorker::new(client.clone(), chains, cmd_rx)) - } + Object::Client(client) => Self::Client( + id, + ClientWorker::new( + client.clone(), + chains, + cmd_rx, + config.mode.clients.misbehaviour, + ), + ), Object::Connection(connection) => Self::Connection( id, ConnectionWorker::new(connection.clone(), chains, cmd_rx), @@ -108,23 +114,24 @@ impl Worker) { + fn run(self, msg_tx: Sender) { let id = self.id(); let object = self.object(); let name = format!("{}#{}", object.short_name(), id); let result = match self { - Self::Client(_, w) => w.run(config), + Self::Client(_, w) => w.run(), Self::Connection(_, w) => w.run(), Self::Channel(_, w) => w.run(), Self::Packet(_, w) => w.run(), diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index 2f3f500b54..353983a89e 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -8,7 +8,6 @@ use ibc::{core::ics02_client::events::UpdateClient, events::IbcEvent}; use crate::{ chain::handle::{ChainHandle, ChainHandlePair}, - config::Config, foreign_client::{ForeignClient, ForeignClientErrorDetail, MisbehaviourResults}, object::Client, telemetry, @@ -21,6 +20,7 @@ pub struct ClientWorker { client: Client, chains: ChainHandlePair, cmd_rx: Receiver, + misbehaviour: bool, } impl ClientWorker { @@ -28,16 +28,18 @@ impl ClientWorker { client: Client, chains: ChainHandlePair, cmd_rx: Receiver, + misbehaviour: bool, ) -> Self { Self { client, chains, cmd_rx, + misbehaviour, } } /// Run the event loop for events associated with a [`Client`]. - pub fn run(self, config: &Config) -> Result<(), RunError> { + pub fn run(self) -> Result<(), RunError> { let mut client = ForeignClient::restore( self.client.dst_client_id.clone(), self.chains.b.clone(), @@ -50,8 +52,7 @@ impl ClientWorker { ); // initial check for evidence of misbehaviour for all updates - let skip_misbehaviour = - !config.mode.clients.misbehaviour && self.detect_misbehaviour(&client, None); + let skip_misbehaviour = !self.misbehaviour || self.detect_misbehaviour(&client, None); // remember the time of the last refresh so we backoff let mut last_refresh = Instant::now() - Duration::from_secs(61); diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index 99045b72e9..758b91b702 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -26,6 +26,7 @@ pub struct PacketWorker { chains: ChainHandlePair, cmd_rx: Receiver, clear_packets_interval: u64, + clear_packets_on_start: Option, with_tx_confirmation: bool, } @@ -35,19 +36,22 @@ impl PacketWorker { chains: ChainHandlePair, cmd_rx: Receiver, clear_packets_interval: u64, + clear_packets_on_start: bool, with_tx_confirmation: bool, ) -> Self { + let clear_packets_on_start = Some(clear_packets_on_start); Self { path, chains, cmd_rx, clear_packets_interval, + clear_packets_on_start, with_tx_confirmation, } } /// Run the event loop for events associated with a [`Packet`]. - pub fn run(self) -> Result<(), RunError> { + pub fn run(mut self) -> Result<(), RunError> { let mut link = Link::new_from_opts( self.chains.a.clone(), self.chains.b.clone(), @@ -110,7 +114,7 @@ impl PacketWorker { /// also refreshes and executes any scheduled operational /// data that is ready. fn step( - &self, + &mut self, cmd: Option, link: &mut Link, index: u64, @@ -125,10 +129,18 @@ impl PacketWorker { height, new_block: _, } => { - // Schedule the clearing of pending packets. This should happen - // once at start, and _forced_ at predefined block intervals. - let force_packet_clearing = self.clear_packets_interval != 0 - && height.revision_height % self.clear_packets_interval == 0; + let (first_run, clear_on_start) = { + match self.clear_packets_on_start.take() { + None => (false, false), + Some(clear_on_start) => (true, clear_on_start), + } + }; + + // Schedule the clearing of pending packets. This may happen once at start, + // and may be _forced_ at predefined block intervals. + let force_packet_clearing = (first_run && clear_on_start) + || (self.clear_packets_interval != 0 + && height.revision_height % self.clear_packets_interval == 0); link.a_to_b .schedule_packet_clearing(Some(height), force_packet_clearing) From 0fb2d6cef857d61e076d6bd5390fcf6bbe6170fb Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 3 Nov 2021 20:13:01 +0530 Subject: [PATCH 06/36] Refresh clients only if enabled in config --- relayer/src/worker.rs | 1 + relayer/src/worker/client.rs | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index 90e8389cdc..74787829ff 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -98,6 +98,7 @@ impl Worker Self::Connection( diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index 353983a89e..0ce6c8c0da 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -21,6 +21,7 @@ pub struct ClientWorker { chains: ChainHandlePair, cmd_rx: Receiver, misbehaviour: bool, + refresh: bool, } impl ClientWorker { @@ -29,12 +30,14 @@ impl ClientWorker { chains: ChainHandlePair, cmd_rx: Receiver, misbehaviour: bool, + refresh: bool, ) -> Self { Self { client, chains, cmd_rx, misbehaviour, + refresh, } } @@ -63,7 +66,7 @@ impl ClientWorker { // Clients typically need refresh every 2/3 of their // trusting period (which can e.g., two weeks). // Backoff refresh checking to attempt it every minute. - if last_refresh.elapsed() > Duration::from_secs(60) { + if self.refresh && last_refresh.elapsed() > Duration::from_secs(60) { // Run client refresh, exit only if expired or frozen match client.refresh() { Ok(Some(_)) => { From 3957b2e2dc9e2bd161200770fe8659272797db46 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 19:41:54 +0530 Subject: [PATCH 07/36] Collect event based on config mode --- relayer/src/supervisor.rs | 187 ++++++++++++++------------------------ 1 file changed, 69 insertions(+), 118 deletions(-) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 1f881386cc..75d5a9c63c 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -171,6 +171,28 @@ impl Supervisor { } } + fn collect_event( + &self, + collected: &mut CollectedEvents, + event: &IbcEvent, + enabled: bool, + object_ctor: F, + ) where + F: FnOnce() -> Option, + { + if enabled { + if let Some(object) = object_ctor() { + if self.workers.contains(&object) { + collected + .per_object + .entry(object) + .or_default() + .push(event.clone()); + } + } + } + } + /// Collect the events we are interested in from an [`EventBatch`], /// and maps each [`IbcEvent`] to their corresponding [`Object`]. pub fn collect_events( @@ -180,11 +202,8 @@ impl Supervisor { ) -> CollectedEvents { let mut collected = CollectedEvents::new(batch.height, batch.chain_id.clone()); - let handshake_enabled = self - .config - .read() - .expect("poisoned lock") - .handshake_enabled(); + let config = self.config.read().expect("poisoned lock"); + let mode = &config.mode; for event in &batch.events { match event { @@ -192,144 +211,76 @@ impl Supervisor { collected.new_block = Some(event.clone()); } IbcEvent::UpdateClient(ref update) => { - if let Ok(object) = Object::for_update_client(update, src_chain) { - // Collect update client events only if the worker exists - if self.workers.contains(&object) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } - } + self.collect_event(&mut collected, event, mode.clients.enabled, || { + Object::for_update_client(update, src_chain).ok() + }); } IbcEvent::OpenInitConnection(..) | IbcEvent::OpenTryConnection(..) | IbcEvent::OpenAckConnection(..) => { - if !handshake_enabled { - continue; - } - - let object = event - .connection_attributes() - .map(|attr| Object::connection_from_conn_open_events(attr, src_chain)); - - if let Some(Ok(object)) = object { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.connections.enabled, || { + event + .connection_attributes() + .map(|attr| { + Object::connection_from_conn_open_events(attr, src_chain).ok() + }) + .flatten() + }); } IbcEvent::OpenInitChannel(..) | IbcEvent::OpenTryChannel(..) => { - if !handshake_enabled { - continue; - } - - let object = event - .channel_attributes() - .map(|attr| Object::channel_from_chan_open_events(attr, src_chain)); - - if let Some(Ok(object)) = object { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.channels.enabled, || { + event + .channel_attributes() + .map(|attr| Object::channel_from_chan_open_events(attr, src_chain).ok()) + .flatten() + }); } IbcEvent::OpenAckChannel(ref open_ack) => { // Create client and packet workers here as channel end must be opened - if let Ok(client_object) = - Object::client_from_chan_open_events(open_ack.attributes(), src_chain) - { - collected - .per_object - .entry(client_object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.clients.enabled, || { + Object::client_from_chan_open_events(open_ack.attributes(), src_chain).ok() + }); - if let Ok(packet_object) = - Object::packet_from_chan_open_events(open_ack.attributes(), src_chain) - { - collected - .per_object - .entry(packet_object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.packets.enabled, || { + Object::packet_from_chan_open_events(open_ack.attributes(), src_chain).ok() + }); // If handshake message relaying is enabled create worker to send the MsgChannelOpenConfirm message - if handshake_enabled { - if let Ok(channel_object) = - Object::channel_from_chan_open_events(open_ack.attributes(), src_chain) - { - collected - .per_object - .entry(channel_object) - .or_default() - .push(event.clone()); - } - } + self.collect_event(&mut collected, event, mode.channels.enabled, || { + Object::channel_from_chan_open_events(open_ack.attributes(), src_chain).ok() + }); } IbcEvent::OpenConfirmChannel(ref open_confirm) => { // Create client worker here as channel end must be opened - if let Ok(client_object) = + self.collect_event(&mut collected, event, mode.clients.enabled, || { Object::client_from_chan_open_events(open_confirm.attributes(), src_chain) - { - collected - .per_object - .entry(client_object) - .or_default() - .push(event.clone()); - } - if let Ok(packet_object) = + .ok() + }); + + self.collect_event(&mut collected, event, mode.packets.enabled, || { Object::packet_from_chan_open_events(open_confirm.attributes(), src_chain) - { - collected - .per_object - .entry(packet_object) - .or_default() - .push(event.clone()); - } + .ok() + }); } IbcEvent::SendPacket(ref packet) => { - if let Ok(object) = Object::for_send_packet(packet, src_chain) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.packets.enabled, || { + Object::for_send_packet(packet, src_chain).ok() + }); } IbcEvent::TimeoutPacket(ref packet) => { - if let Ok(object) = Object::for_timeout_packet(packet, src_chain) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.packets.enabled, || { + Object::for_timeout_packet(packet, src_chain).ok() + }); } IbcEvent::WriteAcknowledgement(ref packet) => { - if let Ok(object) = Object::for_write_ack(packet, src_chain) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.packets.enabled, || { + Object::for_write_ack(packet, src_chain).ok() + }); } IbcEvent::CloseInitChannel(ref packet) => { - if let Ok(object) = Object::for_close_init_channel(packet, src_chain) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + self.collect_event(&mut collected, event, mode.packets.enabled, || { + Object::for_close_init_channel(packet, src_chain).ok() + }); } _ => (), } From 867c42dba183ab8416883234bbfb65a20d1bd2a5 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 20:03:18 +0530 Subject: [PATCH 08/36] Fix collect_event() --- relayer/src/supervisor.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 75d5a9c63c..1776e6c710 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -182,13 +182,11 @@ impl Supervisor { { if enabled { if let Some(object) = object_ctor() { - if self.workers.contains(&object) { - collected - .per_object - .entry(object) - .or_default() - .push(event.clone()); - } + collected + .per_object + .entry(object) + .or_default() + .push(event.clone()); } } } @@ -212,7 +210,12 @@ impl Supervisor { } IbcEvent::UpdateClient(ref update) => { self.collect_event(&mut collected, event, mode.clients.enabled, || { - Object::for_update_client(update, src_chain).ok() + // Collect update client events only if the worker exists + if let Ok(object) = Object::for_update_client(update, src_chain) { + self.workers.contains(&object).then(|| object) + } else { + None + } }); } IbcEvent::OpenInitConnection(..) From cac31a6f975a8435388f91161a0b47de665954d2 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 20:44:59 +0530 Subject: [PATCH 09/36] Spawn workers based on config mode --- relayer/src/supervisor/spawn.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/relayer/src/supervisor/spawn.rs b/relayer/src/supervisor/spawn.rs index 1b0536819d..3f4c099d2e 100644 --- a/relayer/src/supervisor/spawn.rs +++ b/relayer/src/supervisor/spawn.rs @@ -393,11 +393,13 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { client: IdentifiedAnyClientState, connection: IdentifiedConnectionEnd, ) -> Result<(), Error> { - let handshake_enabled = self + let config_conn_enabled = self .config .read() .expect("poisoned lock") - .handshake_enabled(); + .mode + .connections + .enabled; let counterparty_chain = self .registry @@ -425,7 +427,7 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { ); } else if !conn_state_dst.is_open() && conn_state_dst.less_or_equal_progress(conn_state_src) - && handshake_enabled + && config_conn_enabled { // create worker for connection handshake that will advance the remote state let connection_object = Object::Connection(Connection { @@ -460,11 +462,13 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { connection: &IdentifiedConnectionEnd, channel: IdentifiedChannelEnd, ) -> Result<(), Error> { - let handshake_enabled = self + let config_chan_enabled = self .config .read() .expect("poisoned lock") - .handshake_enabled(); + .mode + .connections + .enabled; let counterparty_chain = self .registry @@ -548,7 +552,7 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { } } else if !chan_state_dst.is_open() && chan_state_dst.less_or_equal_progress(chan_state_src) - && handshake_enabled + && config_chan_enabled { // create worker for channel handshake that will advance the remote state let channel_object = Object::Channel(Channel { From 5cbe960b34d04583d83ec9d087047292b4c845da Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 21:00:42 +0530 Subject: [PATCH 10/36] Update E2E hermes config --- ci/simple_config.toml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ci/simple_config.toml b/ci/simple_config.toml index 5f82c5b6a3..a684267477 100644 --- a/ci/simple_config.toml +++ b/ci/simple_config.toml @@ -2,6 +2,26 @@ log_level = 'trace' filter = false +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true + [telemetry] enabled = false host = '127.0.0.1' From 7fbe9a858b05867db4195c0470d62631a072b9d7 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 21:13:19 +0530 Subject: [PATCH 11/36] Fix tests --- .../config/fixtures/relayer_conf_example.toml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/relayer/tests/config/fixtures/relayer_conf_example.toml b/relayer/tests/config/fixtures/relayer_conf_example.toml index b07289291e..13bcfc0162 100644 --- a/relayer/tests/config/fixtures/relayer_conf_example.toml +++ b/relayer/tests/config/fixtures/relayer_conf_example.toml @@ -1,6 +1,26 @@ [global] log_level = 'error' +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true + [[chains]] id = 'chain_A' rpc_addr = 'http://127.0.0.1:26657' From 86b5e5da3065b632dcfcad85cf0ec39c3e12cb74 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 5 Nov 2021 22:36:26 +0530 Subject: [PATCH 12/36] Fix E2E tests --- e2e/e2e/channel.py | 14 +++++++++----- e2e/e2e/connection.py | 11 +++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/e2e/e2e/channel.py b/e2e/e2e/channel.py index 960d20695b..9f5589fb6c 100644 --- a/e2e/e2e/channel.py +++ b/e2e/e2e/channel.py @@ -553,10 +553,14 @@ def verify_state(c: Config, ibc1: ChainId, ibc0: ChainId, ibc1_chan_id: ChannelId, port_id: PortId): - strategy = toml.load(c.config_file)['global']['strategy'] - # verify channel state on both chains, should be 'Open' for 'all' strategy, 'Init' otherwise - - if strategy == 'all': + mode = toml.load(c.config_file)['mode'] + clients_enabled = mode['clients']['enabled'] + conn_enabled = mode['connections']['enabled'] + chan_enabled = mode['channels']['enabled'] + packets_enabled = mode['packets']['enabled'] + + # verify connection state on both chains, should be 'Open' for 'all' strategy, 'Init' otherwise + if clients_enabled and conn_enabled and chan_enabled and packets_enabled: sleep(10.0) for i in range(20): sleep(2.0) @@ -569,7 +573,7 @@ def verify_state(c: Config, assert (ibc0_chan_end.state == 'Open'), (ibc0_chan_end, "state is not Open") assert (ibc1_chan_end.state == 'Open'), (ibc1_chan_end, "state is not Open") - elif strategy == 'packets': + else: sleep(5.0) ibc1_chan_end = query_channel_end(c, ibc1, port_id, ibc1_chan_id) assert (ibc1_chan_end.state == 'Init'), (ibc1_chan_end, "state is not Init") diff --git a/e2e/e2e/connection.py b/e2e/e2e/connection.py index 5ebb113824..34da6a5b59 100644 --- a/e2e/e2e/connection.py +++ b/e2e/e2e/connection.py @@ -263,11 +263,14 @@ def verify_state(c: Config, ibc1: ChainId, ibc0: ChainId, ibc1_conn_id: ConnectionId): - strategy = toml.load(c.config_file)['global']['strategy'] - l.debug(f'Using strategy: {strategy}') + mode = toml.load(c.config_file)['mode'] + clients_enabled = mode['clients']['enabled'] + conn_enabled = mode['connections']['enabled'] + chan_enabled = mode['channels']['enabled'] + packets_enabled = mode['packets']['enabled'] # verify connection state on both chains, should be 'Open' for 'all' strategy, 'Init' otherwise - if strategy == 'all': + if clients_enabled and conn_enabled and chan_enabled and packets_enabled: sleep(10.0) for i in range(20): sleep(5.0) @@ -280,7 +283,7 @@ def verify_state(c: Config, assert (ibc0_conn_end.state == 'Open'), (ibc0_conn_end, "state is not Open") assert (ibc1_conn_end.state == 'Open'), (ibc1_conn_end, "state is not Open") - elif strategy == 'packets': + else: sleep(5.0) ibc1_conn_end = query_connection_end(c, ibc1, ibc1_conn_id) assert (ibc1_conn_end.state == 'Init'), (ibc1_conn_end, "state is not Init") From 2f2e4966dcf71132692adc6a211d041e6e6d8486 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 13:46:48 +0530 Subject: [PATCH 13/36] Fix comments that referenced 'strategy' in py files --- e2e/e2e/channel.py | 2 +- e2e/e2e/connection.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/e2e/channel.py b/e2e/e2e/channel.py index 9f5589fb6c..90921aff7b 100644 --- a/e2e/e2e/channel.py +++ b/e2e/e2e/channel.py @@ -559,7 +559,7 @@ def verify_state(c: Config, chan_enabled = mode['channels']['enabled'] packets_enabled = mode['packets']['enabled'] - # verify connection state on both chains, should be 'Open' for 'all' strategy, 'Init' otherwise + # verify connection state on both chains, should be 'Open' or 'Init' depending on config 'mode' if clients_enabled and conn_enabled and chan_enabled and packets_enabled: sleep(10.0) for i in range(20): diff --git a/e2e/e2e/connection.py b/e2e/e2e/connection.py index 34da6a5b59..876f2917d3 100644 --- a/e2e/e2e/connection.py +++ b/e2e/e2e/connection.py @@ -269,7 +269,7 @@ def verify_state(c: Config, chan_enabled = mode['channels']['enabled'] packets_enabled = mode['packets']['enabled'] - # verify connection state on both chains, should be 'Open' for 'all' strategy, 'Init' otherwise + # verify connection state on both chains, should be 'Open' or 'Init' depending on config 'mode' if clients_enabled and conn_enabled and chan_enabled and packets_enabled: sleep(10.0) for i in range(20): From d3bb531231b5f56b5a021eb2b4a1248a5cf02ec1 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 13:52:44 +0530 Subject: [PATCH 14/36] Spawn workers conditionally depending on mode config --- relayer/src/supervisor/spawn.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/relayer/src/supervisor/spawn.rs b/relayer/src/supervisor/spawn.rs index 3f4c099d2e..04805d94f1 100644 --- a/relayer/src/supervisor/spawn.rs +++ b/relayer/src/supervisor/spawn.rs @@ -425,9 +425,9 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { connection.connection_id, chain.id() ); - } else if !conn_state_dst.is_open() + } else if config_conn_enabled + && !conn_state_dst.is_open() && conn_state_dst.less_or_equal_progress(conn_state_src) - && config_conn_enabled { // create worker for connection handshake that will advance the remote state let connection_object = Object::Connection(Connection { @@ -462,13 +462,8 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { connection: &IdentifiedConnectionEnd, channel: IdentifiedChannelEnd, ) -> Result<(), Error> { - let config_chan_enabled = self - .config - .read() - .expect("poisoned lock") - .mode - .connections - .enabled; + let config = self.config.read().expect("poisoned lock"); + let mode = &config.mode; let counterparty_chain = self .registry @@ -492,7 +487,8 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { chan_state_dst ); - if chan_state_src.is_open() + if mode.clients.enabled + && chan_state_src.is_open() && chan_state_dst.is_open() && self.relay_packets_on_channel(&chain, &channel) { @@ -532,7 +528,7 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { }; // If there are any outstanding packets or acks to send, spawn the worker - if has_packets() || has_acks() { + if mode.packets.enabled && has_packets() || has_acks() { // create the Packet object and spawn worker let path_object = Object::Packet(Packet { dst_chain_id: counterparty_chain.id(), @@ -550,9 +546,9 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { ) .then(|| debug!("spawned Packet worker: {}", path_object.short_name())); } - } else if !chan_state_dst.is_open() + } else if mode.channels.enabled + && !chan_state_dst.is_open() && chan_state_dst.less_or_equal_progress(chan_state_src) - && config_chan_enabled { // create worker for channel handshake that will advance the remote state let channel_object = Object::Channel(Channel { From 1b2463c3e15c4ca5cce9901c22a8c12af0c767c7 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 16:21:07 +0530 Subject: [PATCH 15/36] Remove references to strategy in guide and docs --- guide/src/config.md | 22 +++++++++++++++++-- guide/src/help.md | 1 - .../relay-paths/multiple-paths.md | 21 +++++++++++++++++- relayer-cli/tests/fixtures/two_chains.toml | 21 +++++++++++++++++- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/guide/src/config.md b/guide/src/config.md index 75fdc6e8f5..d6c649efb1 100644 --- a/guide/src/config.md +++ b/guide/src/config.md @@ -49,10 +49,28 @@ Here is a full example of a configuration file with two chains configured: ```toml [global] -strategy = 'all' log_level = 'info' filter = false -clear_packets_interval = 100 + +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true [rest] enabled = true diff --git a/guide/src/help.md b/guide/src/help.md index 93f2426c58..7147969495 100644 --- a/guide/src/help.md +++ b/guide/src/help.md @@ -89,7 +89,6 @@ Relevant snippet: ```toml [global] -strategy = 'packets' log_level = 'error' ``` diff --git a/guide/src/tutorials/local-chains/relay-paths/multiple-paths.md b/guide/src/tutorials/local-chains/relay-paths/multiple-paths.md index 0c0839e1c2..1810d8fdfd 100644 --- a/guide/src/tutorials/local-chains/relay-paths/multiple-paths.md +++ b/guide/src/tutorials/local-chains/relay-paths/multiple-paths.md @@ -8,8 +8,27 @@ Follow the steps below to connect three chains together and relay packets betwee ```toml [global] - strategy = 'packets' log_level = 'info' + + [mode] + + [mode.clients] + enabled = true + refresh = true + misbehaviour = true + + [mode.connections] + enabled = false + + [mode.channels] + enabled = false + + [mode.packets] + enabled = true + clear_interval = 100 + clear_on_start = true + filter = false + tx_confirmation = true [[chains]] id = 'ibc-0' diff --git a/relayer-cli/tests/fixtures/two_chains.toml b/relayer-cli/tests/fixtures/two_chains.toml index 2c04d671c8..92a34cf422 100644 --- a/relayer-cli/tests/fixtures/two_chains.toml +++ b/relayer-cli/tests/fixtures/two_chains.toml @@ -1,7 +1,26 @@ [global] -strategy = 'naive' log_level = 'error' # valid options: 'error', 'warn', 'info', 'debug', 'trace' +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true + [[chains]] id = 'ibc-0' rpc_addr = 'http://127.0.0.1:26657' From ce1bee52757061682bede3a0387e08b63150bf3c Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 16:26:08 +0530 Subject: [PATCH 16/36] Remove more refs to strategy --- guide/src/commands/relaying/handshakes.md | 25 ++++++++++++++++++----- guide/src/commands/relaying/packets.md | 19 +++++++++++++++-- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/guide/src/commands/relaying/handshakes.md b/guide/src/commands/relaying/handshakes.md index e99f3bf0bc..a504cffafd 100644 --- a/guide/src/commands/relaying/handshakes.md +++ b/guide/src/commands/relaying/handshakes.md @@ -5,11 +5,26 @@ for connections and channels. ## The `start` Command -To relay packets and handshake messages use `all` as strategy in the `global` section of the configuration file: +To relay packets and handshake messages configure the `mode` section of the configuration file like so: ```toml [global] -strategy = 'all' log_level = 'info' + +[mode] + +[mode.clients] +enabled = true +# ... + +[mode.connections] +enabled = true + +[mode.channels] +enabled = true + +[mode.packets] +enabled = true +# ... ``` Then start hermes using the start command: @@ -28,15 +43,15 @@ the configured chains. Assuming the events are coming from a `source` chain, the relayer determines the `destination` chain and builds the handshake messages based on these events. These are then sent to the `destination` chain. -In addition to the events described in [Packet Relaying](packets.md#packet-relaying), in the `all` strategy mode the following IBC events are handled: +In addition to the events described in [Packet Relaying](packets.md#packet-relaying), the following IBC events may be handled: -- Channels: +- Channels (if `mode.channels.enabled=true`): - `chan_open_init`: the relayer builds a `MsgChannelOpenTry` message - `chan_open_try`: the relayer builds a `MsgChannelOpenAck` message - `chan_open_ack`: the relayer builds a `MsgChannelOpenConfirm` message - `chan_open_confirm`: no message is sent out, channel opening is finished -- Connections: +- Connections (if `mode.connections.enabled=true`): - `conn_open_init`: the relayer builds a `MsgConnOpenTry` message - `conn_open_try`: the relayer builds a `MsgConnOpenAck` message - `conn_open_ack`: the relayer builds a `MsgConnOpenConfirm` message diff --git a/guide/src/commands/relaying/packets.md b/guide/src/commands/relaying/packets.md index 8ee0a6925c..88644dcafe 100644 --- a/guide/src/commands/relaying/packets.md +++ b/guide/src/commands/relaying/packets.md @@ -9,11 +9,26 @@ This section describes the configuration and commands that can be used to start ## The `start` Command -To relay packets only use `packets` as strategy in the `global` section of the configuration file: +To relay packets only configure the `mode` section of the configuration file like so: ```toml [global] -strategy = 'packets' log_level = 'info' + +[mode] + +[mode.clients] +enabled = true +# ... + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +# ... ``` Then start hermes using the start command: From 47a3c1f85cd6aee3fb07ac5bd0e2ba5b94ec2208 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 16:45:59 +0530 Subject: [PATCH 17/36] Add .changelog entry --- .changelog/unreleased/features/ibc-relayer/1518-config-modes.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/ibc-relayer/1518-config-modes.md diff --git a/.changelog/unreleased/features/ibc-relayer/1518-config-modes.md b/.changelog/unreleased/features/ibc-relayer/1518-config-modes.md new file mode 100644 index 0000000000..c42e7de9b0 --- /dev/null +++ b/.changelog/unreleased/features/ibc-relayer/1518-config-modes.md @@ -0,0 +1,2 @@ +- Allow for more granular control of relaying modes. The `mode` configuration section replaces the `strategy` option. + ([#1518](https://github.com/informalsystems/ibc-rs/issues/1518)) From 28cae96fb73e9671216a5c03887b558c8417f713 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 19:50:57 +0530 Subject: [PATCH 18/36] Remove global config options - filter & tx_confirmations --- ci/simple_config.toml | 1 - config.toml | 48 ++++++++++++++++++++++++++++----- guide/src/config.md | 1 - relayer/src/config.rs | 30 ++++++++++----------- relayer/src/supervisor.rs | 14 ++++++++-- relayer/src/supervisor/spawn.rs | 7 ++++- relayer/src/worker.rs | 2 +- 7 files changed, 75 insertions(+), 28 deletions(-) diff --git a/ci/simple_config.toml b/ci/simple_config.toml index a684267477..9e23366d5e 100644 --- a/ci/simple_config.toml +++ b/ci/simple_config.toml @@ -1,6 +1,5 @@ [global] log_level = 'trace' -filter = false [mode] diff --git a/config.toml b/config.toml index 82fba076d6..22c686c05d 100644 --- a/config.toml +++ b/config.toml @@ -1,7 +1,45 @@ # The global section has parameters that apply globally to the relayer operation. [global] -# Enable or disable the filtering mechanism. Default: 'false' +# Specify the verbosity for the relayer logging output. Default: 'info' +# Valid options are 'error', 'warn', 'info', 'debug', 'trace'. +log_level = 'info' + + +# Specify the mode to be used by the relayer. [Required] +[mode] + +# Specify the client mode. +[mode.clients] +# Whether or not to enable the client workers. [Required] +enabled = true +# Whether or not to enable periodic refresh of clients. [Default: false] +refresh = true +# Whether or not to enable misbehaviour detection for clients. [Default: false] +misbehaviour = true + +# Specify the connections mode. +[mode.connections] +# Whether or not to enable the connection workers. [Required] +enabled = false + +# Specify the channels mode. +[mode.channels] +# Whether or not to enable the channel workers. [Required] +enabled = false + +# Specify the packets mode. +[mode.packets] +# Whether or not to enable the packet workers. [Required] +enabled = true +# Parametrize the periodic packet clearing feature. +# Interval (in number of blocks) at which pending packets +# should be eagerly cleared. A value of '0' will disable +# periodic packet clearing. [Default: 100] +clear_interval = 100 +# Whether or not to clear packets on start. [Default: false] +clear_on_start = true +# Enable or disable the filtering mechanism. # Valid options are 'true', 'false'. # Currently Hermes supports two filters: # 1. Packet filtering on a per-chain basis; see the chain-specific @@ -10,18 +48,14 @@ # is parametrized with (numerator = 1, denominator = 3), so that clients with # thresholds different than this will be ignored. # If set to 'true', both of the above filters will be enabled. +# [Default: false] filter = false - -# Specify the verbosity for the relayer logging output. Default: 'info' -# Valid options are 'error', 'warn', 'info', 'debug', 'trace'. -log_level = 'info' - # Toggle the transaction confirmation mechanism. # The tx confirmation mechanism periodically queries the `/tx_search` RPC # endpoint to check that previously-submitted transactions # (to any chain in this config file) have delivered successfully. # Experimental feature. Affects telemetry if set to false. -# Default: true. +# [Default: true] tx_confirmation = true diff --git a/guide/src/config.md b/guide/src/config.md index d6c649efb1..7cb72fdc7d 100644 --- a/guide/src/config.md +++ b/guide/src/config.md @@ -50,7 +50,6 @@ Here is a full example of a configuration file with two chains configured: ```toml [global] log_level = 'info' -filter = false [mode] diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 4e8197d1cb..0bd2ddb0fc 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -146,7 +146,7 @@ impl Config { port_id: &PortId, channel_id: &ChannelId, ) -> bool { - if !self.global.filter { + if !self.mode.packets.filter { return true; } @@ -156,11 +156,6 @@ impl Config { } } - pub fn handshake_enabled(&self) -> bool { - // FIXME(hu55a1n1) - unimplemented!() - } - pub fn chains_map(&self) -> HashMap<&ChainId, &ChainConfig> { self.chains.iter().map(|c| (&c.id, c)).collect() } @@ -193,19 +188,30 @@ pub struct Channels { pub enabled: bool, } -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct Packets { pub enabled: bool, #[serde(default = "default::clear_packets_interval")] pub clear_interval: u64, #[serde(default)] pub clear_on_start: bool, - #[serde(default)] + #[serde(default = "default::filter")] pub filter: bool, - #[serde(default)] + #[serde(default = "default::tx_confirmation")] pub tx_confirmation: bool, } +impl Default for Packets { + fn default() -> Self { + Self { + clear_interval: default::clear_packets_interval(), + filter: default::filter(), + tx_confirmation: default::tx_confirmation(), + ..Default::default() + } + } +} + /// Log levels are wrappers over [`tracing_core::Level`]. /// /// [`tracing_core::Level`]: https://docs.rs/tracing-core/0.1.17/tracing_core/struct.Level.html @@ -241,18 +247,12 @@ impl fmt::Display for LogLevel { #[serde(default, deny_unknown_fields)] pub struct GlobalConfig { pub log_level: LogLevel, - #[serde(default = "default::filter")] - pub filter: bool, - #[serde(default = "default::tx_confirmation")] - pub tx_confirmation: bool, } impl Default for GlobalConfig { fn default() -> Self { Self { log_level: LogLevel::default(), - filter: default::filter(), - tx_confirmation: default::tx_confirmation(), } } } diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 1776e6c710..cf5a5679e8 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -93,14 +93,24 @@ impl Supervisor { /// Returns `false` otherwise. fn client_filter_enabled(&self) -> bool { // Currently just a wrapper over the global filter. - self.config.read().expect("poisoned lock").global.filter + self.config + .read() + .expect("poisoned lock") + .mode + .packets + .filter } /// Returns `true` if the relayer should filter based on /// channel identifiers. /// Returns `false` otherwise. fn channel_filter_enabled(&self) -> bool { - self.config.read().expect("poisoned lock").global.filter + self.config + .read() + .expect("poisoned lock") + .mode + .packets + .filter } fn relay_packets_on_channel( diff --git a/relayer/src/supervisor/spawn.rs b/relayer/src/supervisor/spawn.rs index 04805d94f1..5ba05a3bb5 100644 --- a/relayer/src/supervisor/spawn.rs +++ b/relayer/src/supervisor/spawn.rs @@ -66,7 +66,12 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { fn client_filter_enabled(&self) -> bool { // Currently just a wrapper over the global filter. - self.config.read().expect("poisoned lock").global.filter + self.config + .read() + .expect("poisoned lock") + .mode + .packets + .filter } pub fn spawn_workers(&mut self) { diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index 74787829ff..a8553fe196 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -116,7 +116,7 @@ impl Worker Date: Mon, 8 Nov 2021 21:21:14 +0530 Subject: [PATCH 19/36] Fix Packet::default() recursion bug --- relayer/src/config.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 0bd2ddb0fc..0b0d882d99 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -115,6 +115,7 @@ pub mod default { pub struct Config { #[serde(default)] pub global: GlobalConfig, + #[serde(default)] pub mode: ModeConfig, #[serde(default)] pub rest: RestConfig, @@ -204,10 +205,12 @@ pub struct Packets { impl Default for Packets { fn default() -> Self { Self { + enabled: false, clear_interval: default::clear_packets_interval(), + clear_on_start: false, filter: default::filter(), tx_confirmation: default::tx_confirmation(), - ..Default::default() + } } } From 3b6129477376917fd40f5a3ea761fff51df15696 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 8 Nov 2021 21:29:31 +0530 Subject: [PATCH 20/36] cargo fmt --- relayer/src/config.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 0b0d882d99..cc313f74c1 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -210,7 +210,6 @@ impl Default for Packets { clear_on_start: false, filter: default::filter(), tx_confirmation: default::tx_confirmation(), - } } } From a12be44bd4852b74c0d5fb936d6ff9021e96f96a Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Tue, 9 Nov 2021 18:06:46 +0530 Subject: [PATCH 21/36] Address feedback --- relayer/src/worker.rs | 17 ++--------------- relayer/src/worker/client.rs | 15 +++++++-------- relayer/src/worker/packet.rs | 32 +++++++++++++------------------- 3 files changed, 22 insertions(+), 42 deletions(-) diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index a8553fe196..dd2949fcf3 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -93,13 +93,7 @@ impl Worker Self::Client( id, - ClientWorker::new( - client.clone(), - chains, - cmd_rx, - config.mode.clients.misbehaviour, - config.mode.clients.refresh, - ), + ClientWorker::new(client.clone(), chains, cmd_rx, config.mode.clients.clone()), ), Object::Connection(connection) => Self::Connection( id, @@ -110,14 +104,7 @@ impl Worker Self::Packet( id, - PacketWorker::new( - path.clone(), - chains, - cmd_rx, - config.mode.packets.clear_interval, - config.mode.packets.clear_on_start, - config.mode.packets.tx_confirmation, - ), + PacketWorker::new(path.clone(), chains, cmd_rx, config.mode.packets.clone()), ), }; diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index 0ce6c8c0da..f65f2c19dd 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -8,6 +8,7 @@ use ibc::{core::ics02_client::events::UpdateClient, events::IbcEvent}; use crate::{ chain::handle::{ChainHandle, ChainHandlePair}, + config::Clients as ClientsConfig, foreign_client::{ForeignClient, ForeignClientErrorDetail, MisbehaviourResults}, object::Client, telemetry, @@ -20,8 +21,7 @@ pub struct ClientWorker { client: Client, chains: ChainHandlePair, cmd_rx: Receiver, - misbehaviour: bool, - refresh: bool, + clients_cfg: ClientsConfig, } impl ClientWorker { @@ -29,15 +29,13 @@ impl ClientWorker { client: Client, chains: ChainHandlePair, cmd_rx: Receiver, - misbehaviour: bool, - refresh: bool, + clients_cfg: ClientsConfig, ) -> Self { Self { client, chains, cmd_rx, - misbehaviour, - refresh, + clients_cfg, } } @@ -55,7 +53,8 @@ impl ClientWorker { ); // initial check for evidence of misbehaviour for all updates - let skip_misbehaviour = !self.misbehaviour || self.detect_misbehaviour(&client, None); + let skip_misbehaviour = + !self.clients_cfg.misbehaviour || self.detect_misbehaviour(&client, None); // remember the time of the last refresh so we backoff let mut last_refresh = Instant::now() - Duration::from_secs(61); @@ -66,7 +65,7 @@ impl ClientWorker { // Clients typically need refresh every 2/3 of their // trusting period (which can e.g., two weeks). // Backoff refresh checking to attempt it every minute. - if self.refresh && last_refresh.elapsed() > Duration::from_secs(60) { + if self.clients_cfg.refresh && last_refresh.elapsed() > Duration::from_secs(60) { // Run client refresh, exit only if expired or frozen match client.refresh() { Ok(Some(_)) => { diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index 758b91b702..a0d812e59a 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -5,6 +5,7 @@ use tracing::{error, info, trace, warn}; use crate::{ chain::handle::{ChainHandle, ChainHandlePair}, + config::Packets as PacketsConfig, link::{Link, LinkParameters, RelaySummary}, object::Packet, telemetry, @@ -25,9 +26,7 @@ pub struct PacketWorker { path: Packet, chains: ChainHandlePair, cmd_rx: Receiver, - clear_packets_interval: u64, - clear_packets_on_start: Option, - with_tx_confirmation: bool, + packets_cfg: PacketsConfig, } impl PacketWorker { @@ -35,18 +34,13 @@ impl PacketWorker { path: Packet, chains: ChainHandlePair, cmd_rx: Receiver, - clear_packets_interval: u64, - clear_packets_on_start: bool, - with_tx_confirmation: bool, + packets_cfg: PacketsConfig, ) -> Self { - let clear_packets_on_start = Some(clear_packets_on_start); Self { path, chains, cmd_rx, - clear_packets_interval, - clear_packets_on_start, - with_tx_confirmation, + packets_cfg, } } @@ -59,7 +53,7 @@ impl PacketWorker { src_port_id: self.path.src_port_id.clone(), src_channel_id: self.path.src_channel_id.clone(), }, - self.with_tx_confirmation, + self.packets_cfg.tx_confirmation, ) .map_err(RunError::link)?; @@ -129,18 +123,18 @@ impl PacketWorker { height, new_block: _, } => { - let (first_run, clear_on_start) = { - match self.clear_packets_on_start.take() { - None => (false, false), - Some(clear_on_start) => (true, clear_on_start), - } + let clear_on_start = if self.packets_cfg.clear_on_start { + self.packets_cfg.clear_on_start = false; + true + } else { + false }; // Schedule the clearing of pending packets. This may happen once at start, // and may be _forced_ at predefined block intervals. - let force_packet_clearing = (first_run && clear_on_start) - || (self.clear_packets_interval != 0 - && height.revision_height % self.clear_packets_interval == 0); + let force_packet_clearing = clear_on_start + || (self.packets_cfg.clear_interval != 0 + && height.revision_height % self.packets_cfg.clear_interval == 0); link.a_to_b .schedule_packet_clearing(Some(height), force_packet_clearing) From c51992c791bdb26950d32d60b6467ddf2c728c5d Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 00:28:09 +0530 Subject: [PATCH 22/36] Add serde(deny_unknown_fields) for mode config --- relayer/src/config.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index cc313f74c1..c5dea6d047 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -163,6 +163,7 @@ impl Config { } #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ModeConfig { pub clients: Clients, pub connections: Connections, @@ -171,6 +172,7 @@ pub struct ModeConfig { } #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct Clients { pub enabled: bool, #[serde(default)] @@ -180,16 +182,19 @@ pub struct Clients { } #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct Connections { pub enabled: bool, } #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct Channels { pub enabled: bool, } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct Packets { pub enabled: bool, #[serde(default = "default::clear_packets_interval")] From a743ea0850015de729bb1ee5df31c12b5f24b1d4 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 00:46:40 +0530 Subject: [PATCH 23/36] Add check for valid mode config --- relayer-cli/src/config.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/relayer-cli/src/config.rs b/relayer-cli/src/config.rs index 40481ca3bc..e84cabc239 100644 --- a/relayer-cli/src/config.rs +++ b/relayer-cli/src/config.rs @@ -9,7 +9,7 @@ use std::path::PathBuf; use flex_error::{define_error, TraceError}; use ibc::core::ics24_host::identifier::ChainId; -use ibc_relayer::config::Config; +use ibc_relayer::config::{Config, ModeConfig}; use tendermint_light_client::types::TrustThreshold; use tracing_subscriber::filter::ParseError; @@ -36,6 +36,13 @@ define_error! { e.log_level) }, + InvalidMode + { reason: String, } + |e| { + format!("config file specifies invalid mode config, caused by: {0}", + e.reason) + }, + DuplicateChains { chain_id: ChainId } |e| { @@ -68,6 +75,19 @@ pub fn validate_config(config: &Config) -> Result<(), Error> { validate_trust_threshold(&c.id, c.trust_threshold)?; } + // Check for invalid mode config + validate_mode(&config.mode)?; + + Ok(()) +} + +fn validate_mode(mode: &ModeConfig) -> Result<(), Error> { + if mode.clients.enabled && !mode.clients.refresh && !mode.clients.misbehaviour { + return Err(Error::invalid_mode( + "either `refresh` or `misbehaviour` must be set to true if `clients.enabled` is set to true".to_string(), + )); + } + Ok(()) } From bfc839ced4ffd338e6166e9264799bac9406edb8 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 01:00:17 +0530 Subject: [PATCH 24/36] Update ADRs that referenced strategy config --- docs/architecture/adr-002-ibc-relayer.md | 28 +++++++++++++------ .../adr-006-hermes-v0.2-usecases.md | 1 - 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/architecture/adr-002-ibc-relayer.md b/docs/architecture/adr-002-ibc-relayer.md index e2aa1cec98..cb80078628 100644 --- a/docs/architecture/adr-002-ibc-relayer.md +++ b/docs/architecture/adr-002-ibc-relayer.md @@ -117,9 +117,28 @@ Below is an example of a configuration file. ```toml [global] -strategy = "packets" log_level = "error" +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true + [[chains]] id = "chain_A" rpc_addr = "http://localhost:26657" @@ -172,14 +191,7 @@ pub struct Config { pub connections: Option>, } -pub enum Strategy { - Packets, - HandshakeAndPackets, -} - pub struct GlobalConfig { - pub strategy: Strategy, - /// All valid log levels, as defined in tracing: /// https://docs.rs/tracing-core/0.1.17/tracing_core/struct.Level.html pub log_level: String, diff --git a/docs/architecture/adr-006-hermes-v0.2-usecases.md b/docs/architecture/adr-006-hermes-v0.2-usecases.md index 7a9b361f65..bbbabc5596 100644 --- a/docs/architecture/adr-006-hermes-v0.2-usecases.md +++ b/docs/architecture/adr-006-hermes-v0.2-usecases.md @@ -252,7 +252,6 @@ of the config file will look as follows: ```toml [global] -strategy = 'packets' log_level = 'error' log_json = 'false' ``` From a3137e2b53c6ecb91af7a34dbb0baa43035548a2 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 01:02:23 +0530 Subject: [PATCH 25/36] Modify gm scripts for config changes --- scripts/gm/bin/lib-gm | 24 ++++++++++++++++++++---- scripts/gm/gm.toml | 3 --- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/gm/bin/lib-gm b/scripts/gm/bin/lib-gm index a3278b978f..f0543f0e47 100644 --- a/scripts/gm/bin/lib-gm +++ b/scripts/gm/bin/lib-gm @@ -46,7 +46,6 @@ set_config_defaults() { GLOBAL_HDPATH="" GLOBAL_HERMES_BINARY="$(which hermes || echo "./hermes")" GLOBAL_HERMES_CONFIG="${HOME}/.hermes/config.toml" - GLOBAL_HERMES_STRATEGY="packets" GLOBAL_HERMES_LOG_LEVEL="info" GLOBAL_HERMES_TELEMETRY_ENABLED="true" GLOBAL_HERMES_TELEMETRY_HOST="127.0.0.1" @@ -83,8 +82,6 @@ parse_config_file() { # shellcheck disable=SC2155 export GLOBAL_HERMES_CONFIG="$(eval echo "$(stoml -sq "$CONFIG_FILE" global.hermes.config || echo "$GLOBAL_HERMES_CONFIG")")" # shellcheck disable=SC2155 - export GLOBAL_HERMES_STRATEGY="$(stoml -sq "$CONFIG_FILE" global.hermes.strategy || echo "$GLOBAL_HERMES_STRATEGY")" - # shellcheck disable=SC2155 export GLOBAL_HERMES_LOG_LEVEL="$(stoml -sq "$CONFIG_FILE" global.hermes.log_level || echo "$GLOBAL_HERMES_LOG_LEVEL")" # shellcheck disable=SC2155 export GLOBAL_HERMES_TELEMETRY_ENABLED="$(stoml -sq "$CONFIG_FILE" global.hermes.telemetry_enabled || echo "$GLOBAL_HERMES_TELEMETRY_ENABLED")" @@ -830,9 +827,28 @@ hermes_config() { fi cat < "$GLOBAL_HERMES_CONFIG" [global] -strategy = '${GLOBAL_HERMES_STRATEGY}' log_level = '${GLOBAL_HERMES_LOG_LEVEL}' +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +filter = false +tx_confirmation = true + [telemetry] enabled = ${GLOBAL_HERMES_TELEMETRY_ENABLED} host = '${GLOBAL_HERMES_TELEMETRY_HOST}' diff --git a/scripts/gm/gm.toml b/scripts/gm/gm.toml index 56d4361eec..0d7c393008 100644 --- a/scripts/gm/gm.toml +++ b/scripts/gm/gm.toml @@ -59,9 +59,6 @@ binary="./hermes" # Hermes configuration file path. config="$HOME/.hermes/config.toml" -# Hermes configuration strategy paremeter. -strategy="packets" - # Hermes configuration log_level parameter. log_level="info" From 891251130a421c9146182f30fb399cee5367a53e Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 16:25:17 +0530 Subject: [PATCH 26/36] Trck packet worker's first run using a boolean --- relayer/src/worker/packet.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index a0d812e59a..7075a822f5 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -27,6 +27,7 @@ pub struct PacketWorker { chains: ChainHandlePair, cmd_rx: Receiver, packets_cfg: PacketsConfig, + first_run: bool, } impl PacketWorker { @@ -41,6 +42,16 @@ impl PacketWorker { chains, cmd_rx, packets_cfg, + first_run: true, + } + } + + fn clear_packets_on_start(&mut self) -> bool { + if self.first_run { + self.first_run = false; + self.packets_cfg.clear_on_start + } else { + false } } @@ -123,16 +134,9 @@ impl PacketWorker { height, new_block: _, } => { - let clear_on_start = if self.packets_cfg.clear_on_start { - self.packets_cfg.clear_on_start = false; - true - } else { - false - }; - // Schedule the clearing of pending packets. This may happen once at start, // and may be _forced_ at predefined block intervals. - let force_packet_clearing = clear_on_start + let force_packet_clearing = self.clear_packets_on_start() || (self.packets_cfg.clear_interval != 0 && height.revision_height % self.packets_cfg.clear_interval == 0); From f87720d13df3ba472edc06ecfe4382b29304c2e8 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 18:44:22 +0530 Subject: [PATCH 27/36] Enable connections and channels mode for gm default config --- scripts/gm/bin/lib-gm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/gm/bin/lib-gm b/scripts/gm/bin/lib-gm index f0543f0e47..fd1e21e7af 100644 --- a/scripts/gm/bin/lib-gm +++ b/scripts/gm/bin/lib-gm @@ -837,10 +837,10 @@ refresh = true misbehaviour = true [mode.connections] -enabled = false +enabled = true [mode.channels] -enabled = false +enabled = true [mode.packets] enabled = true From 4dc73ca54b04f1cba2dbdc4f47dda3bc95d94cba Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 19:38:29 +0530 Subject: [PATCH 28/36] Apply suggestion Co-authored-by: Romain Ruetschi --- relayer/src/worker/packet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index 7075a822f5..2ee80ef8a1 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -46,7 +46,7 @@ impl PacketWorker { } } - fn clear_packets_on_start(&mut self) -> bool { + fn clear_packets(&mut self) -> bool { if self.first_run { self.first_run = false; self.packets_cfg.clear_on_start From df9cc0f45a27e1df8ace5c202d506a64a385b25a Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 10 Nov 2021 19:43:06 +0530 Subject: [PATCH 29/36] Fix build --- relayer/src/worker/packet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index 2ee80ef8a1..d5502f2813 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -136,7 +136,7 @@ impl PacketWorker { } => { // Schedule the clearing of pending packets. This may happen once at start, // and may be _forced_ at predefined block intervals. - let force_packet_clearing = self.clear_packets_on_start() + let force_packet_clearing = self.clear_packets() || (self.packets_cfg.clear_interval != 0 && height.revision_height % self.packets_cfg.clear_interval == 0); From d09f6a5d77fe5dfe8eb8a45f068ad3dd9e37afc4 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 11 Nov 2021 10:03:13 +0100 Subject: [PATCH 30/36] Add comment for `Supervisor::collect_events` --- relayer/src/supervisor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index cf5a5679e8..2efe639ed3 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -181,6 +181,8 @@ impl Supervisor { } } + /// If `enabled`, build an `Object` using the provided `object_ctor` + /// and add the given `event` to the `collected` events for this `object`. fn collect_event( &self, collected: &mut CollectedEvents, From 1b294a177b9e9f4f38ffc310e3a8402550d210ea Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 11 Nov 2021 10:16:38 +0100 Subject: [PATCH 31/36] Avoid holding the lock on the config for too long in `Supervisor::collect_events` --- relayer/src/config.rs | 10 +++++----- relayer/src/supervisor.rs | 3 +-- relayer/src/worker.rs | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index c5dea6d047..72f0ef7463 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -162,7 +162,7 @@ impl Config { } } -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct ModeConfig { pub clients: Clients, @@ -171,7 +171,7 @@ pub struct ModeConfig { pub packets: Packets, } -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Clients { pub enabled: bool, @@ -181,19 +181,19 @@ pub struct Clients { pub misbehaviour: bool, } -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Connections { pub enabled: bool, } -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Channels { pub enabled: bool, } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Packets { pub enabled: bool, diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 2efe639ed3..8e4db3ed41 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -212,8 +212,7 @@ impl Supervisor { ) -> CollectedEvents { let mut collected = CollectedEvents::new(batch.height, batch.chain_id.clone()); - let config = self.config.read().expect("poisoned lock"); - let mode = &config.mode; + let mode = self.config.read().expect("poisoned lock").mode; for event in &batch.events { match event { diff --git a/relayer/src/worker.rs b/relayer/src/worker.rs index dd2949fcf3..fd3f80980c 100644 --- a/relayer/src/worker.rs +++ b/relayer/src/worker.rs @@ -93,7 +93,7 @@ impl Worker Self::Client( id, - ClientWorker::new(client.clone(), chains, cmd_rx, config.mode.clients.clone()), + ClientWorker::new(client.clone(), chains, cmd_rx, config.mode.clients), ), Object::Connection(connection) => Self::Connection( id, @@ -104,7 +104,7 @@ impl Worker Self::Packet( id, - PacketWorker::new(path.clone(), chains, cmd_rx, config.mode.packets.clone()), + PacketWorker::new(path.clone(), chains, cmd_rx, config.mode.packets), ), }; From 81f96f9104d9f7c9dc04b1f655a4bf732b13944a Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 11 Nov 2021 10:42:28 +0100 Subject: [PATCH 32/36] Small refactor in the packet worker --- relayer/src/worker/packet.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/relayer/src/worker/packet.rs b/relayer/src/worker/packet.rs index d5502f2813..1215d54c4f 100644 --- a/relayer/src/worker/packet.rs +++ b/relayer/src/worker/packet.rs @@ -1,6 +1,7 @@ use core::time::Duration; use crossbeam_channel::Receiver; +use ibc::Height; use tracing::{error, info, trace, warn}; use crate::{ @@ -46,12 +47,17 @@ impl PacketWorker { } } - fn clear_packets(&mut self) -> bool { + /// Whether or not to clear pending packets at this `step` for the given height. + /// Packets are cleared on the first iteration if `clear_on_start` is true. + /// Subsequently, packets are cleared only if `clear_interval` is not `0` and + /// if we have reached the interval. + fn clear_packets(&mut self, height: Height) -> bool { if self.first_run { self.first_run = false; self.packets_cfg.clear_on_start } else { - false + self.packets_cfg.clear_interval != 0 + && height.revision_height % self.packets_cfg.clear_interval == 0 } } @@ -136,12 +142,8 @@ impl PacketWorker { } => { // Schedule the clearing of pending packets. This may happen once at start, // and may be _forced_ at predefined block intervals. - let force_packet_clearing = self.clear_packets() - || (self.packets_cfg.clear_interval != 0 - && height.revision_height % self.packets_cfg.clear_interval == 0); - link.a_to_b - .schedule_packet_clearing(Some(height), force_packet_clearing) + .schedule_packet_clearing(Some(height), self.clear_packets(height)) } WorkerCmd::ClearPendingPackets => link.a_to_b.schedule_packet_clearing(None, true), From 2e3bca3cc9617f102cfff02a56b1aa4bc0264f19 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 11 Nov 2021 11:25:28 +0100 Subject: [PATCH 33/36] Emit a warning if all operations modes are disabled --- config.toml | 4 +- relayer-cli/src/application.rs | 15 ++++++-- relayer-cli/src/commands/config/validate.rs | 6 ++- relayer-cli/src/config.rs | 42 ++++++++++++++------- relayer/src/config.rs | 9 +++++ 5 files changed, 56 insertions(+), 20 deletions(-) diff --git a/config.toml b/config.toml index 22c686c05d..a2502e438a 100644 --- a/config.toml +++ b/config.toml @@ -12,7 +12,7 @@ log_level = 'info' # Specify the client mode. [mode.clients] # Whether or not to enable the client workers. [Required] -enabled = true +enabled = false # Whether or not to enable periodic refresh of clients. [Default: false] refresh = true # Whether or not to enable misbehaviour detection for clients. [Default: false] @@ -31,7 +31,7 @@ enabled = false # Specify the packets mode. [mode.packets] # Whether or not to enable the packet workers. [Required] -enabled = true +enabled = false # Parametrize the periodic packet clearing feature. # Interval (in number of blocks) at which pending packets # should be eagerly cleared. A value of '0' will disable diff --git a/relayer-cli/src/application.rs b/relayer-cli/src/application.rs index 96b5121a7a..2548e6c66f 100644 --- a/relayer-cli/src/application.rs +++ b/relayer-cli/src/application.rs @@ -123,12 +123,21 @@ impl Application for CliApp { /// time in app lifecycle when configuration would be loaded if /// possible. fn after_config(&mut self, config: Self::Cfg) -> Result<(), FrameworkError> { + use crate::config::Diagnostic; + // Configure components self.state.components.after_config(&config)?; - validate_config(&config).map_err(|validation_err| { - FrameworkErrorKind::ConfigError.context(format!("{}", validation_err)) - })?; + if let Err(diagnostic) = validate_config(&config) { + match diagnostic { + Diagnostic::Warning(e) => { + tracing::warn!("relayer may be misconfigured: {}", e.detail()); + } + Diagnostic::Error(e) => { + return Err(FrameworkErrorKind::ConfigError.context(e).into()); + } + } + }; self.config = Some(config); diff --git a/relayer-cli/src/commands/config/validate.rs b/relayer-cli/src/commands/config/validate.rs index 79c2fd1939..9cf61b9236 100644 --- a/relayer-cli/src/commands/config/validate.rs +++ b/relayer-cli/src/commands/config/validate.rs @@ -13,9 +13,11 @@ impl Runnable for ValidateCmd { let config = app_config(); trace!("loaded configuration: {:#?}", *config); + // No need to output the underlying error, this is done already when the application boots. + // See `application::CliApp::after_config`. match config::validate_config(&config) { - Ok(_) => Output::success("validation passed successfully").exit(), - Err(e) => Output::error(format!("{}", e)).exit(), + Ok(_) => Output::success("configuration is valid").exit(), + Err(_) => Output::error("configuration is invalid").exit(), } } } diff --git a/relayer-cli/src/config.rs b/relayer-cli/src/config.rs index e84cabc239..ff48da2c8e 100644 --- a/relayer-cli/src/config.rs +++ b/relayer-cli/src/config.rs @@ -63,13 +63,20 @@ define_error! { } } +#[derive(Clone, Debug)] +pub enum Diagnostic { + Warning(E), + Error(E), +} + /// Method for syntactic validation of the input configuration file. -pub fn validate_config(config: &Config) -> Result<(), Error> { +pub fn validate_config(config: &Config) -> Result<(), Diagnostic> { // Check for duplicate chain configuration and invalid trust thresholds let mut unique_chain_ids = BTreeSet::new(); for c in config.chains.iter() { - if !unique_chain_ids.insert(c.id.clone()) { - return Err(Error::duplicate_chains(c.id.clone())); + let already_present = !unique_chain_ids.insert(c.id.clone()); + if already_present { + return Err(Diagnostic::Error(Error::duplicate_chains(c.id.clone()))); } validate_trust_threshold(&c.id, c.trust_threshold)?; @@ -81,11 +88,17 @@ pub fn validate_config(config: &Config) -> Result<(), Error> { Ok(()) } -fn validate_mode(mode: &ModeConfig) -> Result<(), Error> { +fn validate_mode(mode: &ModeConfig) -> Result<(), Diagnostic> { + if mode.all_disabled() { + return Err(Diagnostic::Warning(Error::invalid_mode( + "all operation modes of Hermes are disabled, relayer won't perform any action aside from subscribing to events".to_string(), + ))); + } + if mode.clients.enabled && !mode.clients.refresh && !mode.clients.misbehaviour { - return Err(Error::invalid_mode( + return Err(Diagnostic::Error(Error::invalid_mode( "either `refresh` or `misbehaviour` must be set to true if `clients.enabled` is set to true".to_string(), - )); + ))); } Ok(()) @@ -96,29 +109,32 @@ fn validate_mode(mode: &ModeConfig) -> Result<(), Error> { /// a) non-zero /// b) greater or equal to 1/3 /// c) strictly less than 1 -fn validate_trust_threshold(id: &ChainId, trust_threshold: TrustThreshold) -> Result<(), Error> { +fn validate_trust_threshold( + id: &ChainId, + trust_threshold: TrustThreshold, +) -> Result<(), Diagnostic> { if trust_threshold.denominator() == 0 { - return Err(Error::invalid_trust_threshold( + return Err(Diagnostic::Error(Error::invalid_trust_threshold( trust_threshold, id.clone(), "trust threshold denominator cannot be zero".to_string(), - )); + ))); } if trust_threshold.numerator() * 3 < trust_threshold.denominator() { - return Err(Error::invalid_trust_threshold( + return Err(Diagnostic::Error(Error::invalid_trust_threshold( trust_threshold, id.clone(), "trust threshold cannot be < 1/3".to_string(), - )); + ))); } if trust_threshold.numerator() >= trust_threshold.denominator() { - return Err(Error::invalid_trust_threshold( + return Err(Diagnostic::Error(Error::invalid_trust_threshold( trust_threshold, id.clone(), "trust threshold cannot be >= 1".to_string(), - )); + ))); } Ok(()) diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 72f0ef7463..32e6ae2a3d 100644 --- a/relayer/src/config.rs +++ b/relayer/src/config.rs @@ -171,6 +171,15 @@ pub struct ModeConfig { pub packets: Packets, } +impl ModeConfig { + pub fn all_disabled(&self) -> bool { + !self.clients.enabled + && !self.connections.enabled + && !self.channels.enabled + && !self.packets.enabled + } +} + #[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Clients { From 04b501e26181b6c9a418dfb6e59243148a689001 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 11 Nov 2021 11:26:09 +0100 Subject: [PATCH 34/36] Fix typo --- relayer-cli/src/application.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer-cli/src/application.rs b/relayer-cli/src/application.rs index 2548e6c66f..6c16de3bde 100644 --- a/relayer-cli/src/application.rs +++ b/relayer-cli/src/application.rs @@ -131,7 +131,7 @@ impl Application for CliApp { if let Err(diagnostic) = validate_config(&config) { match diagnostic { Diagnostic::Warning(e) => { - tracing::warn!("relayer may be misconfigured: {}", e.detail()); + tracing::warn!("relayer may be misconfigured: {}", e); } Diagnostic::Error(e) => { return Err(FrameworkErrorKind::ConfigError.context(e).into()); From 4ec957bd1c46177e57678fc9050947ddc85cdc9b Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 11 Nov 2021 17:05:02 +0530 Subject: [PATCH 35/36] Fix case where only packet mode in enabled --- relayer/src/supervisor/spawn.rs | 98 +++++++++++++++++---------------- 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/relayer/src/supervisor/spawn.rs b/relayer/src/supervisor/spawn.rs index 5ba05a3bb5..489c35db62 100644 --- a/relayer/src/supervisor/spawn.rs +++ b/relayer/src/supervisor/spawn.rs @@ -492,64 +492,68 @@ impl<'a, Chain: ChainHandle + 'static> SpawnContext<'a, Chain> { chan_state_dst ); - if mode.clients.enabled + if (mode.clients.enabled || mode.packets.enabled) && chan_state_src.is_open() && chan_state_dst.is_open() && self.relay_packets_on_channel(&chain, &channel) { - // spawn the client worker - let client_object = Object::Client(Client { - dst_client_id: client.client_id.clone(), - dst_chain_id: chain.id(), - src_chain_id: client.client_state.chain_id(), - }); - - self.workers - .spawn( - counterparty_chain.clone(), - chain.clone(), - &client_object, - &self.config.read().expect("poisoned lock"), - ) - .then(|| debug!("spawned Client worker: {}", client_object.short_name())); - - // Safe to unwrap because the inner channel end has state open - let counterparty_channel = counterparty_channel.unwrap(); - - let has_packets = || -> bool { - !unreceived_packets(&counterparty_chain, &chain, counterparty_channel.clone()) - .unwrap_or_default() - .is_empty() - }; - - let has_acks = || -> bool { - !unreceived_acknowledgements( - &counterparty_chain, - &chain, - counterparty_channel.clone(), - ) - .unwrap_or_default() - .is_empty() - }; - - // If there are any outstanding packets or acks to send, spawn the worker - if mode.packets.enabled && has_packets() || has_acks() { - // create the Packet object and spawn worker - let path_object = Object::Packet(Packet { - dst_chain_id: counterparty_chain.id(), - src_chain_id: chain.id(), - src_channel_id: channel.channel_id, - src_port_id: channel.port_id, + if mode.clients.enabled { + // spawn the client worker + let client_object = Object::Client(Client { + dst_client_id: client.client_id.clone(), + dst_chain_id: chain.id(), + src_chain_id: client.client_state.chain_id(), }); self.workers .spawn( - chain.clone(), counterparty_chain.clone(), - &path_object, + chain.clone(), + &client_object, &self.config.read().expect("poisoned lock"), ) - .then(|| debug!("spawned Packet worker: {}", path_object.short_name())); + .then(|| debug!("spawned Client worker: {}", client_object.short_name())); + } + + if mode.packets.enabled { + // Safe to unwrap because the inner channel end has state open + let counterparty_channel = counterparty_channel.unwrap(); + + let has_packets = || -> bool { + !unreceived_packets(&counterparty_chain, &chain, counterparty_channel.clone()) + .unwrap_or_default() + .is_empty() + }; + + let has_acks = || -> bool { + !unreceived_acknowledgements( + &counterparty_chain, + &chain, + counterparty_channel.clone(), + ) + .unwrap_or_default() + .is_empty() + }; + + // If there are any outstanding packets or acks to send, spawn the worker + if has_packets() || has_acks() { + // create the Packet object and spawn worker + let path_object = Object::Packet(Packet { + dst_chain_id: counterparty_chain.id(), + src_chain_id: chain.id(), + src_channel_id: channel.channel_id, + src_port_id: channel.port_id, + }); + + self.workers + .spawn( + chain.clone(), + counterparty_chain.clone(), + &path_object, + &self.config.read().expect("poisoned lock"), + ) + .then(|| debug!("spawned Packet worker: {}", path_object.short_name())); + } } } else if mode.channels.enabled && !chan_state_dst.is_open() From db3b6a5a11eff6f85f7f5f3ee64d4e87bc092a87 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 12 Nov 2021 12:21:50 +0100 Subject: [PATCH 36/36] Improve log message in cient worker --- relayer/src/worker/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index f65f2c19dd..7020655428 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -48,8 +48,8 @@ impl ClientWorker { ); info!( - "[{}] running client worker & initial misbehaviour detection", - client + "[{}] running client worker with misbehaviour={} and refresh={}", + client, self.clients_cfg.misbehaviour, self.clients_cfg.refresh ); // initial check for evidence of misbehaviour for all updates