From 2f5196461ccbf601c54f1c5e203de127b3ede20c Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Mon, 3 Aug 2020 12:24:41 +0200 Subject: [PATCH 01/28] Add doc comments and error types to ICS 02 module --- modules/src/ics02_client/error.rs | 8 ++++++++ modules/src/ics02_client/events.rs | 6 +++++- modules/src/ics02_client/msgs.rs | 9 ++++++--- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs index 8bca154a27..b40ea6bed7 100644 --- a/modules/src/ics02_client/error.rs +++ b/modules/src/ics02_client/error.rs @@ -1,12 +1,20 @@ use anomaly::{BoxError, Context}; use thiserror::Error; +use crate::ics24_host::identifier::ClientId; + pub type Error = anomaly::Error; #[derive(Clone, Debug, Error)] pub enum Kind { #[error("unknown client type")] UnknownClientType, + + #[error("client already exists: {0}")] + ClientAlreadyExists(ClientId), + + #[error("client not found: {0}")] + ClientNotFound(ClientId), } impl Kind { diff --git a/modules/src/ics02_client/events.rs b/modules/src/ics02_client/events.rs index d2c6b364b6..9cc33037b4 100644 --- a/modules/src/ics02_client/events.rs +++ b/modules/src/ics02_client/events.rs @@ -9,6 +9,7 @@ use serde_derive::{Deserialize, Serialize}; use std::convert::TryFrom; use tendermint::block; +/// NewBlock event signals the committing & execution of a new block. // TODO - find a better place for NewBlock #[derive(Debug, Deserialize, Serialize, Clone)] pub struct NewBlock { @@ -27,6 +28,7 @@ impl From for IBCEvent { } } +/// CreateClient event signals the creation of a new on-chain client (IBC client). #[derive(Debug, Deserialize, Serialize, Clone)] pub struct CreateClient { pub height: block::Height, @@ -51,6 +53,7 @@ impl From for IBCEvent { } } +/// UpdateClient event signals a recent update of an on-chain client (IBC Client). #[derive(Debug, Deserialize, Serialize, Clone)] pub struct UpdateClient { pub height: block::Height, @@ -75,7 +78,8 @@ impl From for IBCEvent { } } -#[derive(Debug, Deserialize, Serialize, Clone)] +/// ClientMisbehavior event signals the update of an on-chain client (IBC Client) with evidence of +/// misbehavior.#[derive(Debug, Deserialize, Serialize, Clone)] pub struct ClientMisbehavior { pub height: block::Height, pub client_id: ClientId, diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index a25d49ba4d..8aaa04965f 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -1,6 +1,9 @@ -use super::client_type::ClientType; -use super::header::Header; -use super::state::ConsensusState; +//! These are definitions of messages that a relayer submits to a chain. Specific implementations of +//! these messages can be found, for instance, in ICS 07 for Tendermint-specific chains. A chain +//! handles these messages in two layers: first with the general ICS 02 client handler, which +//! subsequently calls into the chain-specific (e.g., ICS 07) client handler. See: +//! https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create. + use crate::ics24_host::identifier::ClientId; use crate::tx_msg::Msg; From b1d009150476666a82a392ee9bacb56f38373cff Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 4 Aug 2020 15:32:45 +0200 Subject: [PATCH 02/28] Rename ics02_client::client module to ics02_client::raw --- modules/src/ics02_client/mod.rs | 2 +- modules/src/ics02_client/{client.rs => raw.rs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename modules/src/ics02_client/{client.rs => raw.rs} (100%) diff --git a/modules/src/ics02_client/mod.rs b/modules/src/ics02_client/mod.rs index 81ad985e35..e782335602 100644 --- a/modules/src/ics02_client/mod.rs +++ b/modules/src/ics02_client/mod.rs @@ -1,9 +1,9 @@ //! ICS 02: IBC Client implementation -pub mod client; pub mod client_type; pub mod error; pub mod events; pub mod header; pub mod msgs; +pub mod raw; pub mod state; diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/raw.rs similarity index 100% rename from modules/src/ics02_client/client.rs rename to modules/src/ics02_client/raw.rs From 0f936465fc3831b1c9c65b0fe2d8c65c778dbe64 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Mon, 3 Aug 2020 15:19:51 +0200 Subject: [PATCH 03/28] Replace message traits with structs --- modules/src/ics02_client/msgs.rs | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 8aaa04965f..200f8610c9 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -5,24 +5,16 @@ //! https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create. use crate::ics24_host::identifier::ClientId; -use crate::tx_msg::Msg; -pub trait MsgUpdateClient -where - Self: Msg, -{ - type Header: Header; - fn client_id(&self) -> &ClientId; - fn header(&self) -> &Self::Header; +/// A type of message that triggers the creation of a new on-chain (IBC) client. +pub struct MsgCreateClient { + client_id: ClientId, + client_type: C::ClientType, + consensus_state: C::ConsensusState, } -pub trait MsgCreateClient -where - Self: Msg, -{ - type ConsensusState: ConsensusState; - - fn client_id(&self) -> &ClientId; - fn client_type(&self) -> ClientType; - fn consensus_state(&self) -> Self::ConsensusState; +/// A type of message that triggers the update of an on-chain (IBC) client with new headers. +pub struct MsgUpdateClient { + client_id: ClientId, + header: C::Header, } From c5fce268d5b440c8630a63fd2ae3699c64755270 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 12:14:52 +0200 Subject: [PATCH 04/28] Fix formatting error --- modules/src/ics02_client/events.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/src/ics02_client/events.rs b/modules/src/ics02_client/events.rs index 9cc33037b4..e2ef2eb2a4 100644 --- a/modules/src/ics02_client/events.rs +++ b/modules/src/ics02_client/events.rs @@ -79,7 +79,8 @@ impl From for IBCEvent { } /// ClientMisbehavior event signals the update of an on-chain client (IBC Client) with evidence of -/// misbehavior.#[derive(Debug, Deserialize, Serialize, Clone)] +/// misbehavior. +#[derive(Debug, Deserialize, Serialize, Clone)] pub struct ClientMisbehavior { pub height: block::Height, pub client_id: ClientId, From 64696e02901fa6cedf4d891212b7f7a90e7f9d3a Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 12:15:04 +0200 Subject: [PATCH 05/28] Add implementation specific error kind --- modules/src/ics02_client/error.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs index b40ea6bed7..094ff97811 100644 --- a/modules/src/ics02_client/error.rs +++ b/modules/src/ics02_client/error.rs @@ -15,6 +15,9 @@ pub enum Kind { #[error("client not found: {0}")] ClientNotFound(ClientId), + + #[error("implementation specific")] + ImplementationSpecific, } impl Kind { From 5a045a686cee44d3a8d2eeb03c92c757661127f8 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 12:15:42 +0200 Subject: [PATCH 06/28] Fixup client message structs definitions --- modules/src/ics02_client/msgs.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 200f8610c9..0f1112a3e7 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -4,17 +4,20 @@ //! subsequently calls into the chain-specific (e.g., ICS 07) client handler. See: //! https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create. +use crate::ics02_client::client::ClientDef; +use crate::ics02_client::client_type::ClientType; + use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. -pub struct MsgCreateClient { +pub struct MsgCreateClient { client_id: ClientId, - client_type: C::ClientType, + client_type: ClientType, consensus_state: C::ConsensusState, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. -pub struct MsgUpdateClient { +pub struct MsgUpdateClient { client_id: ClientId, header: C::Header, } From 6949ed61958e11e564df32089e9a0c8484588815 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 12:21:42 +0200 Subject: [PATCH 07/28] Fix clippy warnings --- modules/src/ics02_client/client_type.rs | 6 +++--- modules/src/ics03_connection/msgs.rs | 8 ++++---- modules/src/ics04_channel/channel.rs | 4 ++-- modules/src/ics04_channel/msgs.rs | 12 ++++++------ modules/src/ics07_tendermint/client_state.rs | 2 +- modules/src/ics07_tendermint/header.rs | 2 +- relayer-cli/src/commands/query/channel.rs | 2 +- relayer-cli/src/commands/query/client.rs | 8 ++++---- relayer-cli/src/commands/query/connection.rs | 6 +++--- relayer-cli/tests/acceptance.rs | 2 +- relayer-cli/tests/integration.rs | 2 +- 11 files changed, 27 insertions(+), 27 deletions(-) diff --git a/modules/src/ics02_client/client_type.rs b/modules/src/ics02_client/client_type.rs index 9837514d76..a9d1e7ab60 100644 --- a/modules/src/ics02_client/client_type.rs +++ b/modules/src/ics02_client/client_type.rs @@ -38,8 +38,8 @@ mod tests { let client_type = ClientType::from_str("tendermint"); match client_type { - Ok(ClientType::Tendermint) => assert!(true), - Err(_) => assert!(false, "parse failed"), + Ok(ClientType::Tendermint) => (), + _ => panic!("parse failed"), } } @@ -52,7 +52,7 @@ mod tests { format!("{}", err), "unknown client type: some-random-client-type" ), - _ => assert!(false, "parse didn't fail"), + _ => panic!("parse didn't fail"), } } } diff --git a/modules/src/ics03_connection/msgs.rs b/modules/src/ics03_connection/msgs.rs index 5ffec925e1..9fd1997419 100644 --- a/modules/src/ics03_connection/msgs.rs +++ b/modules/src/ics03_connection/msgs.rs @@ -339,7 +339,7 @@ mod tests { counterparty_connection_id: "abcdefghijksdffjssdkflweldflsfladfsfwjkrekcmmsdfsdfjflddmnopqrstu" .to_string(), - ..default_con_params.clone() + ..default_con_params }, want_pass: false, }, @@ -485,7 +485,7 @@ mod tests { name: "Empty proof".to_string(), params: ConOpenTryParams { proof_init: CommitmentProof { ops: vec![] }, - ..default_con_params.clone() + ..default_con_params }, want_pass: false, }, @@ -585,7 +585,7 @@ mod tests { name: "Bad consensus height, height is 0".to_string(), params: ConOpenAckParams { consensus_height: 0, - ..default_con_params.clone() + ..default_con_params }, want_pass: false, }, @@ -659,7 +659,7 @@ mod tests { name: "Bad proof height, height is 0".to_string(), params: ConOpenConfirmParams { proof_height: 0, - ..default_con_params.clone() + ..default_con_params }, want_pass: false, }, diff --git a/modules/src/ics04_channel/channel.rs b/modules/src/ics04_channel/channel.rs index 28e1f5816c..d07f76e4ff 100644 --- a/modules/src/ics04_channel/channel.rs +++ b/modules/src/ics04_channel/channel.rs @@ -199,7 +199,7 @@ mod tests { let tests: Vec = vec![ Test { name: "Raw channel end with missing counterparty".to_string(), - params: empty_raw_channel_end.clone(), + params: empty_raw_channel_end, want_pass: false, }, Test { @@ -247,7 +247,7 @@ mod tests { }, Test { name: "Raw channel end with correct params".to_string(), - params: raw_channel_end.clone(), + params: raw_channel_end, want_pass: true, }, ] diff --git a/modules/src/ics04_channel/msgs.rs b/modules/src/ics04_channel/msgs.rs index 744c9b58a6..b017444269 100644 --- a/modules/src/ics04_channel/msgs.rs +++ b/modules/src/ics04_channel/msgs.rs @@ -662,7 +662,7 @@ mod tests { name: "Bad connection hops (conn id too short, must be 10 chars)".to_string(), params: OpenInitParams { connection_hops: vec!["conn124".to_string()].into_iter().collect(), - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -866,7 +866,7 @@ mod tests { name: "Correct counterparty channel identifier".to_string(), params: OpenTryParams { counterparty_channel_id: "channelid34".to_string(), - ..default_params.clone() + ..default_params }, want_pass: true, }, @@ -996,7 +996,7 @@ mod tests { name: "Bad proof height, height = 0".to_string(), params: OpenAckParams { proof_height: 0, - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -1111,7 +1111,7 @@ mod tests { name: "Bad proof height, height = 0".to_string(), params: OpenConfirmParams { proof_height: 0, - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -1213,7 +1213,7 @@ mod tests { name: "Bad channel, name too long".to_string(), params: CloseInitParams { channel_id: "abcdeasdfasdfasdfasdfasdfasdfasdfasdfdgasdfasdfasdfghijklmnopqrstu".to_string(), - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -1325,7 +1325,7 @@ mod tests { name: "Bad proof height, height = 0".to_string(), params: CloseConfirmParams { proof_height: 0, - ..default_params.clone() + ..default_params }, want_pass: false, }, diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index fa5f3fa6d5..b38b268892 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -185,7 +185,7 @@ mod tests { params: ClientStateParams { trusting_period: Duration::from_secs(11), unbonding_period: Duration::from_secs(10), - ..default_params.clone() + ..default_params }, want_pass: false, }, diff --git a/modules/src/ics07_tendermint/header.rs b/modules/src/ics07_tendermint/header.rs index ffb1085ca1..695e2beb3d 100644 --- a/modules/src/ics07_tendermint/header.rs +++ b/modules/src/ics07_tendermint/header.rs @@ -64,7 +64,7 @@ pub mod test_util { Header { signed_header: shdr, validator_set: vs.clone(), - next_validator_set: vs.clone(), + next_validator_set: vs, } } } diff --git a/relayer-cli/src/commands/query/channel.rs b/relayer-cli/src/commands/query/channel.rs index 177ecd183f..d4916765e6 100644 --- a/relayer-cli/src/commands/query/channel.rs +++ b/relayer-cli/src/commands/query/channel.rs @@ -189,7 +189,7 @@ mod tests { name: "Bad channel, name too short".to_string(), params: QueryChannelEndCmd { channel_id: Some("chshort".to_string()), - ..default_params.clone() + ..default_params }, want_pass: false, }, diff --git a/relayer-cli/src/commands/query/client.rs b/relayer-cli/src/commands/query/client.rs index 00253b4eeb..4c25117fd2 100644 --- a/relayer-cli/src/commands/query/client.rs +++ b/relayer-cli/src/commands/query/client.rs @@ -345,7 +345,7 @@ mod tests { name: "No client id specified".to_string(), params: QueryClientStateCmd { client_id: None, - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -353,7 +353,7 @@ mod tests { name: "Bad client id, non-alpha".to_string(), params: QueryClientStateCmd { client_id: Some("p34".to_string()), - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -431,7 +431,7 @@ mod tests { name: "No client id specified".to_string(), params: QueryClientConnectionsCmd { client_id: None, - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -439,7 +439,7 @@ mod tests { name: "Bad client id, non-alpha".to_string(), params: QueryClientConnectionsCmd { client_id: Some("p34".to_string()), - ..default_params.clone() + ..default_params }, want_pass: false, }, diff --git a/relayer-cli/src/commands/query/connection.rs b/relayer-cli/src/commands/query/connection.rs index 8ee9662453..9702c97cb8 100644 --- a/relayer-cli/src/commands/query/connection.rs +++ b/relayer-cli/src/commands/query/connection.rs @@ -141,7 +141,7 @@ mod tests { name: "No connection id specified".to_string(), params: QueryConnectionEndCmd { connection_id: None, - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -149,7 +149,7 @@ mod tests { name: "Bad connection, non-alpha".to_string(), params: QueryConnectionEndCmd { connection_id: Some("conn01".to_string()), - ..default_params.clone() + ..default_params }, want_pass: false, }, @@ -157,7 +157,7 @@ mod tests { name: "Bad connection, name too short".to_string(), params: QueryConnectionEndCmd { connection_id: Some("connshort".to_string()), - ..default_params.clone() + ..default_params }, want_pass: false, }, diff --git a/relayer-cli/tests/acceptance.rs b/relayer-cli/tests/acceptance.rs index 2da38d04ca..9063229a18 100644 --- a/relayer-cli/tests/acceptance.rs +++ b/relayer-cli/tests/acceptance.rs @@ -27,7 +27,7 @@ use once_cell::sync::Lazy; /// the runner acquire a mutex when executing commands and inspecting /// exit statuses, serializing what would otherwise be multithreaded /// invocations as `cargo test` executes tests in parallel by default. -pub static RUNNER: Lazy = Lazy::new(|| CmdRunner::default()); +pub static RUNNER: Lazy = Lazy::new(CmdRunner::default); /// Use `Config::default()` value if no config or args #[test] diff --git a/relayer-cli/tests/integration.rs b/relayer-cli/tests/integration.rs index ebc0c82ad1..3e8e38d7b2 100644 --- a/relayer-cli/tests/integration.rs +++ b/relayer-cli/tests/integration.rs @@ -67,7 +67,7 @@ fn query_connection_id() { assert_eq!(query.counterparty().connection_id(), "connectionidtwo"); assert_eq!( query.counterparty().prefix(), - &CommitmentPrefix::new("prefix".as_bytes().to_vec()) + &CommitmentPrefix::new(b"prefix".to_vec()) ); assert_eq!( query.versions(), From 414cfa909f866d03fc5ca50354e18625510b00a2 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 12:23:18 +0200 Subject: [PATCH 08/28] Add basic handler definitions --- modules/src/handler.rs | 60 +++++++++++++++++++++++++++++ modules/src/ics02_client/client.rs | 25 ++++++++++++ modules/src/ics02_client/handler.rs | 21 ++++++++++ modules/src/ics02_client/mod.rs | 2 + 4 files changed, 108 insertions(+) create mode 100644 modules/src/handler.rs create mode 100644 modules/src/ics02_client/client.rs create mode 100644 modules/src/ics02_client/handler.rs diff --git a/modules/src/handler.rs b/modules/src/handler.rs new file mode 100644 index 0000000000..92b3796c9c --- /dev/null +++ b/modules/src/handler.rs @@ -0,0 +1,60 @@ +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Attribute { + key: String, + value: String, +} + +impl Attribute { + fn new(key: String, value: String) -> Self { + Self { key, value } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum EventType { + Message, + Custom(String), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Event { + tpe: EventType, + attributes: Vec, +} + +impl Event { + fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self { + Self { + tpe, + attributes: attrs + .into_iter() + .map(|(k, v)| Attribute::new(k, v)) + .collect(), + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HandlerOutput { + result: T, + log: Vec, + events: Vec, +} + +impl HandlerOutput { + fn new(result: T) -> Self { + Self { + result, + log: vec![], + events: vec![], + } + } + + fn log(&mut self, log: String) { + self.log.push(log); + } + + fn emit(&mut self, event: impl Into) { + self.events.push(event.into()); + } +} diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs new file mode 100644 index 0000000000..40671845ae --- /dev/null +++ b/modules/src/ics02_client/client.rs @@ -0,0 +1,25 @@ +//! ICS 02: Client definitions + +use tendermint::lite::Height; +use tendermint::time::Time; + +pub trait ClientDef: Sized { + type Header: Header; + type ClientState: ClientState; + type ConsensusState: ConsensusState; +} + +pub trait Header {} + +pub trait ClientState { + /// Initialise a client state with a provided consensus state. + fn from_consensus_state(cs: CD::ConsensusState) -> Self; + + /// Height of most recent validated header. + fn latest_client_height(&self) -> Height; +} + +pub trait ConsensusState { + /// Returns the timestamp associated with that consensus state. + fn timestamp(&self) -> Time; +} diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs new file mode 100644 index 0000000000..3d43e3b40a --- /dev/null +++ b/modules/src/ics02_client/handler.rs @@ -0,0 +1,21 @@ +use crate::ics02_client::client::ClientDef; +use crate::ics02_client::client_type::ClientType; +use crate::ics02_client::error::Error; +use crate::ics24_host::identifier::ClientId; + +use tendermint::lite::Height; + +pub trait ClientContext +where + CD: ClientDef, +{ + fn client_type(&self, client_id: ClientId) -> Option; + fn client_state(&self, client_id: ClientId) -> Option; + fn consensus_state(&self, client_id: ClientId, height: Height) -> Option; +} + +pub trait ClientKeeper { + fn store_client_type(client_type: ClientType) -> Result<(), Error>; + fn store_client_state(client_state: CD::ClientState) -> Result<(), Error>; + fn store_consensus_state(consensus_state: CD::ConsensusState) -> Result<(), Error>; +} diff --git a/modules/src/ics02_client/mod.rs b/modules/src/ics02_client/mod.rs index e782335602..7f8090309c 100644 --- a/modules/src/ics02_client/mod.rs +++ b/modules/src/ics02_client/mod.rs @@ -1,8 +1,10 @@ //! ICS 02: IBC Client implementation +pub mod client; pub mod client_type; pub mod error; pub mod events; +pub mod handler; pub mod header; pub mod msgs; pub mod raw; From 605e62a9315357e758ee975160755f8c784280d9 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 5 Aug 2020 16:14:57 +0200 Subject: [PATCH 09/28] Add initial implementation of CreateClient handler --- modules/src/handler.rs | 54 ++++- modules/src/ics02_client/handler.rs | 79 ++++++- .../src/ics02_client/handler/create_client.rs | 65 ++++++ modules/src/ics02_client/msgs.rs | 11 +- modules/src/ics02_client/state.rs | 4 +- .../ics07_tendermint/msgs/create_client.rs | 194 +++++++++--------- .../ics07_tendermint/msgs/update_client.rs | 110 +++++----- modules/src/lib.rs | 1 + 8 files changed, 345 insertions(+), 173 deletions(-) create mode 100644 modules/src/ics02_client/handler/create_client.rs diff --git a/modules/src/handler.rs b/modules/src/handler.rs index 92b3796c9c..26a39ac4d8 100644 --- a/modules/src/handler.rs +++ b/modules/src/handler.rs @@ -1,3 +1,5 @@ +use std::marker::PhantomData; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Attribute { key: String, @@ -5,7 +7,7 @@ pub struct Attribute { } impl Attribute { - fn new(key: String, value: String) -> Self { + pub fn new(key: String, value: String) -> Self { Self { key, value } } } @@ -23,7 +25,7 @@ pub struct Event { } impl Event { - fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self { + pub fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self { Self { tpe, attributes: attrs @@ -34,27 +36,61 @@ impl Event { } } +pub type HandlerResult = Result, E>; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct HandlerOutput { - result: T, + pub result: T, + pub log: Vec, + pub events: Vec, +} + +impl HandlerOutput { + pub fn builder() -> HandlerOutputBuilder { + HandlerOutputBuilder::new() + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HandlerOutputBuilder { log: Vec, events: Vec, + marker: PhantomData, } -impl HandlerOutput { - fn new(result: T) -> Self { +impl HandlerOutputBuilder { + pub fn new() -> Self { Self { - result, log: vec![], events: vec![], + marker: PhantomData, } } - fn log(&mut self, log: String) { - self.log.push(log); + pub fn with_log(mut self, log: impl Into>) -> Self { + self.log.append(&mut log.into()); + self + } + + pub fn log(&mut self, log: impl Into) { + self.log.push(log.into()); + } + + pub fn with_events(mut self, events: impl Into>) -> Self { + self.events.append(&mut events.into()); + self } - fn emit(&mut self, event: impl Into) { + pub fn emit(&mut self, event: impl Into) { self.events.push(event.into()); } + + pub fn with_result(self, result: T) -> HandlerOutput { + HandlerOutput { + result, + log: self.log, + events: self.events, + } + } } + diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 3d43e3b40a..c4be079e2a 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -1,21 +1,88 @@ +use crate::handler::{Event, EventType, HandlerOutput}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::Error; +use crate::ics02_client::msgs::MsgCreateClient; use crate::ics24_host::identifier::ClientId; use tendermint::lite::Height; +pub mod create_client; + pub trait ClientContext where CD: ClientDef, { - fn client_type(&self, client_id: ClientId) -> Option; - fn client_state(&self, client_id: ClientId) -> Option; - fn consensus_state(&self, client_id: ClientId, height: Height) -> Option; + fn client_type(&self, client_id: &ClientId) -> Option; + fn client_state(&self, client_id: &ClientId) -> Option; + fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; } pub trait ClientKeeper { - fn store_client_type(client_type: ClientType) -> Result<(), Error>; - fn store_client_state(client_state: CD::ClientState) -> Result<(), Error>; - fn store_consensus_state(consensus_state: CD::ConsensusState) -> Result<(), Error>; + fn store_client_type( + &mut self, + client_id: ClientId, + client_type: ClientType, + ) -> Result<(), Error>; + + fn store_client_state( + &mut self, + client_id: ClientId, + client_state: CD::ClientState, + ) -> Result<(), Error>; + + fn store_consensus_state( + &mut self, + client_id: ClientId, + consensus_state: CD::ConsensusState, + ) -> Result<(), Error>; +} + +pub enum ClientEvent { + ClientCreated(ClientId), + ClientUpdated(ClientId), +} + +impl From for Event { + fn from(ce: ClientEvent) -> Event { + match ce { + ClientEvent::ClientCreated(client_id) => Event::new( + EventType::Custom("ClientCreated".to_string()), + vec![("client_id".to_string(), client_id.to_string())], + ), + ClientEvent::ClientUpdated(client_id) => Event::new( + EventType::Custom("ClientUpdated".to_string()), + vec![("client_id".to_string(), client_id.to_string())], + ), + } + } +} + +pub enum ClientMsg { + CreateClient(MsgCreateClient), + // UpdateClient(UpdateClientMsg), } + +pub fn handler(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +where + CD: ClientDef, + Ctx: ClientContext + ClientKeeper, +{ + match msg { + ClientMsg::CreateClient(msg) => { + let HandlerOutput { + result, + log, + events, + } = create_client::process(ctx, msg)?; + + create_client::keep(ctx, result)?; + + Ok(HandlerOutput::builder() + .with_log(log) + .with_events(events) + .with_result(())) + } + } +} + diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs new file mode 100644 index 0000000000..6c369dfc5c --- /dev/null +++ b/modules/src/ics02_client/handler/create_client.rs @@ -0,0 +1,65 @@ +use crate::handler::{HandlerOutput, HandlerResult}; +use crate::ics02_client::client::{ClientDef, ClientState}; +use crate::ics02_client::client_type::ClientType; +use crate::ics02_client::error::{Error, Kind}; +use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; +use crate::ics02_client::msgs::MsgCreateClient; +use crate::ics24_host::identifier::ClientId; + +pub struct CreateClientResult { + client_id: ClientId, + client_type: ClientType, + client_state: CD::ClientState, +} + +pub fn process( + ctx: &dyn ClientContext, + msg: MsgCreateClient, +) -> HandlerResult, Error> +where + CD: ClientDef, +{ + let mut output = HandlerOutput::builder(); + + let MsgCreateClient { + client_id, + client_type, + consensus_state, + } = msg; + + if ctx.client_state(&client_id).is_some() { + return Err(Kind::ClientAlreadyExists(client_id).into()); + } + + output.log("success: no client state found"); + + if ctx.client_type(&client_id).is_some() { + return Err(Kind::ClientAlreadyExists(client_id).into()); + } + + output.log("success: no client type found"); + + let client_state = CD::ClientState::from_consensus_state(consensus_state); + + output.emit(ClientEvent::ClientCreated(client_id.clone())); + + Ok(output.with_result(CreateClientResult { + client_id, + client_type, + client_state, + })) +} + +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: CreateClientResult, +) -> Result<(), Error> +where + CD: ClientDef, +{ + keeper.store_client_state(result.client_id.clone(), result.client_state)?; + keeper.store_client_type(result.client_id, result.client_type)?; + + Ok(()) +} + diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 0f1112a3e7..63abd6fa9c 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -11,13 +11,14 @@ use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. pub struct MsgCreateClient { - client_id: ClientId, - client_type: ClientType, - consensus_state: C::ConsensusState, + pub client_id: ClientId, + pub client_type: ClientType, + pub consensus_state: C::ConsensusState, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. pub struct MsgUpdateClient { - client_id: ClientId, - header: C::Header, + pub client_id: ClientId, + pub header: C::Header, } + diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index d441112fcf..9b69cebf5a 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -35,7 +35,9 @@ pub trait ClientState { /// Freeze status of the client fn is_frozen(&self) -> bool; - // TODO: It's unclear what this function is expected to achieve. Document this. + /// Verifies a proof of the consensus state of the specified client stored on the target machine. + /// FIXME: Definition is incomplete. + /// See https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#required-functions fn verify_client_consensus_state( &self, root: &CommitmentRoot, diff --git a/modules/src/ics07_tendermint/msgs/create_client.rs b/modules/src/ics07_tendermint/msgs/create_client.rs index 53e8935ad7..a562b9ea23 100644 --- a/modules/src/ics07_tendermint/msgs/create_client.rs +++ b/modules/src/ics07_tendermint/msgs/create_client.rs @@ -1,97 +1,97 @@ -use crate::ics02_client::client_type::ClientType; -use crate::ics07_tendermint::consensus_state::ConsensusState; -use crate::ics07_tendermint::header::Header; -use crate::ics23_commitment::CommitmentRoot; -use crate::ics24_host::identifier::ClientId; -use crate::tx_msg::Msg; - -use std::time::Duration; - -use serde_derive::{Deserialize, Serialize}; -use tendermint::account::Id as AccountId; - -pub const TYPE_MSG_CREATE_CLIENT: &str = "create_client"; - -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -pub struct MsgCreateClient { - client_id: ClientId, - header: Header, - trusting_period: Duration, - bonding_period: Duration, - signer: AccountId, -} - -impl MsgCreateClient { - pub fn new( - client_id: ClientId, - header: Header, - trusting_period: Duration, - bonding_period: Duration, - signer: AccountId, - ) -> Self { - Self { - client_id, - header, - trusting_period, - bonding_period, - signer, - } - } - - fn get_client_id(&self) -> &ClientId { - &self.client_id - } - - fn get_header(&self) -> &Header { - &self.header - } -} - -impl Msg for MsgCreateClient { - type ValidationError = crate::ics24_host::error::ValidationError; - - fn route(&self) -> String { - crate::keys::ROUTER_KEY.to_string() - } - - fn get_type(&self) -> String { - TYPE_MSG_CREATE_CLIENT.to_string() - } - - fn validate_basic(&self) -> Result<(), Self::ValidationError> { - // Nothing to validate since both ClientId and AccountId perform validation on creation. - Ok(()) - } - - fn get_sign_bytes(&self) -> Vec { - todo!() - } - - fn get_signers(&self) -> Vec { - vec![self.signer] - } -} - -impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { - type ConsensusState = ConsensusState; - - fn client_id(&self) -> &ClientId { - &self.client_id - } - - fn client_type(&self) -> ClientType { - ClientType::Tendermint - } - - fn consensus_state(&self) -> Self::ConsensusState { - let root = CommitmentRoot; // TODO - let header = &self.header.signed_header.header; - - ConsensusState::new( - root, - header.height.into(), - header.time, - self.header.validator_set.clone(), - ) - } -} +// use crate::ics02_client::client_type::ClientType; +// use crate::ics07_tendermint::consensus_state::ConsensusState; +// use crate::ics07_tendermint::header::Header; +// use crate::ics23_commitment::CommitmentRoot; +// use crate::ics24_host::identifier::ClientId; +// use crate::tx_msg::Msg; + +// use std::time::Duration; + +// use serde_derive::{Deserialize, Serialize}; +// use tendermint::account::Id as AccountId; + +// pub const TYPE_MSG_CREATE_CLIENT: &str = "create_client"; + +// #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +// pub struct MsgCreateClient { +// client_id: ClientId, +// header: Header, +// trusting_period: Duration, +// bonding_period: Duration, +// signer: AccountId, +// } + +// impl MsgCreateClient { +// pub fn new( +// client_id: ClientId, +// header: Header, +// trusting_period: Duration, +// bonding_period: Duration, +// signer: AccountId, +// ) -> Self { +// Self { +// client_id, +// header, +// trusting_period, +// bonding_period, +// signer, +// } +// } + +// fn get_client_id(&self) -> &ClientId { +// &self.client_id +// } + +// fn get_header(&self) -> &Header { +// &self.header +// } +// } + +// impl Msg for MsgCreateClient { +// type ValidationError = crate::ics24_host::error::ValidationError; + +// fn route(&self) -> String { +// crate::keys::ROUTER_KEY.to_string() +// } + +// fn get_type(&self) -> String { +// TYPE_MSG_CREATE_CLIENT.to_string() +// } + +// fn validate_basic(&self) -> Result<(), Self::ValidationError> { +// // Nothing to validate since both ClientId and AccountId perform validation on creation. +// Ok(()) +// } + +// fn get_sign_bytes(&self) -> Vec { +// todo!() +// } + +// fn get_signers(&self) -> Vec { +// vec![self.signer] +// } +// } + +// impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { +// type ConsensusState = ConsensusState; + +// fn client_id(&self) -> &ClientId { +// &self.client_id +// } + +// fn client_type(&self) -> ClientType { +// ClientType::Tendermint +// } + +// fn consensus_state(&self) -> Self::ConsensusState { +// let root = CommitmentRoot; // TODO +// let header = &self.header.signed_header.header; + +// ConsensusState::new( +// root, +// header.height.into(), +// header.time, +// self.header.validator_set.clone(), +// ) +// } +// } diff --git a/modules/src/ics07_tendermint/msgs/update_client.rs b/modules/src/ics07_tendermint/msgs/update_client.rs index 5b5cb1350a..725b53f6fa 100644 --- a/modules/src/ics07_tendermint/msgs/update_client.rs +++ b/modules/src/ics07_tendermint/msgs/update_client.rs @@ -1,70 +1,70 @@ -use crate::ics07_tendermint::header::Header; -use crate::ics24_host::identifier::ClientId; -use crate::tx_msg::Msg; +// use crate::ics07_tendermint::header::Header; +// use crate::ics24_host::identifier::ClientId; +// use crate::tx_msg::Msg; -use serde_derive::{Deserialize, Serialize}; -use tendermint::account::Id as AccountId; +// use serde_derive::{Deserialize, Serialize}; +// use tendermint::account::Id as AccountId; -pub const TYPE_MSG_UPDATE_CLIENT: &str = "update_client"; +// pub const TYPE_MSG_UPDATE_CLIENT: &str = "update_client"; -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -pub struct MsgUpdateClient { - client_id: ClientId, - header: Header, - signer: AccountId, -} +// #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +// pub struct MsgUpdateClient { +// client_id: ClientId, +// header: Header, +// signer: AccountId, +// } -impl MsgUpdateClient { - pub fn new(client_id: ClientId, header: Header, signer: AccountId) -> Self { - Self { - client_id, - header, - signer, - } - } +// impl MsgUpdateClient { +// pub fn new(client_id: ClientId, header: Header, signer: AccountId) -> Self { +// Self { +// client_id, +// header, +// signer, +// } +// } - fn get_client_id(&self) -> &ClientId { - &self.client_id - } +// fn get_client_id(&self) -> &ClientId { +// &self.client_id +// } - fn get_header(&self) -> &Header { - &self.header - } -} +// fn get_header(&self) -> &Header { +// &self.header +// } +// } -impl Msg for MsgUpdateClient { - type ValidationError = crate::ics24_host::error::ValidationError; +// impl Msg for MsgUpdateClient { +// type ValidationError = crate::ics24_host::error::ValidationError; - fn route(&self) -> String { - crate::keys::ROUTER_KEY.to_string() - } +// fn route(&self) -> String { +// crate::keys::ROUTER_KEY.to_string() +// } - fn get_type(&self) -> String { - TYPE_MSG_UPDATE_CLIENT.to_string() - } +// fn get_type(&self) -> String { +// TYPE_MSG_UPDATE_CLIENT.to_string() +// } - fn validate_basic(&self) -> Result<(), Self::ValidationError> { - // Nothing to validate since both ClientId and AccountId perform validation on creation. - Ok(()) - } +// fn validate_basic(&self) -> Result<(), Self::ValidationError> { +// // Nothing to validate since both ClientId and AccountId perform validation on creation. +// Ok(()) +// } - fn get_sign_bytes(&self) -> Vec { - todo!() - } +// fn get_sign_bytes(&self) -> Vec { +// todo!() +// } - fn get_signers(&self) -> Vec { - vec![self.signer] - } -} +// fn get_signers(&self) -> Vec { +// vec![self.signer] +// } +// } -impl crate::ics02_client::msgs::MsgUpdateClient for MsgUpdateClient { - type Header = Header; +// impl crate::ics02_client::msgs::MsgUpdateClient for MsgUpdateClient { +// type Header = Header; - fn client_id(&self) -> &ClientId { - &self.client_id - } +// fn client_id(&self) -> &ClientId { +// &self.client_id +// } - fn header(&self) -> &Self::Header { - &self.header - } -} +// fn header(&self) -> &Self::Header { +// &self.header +// } +// } diff --git a/modules/src/lib.rs b/modules/src/lib.rs index eef337d78d..88020d6976 100644 --- a/modules/src/lib.rs +++ b/modules/src/lib.rs @@ -20,6 +20,7 @@ //! - ICS 24: Host Requirements pub mod events; +pub mod handler; pub mod ics02_client; pub mod ics03_connection; pub mod ics04_channel; From 46cf57a7ba4a8a17f58b5be2155ffe2a2faa667b Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 11:12:40 +0200 Subject: [PATCH 10/28] Add implementation of ClientDef for Tendermint --- modules/src/ics07_tendermint/client_state.rs | 11 +- .../src/ics07_tendermint/consensus_state.rs | 9 +- modules/src/ics07_tendermint/mod.rs | 34 ++- .../ics07_tendermint/msgs/create_client.rs | 194 +++++++++--------- .../ics07_tendermint/msgs/update_client.rs | 110 +++++----- 5 files changed, 196 insertions(+), 162 deletions(-) diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index b38b268892..9693049bde 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -10,11 +10,11 @@ use tendermint::lite::Header as liteHeader; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ClientState { - id: ClientId, - trusting_period: Duration, - unbonding_period: Duration, - frozen_height: crate::Height, - latest_header: Header, + pub id: ClientId, + pub trusting_period: Duration, + pub unbonding_period: Duration, + pub frozen_height: crate::Height, + pub latest_header: Header, } impl ClientState { @@ -215,3 +215,4 @@ mod tests { } } } + diff --git a/modules/src/ics07_tendermint/consensus_state.rs b/modules/src/ics07_tendermint/consensus_state.rs index dd4ba0ab80..ddea78f6b9 100644 --- a/modules/src/ics07_tendermint/consensus_state.rs +++ b/modules/src/ics07_tendermint/consensus_state.rs @@ -5,10 +5,10 @@ use serde_derive::{Deserialize, Serialize}; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ConsensusState { - root: CommitmentRoot, - height: crate::Height, - timestamp: tendermint::time::Time, - validator_set: tendermint::validator::Set, + pub root: CommitmentRoot, + pub height: crate::Height, + pub timestamp: tendermint::time::Time, + pub validator_set: tendermint::validator::Set, } impl ConsensusState { @@ -68,3 +68,4 @@ mod tests { test_serialization_roundtrip::(json_data); } } + diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index d384cb4bc0..7ee2323451 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -4,4 +4,36 @@ pub mod client_state; pub mod consensus_state; pub mod error; pub mod header; -pub mod msgs; +// pub mod msgs; + +use tendermint::lite::Height; +use tendermint::time::Time; + +use crate::ics02_client::client as ics02; + +pub struct TendermintClient; + +impl ics02::Header for header::Header {} + +impl ics02::ClientState for client_state::ClientState { + fn from_consensus_state(_cs: consensus_state::ConsensusState) -> Self { + todo!() + } + + fn latest_client_height(&self) -> Height { + self.latest_header.signed_header.header.height.into() + } +} + +impl ics02::ConsensusState for consensus_state::ConsensusState { + fn timestamp(&self) -> Time { + self.timestamp + } +} + +impl ics02::ClientDef for TendermintClient { + type Header = header::Header; + type ClientState = client_state::ClientState; + type ConsensusState = consensus_state::ConsensusState; +} + diff --git a/modules/src/ics07_tendermint/msgs/create_client.rs b/modules/src/ics07_tendermint/msgs/create_client.rs index a562b9ea23..53e8935ad7 100644 --- a/modules/src/ics07_tendermint/msgs/create_client.rs +++ b/modules/src/ics07_tendermint/msgs/create_client.rs @@ -1,97 +1,97 @@ -// use crate::ics02_client::client_type::ClientType; -// use crate::ics07_tendermint::consensus_state::ConsensusState; -// use crate::ics07_tendermint::header::Header; -// use crate::ics23_commitment::CommitmentRoot; -// use crate::ics24_host::identifier::ClientId; -// use crate::tx_msg::Msg; - -// use std::time::Duration; - -// use serde_derive::{Deserialize, Serialize}; -// use tendermint::account::Id as AccountId; - -// pub const TYPE_MSG_CREATE_CLIENT: &str = "create_client"; - -// #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -// pub struct MsgCreateClient { -// client_id: ClientId, -// header: Header, -// trusting_period: Duration, -// bonding_period: Duration, -// signer: AccountId, -// } - -// impl MsgCreateClient { -// pub fn new( -// client_id: ClientId, -// header: Header, -// trusting_period: Duration, -// bonding_period: Duration, -// signer: AccountId, -// ) -> Self { -// Self { -// client_id, -// header, -// trusting_period, -// bonding_period, -// signer, -// } -// } - -// fn get_client_id(&self) -> &ClientId { -// &self.client_id -// } - -// fn get_header(&self) -> &Header { -// &self.header -// } -// } - -// impl Msg for MsgCreateClient { -// type ValidationError = crate::ics24_host::error::ValidationError; - -// fn route(&self) -> String { -// crate::keys::ROUTER_KEY.to_string() -// } - -// fn get_type(&self) -> String { -// TYPE_MSG_CREATE_CLIENT.to_string() -// } - -// fn validate_basic(&self) -> Result<(), Self::ValidationError> { -// // Nothing to validate since both ClientId and AccountId perform validation on creation. -// Ok(()) -// } - -// fn get_sign_bytes(&self) -> Vec { -// todo!() -// } - -// fn get_signers(&self) -> Vec { -// vec![self.signer] -// } -// } - -// impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { -// type ConsensusState = ConsensusState; - -// fn client_id(&self) -> &ClientId { -// &self.client_id -// } - -// fn client_type(&self) -> ClientType { -// ClientType::Tendermint -// } - -// fn consensus_state(&self) -> Self::ConsensusState { -// let root = CommitmentRoot; // TODO -// let header = &self.header.signed_header.header; - -// ConsensusState::new( -// root, -// header.height.into(), -// header.time, -// self.header.validator_set.clone(), -// ) -// } -// } +use crate::ics02_client::client_type::ClientType; +use crate::ics07_tendermint::consensus_state::ConsensusState; +use crate::ics07_tendermint::header::Header; +use crate::ics23_commitment::CommitmentRoot; +use crate::ics24_host::identifier::ClientId; +use crate::tx_msg::Msg; + +use std::time::Duration; + +use serde_derive::{Deserialize, Serialize}; +use tendermint::account::Id as AccountId; + +pub const TYPE_MSG_CREATE_CLIENT: &str = "create_client"; + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct MsgCreateClient { + client_id: ClientId, + header: Header, + trusting_period: Duration, + bonding_period: Duration, + signer: AccountId, +} + +impl MsgCreateClient { + pub fn new( + client_id: ClientId, + header: Header, + trusting_period: Duration, + bonding_period: Duration, + signer: AccountId, + ) -> Self { + Self { + client_id, + header, + trusting_period, + bonding_period, + signer, + } + } + + fn get_client_id(&self) -> &ClientId { + &self.client_id + } + + fn get_header(&self) -> &Header { + &self.header + } +} + +impl Msg for MsgCreateClient { + type ValidationError = crate::ics24_host::error::ValidationError; + + fn route(&self) -> String { + crate::keys::ROUTER_KEY.to_string() + } + + fn get_type(&self) -> String { + TYPE_MSG_CREATE_CLIENT.to_string() + } + + fn validate_basic(&self) -> Result<(), Self::ValidationError> { + // Nothing to validate since both ClientId and AccountId perform validation on creation. + Ok(()) + } + + fn get_sign_bytes(&self) -> Vec { + todo!() + } + + fn get_signers(&self) -> Vec { + vec![self.signer] + } +} + +impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { + type ConsensusState = ConsensusState; + + fn client_id(&self) -> &ClientId { + &self.client_id + } + + fn client_type(&self) -> ClientType { + ClientType::Tendermint + } + + fn consensus_state(&self) -> Self::ConsensusState { + let root = CommitmentRoot; // TODO + let header = &self.header.signed_header.header; + + ConsensusState::new( + root, + header.height.into(), + header.time, + self.header.validator_set.clone(), + ) + } +} diff --git a/modules/src/ics07_tendermint/msgs/update_client.rs b/modules/src/ics07_tendermint/msgs/update_client.rs index 725b53f6fa..5b5cb1350a 100644 --- a/modules/src/ics07_tendermint/msgs/update_client.rs +++ b/modules/src/ics07_tendermint/msgs/update_client.rs @@ -1,70 +1,70 @@ -// use crate::ics07_tendermint::header::Header; -// use crate::ics24_host::identifier::ClientId; -// use crate::tx_msg::Msg; +use crate::ics07_tendermint::header::Header; +use crate::ics24_host::identifier::ClientId; +use crate::tx_msg::Msg; -// use serde_derive::{Deserialize, Serialize}; -// use tendermint::account::Id as AccountId; +use serde_derive::{Deserialize, Serialize}; +use tendermint::account::Id as AccountId; -// pub const TYPE_MSG_UPDATE_CLIENT: &str = "update_client"; +pub const TYPE_MSG_UPDATE_CLIENT: &str = "update_client"; -// #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -// pub struct MsgUpdateClient { -// client_id: ClientId, -// header: Header, -// signer: AccountId, -// } +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct MsgUpdateClient { + client_id: ClientId, + header: Header, + signer: AccountId, +} -// impl MsgUpdateClient { -// pub fn new(client_id: ClientId, header: Header, signer: AccountId) -> Self { -// Self { -// client_id, -// header, -// signer, -// } -// } +impl MsgUpdateClient { + pub fn new(client_id: ClientId, header: Header, signer: AccountId) -> Self { + Self { + client_id, + header, + signer, + } + } -// fn get_client_id(&self) -> &ClientId { -// &self.client_id -// } + fn get_client_id(&self) -> &ClientId { + &self.client_id + } -// fn get_header(&self) -> &Header { -// &self.header -// } -// } + fn get_header(&self) -> &Header { + &self.header + } +} -// impl Msg for MsgUpdateClient { -// type ValidationError = crate::ics24_host::error::ValidationError; +impl Msg for MsgUpdateClient { + type ValidationError = crate::ics24_host::error::ValidationError; -// fn route(&self) -> String { -// crate::keys::ROUTER_KEY.to_string() -// } + fn route(&self) -> String { + crate::keys::ROUTER_KEY.to_string() + } -// fn get_type(&self) -> String { -// TYPE_MSG_UPDATE_CLIENT.to_string() -// } + fn get_type(&self) -> String { + TYPE_MSG_UPDATE_CLIENT.to_string() + } -// fn validate_basic(&self) -> Result<(), Self::ValidationError> { -// // Nothing to validate since both ClientId and AccountId perform validation on creation. -// Ok(()) -// } + fn validate_basic(&self) -> Result<(), Self::ValidationError> { + // Nothing to validate since both ClientId and AccountId perform validation on creation. + Ok(()) + } -// fn get_sign_bytes(&self) -> Vec { -// todo!() -// } + fn get_sign_bytes(&self) -> Vec { + todo!() + } -// fn get_signers(&self) -> Vec { -// vec![self.signer] -// } -// } + fn get_signers(&self) -> Vec { + vec![self.signer] + } +} -// impl crate::ics02_client::msgs::MsgUpdateClient for MsgUpdateClient { -// type Header = Header; +impl crate::ics02_client::msgs::MsgUpdateClient for MsgUpdateClient { + type Header = Header; -// fn client_id(&self) -> &ClientId { -// &self.client_id -// } + fn client_id(&self) -> &ClientId { + &self.client_id + } -// fn header(&self) -> &Self::Header { -// &self.header -// } -// } + fn header(&self) -> &Self::Header { + &self.header + } +} From 2014925d1f6de89b62708900faf8a9b9c3df9f04 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 11:12:46 +0200 Subject: [PATCH 11/28] Re-use existing traits --- modules/src/ics02_client/client.rs | 23 ++++--------------- .../src/ics02_client/handler/create_client.rs | 4 ++-- modules/src/ics02_client/state.rs | 1 + modules/src/ics07_tendermint/client_state.rs | 11 +++++++-- modules/src/ics07_tendermint/mod.rs | 21 ----------------- 5 files changed, 16 insertions(+), 44 deletions(-) diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs index 40671845ae..cfc71a6b6d 100644 --- a/modules/src/ics02_client/client.rs +++ b/modules/src/ics02_client/client.rs @@ -1,25 +1,10 @@ //! ICS 02: Client definitions -use tendermint::lite::Height; -use tendermint::time::Time; +use crate::ics02_client::state::{ClientState, ConsensusState}; pub trait ClientDef: Sized { - type Header: Header; - type ClientState: ClientState; - type ConsensusState: ConsensusState; + type Header; + type ClientState: ClientState + From; + type ConsensusState: ConsensusState; } -pub trait Header {} - -pub trait ClientState { - /// Initialise a client state with a provided consensus state. - fn from_consensus_state(cs: CD::ConsensusState) -> Self; - - /// Height of most recent validated header. - fn latest_client_height(&self) -> Height; -} - -pub trait ConsensusState { - /// Returns the timestamp associated with that consensus state. - fn timestamp(&self) -> Time; -} diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 6c369dfc5c..2aeabc9054 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -1,5 +1,5 @@ use crate::handler::{HandlerOutput, HandlerResult}; -use crate::ics02_client::client::{ClientDef, ClientState}; +use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; @@ -39,7 +39,7 @@ where output.log("success: no client type found"); - let client_state = CD::ClientState::from_consensus_state(consensus_state); + let client_state = consensus_state.into(); output.emit(ClientEvent::ClientCreated(client_id.clone())); diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index 9b69cebf5a..55e3fd0b78 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -43,3 +43,4 @@ pub trait ClientState { root: &CommitmentRoot, ) -> Result<(), Self::ValidationError>; } + diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index 9693049bde..b4bdc76f3c 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -1,9 +1,10 @@ use crate::ics02_client::client_type::ClientType; -use crate::ics23_commitment::CommitmentRoot; - +use crate::ics07_tendermint::consensus_state::ConsensusState; use crate::ics07_tendermint::error::{Error, Kind}; use crate::ics07_tendermint::header::Header; +use crate::ics23_commitment::CommitmentRoot; use crate::ics24_host::identifier::ClientId; + use serde_derive::{Deserialize, Serialize}; use std::time::Duration; use tendermint::lite::Header as liteHeader; @@ -64,6 +65,12 @@ impl ClientState { } } +impl From for ClientState { + fn from(_: ConsensusState) -> Self { + todo!() + } +} + impl crate::ics02_client::state::ClientState for ClientState { type ValidationError = Error; diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index 7ee2323451..075c7f53c0 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -6,31 +6,10 @@ pub mod error; pub mod header; // pub mod msgs; -use tendermint::lite::Height; -use tendermint::time::Time; - use crate::ics02_client::client as ics02; pub struct TendermintClient; -impl ics02::Header for header::Header {} - -impl ics02::ClientState for client_state::ClientState { - fn from_consensus_state(_cs: consensus_state::ConsensusState) -> Self { - todo!() - } - - fn latest_client_height(&self) -> Height { - self.latest_header.signed_header.header.height.into() - } -} - -impl ics02::ConsensusState for consensus_state::ConsensusState { - fn timestamp(&self) -> Time { - self.timestamp - } -} - impl ics02::ClientDef for TendermintClient { type Header = header::Header; type ClientState = client_state::ClientState; From d80274a906904eb83b83da66dc099370628a7dfe Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 11:12:47 +0200 Subject: [PATCH 12/28] Add valid test for create_client handler --- modules/src/handler.rs | 2 +- modules/src/ics02_client/client.rs | 2 +- modules/src/ics02_client/handler.rs | 3 +- .../src/ics02_client/handler/create_client.rs | 161 ++++++++++++++++++ modules/src/ics02_client/msgs.rs | 2 + .../ics07_tendermint/msgs/create_client.rs | 1 + 6 files changed, 168 insertions(+), 3 deletions(-) diff --git a/modules/src/handler.rs b/modules/src/handler.rs index 26a39ac4d8..f97fc8e035 100644 --- a/modules/src/handler.rs +++ b/modules/src/handler.rs @@ -51,7 +51,7 @@ impl HandlerOutput { } } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct HandlerOutputBuilder { log: Vec, events: Vec, diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs index cfc71a6b6d..36dc440f2d 100644 --- a/modules/src/ics02_client/client.rs +++ b/modules/src/ics02_client/client.rs @@ -2,7 +2,7 @@ use crate::ics02_client::state::{ClientState, ConsensusState}; -pub trait ClientDef: Sized { +pub trait ClientDef { type Header; type ClientState: ClientState + From; type ConsensusState: ConsensusState; diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index c4be079e2a..1e843df869 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -5,7 +5,7 @@ use crate::ics02_client::error::Error; use crate::ics02_client::msgs::MsgCreateClient; use crate::ics24_host::identifier::ClientId; -use tendermint::lite::Height; +use crate::Height; pub mod create_client; @@ -38,6 +38,7 @@ pub trait ClientKeeper { ) -> Result<(), Error>; } +#[derive(Clone, Debug, PartialEq, Eq)] pub enum ClientEvent { ClientCreated(ClientId), ClientUpdated(ClientId), diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 2aeabc9054..5dcbf3b5b5 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -63,3 +63,164 @@ where Ok(()) } +#[cfg(test)] +mod tests { + use super::*; + use crate::ics02_client::header::Header; + use crate::ics02_client::state::{ClientState, ConsensusState}; + use crate::ics23_commitment::CommitmentRoot; + use crate::Height; + use thiserror::Error; + + #[derive(Debug, Error)] + enum MockError {} + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockHeader(u32); + + impl Header for MockHeader { + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + } + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockClientState(u32); + + impl ClientState for MockClientState { + type ValidationError = MockError; + + fn client_id(&self) -> ClientId { + todo!() + } + + fn client_type(&self) -> ClientType { + todo!() + } + + fn get_latest_height(&self) -> Height { + todo!() + } + + fn is_frozen(&self) -> bool { + todo!() + } + + fn verify_client_consensus_state( + &self, + _root: &CommitmentRoot, + ) -> Result<(), Self::ValidationError> { + todo!() + } + } + + impl From for MockClientState { + fn from(cs: MockConsensusState) -> Self { + Self(cs.0) + } + } + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockConsensusState(u32); + + impl ConsensusState for MockConsensusState { + type ValidationError = MockError; + + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + + fn root(&self) -> &CommitmentRoot { + todo!() + } + + fn validate_basic(&self) -> Result<(), Self::ValidationError> { + todo!() + } + } + + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + struct MockClient; + + impl ClientDef for MockClient { + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; + } + + #[derive(Clone, Debug, PartialEq, Eq)] + struct MockClientContext { + client_state: Option, + client_type: Option, + consensus_state: Option, + } + + impl ClientContext for MockClientContext { + fn client_type(&self, _client_id: &ClientId) -> Option { + self.client_type.clone() + } + + fn client_state(&self, _client_id: &ClientId) -> Option { + self.client_state + } + + fn consensus_state( + &self, + _client_id: &ClientId, + _height: Height, + ) -> Option { + self.consensus_state + } + } + + #[test] + fn test_create_client_ok() { + let mock = MockClientContext { + client_type: None, + client_state: None, + consensus_state: None, + }; + + let msg = MsgCreateClient { + client_id: "mockclient".parse().unwrap(), + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42), + }; + + let output = process(&mock, msg.clone()); + + match output { + Ok(HandlerOutput { + result, + events, + log, + }) => { + assert_eq!(result.client_type, ClientType::Tendermint); + assert_eq!(result.client_state, MockClientState(msg.consensus_state.0)); + assert_eq!( + events, + vec![ClientEvent::ClientCreated(msg.client_id).into()] + ); + assert_eq!( + log, + vec![ + "success: no client state found".to_string(), + "success: no client type found".to_string() + ] + ); + } + Err(err) => { + panic!("unexpected error: {}", err); + } + } + } +} + diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 63abd6fa9c..807ad605d3 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -10,6 +10,7 @@ use crate::ics02_client::client_type::ClientType; use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgCreateClient { pub client_id: ClientId, pub client_type: ClientType, @@ -17,6 +18,7 @@ pub struct MsgCreateClient { } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgUpdateClient { pub client_id: ClientId, pub header: C::Header, diff --git a/modules/src/ics07_tendermint/msgs/create_client.rs b/modules/src/ics07_tendermint/msgs/create_client.rs index 53e8935ad7..19b56ed84f 100644 --- a/modules/src/ics07_tendermint/msgs/create_client.rs +++ b/modules/src/ics07_tendermint/msgs/create_client.rs @@ -95,3 +95,4 @@ impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { ) } } + From 302b083903b77f4a211d3ee70bf1fb7478074d6c Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 14:09:25 +0200 Subject: [PATCH 13/28] Add tests for case where client already exists --- modules/src/ics02_client/error.rs | 3 +- .../src/ics02_client/handler/create_client.rs | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs index 094ff97811..4f4845681e 100644 --- a/modules/src/ics02_client/error.rs +++ b/modules/src/ics02_client/error.rs @@ -5,7 +5,7 @@ use crate::ics24_host::identifier::ClientId; pub type Error = anomaly::Error; -#[derive(Clone, Debug, Error)] +#[derive(Clone, Debug, Error, PartialEq, Eq)] pub enum Kind { #[error("unknown client type")] UnknownClientType, @@ -25,3 +25,4 @@ impl Kind { Context::new(self, Some(source.into())) } } + diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 5dcbf3b5b5..449159e96c 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -6,6 +6,7 @@ use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; use crate::ics02_client::msgs::MsgCreateClient; use crate::ics24_host::identifier::ClientId; +#[derive(Clone, Debug, PartialEq, Eq)] pub struct CreateClientResult { client_id: ClientId, client_type: ClientType, @@ -222,5 +223,51 @@ mod tests { } } } + + #[test] + fn test_create_client_existing_client_type() { + let mock = MockClientContext { + client_type: Some(ClientType::Tendermint), + client_state: None, + consensus_state: None, + }; + + let msg = MsgCreateClient { + client_id: "mockclient".parse().unwrap(), + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42), + }; + + let output = process(&mock, msg.clone()); + + if let Err(err) = output { + assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); + } else { + panic!("expected an error"); + } + } + + #[test] + fn test_create_client_existing_client_state() { + let mock = MockClientContext { + client_type: None, + client_state: Some(MockClientState(0)), + consensus_state: None, + }; + + let msg = MsgCreateClient { + client_id: "mockclient".parse().unwrap(), + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42), + }; + + let output = process(&mock, msg.clone()); + + if let Err(err) = output { + assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); + } else { + panic!("expected an error"); + } + } } From e335fa26311c5f20255b4d133d564d61d789f916 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 14:09:38 +0200 Subject: [PATCH 14/28] WIP: Update client handler --- modules/src/ics02_client/error.rs | 4 + modules/src/ics02_client/handler.rs | 1 + .../src/ics02_client/handler/update_client.rs | 270 ++++++++++++++++++ 3 files changed, 275 insertions(+) create mode 100644 modules/src/ics02_client/handler/update_client.rs diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs index 4f4845681e..f1bfe6f1b1 100644 --- a/modules/src/ics02_client/error.rs +++ b/modules/src/ics02_client/error.rs @@ -2,6 +2,7 @@ use anomaly::{BoxError, Context}; use thiserror::Error; use crate::ics24_host::identifier::ClientId; +use crate::Height; pub type Error = anomaly::Error; @@ -16,6 +17,9 @@ pub enum Kind { #[error("client not found: {0}")] ClientNotFound(ClientId), + #[error("consensus state not found at: {0} at height {1}")] + ConsensusStateNotFound(ClientId, Height), + #[error("implementation specific")] ImplementationSpecific, } diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 1e843df869..650b1f6e7e 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -8,6 +8,7 @@ use crate::ics24_host::identifier::ClientId; use crate::Height; pub mod create_client; +// pub mod update_client; pub trait ClientContext where diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs new file mode 100644 index 0000000000..229e2da9a4 --- /dev/null +++ b/modules/src/ics02_client/handler/update_client.rs @@ -0,0 +1,270 @@ +use crate::handler::{HandlerOutput, HandlerResult}; +use crate::ics02_client::client::ClientDef; +use crate::ics02_client::error::{Error, Kind}; +use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; +use crate::ics02_client::msgs::MsgUpdateClient; +use crate::ics24_host::identifier::ClientId; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpdateClientResult { + client_id: ClientId, + client_state: C::ClientState, + consensus_state: C::ConsensusState, +} + +pub fn process( + ctx: &dyn ClientContext, + msg: MsgUpdateClient, +) -> HandlerResult, Error> +where + CD: ClientDef, +{ + let mut output = HandlerOutput::builder(); + + let MsgUpdateClient { client_id, header } = msg; + + let client_type = ctx + .client_type(&client_id) + .ok_or_else(|| Kind::ClientNotFound(msg.client_id))?; + + let client_state = ctx + .client_state(&client_id) + .ok_or_else(|| Kind::ClientNotFound(msg.client_id))?; + + let consensus_state = ctx + .consensus_state(&client_id, client_state.latest_height()) + .ok_or_else(|| Kind::ConsensusStateNotFound(msg.client_id, client_state.latest_height()))?; + + // client_type.check_validity_and_update_state( + // &mut client_state, + // &mut consensus_state, + // &header, + // )?; + + Ok(output.with_result(UpdateClientResult { + client_id, + client_state, + consensus_state, + })) +} + +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: UpdateClientResult, +) -> Result<(), Error> +where + CD: ClientDef, +{ + keeper.store_client_state(result.client_id.clone(), result.client_state)?; + keeper.store_consensus_state(result.client_id, result.consensus_state)?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ics02_client::header::Header; + use crate::ics02_client::state::{ClientState, ConsensusState}; + use crate::ics23_commitment::CommitmentRoot; + use crate::Height; + use thiserror::Error; + + #[derive(Debug, Error)] + enum MockError {} + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockHeader(u32); + + impl Header for MockHeader { + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + } + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockClientState(u32); + + impl ClientState for MockClientState { + type ValidationError = MockError; + + fn client_id(&self) -> ClientId { + todo!() + } + + fn client_type(&self) -> ClientType { + todo!() + } + + fn get_latest_height(&self) -> Height { + todo!() + } + + fn is_frozen(&self) -> bool { + todo!() + } + + fn verify_client_consensus_state( + &self, + _root: &CommitmentRoot, + ) -> Result<(), Self::ValidationError> { + todo!() + } + } + + impl From for MockClientState { + fn from(cs: MockConsensusState) -> Self { + Self(cs.0) + } + } + + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + struct MockConsensusState(u32); + + impl ConsensusState for MockConsensusState { + type ValidationError = MockError; + + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + + fn root(&self) -> &CommitmentRoot { + todo!() + } + + fn validate_basic(&self) -> Result<(), Self::ValidationError> { + todo!() + } + } + + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + struct MockClient; + + impl ClientDef for MockClient { + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; + } + + #[derive(Clone, Debug, PartialEq, Eq)] + struct MockClientContext { + client_state: Option, + client_type: Option, + consensus_state: Option, + } + + impl ClientContext for MockClientContext { + fn client_type(&self, _client_id: &ClientId) -> Option { + self.client_type.clone() + } + + fn client_state(&self, _client_id: &ClientId) -> Option { + self.client_state + } + + fn consensus_state( + &self, + _client_id: &ClientId, + _height: Height, + ) -> Option { + self.consensus_state + } + } + + #[test] + fn test_update_client_ok() { + let mock = MockClientContext { + client_type: None, + client_state: None, + consensus_state: None, + }; + + let msg = MsgUpdateClient { + client_id: "mockclient".parse().unwrap(), + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42), + }; + + let output = process(&mock, msg.clone()); + + match output { + Ok(HandlerOutput { + result, + events, + log, + }) => { + assert_eq!(result.client_type, ClientType::Tendermint); + assert_eq!(result.client_state, MockClientState(msg.consensus_state.0)); + assert_eq!( + events, + vec![ClientEvent::ClientUpdated(msg.client_id).into()] + ); + assert_eq!( + log, + vec![ + "success: no client state found".to_string(), + "success: no client type found".to_string() + ] + ); + } + Err(err) => { + panic!("unexpected error: {}", err); + } + } + } + + #[test] + fn test_update_client_existing_client_type() { + let mock = MockClientContext { + client_type: Some(ClientType::Tendermint), + client_state: None, + consensus_state: None, + }; + + let msg = MsgUpdateClient { + client_id: "mockclient".parse().unwrap(), + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42), + }; + + let output = process(&mock, msg.clone()); + + if let Err(err) = output { + assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); + } else { + panic!("expected an error"); + } + } + + #[test] + fn test_update_client_existing_client_state() { + let mock = MockClientContext { + client_type: None, + client_state: Some(MockClientState(0)), + consensus_state: None, + }; + + #[allow(unreachable_code)] + let msg = MsgUpdateClient { + client_id: "mockclient".parse().unwrap(), + header: todo!(), + }; + + let output = process(&mock, msg.clone()); + + if let Err(err) = output { + assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); + } else { + panic!("expected an error"); + } + } +} + From fb972a94014e82e390571a7beda4a0b93664db1f Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 7 Aug 2020 16:57:13 +0200 Subject: [PATCH 15/28] Add initial proposal for ADR 003 --- .../003-handler-implementation.md | 552 ++++++++++++++++++ .../src/ics02_client/handler/create_client.rs | 25 +- 2 files changed, 571 insertions(+), 6 deletions(-) create mode 100644 docs/architecture/003-handler-implementation.md diff --git a/docs/architecture/003-handler-implementation.md b/docs/architecture/003-handler-implementation.md new file mode 100644 index 0000000000..f79e706538 --- /dev/null +++ b/docs/architecture/003-handler-implementation.md @@ -0,0 +1,552 @@ +# ADR 003: Handler implementation + +## Changelog +* 2020-08-06: Initial proposal + +## Context + +> This section contains all the context one needs to understand the current state, and why there is a problem. It should be as succinct as possible and introduce the high level idea behind the solution. + +TODO + +## Decision + +In this chapter, we provide recommendation for implementing IBC handlers within the `ibc-rs` crate. +Concepts are introduced in the order given by a topological sort of their dependencies on each other. + +### Events + +IBC handlers must be able to emit events which will then be broadcasted via the node's pub/sub mechanism, +and eventually picked up by the IBC relayer. + +A generic interface for events is provided below, where an event is represented +as a pair of an event type and a list of attributes. An attribute is simply a pair +of a key and a value, both represented as strings. TODO: Describe event type. + +```rust +pub struct Attribute { + key: String, + value: String, +} + +impl Attribute { + pub fn new(key: String, value: String) -> Self; +} + +pub enum EventType { + Message, + Custom(String), +} + +pub struct Event { + tpe: EventType, + attributes: Vec, +} + +impl Event { + pub fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self; +} +``` + +### Logging + +IBC handlers must be able to log information for introspectability and ease of debugging. +A handler can output multiple log records, which are expressed as a pair of a status and a +log line. The interface for emitting log records is described in the next section. + +```rust +pub enum LogStatus { + Success, + Info, + Warning, + Error, +} + +pub struct Log { + status: LogStatus, + body: String, +} + +impl Log { + fn success(msg: impl Display) -> Self; + fn info(msg: impl Display) -> Self; + fn warning(msg: impl Display) -> Self; + fn error(msg: impl Display) -> Self; +} +``` + +### Handler output + +IBC handlers must be able to return arbitrary data, together with events and log records, as descibed above. +As a handler may fail, it is necessary to keep track of errors. + +To this end, we introduce a type for the return value of a handler: + +```rust +pub type HandlerResult = Result, E>; + +pub struct HandlerOutput { + pub result: T, + pub log: Vec, + pub events: Vec, +} +``` + +We introduce a builder interface to be used within the handler implementation to incrementally build a `HandlerOutput` value. + +```rust +impl HandlerOutput { + pub fn builder() -> HandlerOutputBuilder { + HandlerOutputBuilder::new() + } +} + +pub struct HandlerOutputBuilder { + log: Vec, + events: Vec, + marker: PhantomData, +} + +impl HandlerOutputBuilder { + pub fn log(&mut self, log: impl Into); + pub fn emit(&mut self, event: impl Into); + pub fn with_result(self, result: T) -> HandlerOutput; +} +``` + +We provide below an example usage of the builder API: + +```rust +fn some_ibc_handler() -> HandlerResult { + let mut output = HandlerOutput::builder(); + + // ... + + output.log(Log::info("did something")) + + // ... + + output.log(Log::success("all good")); + output.emit(SomeEvent::AllGood); + + Ok(output.with_result(42)); +} +``` + +### IBC Submodule + +The various IBC messages and their handlers, as described in the IBC specification, +are split into a collection of submodules, each pertaining to a specific aspect of +the IBC protocol, eg. client lifecycle management, connection lifecycle management, +packet relay, etc. + +In this section we propose a general approach to implement the handlers for a submodule. +To make things more concrete, we will use the ICS 002 Client submodule as a +running example, but the methodology outlined here should apply to any submodule. +This specific module also has the peculiarity of dealing with datatypes which +are specific to a given type of chain, eg. Tendermint. This incurs the need for +abstracting over these datatypes, which introduces additional abstractions which +may not be needed for other submodules. + +#### Events + +The events which may be emitted by the handlers of a submodule should be defined +as an enumeration, while a way of converting those into the generic `Event` type +defined in a previous section should be provided via the `From` trait. + +We provide below an implementation of both for the ICS 002 Client submodule: + +```rust +pub enum ClientEvent { + ClientCreated(ClientId), + ClientUpdated(ClientId), +} + +impl From for Event { + fn from(ce: ClientEvent) -> Event { + match ce { + ClientEvent::ClientCreated(client_id) => Event::new( + EventType::Custom("ClientCreated".to_string()), + vec![("client_id".to_string(), client_id.to_string())], + ), + ClientEvent::ClientUpdated(client_id) => Event::new( + EventType::Custom("ClientUpdated".to_string()), + vec![("client_id".to_string(), client_id.to_string())], + ), + } + } +} +``` + +#### Abstracting over chain-specific datatypes + +To abstract over chain-specific datatypes, we introduce a trait which specifies +both which types we need to abstract over and their interface. For the Client submodule, +this trait looks as follow: + +```rust +pub trait ClientDef { + type Header: Header; + type ClientState: ClientState + From; + type ConsensusState: ConsensusState; +} +``` + +This trait specifies three datatypes, and their corresponding interface, which is provided +via a trait defined in the same submodule. +Additionally, this trait expresses the requirement for a `ClientState` to be built from +a `ConsensusState`. + +A production implementation of this interface would instantiate these types with the concrete +types used by the chain. + +For the purpose of unit-testing, a mock implementation of the `ClientDef` trait could look as +follows: + +```rust +struct MockHeader(u32); +impl Header for MockHeader { + // omitted +} + +struct MockClientState(u32); +impl ClientState for MockClientState { + // omitted +} + +impl From for MockClientState { + fn from(cs: MockConsensusState) -> Self { + Self(cs.0) + } +} + +struct MockConsensusState(u32); +impl ConsensusState for MockConsensusState { + // omitted +} + +struct MockClient; + +impl ClientDef for MockClient { + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; +} +``` + +#### Reader + +A typical handler will need to read data from the chain state at the current height, +via the private and provable stores. + +To avoid coupling between the handler interface and the store API, we introduce an interface +for accessing this data. This interface is shared between all handlers in a submodule, as +those typically access the same data. + +Having a high-level interface for this purpose helps avoiding coupling which makes +writing unit tests for the handlers easier, as one does not need to provide a concrete +store, or to mock one. + +We provide below the definition of such an interface, called a `Reader` for the ICS 02 Client submodule: + +```rust +pub trait ClientReader +where + CD: ClientDef, +{ + fn client_type(&self, client_id: &ClientId) -> Option; + fn client_state(&self, client_id: &ClientId) -> Option; + fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; +} +``` + +Because the data exposed by this `Reader` is chain-specific, the `Reader` trait is parametrized +by the type of chain, via the `ClientDef` trait bound. Other submodules may not need the generality +and do away with the type parameter and trait bound altogether. + +A production implementation of this `Reader` would hold references to both the private and provable +store at the current height where the handler executes, but we omit the actual implementation as +the store interfaces are yet to be defined, as is the general IBC top-level module machinery. + +A mock implementation of the `ClientReader` trait could look as follows, given the `MockClient` +definition provided in the previous section. + +```rust +struct MockClientContext { + client_id: ClientId, + client_state: Option, + client_type: Option, + consensus_state: Option, +} + +impl ClientContext for MockClientContext { + fn client_type(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_type.clone() + } else { + None + } + } + + fn client_state(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_state + } else { + None + } + } + + fn consensus_state(&self, client_id: &ClientId, _height: Height) -> Option { + if client_id == &self.client_id { + self.consensus_state + } else { + None + } + } +} +``` + +#### Keeper + +Once a handler executes successfully, some data will typically need to be persisted in the chain state +via the private/provable store interfaces. In the same vein as for the reader defined in the previous section, +a submodule should define a trait which provides operations to persist such data. +The same considerations w.r.t. to coupling and unit-testing apply here as well. + +We give below a version of this keeper trait for the Client submodule: + +```rust +pub trait ClientKeeper { + fn store_client_type( + &mut self, + client_id: ClientId, + client_type: ClientType, + ) -> Result<(), Error>; + + fn store_client_state( + &mut self, + client_id: ClientId, + client_state: CD::ClientState, + ) -> Result<(), Error>; + + fn store_consensus_state( + &mut self, + client_id: ClientId, + consensus_state: CD::ConsensusState, + ) -> Result<(), Error>; +} +``` + +Other submodules may not need the generality and do away with the type parameter and trait +bound altogether. + +#### Handler implementation + +We now come to the actual definition of a handler for a submodule. + +We recommend each handler to be defined within its own Rust module, named +after the handler itself. For example, the "Create Client" handler of ICS 002 would +be defined in `ibc_modules::ics02_client::handler::create_client`. + +##### Message type + +Each handler must define a datatype which represent the message it can process. +For the "Create Client" handler of ICS 002, the message would look as follows: + +```rust +pub struct MsgCreateClient { + pub client_id: ClientId, + pub client_type: ClientType, + pub consensus_state: C::ConsensusState, +} +``` + +Again, because the message mentions chain-specific datatypes, it must be parametrized by +the type of chain, bounded by the `ClientDef` trait defined in an earlier section. +Other submodules may not need the generality and do away with the type parameter and trait +bound altogether. + +##### Handler implementation + +In this section we provide guidelines for implementating an actual handler. + +We divide the handler in two parts: processing and persistance. + +###### Processing + +The actual logic of the handler is expressed as a pure function, typically named +`process`, which takes as arguments a `Reader` and the corresponding message, and returns +a `HandlerOutput`, where `T` is a concrete datatype and `E` is an error type which defines +all potential errors yielded by the handlers of the current submodule. + +For the "Create Client" handler of ICS 002, `T` would be defined as the following datatype: + +```rust +pub struct CreateClientResult { + client_id: ClientId, + client_type: ClientType, + client_state: CD::ClientState, +} +``` + +The `process` function will typically read data via the `Reader`, perform checks and validation, construct new +datatypes, emit log records and events, and eventually return some data together with objects to be persisted. + +To this end, this `process` function will create and manipulate a `HandlerOutput` value like described in +the corresponding section. + +We provide below the actual implementation of the `process` function for the "Create Client" handler of ICS 002: + +```rust +pub fn process( + reader: &dyn ClientReader, + msg: MsgCreateClient, +) -> HandlerResult, Error> +where + CD: ClientDef, +{ + let mut output = HandlerOutput::builder(); + + let MsgCreateClient { + client_id, + client_type, + consensus_state, + } = msg; + + if reader.client_state(&client_id).is_some() { + return Err(Kind::ClientAlreadyExists(client_id).into()); + } + + output.log("success: no client state found"); + + if reader.client_type(&client_id).is_some() { + return Err(Kind::ClientAlreadyExists(client_id).into()); + } + + output.log("success: no client type found"); + + let client_state = consensus_state.into(); + + output.emit(ClientEvent::ClientCreated(client_id.clone())); + + Ok(output.with_result(CreateClientResult { + client_id, + client_type, + client_state, + })) +} +``` + +Again, because this handler deals with chain-specific data, the `process` function is parametrized +by the type of chain, via the `ClientDef` trait bound. Other submodules or handlers may not need the generality +and do away with the type parameter and trait bound altogether. + +###### Persistence + +If the `process` function specified above succeeds, the result value it yielded is then +passed to a function named `keep`, which is responsible for persisting the objects constructed +by the processing function. This `keep` function takes the submodule's `Keeper` and the result +type defined above, and performs side-effecting calls to the keeper's methods to persist the result. + +Below is given an implementation of the `keep` function for the "Create Client" handler: + +```rust +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: CreateClientResult, +) -> Result<(), Error> +where + CD: ClientDef, +{ + keeper.store_client_state(result.client_id.clone(), result.client_state)?; + keeper.store_client_type(result.client_id, result.client_type)?; + + Ok(()) +} +``` + +##### Submodule handler + +> This section is very much a work in progress, as further investigation into what +> a production-ready implementation of the `ctx` parameter of the top-level handler +> is required. As such, implementors should feel free to disregard the recommendations +> below, and are encouraged to come up with amendments to this ADR to better capture +> the actual requirements. + +Each submodule is responsible for dispatching the messages it is given to the appropriate +handler processing function and, if successful, pass the resulting data to the persistance +function defined in the previous section. + +To this end, the submodule should define an enumeration of all handlers messages, in order +for the top-level submodule handler to dispatch them. Such a definition for the ICS 002 Client +submodule is given below. + +```rust +pub enum ClientMsg { + CreateClient(MsgCreateClient), + UpdateClient(UpdateClientMsg), +} +``` + +Because the messages mention chain-specific datatypes, the whole enumeration must be parametrized by +the type of chain, bounded by the `ClientDef` trait defined in an earlier section. +Other submodules may not need the generality and do away with the type parameter and trait +bound altogether. + +The actual implementation of a submodule handler is quite straightforward and unlikely to vary +much in substance between submodules. We give an implementation for the ICS 002 Client module below. + +```rust +pub fn handler(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +where + Client: ClientDef, + Ctx: ClientContext + ClientKeeper, +{ + match msg { + ClientMsg::CreateClient(msg) => { + let HandlerOutput { + result, + log, + events, + } = create_client::process(ctx, msg)?; + + create_client::keep(ctx, result)?; + + Ok(HandlerOutput::builder() + .with_log(log) + .with_events(events) + .with_result(())) + } + + ClientMsg::UpdateClient(msg) => // omitted + } +} +``` + +In essence, a top-level handler is a function of a message wrapped in the enumeration introduced above, +and a "context" which implements both the `Reader` and `Keeper` interfaces. + +It is currently not clear if such a requirement is actually viable, as handlers might need to access +a `Keeper` for various chain types known only at runtime, which would prevent having a static bound +on the context, as expressed above. Further investigations are required to sort this out, hence the +disclaimer at the beginning of this section. + +## Status + +Proposed + +## Consequences + +> This section describes the consequences, after applying the decision. All consequences should be summarized here, not just the "positive" ones. + +### Positive + +### Negative + +### Neutral + +## References + +> Are there any relevant PR comments, issues that led up to this, or articles refernced for why we made the given design choice? If so link them here! + +* {reference link} diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 449159e96c..fe3bf245d7 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -159,26 +159,39 @@ mod tests { #[derive(Clone, Debug, PartialEq, Eq)] struct MockClientContext { + client_id: ClientId, client_state: Option, client_type: Option, consensus_state: Option, } impl ClientContext for MockClientContext { - fn client_type(&self, _client_id: &ClientId) -> Option { - self.client_type.clone() + fn client_type(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_type.clone() + } else { + None + } } - fn client_state(&self, _client_id: &ClientId) -> Option { - self.client_state + fn client_state(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_state + } else { + None + } } fn consensus_state( &self, - _client_id: &ClientId, + client_id: &ClientId, _height: Height, ) -> Option { - self.consensus_state + if client_id == &self.client_id { + self.consensus_state + } else { + None + } } } From add693e04971de62462f3484eb28329dbf6e3c16 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Mon, 10 Aug 2020 15:06:09 +0200 Subject: [PATCH 16/28] Rename file to adr-003-handler-implementation.md --- ...andler-implementation.md => adr-003-handler-implementation.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/architecture/{003-handler-implementation.md => adr-003-handler-implementation.md} (100%) diff --git a/docs/architecture/003-handler-implementation.md b/docs/architecture/adr-003-handler-implementation.md similarity index 100% rename from docs/architecture/003-handler-implementation.md rename to docs/architecture/adr-003-handler-implementation.md From 9939b39b6e3f2a1f7b515faaa736275c2b9f2046 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Mon, 10 Aug 2020 15:20:26 +0200 Subject: [PATCH 17/28] Replace "handler" with "message processor" in plain text --- .../adr-003-handler-implementation.md | 102 +++++++++--------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/docs/architecture/adr-003-handler-implementation.md b/docs/architecture/adr-003-handler-implementation.md index f79e706538..08b1110753 100644 --- a/docs/architecture/adr-003-handler-implementation.md +++ b/docs/architecture/adr-003-handler-implementation.md @@ -1,4 +1,4 @@ -# ADR 003: Handler implementation +# ADR 003: IBC protocol implementation ## Changelog * 2020-08-06: Initial proposal @@ -11,17 +11,23 @@ TODO ## Decision -In this chapter, we provide recommendation for implementing IBC handlers within the `ibc-rs` crate. +In this ADR, we provide recommendations for implementing the IBC message processing logic within the `ibc-rs` crate. Concepts are introduced in the order given by a topological sort of their dependencies on each other. ### Events -IBC handlers must be able to emit events which will then be broadcasted via the node's pub/sub mechanism, +IBC message processors must be able to emit events which will then be broadcasted via the node's pub/sub mechanism, and eventually picked up by the IBC relayer. A generic interface for events is provided below, where an event is represented as a pair of an event type and a list of attributes. An attribute is simply a pair -of a key and a value, both represented as strings. TODO: Describe event type. +of a key and a value, both represented as strings. + +Here is the [list of all IBB-related events][events], as seen by the relayer. +Because the structure of these events do not match the ones which are emitted by the IBC message processors, +each IBC submodule should defined its own event type and associated variants. + +[events]: https://github.com/informalsystems/ibc-rs/blob/bf84a73ef7b3d5e9a434c9af96165997382dcc9d/modules/src/events.rs#L15-L43 ```rust pub struct Attribute { @@ -39,19 +45,19 @@ pub enum EventType { } pub struct Event { - tpe: EventType, + typ: EventType, attributes: Vec, } impl Event { - pub fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self; + pub fn new(typ: EventType, attrs: Vec<(String, String)>) -> Self; } ``` ### Logging -IBC handlers must be able to log information for introspectability and ease of debugging. -A handler can output multiple log records, which are expressed as a pair of a status and a +IBC message processors must be able to log information for introspectability and ease of debugging. +A message processor can output multiple log records, which are expressed as a pair of a status and a log line. The interface for emitting log records is described in the next section. ```rust @@ -75,12 +81,12 @@ impl Log { } ``` -### Handler output +### Message processor output -IBC handlers must be able to return arbitrary data, together with events and log records, as descibed above. -As a handler may fail, it is necessary to keep track of errors. +IBC message processors must be able to return arbitrary data, together with events and log records, as descibed above. +As a message processor may fail, it is necessary to keep track of errors. -To this end, we introduce a type for the return value of a handler: +To this end, we introduce a type for the return value of a message processor: ```rust pub type HandlerResult = Result, E>; @@ -92,7 +98,7 @@ pub struct HandlerOutput { } ``` -We introduce a builder interface to be used within the handler implementation to incrementally build a `HandlerOutput` value. +We introduce a builder interface to be used within the message processor implementation to incrementally build a `HandlerOutput` value. ```rust impl HandlerOutput { @@ -135,12 +141,12 @@ fn some_ibc_handler() -> HandlerResult { ### IBC Submodule -The various IBC messages and their handlers, as described in the IBC specification, +The various IBC messages and their processing logic, as described in the IBC specification, are split into a collection of submodules, each pertaining to a specific aspect of the IBC protocol, eg. client lifecycle management, connection lifecycle management, packet relay, etc. -In this section we propose a general approach to implement the handlers for a submodule. +In this section we propose a general approach to implement the message processors for a submodule. To make things more concrete, we will use the ICS 002 Client submodule as a running example, but the methodology outlined here should apply to any submodule. This specific module also has the peculiarity of dealing with datatypes which @@ -150,7 +156,7 @@ may not be needed for other submodules. #### Events -The events which may be emitted by the handlers of a submodule should be defined +The events which may be emitted by the message processors of a submodule should be defined as an enumeration, while a way of converting those into the generic `Event` type defined in a previous section should be provided via the `From` trait. @@ -236,15 +242,15 @@ impl ClientDef for MockClient { #### Reader -A typical handler will need to read data from the chain state at the current height, +A typical message processor will need to read data from the chain state at the current height, via the private and provable stores. -To avoid coupling between the handler interface and the store API, we introduce an interface -for accessing this data. This interface is shared between all handlers in a submodule, as +To avoid coupling between the message processor interface and the store API, we introduce an interface +for accessing this data. This interface is shared between all message processors in a submodule, as those typically access the same data. Having a high-level interface for this purpose helps avoiding coupling which makes -writing unit tests for the handlers easier, as one does not need to provide a concrete +writing unit tests for the message processors easier, as one does not need to provide a concrete store, or to mock one. We provide below the definition of such an interface, called a `Reader` for the ICS 02 Client submodule: @@ -265,7 +271,7 @@ by the type of chain, via the `ClientDef` trait bound. Other submodules may not and do away with the type parameter and trait bound altogether. A production implementation of this `Reader` would hold references to both the private and provable -store at the current height where the handler executes, but we omit the actual implementation as +store at the current height where the message processor executes, but we omit the actual implementation as the store interfaces are yet to be defined, as is the general IBC top-level module machinery. A mock implementation of the `ClientReader` trait could look as follows, given the `MockClient` @@ -308,7 +314,7 @@ impl ClientContext for MockClientContext { #### Keeper -Once a handler executes successfully, some data will typically need to be persisted in the chain state +Once a message processor executes successfully, some data will typically need to be persisted in the chain state via the private/provable store interfaces. In the same vein as for the reader defined in the previous section, a submodule should define a trait which provides operations to persist such data. The same considerations w.r.t. to coupling and unit-testing apply here as well. @@ -340,18 +346,18 @@ pub trait ClientKeeper { Other submodules may not need the generality and do away with the type parameter and trait bound altogether. -#### Handler implementation +#### Submodule implementation -We now come to the actual definition of a handler for a submodule. +We now come to the actual definition of a message processor for a submodule. -We recommend each handler to be defined within its own Rust module, named -after the handler itself. For example, the "Create Client" handler of ICS 002 would +We recommend each message processor to be defined within its own Rust module, named +after the message processor itself. For example, the "Create Client" message processor of ICS 002 would be defined in `ibc_modules::ics02_client::handler::create_client`. ##### Message type -Each handler must define a datatype which represent the message it can process. -For the "Create Client" handler of ICS 002, the message would look as follows: +Each message processor must define a datatype which represent the message it can process. +For the "Create Client" sub-protocol of ICS 002, the message would look as follows: ```rust pub struct MsgCreateClient { @@ -366,20 +372,20 @@ the type of chain, bounded by the `ClientDef` trait defined in an earlier sectio Other submodules may not need the generality and do away with the type parameter and trait bound altogether. -##### Handler implementation +##### Message processor implementation -In this section we provide guidelines for implementating an actual handler. +In this section we provide guidelines for implementating an actual message processor. -We divide the handler in two parts: processing and persistance. +We divide the message processor in two parts: processing and persistance. ###### Processing -The actual logic of the handler is expressed as a pure function, typically named +The actual logic of the message processor is expressed as a pure function, typically named `process`, which takes as arguments a `Reader` and the corresponding message, and returns a `HandlerOutput`, where `T` is a concrete datatype and `E` is an error type which defines -all potential errors yielded by the handlers of the current submodule. +all potential errors yielded by the message processors of the current submodule. -For the "Create Client" handler of ICS 002, `T` would be defined as the following datatype: +For the "Create Client" sub-protocol of ICS 002, `T` would be defined as the following datatype: ```rust pub struct CreateClientResult { @@ -395,7 +401,7 @@ datatypes, emit log records and events, and eventually return some data together To this end, this `process` function will create and manipulate a `HandlerOutput` value like described in the corresponding section. -We provide below the actual implementation of the `process` function for the "Create Client" handler of ICS 002: +We provide below the actual implementation of the `process` function for the "Create Client" sub-protocol of ICS 002: ```rust pub fn process( @@ -437,8 +443,8 @@ where } ``` -Again, because this handler deals with chain-specific data, the `process` function is parametrized -by the type of chain, via the `ClientDef` trait bound. Other submodules or handlers may not need the generality +Again, because this message processor deals with chain-specific data, the `process` function is parametrized +by the type of chain, via the `ClientDef` trait bound. Other submodules or messages may not need the generality and do away with the type parameter and trait bound altogether. ###### Persistence @@ -448,7 +454,7 @@ passed to a function named `keep`, which is responsible for persisting the objec by the processing function. This `keep` function takes the submodule's `Keeper` and the result type defined above, and performs side-effecting calls to the keeper's methods to persist the result. -Below is given an implementation of the `keep` function for the "Create Client" handler: +Below is given an implementation of the `keep` function for the "Create Client" message processors: ```rust pub fn keep( @@ -465,21 +471,21 @@ where } ``` -##### Submodule handler +##### Submodule dispatcher > This section is very much a work in progress, as further investigation into what -> a production-ready implementation of the `ctx` parameter of the top-level handler +> a production-ready implementation of the `ctx` parameter of the top-level dispatcher > is required. As such, implementors should feel free to disregard the recommendations > below, and are encouraged to come up with amendments to this ADR to better capture > the actual requirements. Each submodule is responsible for dispatching the messages it is given to the appropriate -handler processing function and, if successful, pass the resulting data to the persistance +message processing function and, if successful, pass the resulting data to the persistance function defined in the previous section. -To this end, the submodule should define an enumeration of all handlers messages, in order -for the top-level submodule handler to dispatch them. Such a definition for the ICS 002 Client -submodule is given below. +To this end, the submodule should define an enumeration of all messages, in order +for the top-level submodule dispatcher to forward them to the appropriate processor. +Such a definition for the ICS 002 Client submodule is given below. ```rust pub enum ClientMsg { @@ -493,11 +499,11 @@ the type of chain, bounded by the `ClientDef` trait defined in an earlier sectio Other submodules may not need the generality and do away with the type parameter and trait bound altogether. -The actual implementation of a submodule handler is quite straightforward and unlikely to vary +The actual implementation of a submodule dispatcher is quite straightforward and unlikely to vary much in substance between submodules. We give an implementation for the ICS 002 Client module below. ```rust -pub fn handler(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where Client: ClientDef, Ctx: ClientContext + ClientKeeper, @@ -523,10 +529,10 @@ where } ``` -In essence, a top-level handler is a function of a message wrapped in the enumeration introduced above, +In essence, a top-level dispatcher is a function of a message wrapped in the enumeration introduced above, and a "context" which implements both the `Reader` and `Keeper` interfaces. -It is currently not clear if such a requirement is actually viable, as handlers might need to access +It is currently not clear if such a requirement is actually viable, as message processors might need to access a `Keeper` for various chain types known only at runtime, which would prevent having a static bound on the context, as expressed above. Further investigations are required to sort this out, hence the disclaimer at the beginning of this section. From 5b85658e55994b0d19f480afcae7e3ed042423b2 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 11 Aug 2020 10:53:50 +0200 Subject: [PATCH 18/28] Formatting --- modules/src/handler.rs | 1 - modules/src/ics02_client/client.rs | 1 - modules/src/ics02_client/error.rs | 1 - modules/src/ics02_client/handler.rs | 1 - modules/src/ics02_client/handler/create_client.rs | 1 - modules/src/ics02_client/msgs.rs | 1 - modules/src/ics02_client/state.rs | 1 - modules/src/ics07_tendermint/client_state.rs | 1 - modules/src/ics07_tendermint/consensus_state.rs | 1 - modules/src/ics07_tendermint/mod.rs | 1 - 10 files changed, 10 deletions(-) diff --git a/modules/src/handler.rs b/modules/src/handler.rs index f97fc8e035..05448f412f 100644 --- a/modules/src/handler.rs +++ b/modules/src/handler.rs @@ -93,4 +93,3 @@ impl HandlerOutputBuilder { } } } - diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs index 36dc440f2d..55491b30ec 100644 --- a/modules/src/ics02_client/client.rs +++ b/modules/src/ics02_client/client.rs @@ -7,4 +7,3 @@ pub trait ClientDef { type ClientState: ClientState + From; type ConsensusState: ConsensusState; } - diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs index f1bfe6f1b1..7c1acb0b4b 100644 --- a/modules/src/ics02_client/error.rs +++ b/modules/src/ics02_client/error.rs @@ -29,4 +29,3 @@ impl Kind { Context::new(self, Some(source.into())) } } - diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 650b1f6e7e..99f6c2c1e4 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -87,4 +87,3 @@ where } } } - diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index fe3bf245d7..6833471be6 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -283,4 +283,3 @@ mod tests { } } } - diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 807ad605d3..4648734b3e 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -23,4 +23,3 @@ pub struct MsgUpdateClient { pub client_id: ClientId, pub header: C::Header, } - diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index 55e3fd0b78..9b69cebf5a 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -43,4 +43,3 @@ pub trait ClientState { root: &CommitmentRoot, ) -> Result<(), Self::ValidationError>; } - diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index b4bdc76f3c..5da32bda7c 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -222,4 +222,3 @@ mod tests { } } } - diff --git a/modules/src/ics07_tendermint/consensus_state.rs b/modules/src/ics07_tendermint/consensus_state.rs index ddea78f6b9..2e205f54ed 100644 --- a/modules/src/ics07_tendermint/consensus_state.rs +++ b/modules/src/ics07_tendermint/consensus_state.rs @@ -68,4 +68,3 @@ mod tests { test_serialization_roundtrip::(json_data); } } - diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index 075c7f53c0..22fcd9ee2f 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -15,4 +15,3 @@ impl ics02::ClientDef for TendermintClient { type ClientState = client_state::ClientState; type ConsensusState = consensus_state::ConsensusState; } - From 6b86998cef2a6776639fee9f9ab5c747439c9293 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 11 Aug 2020 13:42:02 +0200 Subject: [PATCH 19/28] Fix create client handler tests --- .../src/ics02_client/handler/create_client.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 6833471be6..f33f014f3f 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -197,14 +197,17 @@ mod tests { #[test] fn test_create_client_ok() { + let client_id: ClientId = "mockclient".parse().unwrap(); + let mock = MockClientContext { + client_id: client_id.clone(), client_type: None, client_state: None, consensus_state: None, }; let msg = MsgCreateClient { - client_id: "mockclient".parse().unwrap(), + client_id, client_type: ClientType::Tendermint, consensus_state: MockConsensusState(42), }; @@ -239,14 +242,17 @@ mod tests { #[test] fn test_create_client_existing_client_type() { + let client_id: ClientId = "mockclient".parse().unwrap(); + let mock = MockClientContext { + client_id: client_id.clone(), client_type: Some(ClientType::Tendermint), client_state: None, consensus_state: None, }; let msg = MsgCreateClient { - client_id: "mockclient".parse().unwrap(), + client_id, client_type: ClientType::Tendermint, consensus_state: MockConsensusState(42), }; @@ -262,14 +268,17 @@ mod tests { #[test] fn test_create_client_existing_client_state() { + let client_id: ClientId = "mockclient".parse().unwrap(); + let mock = MockClientContext { + client_id: client_id.clone(), client_type: None, client_state: Some(MockClientState(0)), consensus_state: None, }; let msg = MsgCreateClient { - client_id: "mockclient".parse().unwrap(), + client_id, client_type: ClientType::Tendermint, consensus_state: MockConsensusState(42), }; @@ -283,3 +292,4 @@ mod tests { } } } + From b1c2f2d88a79c1f62c089815f907cae4fdffda49 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 11 Aug 2020 13:42:30 +0200 Subject: [PATCH 20/28] Rename module handler to dispatch --- modules/src/ics02_client/handler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 99f6c2c1e4..ca62cb27be 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -65,7 +65,7 @@ pub enum ClientMsg { // UpdateClient(UpdateClientMsg), } -pub fn handler(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where CD: ClientDef, Ctx: ClientContext + ClientKeeper, @@ -87,3 +87,4 @@ where } } } + From 375d61efbe07876f0ebb736953619d935f53ebb1 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 11 Aug 2020 14:13:50 +0200 Subject: [PATCH 21/28] Formatting --- modules/src/ics02_client/handler.rs | 1 - modules/src/ics02_client/handler/create_client.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index ca62cb27be..b40cd24946 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -87,4 +87,3 @@ where } } } - diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index f33f014f3f..adae41057c 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -292,4 +292,3 @@ mod tests { } } } - From 8551bbce22ba523f2f3b7f8501203f002a35d4d3 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 13 Aug 2020 13:42:56 +0200 Subject: [PATCH 22/28] Add sketch of update client processor --- modules/src/ics02_client/client.rs | 12 +++- modules/src/ics02_client/handler.rs | 26 +++++-- .../src/ics02_client/handler/create_client.rs | 24 +++++-- .../src/ics02_client/handler/update_client.rs | 68 +++++++++++-------- modules/src/ics02_client/state.rs | 1 + modules/src/ics07_tendermint/mod.rs | 10 +++ 6 files changed, 99 insertions(+), 42 deletions(-) diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs index 55491b30ec..7f2d74866f 100644 --- a/modules/src/ics02_client/client.rs +++ b/modules/src/ics02_client/client.rs @@ -2,8 +2,18 @@ use crate::ics02_client::state::{ClientState, ConsensusState}; -pub trait ClientDef { +pub trait ClientDef: Sized { + type Error: std::error::Error; + type Header; type ClientState: ClientState + From; type ConsensusState: ConsensusState; + + /// TODO + fn check_validity_and_update_state( + client_state: &mut Self::ClientState, + consensus_state: &Self::ConsensusState, + header: &Self::Header, + ) -> Result<(), Self::Error>; } + diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index b40cd24946..59ad680e7a 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -2,15 +2,15 @@ use crate::handler::{Event, EventType, HandlerOutput}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::Error; -use crate::ics02_client::msgs::MsgCreateClient; +use crate::ics02_client::msgs::{MsgCreateClient, MsgUpdateClient}; use crate::ics24_host::identifier::ClientId; use crate::Height; pub mod create_client; -// pub mod update_client; +pub mod update_client; -pub trait ClientContext +pub trait ClientReader where CD: ClientDef, { @@ -62,13 +62,13 @@ impl From for Event { pub enum ClientMsg { CreateClient(MsgCreateClient), - // UpdateClient(UpdateClientMsg), + UpdateClient(MsgUpdateClient), } pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where CD: ClientDef, - Ctx: ClientContext + ClientKeeper, + Ctx: ClientReader + ClientKeeper, { match msg { ClientMsg::CreateClient(msg) => { @@ -85,5 +85,21 @@ where .with_events(events) .with_result(())) } + + ClientMsg::UpdateClient(msg) => { + let HandlerOutput { + result, + log, + events, + } = update_client::process(ctx, msg)?; + + update_client::keep(ctx, result)?; + + Ok(HandlerOutput::builder() + .with_log(log) + .with_events(events) + .with_result(())) + } } } + diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index adae41057c..4861afa403 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -2,7 +2,7 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; -use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; +use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgCreateClient; use crate::ics24_host::identifier::ClientId; @@ -14,7 +14,7 @@ pub struct CreateClientResult { } pub fn process( - ctx: &dyn ClientContext, + ctx: &dyn ClientReader, msg: MsgCreateClient, ) -> HandlerResult, Error> where @@ -152,20 +152,29 @@ mod tests { struct MockClient; impl ClientDef for MockClient { + type Error = MockError; type Header = MockHeader; type ClientState = MockClientState; type ConsensusState = MockConsensusState; + + fn check_validity_and_update_state( + _client_state: &mut MockClientState, + _consensus_state: &MockConsensusState, + _header: &MockHeader, + ) -> Result<(), Self::Error> { + todo!() + } } #[derive(Clone, Debug, PartialEq, Eq)] - struct MockClientContext { + struct MockClientReader { client_id: ClientId, client_state: Option, client_type: Option, consensus_state: Option, } - impl ClientContext for MockClientContext { + impl ClientReader for MockClientReader { fn client_type(&self, client_id: &ClientId) -> Option { if client_id == &self.client_id { self.client_type.clone() @@ -199,7 +208,7 @@ mod tests { fn test_create_client_ok() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientContext { + let mock = MockClientReader { client_id: client_id.clone(), client_type: None, client_state: None, @@ -244,7 +253,7 @@ mod tests { fn test_create_client_existing_client_type() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientContext { + let mock = MockClientReader { client_id: client_id.clone(), client_type: Some(ClientType::Tendermint), client_state: None, @@ -270,7 +279,7 @@ mod tests { fn test_create_client_existing_client_state() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientContext { + let mock = MockClientReader { client_id: client_id.clone(), client_type: None, client_state: Some(MockClientState(0)), @@ -292,3 +301,4 @@ mod tests { } } } + diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs index 229e2da9a4..e216bb4809 100644 --- a/modules/src/ics02_client/handler/update_client.rs +++ b/modules/src/ics02_client/handler/update_client.rs @@ -1,8 +1,9 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::error::{Error, Kind}; -use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper}; +use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgUpdateClient; +use crate::ics02_client::state::ClientState; use crate::ics24_host::identifier::ClientId; #[derive(Clone, Debug, PartialEq, Eq)] @@ -13,7 +14,7 @@ pub struct UpdateClientResult { } pub fn process( - ctx: &dyn ClientContext, + ctx: &dyn ClientReader, msg: MsgUpdateClient, ) -> HandlerResult, Error> where @@ -23,23 +24,22 @@ where let MsgUpdateClient { client_id, header } = msg; - let client_type = ctx + let _client_type = ctx .client_type(&client_id) - .ok_or_else(|| Kind::ClientNotFound(msg.client_id))?; + .ok_or_else(|| Kind::ClientNotFound(client_id.clone()))?; - let client_state = ctx + let mut client_state = ctx .client_state(&client_id) - .ok_or_else(|| Kind::ClientNotFound(msg.client_id))?; + .ok_or_else(|| Kind::ClientNotFound(client_id.clone()))?; + let latest_height = client_state.get_latest_height(); let consensus_state = ctx - .consensus_state(&client_id, client_state.latest_height()) - .ok_or_else(|| Kind::ConsensusStateNotFound(msg.client_id, client_state.latest_height()))?; + .consensus_state(&client_id, latest_height) + .ok_or_else(|| Kind::ConsensusStateNotFound(client_id.clone(), latest_height))?; - // client_type.check_validity_and_update_state( - // &mut client_state, - // &mut consensus_state, - // &header, - // )?; + CD::check_validity_and_update_state(&mut client_state, &consensus_state, &header).unwrap(); // FIXME + + output.emit(ClientEvent::ClientUpdated(client_id.clone())); Ok(output.with_result(UpdateClientResult { client_id, @@ -64,6 +64,7 @@ where #[cfg(test)] mod tests { use super::*; + use crate::ics02_client::client_type::ClientType; use crate::ics02_client::header::Header; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics23_commitment::CommitmentRoot; @@ -148,10 +149,28 @@ mod tests { #[derive(Clone, Copy, Debug, PartialEq, Eq)] struct MockClient; + fn new_client_state( + client_state: &MockClientState, + consensus_state: &MockConsensusState, + header: &MockHeader, + ) -> MockClientState { + MockClientState(client_state.0 * 17 + consensus_state.0 * 13 + header.0 * 7) + } + impl ClientDef for MockClient { + type Error = MockError; type Header = MockHeader; type ClientState = MockClientState; type ConsensusState = MockConsensusState; + + fn check_validity_and_update_state( + client_state: &mut MockClientState, + consensus_state: &MockConsensusState, + header: &MockHeader, + ) -> Result<(), Self::Error> { + client_state.0 = new_client_state(client_state, consensus_state, header).0; + Ok(()) + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -161,7 +180,7 @@ mod tests { consensus_state: Option, } - impl ClientContext for MockClientContext { + impl ClientReader for MockClientContext { fn client_type(&self, _client_id: &ClientId) -> Option { self.client_type.clone() } @@ -189,8 +208,7 @@ mod tests { let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - client_type: ClientType::Tendermint, - consensus_state: MockConsensusState(42), + header: MockHeader(1), }; let output = process(&mock, msg.clone()); @@ -201,19 +219,12 @@ mod tests { events, log, }) => { - assert_eq!(result.client_type, ClientType::Tendermint); - assert_eq!(result.client_state, MockClientState(msg.consensus_state.0)); + assert_eq!(result.client_state, MockClientState(0)); assert_eq!( events, vec![ClientEvent::ClientUpdated(msg.client_id).into()] ); - assert_eq!( - log, - vec![ - "success: no client state found".to_string(), - "success: no client type found".to_string() - ] - ); + assert!(log.is_empty()); } Err(err) => { panic!("unexpected error: {}", err); @@ -231,8 +242,7 @@ mod tests { let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - client_type: ClientType::Tendermint, - consensus_state: MockConsensusState(42), + header: MockHeader(1), }; let output = process(&mock, msg.clone()); @@ -248,14 +258,14 @@ mod tests { fn test_update_client_existing_client_state() { let mock = MockClientContext { client_type: None, - client_state: Some(MockClientState(0)), + client_state: Some(MockClientState(11)), consensus_state: None, }; #[allow(unreachable_code)] let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - header: todo!(), + header: MockHeader(42), }; let output = process(&mock, msg.clone()); diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index 9b69cebf5a..55e3fd0b78 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -43,3 +43,4 @@ pub trait ClientState { root: &CommitmentRoot, ) -> Result<(), Self::ValidationError>; } + diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index 22fcd9ee2f..40d75a4788 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -11,7 +11,17 @@ use crate::ics02_client::client as ics02; pub struct TendermintClient; impl ics02::ClientDef for TendermintClient { + type Error = error::Error; type Header = header::Header; type ClientState = client_state::ClientState; type ConsensusState = consensus_state::ConsensusState; + + fn check_validity_and_update_state( + _client_state: &mut Self::ClientState, + _consensus_state: &Self::ConsensusState, + _header: &Self::Header, + ) -> Result<(), Self::Error> { + todo!() + } } + From 4a529c156ebd8d118e7d29ca424add5e7b03357f Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 13 Aug 2020 15:02:13 +0200 Subject: [PATCH 23/28] Move mocks into their own module --- modules/src/ics02_client/client.rs | 8 +- modules/src/ics02_client/handler.rs | 3 +- .../src/ics02_client/handler/create_client.rs | 136 +---------------- .../src/ics02_client/handler/update_client.rs | 137 +----------------- modules/src/ics02_client/mocks.rs | 137 ++++++++++++++++++ modules/src/ics02_client/mod.rs | 1 + 6 files changed, 157 insertions(+), 265 deletions(-) create mode 100644 modules/src/ics02_client/mocks.rs diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs index 7f2d74866f..729b222c7d 100644 --- a/modules/src/ics02_client/client.rs +++ b/modules/src/ics02_client/client.rs @@ -2,13 +2,17 @@ use crate::ics02_client::state::{ClientState, ConsensusState}; -pub trait ClientDef: Sized { +pub trait ClientDef { type Error: std::error::Error; type Header; - type ClientState: ClientState + From; + type ClientState: ClientState; type ConsensusState: ConsensusState; + fn new_client_state(_consensus_state: &Self::ConsensusState) -> Self::ClientState { + todo!() + } + /// TODO fn check_validity_and_update_state( client_state: &mut Self::ClientState, diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 59ad680e7a..c84eb4161c 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -1,3 +1,5 @@ +#![allow(unused_imports)] + use crate::handler::{Event, EventType, HandlerOutput}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; @@ -85,7 +87,6 @@ where .with_events(events) .with_result(())) } - ClientMsg::UpdateClient(msg) => { let HandlerOutput { result, diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 4861afa403..3e401b3ff6 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -1,3 +1,5 @@ +#![allow(unreachable_code, unused_variables)] + use crate::handler::{HandlerOutput, HandlerResult}; use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; @@ -40,7 +42,7 @@ where output.log("success: no client type found"); - let client_state = consensus_state.into(); + let client_state = CD::new_client_state(&consensus_state); output.emit(ClientEvent::ClientCreated(client_id.clone())); @@ -68,142 +70,12 @@ where mod tests { use super::*; use crate::ics02_client::header::Header; + use crate::ics02_client::mocks::*; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics23_commitment::CommitmentRoot; use crate::Height; use thiserror::Error; - #[derive(Debug, Error)] - enum MockError {} - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockHeader(u32); - - impl Header for MockHeader { - fn client_type(&self) -> ClientType { - todo!() - } - - fn height(&self) -> Height { - todo!() - } - } - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockClientState(u32); - - impl ClientState for MockClientState { - type ValidationError = MockError; - - fn client_id(&self) -> ClientId { - todo!() - } - - fn client_type(&self) -> ClientType { - todo!() - } - - fn get_latest_height(&self) -> Height { - todo!() - } - - fn is_frozen(&self) -> bool { - todo!() - } - - fn verify_client_consensus_state( - &self, - _root: &CommitmentRoot, - ) -> Result<(), Self::ValidationError> { - todo!() - } - } - - impl From for MockClientState { - fn from(cs: MockConsensusState) -> Self { - Self(cs.0) - } - } - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockConsensusState(u32); - - impl ConsensusState for MockConsensusState { - type ValidationError = MockError; - - fn client_type(&self) -> ClientType { - todo!() - } - - fn height(&self) -> Height { - todo!() - } - - fn root(&self) -> &CommitmentRoot { - todo!() - } - - fn validate_basic(&self) -> Result<(), Self::ValidationError> { - todo!() - } - } - - #[derive(Clone, Copy, Debug, PartialEq, Eq)] - struct MockClient; - - impl ClientDef for MockClient { - type Error = MockError; - type Header = MockHeader; - type ClientState = MockClientState; - type ConsensusState = MockConsensusState; - - fn check_validity_and_update_state( - _client_state: &mut MockClientState, - _consensus_state: &MockConsensusState, - _header: &MockHeader, - ) -> Result<(), Self::Error> { - todo!() - } - } - - #[derive(Clone, Debug, PartialEq, Eq)] - struct MockClientReader { - client_id: ClientId, - client_state: Option, - client_type: Option, - consensus_state: Option, - } - - impl ClientReader for MockClientReader { - fn client_type(&self, client_id: &ClientId) -> Option { - if client_id == &self.client_id { - self.client_type.clone() - } else { - None - } - } - - fn client_state(&self, client_id: &ClientId) -> Option { - if client_id == &self.client_id { - self.client_state - } else { - None - } - } - - fn consensus_state( - &self, - client_id: &ClientId, - _height: Height, - ) -> Option { - if client_id == &self.client_id { - self.consensus_state - } else { - None - } - } - } - #[test] fn test_create_client_ok() { let client_id: ClientId = "mockclient".parse().unwrap(); diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs index e216bb4809..a87978bfe7 100644 --- a/modules/src/ics02_client/handler/update_client.rs +++ b/modules/src/ics02_client/handler/update_client.rs @@ -66,141 +66,16 @@ mod tests { use super::*; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::header::Header; + use crate::ics02_client::mocks::*; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics23_commitment::CommitmentRoot; use crate::Height; use thiserror::Error; - #[derive(Debug, Error)] - enum MockError {} - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockHeader(u32); - - impl Header for MockHeader { - fn client_type(&self) -> ClientType { - todo!() - } - - fn height(&self) -> Height { - todo!() - } - } - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockClientState(u32); - - impl ClientState for MockClientState { - type ValidationError = MockError; - - fn client_id(&self) -> ClientId { - todo!() - } - - fn client_type(&self) -> ClientType { - todo!() - } - - fn get_latest_height(&self) -> Height { - todo!() - } - - fn is_frozen(&self) -> bool { - todo!() - } - - fn verify_client_consensus_state( - &self, - _root: &CommitmentRoot, - ) -> Result<(), Self::ValidationError> { - todo!() - } - } - - impl From for MockClientState { - fn from(cs: MockConsensusState) -> Self { - Self(cs.0) - } - } - - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - struct MockConsensusState(u32); - - impl ConsensusState for MockConsensusState { - type ValidationError = MockError; - - fn client_type(&self) -> ClientType { - todo!() - } - - fn height(&self) -> Height { - todo!() - } - - fn root(&self) -> &CommitmentRoot { - todo!() - } - - fn validate_basic(&self) -> Result<(), Self::ValidationError> { - todo!() - } - } - - #[derive(Clone, Copy, Debug, PartialEq, Eq)] - struct MockClient; - - fn new_client_state( - client_state: &MockClientState, - consensus_state: &MockConsensusState, - header: &MockHeader, - ) -> MockClientState { - MockClientState(client_state.0 * 17 + consensus_state.0 * 13 + header.0 * 7) - } - - impl ClientDef for MockClient { - type Error = MockError; - type Header = MockHeader; - type ClientState = MockClientState; - type ConsensusState = MockConsensusState; - - fn check_validity_and_update_state( - client_state: &mut MockClientState, - consensus_state: &MockConsensusState, - header: &MockHeader, - ) -> Result<(), Self::Error> { - client_state.0 = new_client_state(client_state, consensus_state, header).0; - Ok(()) - } - } - - #[derive(Clone, Debug, PartialEq, Eq)] - struct MockClientContext { - client_state: Option, - client_type: Option, - consensus_state: Option, - } - - impl ClientReader for MockClientContext { - fn client_type(&self, _client_id: &ClientId) -> Option { - self.client_type.clone() - } - - fn client_state(&self, _client_id: &ClientId) -> Option { - self.client_state - } - - fn consensus_state( - &self, - _client_id: &ClientId, - _height: Height, - ) -> Option { - self.consensus_state - } - } - #[test] fn test_update_client_ok() { - let mock = MockClientContext { + let mock = MockClientReader { + client_id: "mockclient".parse().unwrap(), client_type: None, client_state: None, consensus_state: None, @@ -234,7 +109,8 @@ mod tests { #[test] fn test_update_client_existing_client_type() { - let mock = MockClientContext { + let mock = MockClientReader { + client_id: "mockclient".parse().unwrap(), client_type: Some(ClientType::Tendermint), client_state: None, consensus_state: None, @@ -256,7 +132,8 @@ mod tests { #[test] fn test_update_client_existing_client_state() { - let mock = MockClientContext { + let mock = MockClientReader { + client_id: "mockclient".parse().unwrap(), client_type: None, client_state: Some(MockClientState(11)), consensus_state: None, diff --git a/modules/src/ics02_client/mocks.rs b/modules/src/ics02_client/mocks.rs new file mode 100644 index 0000000000..af2e42f8cf --- /dev/null +++ b/modules/src/ics02_client/mocks.rs @@ -0,0 +1,137 @@ +use crate::ics02_client::client::ClientDef; +use crate::ics02_client::client_type::ClientType; +use crate::ics02_client::handler::ClientReader; +use crate::ics02_client::header::Header; +use crate::ics02_client::state::{ClientState, ConsensusState}; +use crate::ics23_commitment::CommitmentRoot; +use crate::ics24_host::identifier::ClientId; + +use crate::Height; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum MockError {} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct MockHeader(pub u32); + +impl Header for MockHeader { + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct MockClientState(pub u32); + +impl ClientState for MockClientState { + type ValidationError = MockError; + + fn client_id(&self) -> ClientId { + todo!() + } + + fn client_type(&self) -> ClientType { + todo!() + } + + fn get_latest_height(&self) -> Height { + todo!() + } + + fn is_frozen(&self) -> bool { + todo!() + } + + fn verify_client_consensus_state( + &self, + _root: &CommitmentRoot, + ) -> Result<(), Self::ValidationError> { + todo!() + } +} + +impl From for MockClientState { + fn from(cs: MockConsensusState) -> Self { + Self(cs.0) + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct MockConsensusState(pub u32); + +impl ConsensusState for MockConsensusState { + type ValidationError = MockError; + + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + + fn root(&self) -> &CommitmentRoot { + todo!() + } + + fn validate_basic(&self) -> Result<(), Self::ValidationError> { + todo!() + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct MockClient; + +impl ClientDef for MockClient { + type Error = MockError; + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; + + fn check_validity_and_update_state( + _client_state: &mut MockClientState, + _consensus_state: &MockConsensusState, + _header: &MockHeader, + ) -> Result<(), Self::Error> { + todo!() + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct MockClientReader { + pub client_id: ClientId, + pub client_state: Option, + pub client_type: Option, + pub consensus_state: Option, +} + +impl ClientReader for MockClientReader { + fn client_type(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_type.clone() + } else { + None + } + } + + fn client_state(&self, client_id: &ClientId) -> Option { + if client_id == &self.client_id { + self.client_state + } else { + None + } + } + + fn consensus_state(&self, client_id: &ClientId, _height: Height) -> Option { + if client_id == &self.client_id { + self.consensus_state + } else { + None + } + } +} diff --git a/modules/src/ics02_client/mod.rs b/modules/src/ics02_client/mod.rs index 7f8090309c..cb3fc9dfaa 100644 --- a/modules/src/ics02_client/mod.rs +++ b/modules/src/ics02_client/mod.rs @@ -6,6 +6,7 @@ pub mod error; pub mod events; pub mod handler; pub mod header; +pub mod mocks; pub mod msgs; pub mod raw; pub mod state; From 05d3dfff7b30008025bd5aac84882d238ebd5b86 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 13 Aug 2020 17:58:33 +0200 Subject: [PATCH 24/28] Remove genericity over client def in client submodule --- modules/Cargo.toml | 1 + modules/src/ics02_client/client.rs | 23 --------- modules/src/ics02_client/handler.rs | 35 ++++++------- .../src/ics02_client/handler/create_client.rs | 43 +++++++--------- .../src/ics02_client/handler/update_client.rs | 49 ++++++++----------- modules/src/ics02_client/header.rs | 3 +- modules/src/ics02_client/mocks.rs | 43 +++++----------- modules/src/ics02_client/mod.rs | 1 - modules/src/ics02_client/msgs.rs | 16 +++--- modules/src/ics02_client/state.rs | 15 +++--- modules/src/ics07_tendermint/client_state.rs | 4 +- .../src/ics07_tendermint/consensus_state.rs | 4 +- modules/src/ics07_tendermint/mod.rs | 20 -------- 13 files changed, 88 insertions(+), 169 deletions(-) delete mode 100644 modules/src/ics02_client/client.rs diff --git a/modules/Cargo.toml b/modules/Cargo.toml index 96a40e3b52..a9d81dff44 100644 --- a/modules/Cargo.toml +++ b/modules/Cargo.toml @@ -23,6 +23,7 @@ serde_json = "1" tracing = "0.1.13" prost = "0.6.1" bytes = "0.5.5" +dyn-clonable = "0.9.0" [dev-dependencies] tokio = { version = "0.2", features = ["macros"] } diff --git a/modules/src/ics02_client/client.rs b/modules/src/ics02_client/client.rs deleted file mode 100644 index 729b222c7d..0000000000 --- a/modules/src/ics02_client/client.rs +++ /dev/null @@ -1,23 +0,0 @@ -//! ICS 02: Client definitions - -use crate::ics02_client::state::{ClientState, ConsensusState}; - -pub trait ClientDef { - type Error: std::error::Error; - - type Header; - type ClientState: ClientState; - type ConsensusState: ConsensusState; - - fn new_client_state(_consensus_state: &Self::ConsensusState) -> Self::ClientState { - todo!() - } - - /// TODO - fn check_validity_and_update_state( - client_state: &mut Self::ClientState, - consensus_state: &Self::ConsensusState, - header: &Self::Header, - ) -> Result<(), Self::Error>; -} - diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index c84eb4161c..519a3c5736 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -1,10 +1,10 @@ #![allow(unused_imports)] use crate::handler::{Event, EventType, HandlerOutput}; -use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::Error; use crate::ics02_client::msgs::{MsgCreateClient, MsgUpdateClient}; +use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; use crate::Height; @@ -12,16 +12,19 @@ use crate::Height; pub mod create_client; pub mod update_client; -pub trait ClientReader -where - CD: ClientDef, -{ +pub trait ClientContext {} + +pub trait ClientReader { fn client_type(&self, client_id: &ClientId) -> Option; - fn client_state(&self, client_id: &ClientId) -> Option; - fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; + fn client_state(&self, client_id: &ClientId) -> Option>; + fn consensus_state( + &self, + client_id: &ClientId, + height: Height, + ) -> Option>; } -pub trait ClientKeeper { +pub trait ClientKeeper { fn store_client_type( &mut self, client_id: ClientId, @@ -31,13 +34,13 @@ pub trait ClientKeeper { fn store_client_state( &mut self, client_id: ClientId, - client_state: CD::ClientState, + client_state: &dyn ClientState, ) -> Result<(), Error>; fn store_consensus_state( &mut self, client_id: ClientId, - consensus_state: CD::ConsensusState, + consensus_state: &dyn ConsensusState, ) -> Result<(), Error>; } @@ -62,15 +65,14 @@ impl From for Event { } } -pub enum ClientMsg { - CreateClient(MsgCreateClient), - UpdateClient(MsgUpdateClient), +pub enum ClientMsg { + CreateClient(MsgCreateClient), + UpdateClient(MsgUpdateClient), } -pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where - CD: ClientDef, - Ctx: ClientReader + ClientKeeper, + Ctx: ClientReader + ClientKeeper, { match msg { ClientMsg::CreateClient(msg) => { @@ -103,4 +105,3 @@ where } } } - diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 3e401b3ff6..7b2bde8e07 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -1,27 +1,24 @@ #![allow(unreachable_code, unused_variables)] use crate::handler::{HandlerOutput, HandlerResult}; -use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgCreateClient; +use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct CreateClientResult { +#[derive(Debug)] +pub struct CreateClientResult { client_id: ClientId, client_type: ClientType, - client_state: CD::ClientState, + client_state: Box, } -pub fn process( - ctx: &dyn ClientReader, - msg: MsgCreateClient, -) -> HandlerResult, Error> -where - CD: ClientDef, -{ +pub fn process( + ctx: &dyn ClientReader, + msg: MsgCreateClient, +) -> HandlerResult { let mut output = HandlerOutput::builder(); let MsgCreateClient { @@ -42,7 +39,7 @@ where output.log("success: no client type found"); - let client_state = CD::new_client_state(&consensus_state); + let client_state = todo!(); // CD::new_client_state(&consensus_state); output.emit(ClientEvent::ClientCreated(client_id.clone())); @@ -53,14 +50,8 @@ where })) } -pub fn keep( - keeper: &mut dyn ClientKeeper, - result: CreateClientResult, -) -> Result<(), Error> -where - CD: ClientDef, -{ - keeper.store_client_state(result.client_id.clone(), result.client_state)?; +pub fn keep(keeper: &mut dyn ClientKeeper, result: CreateClientResult) -> Result<(), Error> { + keeper.store_client_state(result.client_id.clone(), result.client_state.as_ref())?; keeper.store_client_type(result.client_id, result.client_type)?; Ok(()) @@ -90,7 +81,7 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: MockConsensusState(42), + consensus_state: Box::new(MockConsensusState(42)), }; let output = process(&mock, msg.clone()); @@ -102,7 +93,10 @@ mod tests { log, }) => { assert_eq!(result.client_type, ClientType::Tendermint); - assert_eq!(result.client_state, MockClientState(msg.consensus_state.0)); + // assert_eq!( + // result.client_state.as_ref().0, + // msg.consensus_state.as_ref().0 + // ); assert_eq!( events, vec![ClientEvent::ClientCreated(msg.client_id).into()] @@ -135,7 +129,7 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: MockConsensusState(42), + consensus_state: Box::new(MockConsensusState(42)), }; let output = process(&mock, msg.clone()); @@ -161,7 +155,7 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: MockConsensusState(42), + consensus_state: Box::new(MockConsensusState(42)), }; let output = process(&mock, msg.clone()); @@ -173,4 +167,3 @@ mod tests { } } } - diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs index a87978bfe7..513c41968b 100644 --- a/modules/src/ics02_client/handler/update_client.rs +++ b/modules/src/ics02_client/handler/update_client.rs @@ -1,25 +1,21 @@ use crate::handler::{HandlerOutput, HandlerResult}; -use crate::ics02_client::client::ClientDef; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgUpdateClient; -use crate::ics02_client::state::ClientState; +use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct UpdateClientResult { +#[derive(Debug)] +pub struct UpdateClientResult { client_id: ClientId, - client_state: C::ClientState, - consensus_state: C::ConsensusState, + client_state: Box, + consensus_state: Box, } -pub fn process( - ctx: &dyn ClientReader, - msg: MsgUpdateClient, -) -> HandlerResult, Error> -where - CD: ClientDef, -{ +pub fn process( + ctx: &dyn ClientReader, + msg: MsgUpdateClient, +) -> HandlerResult { let mut output = HandlerOutput::builder(); let MsgUpdateClient { client_id, header } = msg; @@ -37,7 +33,9 @@ where .consensus_state(&client_id, latest_height) .ok_or_else(|| Kind::ConsensusStateNotFound(client_id.clone(), latest_height))?; - CD::check_validity_and_update_state(&mut client_state, &consensus_state, &header).unwrap(); // FIXME + let _h = header; + let _cs = client_state.as_mut(); + // CD::check_validity_and_update_state(&mut client_state, &consensus_state, &header).unwrap(); // FIXME output.emit(ClientEvent::ClientUpdated(client_id.clone())); @@ -48,15 +46,9 @@ where })) } -pub fn keep( - keeper: &mut dyn ClientKeeper, - result: UpdateClientResult, -) -> Result<(), Error> -where - CD: ClientDef, -{ - keeper.store_client_state(result.client_id.clone(), result.client_state)?; - keeper.store_consensus_state(result.client_id, result.consensus_state)?; +pub fn keep(keeper: &mut dyn ClientKeeper, result: UpdateClientResult) -> Result<(), Error> { + keeper.store_client_state(result.client_id.clone(), result.client_state.as_ref())?; + keeper.store_consensus_state(result.client_id, result.consensus_state.as_ref())?; Ok(()) } @@ -83,18 +75,18 @@ mod tests { let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - header: MockHeader(1), + header: Box::new(MockHeader(1)), }; let output = process(&mock, msg.clone()); match output { Ok(HandlerOutput { - result, + result: _, events, log, }) => { - assert_eq!(result.client_state, MockClientState(0)); + // assert_eq!(result.client_state, MockClientState(0)); assert_eq!( events, vec![ClientEvent::ClientUpdated(msg.client_id).into()] @@ -118,7 +110,7 @@ mod tests { let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - header: MockHeader(1), + header: Box::new(MockHeader(1)), }; let output = process(&mock, msg.clone()); @@ -142,7 +134,7 @@ mod tests { #[allow(unreachable_code)] let msg = MsgUpdateClient { client_id: "mockclient".parse().unwrap(), - header: MockHeader(42), + header: Box::new(MockHeader(42)), }; let output = process(&mock, msg.clone()); @@ -154,4 +146,3 @@ mod tests { } } } - diff --git a/modules/src/ics02_client/header.rs b/modules/src/ics02_client/header.rs index bc54fd2aa6..255b70ed52 100644 --- a/modules/src/ics02_client/header.rs +++ b/modules/src/ics02_client/header.rs @@ -2,7 +2,8 @@ use super::client_type::ClientType; use crate::Height; /// Abstract of consensus state update information -pub trait Header { +#[dyn_clonable::clonable] +pub trait Header: Clone + std::fmt::Debug { /// The type of client (eg. Tendermint) fn client_type(&self) -> ClientType; diff --git a/modules/src/ics02_client/mocks.rs b/modules/src/ics02_client/mocks.rs index af2e42f8cf..e25b0b13b2 100644 --- a/modules/src/ics02_client/mocks.rs +++ b/modules/src/ics02_client/mocks.rs @@ -1,4 +1,3 @@ -use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::handler::ClientReader; use crate::ics02_client::header::Header; @@ -29,8 +28,6 @@ impl Header for MockHeader { pub struct MockClientState(pub u32); impl ClientState for MockClientState { - type ValidationError = MockError; - fn client_id(&self) -> ClientId { todo!() } @@ -50,7 +47,7 @@ impl ClientState for MockClientState { fn verify_client_consensus_state( &self, _root: &CommitmentRoot, - ) -> Result<(), Self::ValidationError> { + ) -> Result<(), Box> { todo!() } } @@ -65,8 +62,6 @@ impl From for MockClientState { pub struct MockConsensusState(pub u32); impl ConsensusState for MockConsensusState { - type ValidationError = MockError; - fn client_type(&self) -> ClientType { todo!() } @@ -79,25 +74,7 @@ impl ConsensusState for MockConsensusState { todo!() } - fn validate_basic(&self) -> Result<(), Self::ValidationError> { - todo!() - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct MockClient; - -impl ClientDef for MockClient { - type Error = MockError; - type Header = MockHeader; - type ClientState = MockClientState; - type ConsensusState = MockConsensusState; - - fn check_validity_and_update_state( - _client_state: &mut MockClientState, - _consensus_state: &MockConsensusState, - _header: &MockHeader, - ) -> Result<(), Self::Error> { + fn validate_basic(&self) -> Result<(), Box> { todo!() } } @@ -110,7 +87,7 @@ pub struct MockClientReader { pub consensus_state: Option, } -impl ClientReader for MockClientReader { +impl ClientReader for MockClientReader { fn client_type(&self, client_id: &ClientId) -> Option { if client_id == &self.client_id { self.client_type.clone() @@ -119,17 +96,23 @@ impl ClientReader for MockClientReader { } } - fn client_state(&self, client_id: &ClientId) -> Option { + #[allow(trivial_casts)] + fn client_state(&self, client_id: &ClientId) -> Option> { if client_id == &self.client_id { - self.client_state + self.client_state.map(|cs| Box::new(cs) as _) } else { None } } - fn consensus_state(&self, client_id: &ClientId, _height: Height) -> Option { + #[allow(trivial_casts)] + fn consensus_state( + &self, + client_id: &ClientId, + _height: Height, + ) -> Option> { if client_id == &self.client_id { - self.consensus_state + self.consensus_state.map(|cs| Box::new(cs) as _) } else { None } diff --git a/modules/src/ics02_client/mod.rs b/modules/src/ics02_client/mod.rs index cb3fc9dfaa..1d233f467f 100644 --- a/modules/src/ics02_client/mod.rs +++ b/modules/src/ics02_client/mod.rs @@ -1,6 +1,5 @@ //! ICS 02: IBC Client implementation -pub mod client; pub mod client_type; pub mod error; pub mod events; diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 4648734b3e..f3c7890a5f 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -4,22 +4,22 @@ //! subsequently calls into the chain-specific (e.g., ICS 07) client handler. See: //! https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create. -use crate::ics02_client::client::ClientDef; use crate::ics02_client::client_type::ClientType; - +use crate::ics02_client::header::Header; +use crate::ics02_client::state::ConsensusState; use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct MsgCreateClient { +#[derive(Clone, Debug)] +pub struct MsgCreateClient { pub client_id: ClientId, pub client_type: ClientType, - pub consensus_state: C::ConsensusState, + pub consensus_state: Box, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct MsgUpdateClient { +#[derive(Clone, Debug)] +pub struct MsgUpdateClient { pub client_id: ClientId, - pub header: C::Header, + pub header: Box, } diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index 55e3fd0b78..033fb50e9e 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -4,9 +4,8 @@ use crate::ics23_commitment::CommitmentRoot; use crate::ics24_host::identifier::ClientId; use crate::Height; -pub trait ConsensusState { - type ValidationError: std::error::Error; - +#[dyn_clonable::clonable] +pub trait ConsensusState: Clone + std::fmt::Debug { /// Type of client associated with this consensus state (eg. Tendermint) fn client_type(&self) -> ClientType; @@ -17,12 +16,11 @@ pub trait ConsensusState { fn root(&self) -> &CommitmentRoot; /// Performs basic validation of the consensus state - fn validate_basic(&self) -> Result<(), Self::ValidationError>; + fn validate_basic(&self) -> Result<(), Box>; } -pub trait ClientState { - type ValidationError: std::error::Error; - +#[dyn_clonable::clonable] +pub trait ClientState: Clone + std::fmt::Debug { /// Client ID of this state fn client_id(&self) -> ClientId; @@ -41,6 +39,5 @@ pub trait ClientState { fn verify_client_consensus_state( &self, root: &CommitmentRoot, - ) -> Result<(), Self::ValidationError>; + ) -> Result<(), Box>; } - diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index 5da32bda7c..c4d2fe133c 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -72,8 +72,6 @@ impl From for ClientState { } impl crate::ics02_client::state::ClientState for ClientState { - type ValidationError = Error; - fn client_id(&self) -> ClientId { self.id.clone() } @@ -94,7 +92,7 @@ impl crate::ics02_client::state::ClientState for ClientState { fn verify_client_consensus_state( &self, _root: &CommitmentRoot, - ) -> Result<(), Self::ValidationError> { + ) -> Result<(), Box> { unimplemented!() } } diff --git a/modules/src/ics07_tendermint/consensus_state.rs b/modules/src/ics07_tendermint/consensus_state.rs index 2e205f54ed..a8fb21d2b8 100644 --- a/modules/src/ics07_tendermint/consensus_state.rs +++ b/modules/src/ics07_tendermint/consensus_state.rs @@ -28,8 +28,6 @@ impl ConsensusState { } impl crate::ics02_client::state::ConsensusState for ConsensusState { - type ValidationError = crate::ics07_tendermint::error::Error; - fn client_type(&self) -> ClientType { ClientType::Tendermint } @@ -42,7 +40,7 @@ impl crate::ics02_client::state::ConsensusState for ConsensusState { &self.root } - fn validate_basic(&self) -> Result<(), Self::ValidationError> { + fn validate_basic(&self) -> Result<(), Box> { unimplemented!() } } diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index 40d75a4788..76391f8d36 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -5,23 +5,3 @@ pub mod consensus_state; pub mod error; pub mod header; // pub mod msgs; - -use crate::ics02_client::client as ics02; - -pub struct TendermintClient; - -impl ics02::ClientDef for TendermintClient { - type Error = error::Error; - type Header = header::Header; - type ClientState = client_state::ClientState; - type ConsensusState = consensus_state::ConsensusState; - - fn check_validity_and_update_state( - _client_state: &mut Self::ClientState, - _consensus_state: &Self::ConsensusState, - _header: &Self::Header, - ) -> Result<(), Self::Error> { - todo!() - } -} - From 9d23ef3c2111c461c7874be4aea5969b222d91de Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 14 Aug 2020 15:04:48 +0200 Subject: [PATCH 25/28] Lift chain specific data into an enum --- modules/src/ics02_client/client_def.rs | 99 ++++++++ modules/src/ics02_client/handler.rs | 45 ++-- .../src/ics02_client/handler/create_client.rs | 37 +-- .../src/ics02_client/handler/update_client.rs | 214 +++++++++--------- modules/src/ics02_client/mocks.rs | 92 ++++++-- modules/src/ics02_client/mod.rs | 1 + modules/src/ics02_client/msgs.rs | 11 +- 7 files changed, 341 insertions(+), 158 deletions(-) create mode 100644 modules/src/ics02_client/client_def.rs diff --git a/modules/src/ics02_client/client_def.rs b/modules/src/ics02_client/client_def.rs new file mode 100644 index 0000000000..5a07c1296c --- /dev/null +++ b/modules/src/ics02_client/client_def.rs @@ -0,0 +1,99 @@ +use serde_derive::{Deserialize, Serialize}; + +use crate::ics02_client::client_type::ClientType; +use crate::ics02_client::header::Header; +use crate::ics02_client::state::{ClientState, ConsensusState}; +use crate::ics23_commitment::CommitmentRoot; +use crate::ics24_host::identifier::ClientId; +use crate::Height; + +use crate::ics02_client::mocks; +use crate::ics07_tendermint as tendermint; + +pub trait ClientDef: Clone { + type Header: Header; + type ClientState: ClientState; + type ConsensusState: ConsensusState; +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] // TODO: Add Eq +#[allow(clippy::large_enum_variant)] +pub enum AnyHeader { + Mock(mocks::MockHeader), + Tendermint(tendermint::header::Header), +} + +impl Header for AnyHeader { + fn height(&self) -> Height { + todo!() + } + + fn client_type(&self) -> ClientType { + todo!() + } +} + +#[derive(Clone, Debug, PartialEq)] +pub enum AnyClientState { + Mock(mocks::MockClientState), +} + +impl ClientState for AnyClientState { + fn client_id(&self) -> ClientId { + todo!() + } + + fn client_type(&self) -> ClientType { + todo!() + } + + fn get_latest_height(&self) -> Height { + todo!() + } + + fn is_frozen(&self) -> bool { + todo!() + } + + fn verify_client_consensus_state( + &self, + _root: &CommitmentRoot, + ) -> Result<(), Box> { + todo!() + } +} + +#[derive(Clone, Debug, PartialEq)] +pub enum AnyConsensusState { + Mock(mocks::MockConsensusState), +} + +impl ConsensusState for AnyConsensusState { + fn client_type(&self) -> ClientType { + todo!() + } + + fn height(&self) -> Height { + todo!() + } + + fn root(&self) -> &CommitmentRoot { + todo!() + } + + fn validate_basic(&self) -> Result<(), Box> { + todo!() + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum AnyClient { + Mock(mocks::MockClient), +} + +impl ClientDef for AnyClient { + type Header = AnyHeader; + type ClientState = AnyClientState; + type ConsensusState = AnyConsensusState; +} + diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 519a3c5736..2864c643cb 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -1,6 +1,7 @@ #![allow(unused_imports)] use crate::handler::{Event, EventType, HandlerOutput}; +use crate::ics02_client::client_def::{AnyClient, AnyClientState, AnyConsensusState, ClientDef}; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::Error; use crate::ics02_client::msgs::{MsgCreateClient, MsgUpdateClient}; @@ -12,16 +13,16 @@ use crate::Height; pub mod create_client; pub mod update_client; -pub trait ClientContext {} +pub trait ClientContext { + fn client_type(&self, client_id: &ClientId) -> Option; + fn reader(&self, client_type: &ClientType) -> Box; + fn keeper(&self, client_type: &ClientType) -> Box; +} pub trait ClientReader { fn client_type(&self, client_id: &ClientId) -> Option; - fn client_state(&self, client_id: &ClientId) -> Option>; - fn consensus_state( - &self, - client_id: &ClientId, - height: Height, - ) -> Option>; + fn client_state(&self, client_id: &ClientId) -> Option; + fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; } pub trait ClientKeeper { @@ -34,13 +35,13 @@ pub trait ClientKeeper { fn store_client_state( &mut self, client_id: ClientId, - client_state: &dyn ClientState, + client_state: AnyClientState, ) -> Result<(), Error>; fn store_consensus_state( &mut self, client_id: ClientId, - consensus_state: &dyn ConsensusState, + consensus_state: AnyConsensusState, ) -> Result<(), Error>; } @@ -65,24 +66,27 @@ impl From for Event { } } -pub enum ClientMsg { - CreateClient(MsgCreateClient), - UpdateClient(MsgUpdateClient), +pub enum ClientMsg { + CreateClient(MsgCreateClient), + UpdateClient(MsgUpdateClient), } -pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where - Ctx: ClientReader + ClientKeeper, + Ctx: ClientContext, { match msg { ClientMsg::CreateClient(msg) => { + let reader = ctx.reader(&msg.client_type); + let mut keeper = ctx.keeper(&msg.client_type); + let HandlerOutput { result, log, events, - } = create_client::process(ctx, msg)?; + } = create_client::process(&*reader, msg)?; - create_client::keep(ctx, result)?; + create_client::keep(&mut *keeper, result)?; Ok(HandlerOutput::builder() .with_log(log) @@ -90,13 +94,18 @@ where .with_result(())) } ClientMsg::UpdateClient(msg) => { + let client_type = ctx.client_type(&msg.client_id).unwrap(); // FIXME + + let reader = ctx.reader(&client_type); + let mut keeper = ctx.keeper(&client_type); + let HandlerOutput { result, log, events, - } = update_client::process(ctx, msg)?; + } = update_client::process(&*reader, msg)?; - update_client::keep(ctx, result)?; + update_client::keep(&mut *keeper, result)?; Ok(HandlerOutput::builder() .with_log(log) diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 7b2bde8e07..81d1944fed 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -1,24 +1,25 @@ #![allow(unreachable_code, unused_variables)] use crate::handler::{HandlerOutput, HandlerResult}; +use crate::ics02_client::client_def::{AnyClient, ClientDef}; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; -use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; +use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgCreateClient; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; #[derive(Debug)] -pub struct CreateClientResult { +pub struct CreateClientResult { client_id: ClientId, client_type: ClientType, - client_state: Box, + client_state: CD::ClientState, } pub fn process( ctx: &dyn ClientReader, - msg: MsgCreateClient, -) -> HandlerResult { + msg: MsgCreateClient, +) -> HandlerResult, Error> { let mut output = HandlerOutput::builder(); let MsgCreateClient { @@ -50,8 +51,11 @@ pub fn process( })) } -pub fn keep(keeper: &mut dyn ClientKeeper, result: CreateClientResult) -> Result<(), Error> { - keeper.store_client_state(result.client_id.clone(), result.client_state.as_ref())?; +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: CreateClientResult, +) -> Result<(), Error> { + keeper.store_client_state(result.client_id.clone(), result.client_state)?; keeper.store_client_type(result.client_id, result.client_type)?; Ok(()) @@ -71,7 +75,7 @@ mod tests { fn test_create_client_ok() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientReader { + let reader = MockClientReader { client_id: client_id.clone(), client_type: None, client_state: None, @@ -81,10 +85,10 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: Box::new(MockConsensusState(42)), + consensus_state: MockConsensusState(42).into(), }; - let output = process(&mock, msg.clone()); + let output = process(&reader, msg.clone()); match output { Ok(HandlerOutput { @@ -119,7 +123,7 @@ mod tests { fn test_create_client_existing_client_type() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientReader { + let reader = MockClientReader { client_id: client_id.clone(), client_type: Some(ClientType::Tendermint), client_state: None, @@ -129,10 +133,10 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: Box::new(MockConsensusState(42)), + consensus_state: MockConsensusState(42).into(), }; - let output = process(&mock, msg.clone()); + let output = process(&reader, msg.clone()); if let Err(err) = output { assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); @@ -145,7 +149,7 @@ mod tests { fn test_create_client_existing_client_state() { let client_id: ClientId = "mockclient".parse().unwrap(); - let mock = MockClientReader { + let reader = MockClientReader { client_id: client_id.clone(), client_type: None, client_state: Some(MockClientState(0)), @@ -155,10 +159,10 @@ mod tests { let msg = MsgCreateClient { client_id, client_type: ClientType::Tendermint, - consensus_state: Box::new(MockConsensusState(42)), + consensus_state: MockConsensusState(42).into(), }; - let output = process(&mock, msg.clone()); + let output = process(&reader, msg.clone()); if let Err(err) = output { assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); @@ -167,3 +171,4 @@ mod tests { } } } + diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs index 513c41968b..10a23f35ba 100644 --- a/modules/src/ics02_client/handler/update_client.rs +++ b/modules/src/ics02_client/handler/update_client.rs @@ -1,4 +1,7 @@ +#![allow(unreachable_code, unused_variables)] + use crate::handler::{HandlerOutput, HandlerResult}; +use crate::ics02_client::client_def::{AnyClient, ClientDef}; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgUpdateClient; @@ -6,25 +9,25 @@ use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; #[derive(Debug)] -pub struct UpdateClientResult { +pub struct UpdateClientResult { client_id: ClientId, - client_state: Box, - consensus_state: Box, + client_state: CD::ClientState, + consensus_state: CD::ConsensusState, } pub fn process( ctx: &dyn ClientReader, - msg: MsgUpdateClient, -) -> HandlerResult { + msg: MsgUpdateClient, +) -> HandlerResult, Error> { let mut output = HandlerOutput::builder(); let MsgUpdateClient { client_id, header } = msg; - let _client_type = ctx + let client_type = ctx .client_type(&client_id) .ok_or_else(|| Kind::ClientNotFound(client_id.clone()))?; - let mut client_state = ctx + let client_state = ctx .client_state(&client_id) .ok_or_else(|| Kind::ClientNotFound(client_id.clone()))?; @@ -33,8 +36,6 @@ pub fn process( .consensus_state(&client_id, latest_height) .ok_or_else(|| Kind::ConsensusStateNotFound(client_id.clone(), latest_height))?; - let _h = header; - let _cs = client_state.as_mut(); // CD::check_validity_and_update_state(&mut client_state, &consensus_state, &header).unwrap(); // FIXME output.emit(ClientEvent::ClientUpdated(client_id.clone())); @@ -46,103 +47,106 @@ pub fn process( })) } -pub fn keep(keeper: &mut dyn ClientKeeper, result: UpdateClientResult) -> Result<(), Error> { - keeper.store_client_state(result.client_id.clone(), result.client_state.as_ref())?; - keeper.store_consensus_state(result.client_id, result.consensus_state.as_ref())?; +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: UpdateClientResult, +) -> Result<(), Error> { + keeper.store_client_state(result.client_id.clone(), result.client_state)?; + keeper.store_consensus_state(result.client_id, result.consensus_state)?; Ok(()) } -#[cfg(test)] -mod tests { - use super::*; - use crate::ics02_client::client_type::ClientType; - use crate::ics02_client::header::Header; - use crate::ics02_client::mocks::*; - use crate::ics02_client::state::{ClientState, ConsensusState}; - use crate::ics23_commitment::CommitmentRoot; - use crate::Height; - use thiserror::Error; - - #[test] - fn test_update_client_ok() { - let mock = MockClientReader { - client_id: "mockclient".parse().unwrap(), - client_type: None, - client_state: None, - consensus_state: None, - }; - - let msg = MsgUpdateClient { - client_id: "mockclient".parse().unwrap(), - header: Box::new(MockHeader(1)), - }; - - let output = process(&mock, msg.clone()); - - match output { - Ok(HandlerOutput { - result: _, - events, - log, - }) => { - // assert_eq!(result.client_state, MockClientState(0)); - assert_eq!( - events, - vec![ClientEvent::ClientUpdated(msg.client_id).into()] - ); - assert!(log.is_empty()); - } - Err(err) => { - panic!("unexpected error: {}", err); - } - } - } - - #[test] - fn test_update_client_existing_client_type() { - let mock = MockClientReader { - client_id: "mockclient".parse().unwrap(), - client_type: Some(ClientType::Tendermint), - client_state: None, - consensus_state: None, - }; - - let msg = MsgUpdateClient { - client_id: "mockclient".parse().unwrap(), - header: Box::new(MockHeader(1)), - }; - - let output = process(&mock, msg.clone()); - - if let Err(err) = output { - assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); - } else { - panic!("expected an error"); - } - } - - #[test] - fn test_update_client_existing_client_state() { - let mock = MockClientReader { - client_id: "mockclient".parse().unwrap(), - client_type: None, - client_state: Some(MockClientState(11)), - consensus_state: None, - }; - - #[allow(unreachable_code)] - let msg = MsgUpdateClient { - client_id: "mockclient".parse().unwrap(), - header: Box::new(MockHeader(42)), - }; - - let output = process(&mock, msg.clone()); - - if let Err(err) = output { - assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); - } else { - panic!("expected an error"); - } - } -} +// #[cfg(test)] +// mod tests { +// use super::*; +// use crate::ics02_client::client_type::ClientType; +// use crate::ics02_client::header::Header; +// use crate::ics02_client::mocks::*; +// use crate::ics02_client::state::{ClientState, ConsensusState}; +// use crate::ics23_commitment::CommitmentRoot; +// use crate::Height; +// use thiserror::Error; + +// #[test] +// fn test_update_client_ok() { +// let mock = MockClientReader { +// client_id: "mockclient".parse().unwrap(), +// client_type: None, +// client_state: None, +// consensus_state: None, +// }; + +// let msg = MsgUpdateClient { +// client_id: "mockclient".parse().unwrap(), +// header: Box::new(MockHeader(1)), +// }; + +// let output = process(&mock, msg.clone()); + +// match output { +// Ok(HandlerOutput { +// result: _, +// events, +// log, +// }) => { +// // assert_eq!(result.client_state, MockClientState(0)); +// assert_eq!( +// events, +// vec![ClientEvent::ClientUpdated(msg.client_id).into()] +// ); +// assert!(log.is_empty()); +// } +// Err(err) => { +// panic!("unexpected error: {}", err); +// } +// } +// } + +// #[test] +// fn test_update_client_existing_client_type() { +// let mock = MockClientReader { +// client_id: "mockclient".parse().unwrap(), +// client_type: Some(ClientType::Tendermint), +// client_state: None, +// consensus_state: None, +// }; + +// let msg = MsgUpdateClient { +// client_id: "mockclient".parse().unwrap(), +// header: Box::new(MockHeader(1)), +// }; + +// let output = process(&mock, msg.clone()); + +// if let Err(err) = output { +// assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); +// } else { +// panic!("expected an error"); +// } +// } + +// #[test] +// fn test_update_client_existing_client_state() { +// let mock = MockClientReader { +// client_id: "mockclient".parse().unwrap(), +// client_type: None, +// client_state: Some(MockClientState(11).into()), +// consensus_state: None, +// }; + +// #[allow(unreachable_code)] +// let msg = MsgUpdateClient { +// client_id: "mockclient".parse().unwrap(), +// header: MockHeader(42).into(), +// }; + +// let output = process(&mock, msg.clone()); + +// if let Err(err) = output { +// assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); +// } else { +// panic!("expected an error"); +// } +// } +// } diff --git a/modules/src/ics02_client/mocks.rs b/modules/src/ics02_client/mocks.rs index e25b0b13b2..68b482f0e2 100644 --- a/modules/src/ics02_client/mocks.rs +++ b/modules/src/ics02_client/mocks.rs @@ -1,19 +1,28 @@ +use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState, AnyHeader, ClientDef}; use crate::ics02_client::client_type::ClientType; -use crate::ics02_client::handler::ClientReader; +use crate::ics02_client::error::Error; +use crate::ics02_client::handler::{ClientKeeper, ClientReader}; use crate::ics02_client::header::Header; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics23_commitment::CommitmentRoot; use crate::ics24_host::identifier::ClientId; - use crate::Height; + +use serde_derive::{Deserialize, Serialize}; use thiserror::Error; #[derive(Debug, Error)] pub enum MockError {} -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct MockHeader(pub u32); +impl From for AnyHeader { + fn from(mh: MockHeader) -> Self { + Self::Mock(mh) + } +} + impl Header for MockHeader { fn client_type(&self) -> ClientType { todo!() @@ -24,9 +33,15 @@ impl Header for MockHeader { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct MockClientState(pub u32); +impl From for AnyClientState { + fn from(mcs: MockClientState) -> Self { + Self::Mock(mcs) + } +} + impl ClientState for MockClientState { fn client_id(&self) -> ClientId { todo!() @@ -58,9 +73,15 @@ impl From for MockClientState { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct MockConsensusState(pub u32); +impl From for AnyConsensusState { + fn from(mcs: MockConsensusState) -> Self { + Self::Mock(mcs) + } +} + impl ConsensusState for MockConsensusState { fn client_type(&self) -> ClientType { todo!() @@ -80,6 +101,21 @@ impl ConsensusState for MockConsensusState { } #[derive(Clone, Debug, PartialEq, Eq)] +pub struct MockClient; + +impl ClientDef for MockClient { + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; +} + +#[derive(Clone, Debug, PartialEq)] +pub struct MockClientContext { + reader: MockClientReader, + keeper: MockClientKeeper, +} + +#[derive(Clone, Debug, PartialEq)] pub struct MockClientReader { pub client_id: ClientId, pub client_state: Option, @@ -97,24 +133,54 @@ impl ClientReader for MockClientReader { } #[allow(trivial_casts)] - fn client_state(&self, client_id: &ClientId) -> Option> { + fn client_state(&self, client_id: &ClientId) -> Option { if client_id == &self.client_id { - self.client_state.map(|cs| Box::new(cs) as _) + self.client_state.map(Into::into) } else { None } } #[allow(trivial_casts)] - fn consensus_state( - &self, - client_id: &ClientId, - _height: Height, - ) -> Option> { + fn consensus_state(&self, client_id: &ClientId, _height: Height) -> Option { if client_id == &self.client_id { - self.consensus_state.map(|cs| Box::new(cs) as _) + self.consensus_state.map(Into::into) } else { None } } } + +#[derive(Clone, Debug, PartialEq, Default)] +pub struct MockClientKeeper { + pub client_state: Option, + pub client_type: Option, + pub consensus_state: Option, +} + +impl ClientKeeper for MockClientKeeper { + fn store_client_type( + &mut self, + _client_id: ClientId, + _client_type: ClientType, + ) -> Result<(), Error> { + todo!() + } + + fn store_client_state( + &mut self, + _client_id: ClientId, + _client_state: AnyClientState, + ) -> Result<(), Error> { + todo!() + } + + fn store_consensus_state( + &mut self, + _client_id: ClientId, + _consensus_state: AnyConsensusState, + ) -> Result<(), Error> { + todo!() + } +} + diff --git a/modules/src/ics02_client/mod.rs b/modules/src/ics02_client/mod.rs index 1d233f467f..9f5caea073 100644 --- a/modules/src/ics02_client/mod.rs +++ b/modules/src/ics02_client/mod.rs @@ -1,5 +1,6 @@ //! ICS 02: IBC Client implementation +pub mod client_def; pub mod client_type; pub mod error; pub mod events; diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index f3c7890a5f..b9b2238b9d 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -4,22 +4,21 @@ //! subsequently calls into the chain-specific (e.g., ICS 07) client handler. See: //! https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create. +use crate::ics02_client::client_def::ClientDef; use crate::ics02_client::client_type::ClientType; -use crate::ics02_client::header::Header; -use crate::ics02_client::state::ConsensusState; use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. #[derive(Clone, Debug)] -pub struct MsgCreateClient { +pub struct MsgCreateClient { pub client_id: ClientId, pub client_type: ClientType, - pub consensus_state: Box, + pub consensus_state: CD::ConsensusState, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. #[derive(Clone, Debug)] -pub struct MsgUpdateClient { +pub struct MsgUpdateClient { pub client_id: ClientId, - pub header: Box, + pub header: CD::Header, } From 56d0464ee48bcc9e3b4d05dcd07a24b9efd1af72 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 14 Aug 2020 19:15:23 +0200 Subject: [PATCH 26/28] Update ADR to match new handling of chain-specific data --- .../adr-003-handler-implementation.md | 522 +++++++++++------- modules/src/ics02_client/client_def.rs | 10 +- modules/src/ics02_client/handler.rs | 25 +- .../src/ics02_client/handler/create_client.rs | 2 +- modules/src/ics02_client/msgs.rs | 1 + 5 files changed, 349 insertions(+), 211 deletions(-) diff --git a/docs/architecture/adr-003-handler-implementation.md b/docs/architecture/adr-003-handler-implementation.md index 08b1110753..54d8fa7389 100644 --- a/docs/architecture/adr-003-handler-implementation.md +++ b/docs/architecture/adr-003-handler-implementation.md @@ -2,8 +2,10 @@ ## Changelog * 2020-08-06: Initial proposal +* 2020-08-10: Rename Handler to Message Processor +* 2020-08-14: Revamp definition of chain-specific messages, readers and keepers -## Context +## Reader > This section contains all the context one needs to understand the current state, and why there is a problem. It should be as succinct as possible and introduce the high level idea behind the solution. @@ -147,12 +149,8 @@ the IBC protocol, eg. client lifecycle management, connection lifecycle manageme packet relay, etc. In this section we propose a general approach to implement the message processors for a submodule. -To make things more concrete, we will use the ICS 002 Client submodule as a -running example, but the methodology outlined here should apply to any submodule. -This specific module also has the peculiarity of dealing with datatypes which -are specific to a given type of chain, eg. Tendermint. This incurs the need for -abstracting over these datatypes, which introduces additional abstractions which -may not be needed for other submodules. +As a running example we will use a dummy submodule that deals with connections, which should not +be mistaken for the actual ICS 003 Connection submodule. #### Events @@ -160,151 +158,75 @@ The events which may be emitted by the message processors of a submodule should as an enumeration, while a way of converting those into the generic `Event` type defined in a previous section should be provided via the `From` trait. -We provide below an implementation of both for the ICS 002 Client submodule: - ```rust -pub enum ClientEvent { - ClientCreated(ClientId), - ClientUpdated(ClientId), +pub enum ConnectionEvent { + ConnectionCreated(ConnectionId), + ConnectionUpdated(ConnectionId), } -impl From for Event { - fn from(ce: ClientEvent) -> Event { +impl From for Event { + fn from(ce: ConnectionEvent) -> Event { match ce { - ClientEvent::ClientCreated(client_id) => Event::new( - EventType::Custom("ClientCreated".to_string()), - vec![("client_id".to_string(), client_id.to_string())], + ConnectionEvent::ConnectionCreated(client_id) => Event::new( + EventType::Custom("ConnectionCreated".to_string()), + vec![("connection_id".to_string(), client_id.to_string())], ), - ClientEvent::ClientUpdated(client_id) => Event::new( - EventType::Custom("ClientUpdated".to_string()), - vec![("client_id".to_string(), client_id.to_string())], + ConnectionEvent::ConnectionUpdated(client_id) => Event::new( + EventType::Custom("ConnectionUpdated".to_string()), + vec![("connection_id".to_string(), client_id.to_string())], ), } } } ``` -#### Abstracting over chain-specific datatypes - -To abstract over chain-specific datatypes, we introduce a trait which specifies -both which types we need to abstract over and their interface. For the Client submodule, -this trait looks as follow: - -```rust -pub trait ClientDef { - type Header: Header; - type ClientState: ClientState + From; - type ConsensusState: ConsensusState; -} -``` - -This trait specifies three datatypes, and their corresponding interface, which is provided -via a trait defined in the same submodule. -Additionally, this trait expresses the requirement for a `ClientState` to be built from -a `ConsensusState`. - -A production implementation of this interface would instantiate these types with the concrete -types used by the chain. - -For the purpose of unit-testing, a mock implementation of the `ClientDef` trait could look as -follows: - -```rust -struct MockHeader(u32); -impl Header for MockHeader { - // omitted -} - -struct MockClientState(u32); -impl ClientState for MockClientState { - // omitted -} - -impl From for MockClientState { - fn from(cs: MockConsensusState) -> Self { - Self(cs.0) - } -} - -struct MockConsensusState(u32); -impl ConsensusState for MockConsensusState { - // omitted -} - -struct MockClient; - -impl ClientDef for MockClient { - type Header = MockHeader; - type ClientState = MockClientState; - type ConsensusState = MockConsensusState; -} -``` - #### Reader A typical message processor will need to read data from the chain state at the current height, via the private and provable stores. To avoid coupling between the message processor interface and the store API, we introduce an interface -for accessing this data. This interface is shared between all message processors in a submodule, as -those typically access the same data. +for accessing this data. This interface, called a `Reader`, is shared between all message processors +in a submodule, as those typically access the same data. Having a high-level interface for this purpose helps avoiding coupling which makes writing unit tests for the message processors easier, as one does not need to provide a concrete store, or to mock one. -We provide below the definition of such an interface, called a `Reader` for the ICS 02 Client submodule: - ```rust -pub trait ClientReader -where - CD: ClientDef, +pub trait ConnectionReader { - fn client_type(&self, client_id: &ClientId) -> Option; - fn client_state(&self, client_id: &ClientId) -> Option; - fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; + fn connection_type(&self, connection_id: &ConnectionId) -> Option; + fn connection_state(&self, connection_id: &ConnectionId) -> Option; } ``` -Because the data exposed by this `Reader` is chain-specific, the `Reader` trait is parametrized -by the type of chain, via the `ClientDef` trait bound. Other submodules may not need the generality -and do away with the type parameter and trait bound altogether. - A production implementation of this `Reader` would hold references to both the private and provable store at the current height where the message processor executes, but we omit the actual implementation as the store interfaces are yet to be defined, as is the general IBC top-level module machinery. -A mock implementation of the `ClientReader` trait could look as follows, given the `MockClient` +A mock implementation of the `ConnectionReader` trait could look as follows, given the `MockConnection` definition provided in the previous section. ```rust -struct MockClientContext { - client_id: ClientId, - client_state: Option, - client_type: Option, - consensus_state: Option, +struct MockConnectionReader { + connection_id: ConnectionId, + connection_type: Option, + connection_state: Option, } -impl ClientContext for MockClientContext { - fn client_type(&self, client_id: &ClientId) -> Option { - if client_id == &self.client_id { - self.client_type.clone() +impl ConnectionReader for MockConnectionReader { + fn connection_type(&self, connection_id: &ConnectionId) -> Option { + if connection_id == &self.connection_id { + self.connection_type.clone() } else { None } } - fn client_state(&self, client_id: &ClientId) -> Option { - if client_id == &self.client_id { - self.client_state - } else { - None - } - } - - fn consensus_state(&self, client_id: &ClientId, _height: Height) -> Option { - if client_id == &self.client_id { - self.consensus_state + fn connection_state(&self, connection_id: &ConnectionId) -> Option { + if connection_id == &self.connection_id { + self.connection_state } else { None } @@ -319,33 +241,28 @@ via the private/provable store interfaces. In the same vein as for the reader de a submodule should define a trait which provides operations to persist such data. The same considerations w.r.t. to coupling and unit-testing apply here as well. -We give below a version of this keeper trait for the Client submodule: - ```rust -pub trait ClientKeeper { +pub trait ConnectionKeeper { fn store_client_type( &mut self, - client_id: ClientId, - client_type: ClientType, + client_id: ConnectionId, + client_type: ConnectionType, ) -> Result<(), Error>; fn store_client_state( &mut self, - client_id: ClientId, - client_state: CD::ClientState, + client_id: ConnectionId, + client_state: ConnectionState, ) -> Result<(), Error>; fn store_consensus_state( &mut self, - client_id: ClientId, - consensus_state: CD::ConsensusState, + client_id: ConnectionId, + consensus_state: ConsensusState, ) -> Result<(), Error>; } ``` -Other submodules may not need the generality and do away with the type parameter and trait -bound altogether. - #### Submodule implementation We now come to the actual definition of a message processor for a submodule. @@ -357,21 +274,14 @@ be defined in `ibc_modules::ics02_client::handler::create_client`. ##### Message type Each message processor must define a datatype which represent the message it can process. -For the "Create Client" sub-protocol of ICS 002, the message would look as follows: ```rust -pub struct MsgCreateClient { - pub client_id: ClientId, - pub client_type: ClientType, - pub consensus_state: C::ConsensusState, +pub struct MsgCreateConnection { + pub connection_id: ConnectionId, + pub connection_type: ConnectionType, } ``` -Again, because the message mentions chain-specific datatypes, it must be parametrized by -the type of chain, bounded by the `ClientDef` trait defined in an earlier section. -Other submodules may not need the generality and do away with the type parameter and trait -bound altogether. - ##### Message processor implementation In this section we provide guidelines for implementating an actual message processor. @@ -385,13 +295,10 @@ The actual logic of the message processor is expressed as a pure function, typic a `HandlerOutput`, where `T` is a concrete datatype and `E` is an error type which defines all potential errors yielded by the message processors of the current submodule. -For the "Create Client" sub-protocol of ICS 002, `T` would be defined as the following datatype: - ```rust -pub struct CreateClientResult { - client_id: ClientId, - client_type: ClientType, - client_state: CD::ClientState, +pub struct CreateConnectionResult { + client_id: ConnectionId, + client_type: ConnectionType, } ``` @@ -401,52 +308,39 @@ datatypes, emit log records and events, and eventually return some data together To this end, this `process` function will create and manipulate a `HandlerOutput` value like described in the corresponding section. -We provide below the actual implementation of the `process` function for the "Create Client" sub-protocol of ICS 002: - ```rust -pub fn process( - reader: &dyn ClientReader, - msg: MsgCreateClient, -) -> HandlerResult, Error> +pub fn process( + reader: &dyn ConnectionReader, + msg: MsgCreateConnection, +) -> HandlerResult where CD: ClientDef, { let mut output = HandlerOutput::builder(); - let MsgCreateClient { - client_id, - client_type, - consensus_state, - } = msg; + let MsgCreateConnection { connection_id, connection_type, } = msg; - if reader.client_state(&client_id).is_some() { - return Err(Kind::ClientAlreadyExists(client_id).into()); + if reader.connection_state(&connection_id).is_some() { + return Err(Kind::ConnectionAlreadyExists(connection_id).into()); } - output.log("success: no client state found"); + output.log("success: no connection state found"); - if reader.client_type(&client_id).is_some() { - return Err(Kind::ClientAlreadyExists(client_id).into()); + if reader.client_type(&connection_id).is_some() { + return Err(Kind::ConnectionAlreadyExists(connection_id).into()); } - output.log("success: no client type found"); - - let client_state = consensus_state.into(); + output.log("success: no connection type found"); - output.emit(ClientEvent::ClientCreated(client_id.clone())); + output.emit(ConnectionEvent::ConnectionCreated(connection_id.clone())); - Ok(output.with_result(CreateClientResult { - client_id, - client_type, - client_state, + Ok(output.with_result(CreateConnectionResult { + connection_id, + connection_state, })) } ``` -Again, because this message processor deals with chain-specific data, the `process` function is parametrized -by the type of chain, via the `ClientDef` trait bound. Other submodules or messages may not need the generality -and do away with the type parameter and trait bound altogether. - ###### Persistence If the `process` function specified above succeeds, the result value it yielded is then @@ -454,18 +348,18 @@ passed to a function named `keep`, which is responsible for persisting the objec by the processing function. This `keep` function takes the submodule's `Keeper` and the result type defined above, and performs side-effecting calls to the keeper's methods to persist the result. -Below is given an implementation of the `keep` function for the "Create Client" message processors: +Below is given an implementation of the `keep` function for the "Create Connection" message processors: ```rust -pub fn keep( - keeper: &mut dyn ClientKeeper, - result: CreateClientResult, +pub fn keep( + keeper: &mut dyn ConnectionKeeper, + result: CreateConnectionResult, ) -> Result<(), Error> where CD: ClientDef, { - keeper.store_client_state(result.client_id.clone(), result.client_state)?; - keeper.store_client_type(result.client_id, result.client_type)?; + keeper.store_connection_state(result.connection_id.clone(), result.connection_state)?; + keeper.store_connection_type(result.connection_id, result.connection_type)?; Ok(()) } @@ -485,12 +379,12 @@ function defined in the previous section. To this end, the submodule should define an enumeration of all messages, in order for the top-level submodule dispatcher to forward them to the appropriate processor. -Such a definition for the ICS 002 Client submodule is given below. +Such a definition for the ICS 002 Connection submodule is given below. ```rust -pub enum ClientMsg { - CreateClient(MsgCreateClient), - UpdateClient(UpdateClientMsg), +pub enum ConnectionMsg { + CreateConnection(MsgCreateConnection), + UpdateConnection(UpdateConnectionMsg), } ``` @@ -500,23 +394,22 @@ Other submodules may not need the generality and do away with the type parameter bound altogether. The actual implementation of a submodule dispatcher is quite straightforward and unlikely to vary -much in substance between submodules. We give an implementation for the ICS 002 Client module below. +much in substance between submodules. We give an implementation for the ICS 002 Connection module below. ```rust -pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: ConnectionMsg) -> Result, Error> where - Client: ClientDef, - Ctx: ClientContext + ClientKeeper, + Ctx: ConnectionReader + ConnectionKeeper, { match msg { - ClientMsg::CreateClient(msg) => { + ConnectionMsg::CreateConnection(msg) => { let HandlerOutput { result, log, events, - } = create_client::process(ctx, msg)?; + } = create_connection::process(ctx, msg)?; - create_client::keep(ctx, result)?; + create_connection::keep(ctx, result)?; Ok(HandlerOutput::builder() .with_log(log) @@ -524,7 +417,7 @@ where .with_result(())) } - ClientMsg::UpdateClient(msg) => // omitted + ConnectionMsg::UpdateConnection(msg) => // omitted } } ``` @@ -532,10 +425,261 @@ where In essence, a top-level dispatcher is a function of a message wrapped in the enumeration introduced above, and a "context" which implements both the `Reader` and `Keeper` interfaces. -It is currently not clear if such a requirement is actually viable, as message processors might need to access -a `Keeper` for various chain types known only at runtime, which would prevent having a static bound -on the context, as expressed above. Further investigations are required to sort this out, hence the -disclaimer at the beginning of this section. +### Dealing with chain-specific datatypes + +The ICS 002 Client submodule stands out from the other submodules as it needs +to deal with chain-specific datatypes, such as `Header`, `ClientState`, and +`ConsensusState`. + +To abstract over chain-specific datatypes, we introduce a trait which specifies +both which types we need to abstract over, and their interface. + +For the ICS 002 Client submodule, this trait looks as follow: + +```rust +pub trait ClientDef { + type Header: Header; + type ClientState: ClientState; + type ConsensusState: ConsensusState; +} +``` + +The `ClientDef` trait specifies three datatypes, and their corresponding interface, which is provided +via a trait defined in the same submodule. + +A production implementation of this interface would instantiate these types with the concrete +types used by the chain, eg. Tendermint datatypes. Each concrete datatype must be provided +with a `From` instance to lift it into its corresponding `Any...` enumeration. + +For the purpose of unit-testing, a mock implementation of the `ClientDef` trait could look as follows: + +```rust +struct MockHeader(u32); + +impl Header for MockHeader { + // omitted +} + +impl From for AnyHeader { + fn from(mh: MockHeader) -> Self { + Self::Mock(mh) + } +} + +struct MockClientState(u32); + +impl ClientState for MockClientState { + // omitted +} + +impl From for AnyClientState { + fn from(mcs: MockClientState) -> Self { + Self::Mock(mcs) + } +} + +struct MockConsensusState(u32); + +impl ConsensusState for MockConsensusState { + // omitted +} + +impl From for AnyConsensusState { + fn from(mcs: MockConsensusState) -> Self { + Self::Mock(mcs) + } +} + +struct MockClient; + +impl ClientDef for MockClient { + type Header = MockHeader; + type ClientState = MockClientState; + type ConsensusState = MockConsensusState; +} +``` + +Since the actual type of client can only be determined at runtime, we cannot encode +the type of client within the message itself. + +Because of some limitations of the Rust type system, namely the lack of proper support +for existential types, it is currently impossible to define `Reader` and `Keeper` traits +which are agnostic to the actual type of client being used. + +We could alternatively model all chain-specific datatypes as boxed trait objects (`Box`), +but this approach runs into a lot of limitations of trait objects, such as the inability to easily +require such trait objects to be Clonable, or Serializable, or to define an equality relation on them. +Some support for such functionality can be found in third-party libraries, but the overall experience +for the developer is too subpar. + +We thus settle on a different strategy: lifting chain-specific data into an `enum` over all +possible chain types. + +For example, to model a chain-specific `Header` type, we would define an enumeration in the following +way: + +```rust +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] // TODO: Add Eq +pub enum AnyHeader { + Mock(mocks::MockHeader), + Tendermint(tendermint::header::Header), +} + +impl Header for AnyHeader { + fn height(&self) -> Height { + match self { + Self::Mock(header) => header.height(), + Self::Tendermint(header) => header.height(), + } + } + + fn client_type(&self) -> ClientType { + match self { + Self::Mock(header) => header.client_type(), + Self::Tendermint(header) => header.client_type(), + } + } +} +``` + +This enumeration dispatches method calls to the underlying datatype at runtime, while +hiding the latter, and is thus akin to a proper existential type without running +into any limitations of the Rust type system (`impl Header` bounds not being allowed +everywhere, `Header` not being able to be treated as a trait objects because of `Clone`, +`PartialEq` and `Serialize`, `Deserialize` bounds, etc.) + +Other chain-specific datatypes, such as `ClientState` and `ConsensusState` require their own +enumeration over all possible implementations. + +On top of that, we also need to lift the specific client definitions (`ClientDef` instances), +into their own enumeration, as follows: + +```rust +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum AnyClient { + Mock(mocks::MockClient), + Tendermint(tendermint::TendermintClient), +} + +impl ClientDef for AnyClient { + type Header = AnyHeader; + type ClientState = AnyClientState; + type ConsensusState = AnyConsensusState; +} +``` + +Messages can now be defined generically over the `ClientDef` instance: + + +```rust +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct MsgCreateClient { + pub client_id: ClientId, + pub client_type: ClientType, + pub consensus_state: CD::ConsensusState, +} + +pub struct MsgUpdateClient { + pub client_id: ClientId, + pub header: CD::Header, +} +``` + +The `Keeper` and `Reader` traits are defined for any client: + +``` +pub trait ClientReader { + fn client_type(&self, client_id: &ClientId) -> Option; + fn client_state(&self, client_id: &ClientId) -> Option; + fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option; +} + +pub trait ClientKeeper { + fn store_client_type( + &mut self, + client_id: ClientId, + client_type: ClientType, + ) -> Result<(), Error>; + + fn store_client_state( + &mut self, + client_id: ClientId, + client_state: AnyClientState, + ) -> Result<(), Error>; + + fn store_consensus_state( + &mut self, + client_id: ClientId, + consensus_state: AnyConsensusState, + ) -> Result<(), Error>; +} +``` + +This way, only one implementation of the `ClientReader` and `ClientKeeper` trait is required, +as it can delegate eg. the serialization of the underlying datatypes to the `Serialize` bound +of the `Any...` wrappper. + +Both the `process` and `keep` function are defined to take a message generic over +the actual client type: + +```rust +pub fn process( + ctx: &dyn ClientReader, + msg: MsgCreateClient, +) -> HandlerResult, Error>; + +pub fn keep( + keeper: &mut dyn ClientKeeper, + result: CreateClientResult, +) -> Result<(), Error>; +``` + +Same for the top-level dispatcher: + +```rust +pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> +where + Ctx: ClientReader + ClientKeeper; +``` + +With this boilerplate out of way, one can write tests using a mock client, and associated mock datatypes +in a fairly straightforward way, taking advantage of the `From` instance to lift concerete mock datatypes +into the `Any...` enumeration: + +```rust + #[test] + fn test_create_client_ok() { + let client_id: ClientId = "mockclient".parse().unwrap(); + + let reader = MockClientReader { + client_id: client_id.clone(), + client_type: None, + client_state: None, + consensus_state: None, + }; + + let msg = MsgCreateClient { + client_id, + client_type: ClientType::Tendermint, + consensus_state: MockConsensusState(42).into(), // lift into `AnyConsensusState` + }; + + let output = process(&reader, msg.clone()); + + match output { + Ok(HandlerOutput { + result, + events, + log, + }) => { + // snip + } + Err(err) => { + panic!("unexpected error: {}", err); + } + } + } +``` ## Status diff --git a/modules/src/ics02_client/client_def.rs b/modules/src/ics02_client/client_def.rs index 5a07c1296c..ebb5c8a206 100644 --- a/modules/src/ics02_client/client_def.rs +++ b/modules/src/ics02_client/client_def.rs @@ -25,11 +25,17 @@ pub enum AnyHeader { impl Header for AnyHeader { fn height(&self) -> Height { - todo!() + match self { + Self::Mock(header) => header.height(), + Self::Tendermint(header) => header.height(), + } } fn client_type(&self) -> ClientType { - todo!() + match self { + Self::Mock(header) => header.client_type(), + Self::Tendermint(header) => header.client_type(), + } } } diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index 2864c643cb..d19583619a 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -13,12 +13,6 @@ use crate::Height; pub mod create_client; pub mod update_client; -pub trait ClientContext { - fn client_type(&self, client_id: &ClientId) -> Option; - fn reader(&self, client_type: &ClientType) -> Box; - fn keeper(&self, client_type: &ClientType) -> Box; -} - pub trait ClientReader { fn client_type(&self, client_id: &ClientId) -> Option; fn client_state(&self, client_id: &ClientId) -> Option; @@ -73,20 +67,17 @@ pub enum ClientMsg { pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> where - Ctx: ClientContext, + Ctx: ClientReader + ClientKeeper, { match msg { ClientMsg::CreateClient(msg) => { - let reader = ctx.reader(&msg.client_type); - let mut keeper = ctx.keeper(&msg.client_type); - let HandlerOutput { result, log, events, - } = create_client::process(&*reader, msg)?; + } = create_client::process(ctx, msg)?; - create_client::keep(&mut *keeper, result)?; + create_client::keep(ctx, result)?; Ok(HandlerOutput::builder() .with_log(log) @@ -94,18 +85,13 @@ where .with_result(())) } ClientMsg::UpdateClient(msg) => { - let client_type = ctx.client_type(&msg.client_id).unwrap(); // FIXME - - let reader = ctx.reader(&client_type); - let mut keeper = ctx.keeper(&client_type); - let HandlerOutput { result, log, events, - } = update_client::process(&*reader, msg)?; + } = update_client::process(ctx, msg)?; - update_client::keep(&mut *keeper, result)?; + update_client::keep(ctx, result)?; Ok(HandlerOutput::builder() .with_log(log) @@ -114,3 +100,4 @@ where } } } + diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index 81d1944fed..eca75c87d9 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -4,7 +4,7 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::ics02_client::client_def::{AnyClient, ClientDef}; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; -use crate::ics02_client::handler::{ClientContext, ClientEvent, ClientKeeper, ClientReader}; +use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; use crate::ics02_client::msgs::MsgCreateClient; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index b9b2238b9d..3d844511d3 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -22,3 +22,4 @@ pub struct MsgUpdateClient { pub client_id: ClientId, pub header: CD::Header, } + From 458d64b60608ce684cafde916836b2461c9a04c2 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Mon, 17 Aug 2020 11:42:00 +0200 Subject: [PATCH 27/28] Fix the connection specifics in ADR --- .../adr-003-handler-implementation.md | 123 +++++++----------- 1 file changed, 50 insertions(+), 73 deletions(-) diff --git a/docs/architecture/adr-003-handler-implementation.md b/docs/architecture/adr-003-handler-implementation.md index 54d8fa7389..f0f5ada59a 100644 --- a/docs/architecture/adr-003-handler-implementation.md +++ b/docs/architecture/adr-003-handler-implementation.md @@ -160,20 +160,20 @@ defined in a previous section should be provided via the `From` trait. ```rust pub enum ConnectionEvent { - ConnectionCreated(ConnectionId), - ConnectionUpdated(ConnectionId), + ConnectionOpenInit(ConnectionId), + ConnectionOpenTry(ConnectionId), } impl From for Event { fn from(ce: ConnectionEvent) -> Event { match ce { - ConnectionEvent::ConnectionCreated(client_id) => Event::new( - EventType::Custom("ConnectionCreated".to_string()), - vec![("connection_id".to_string(), client_id.to_string())], + ConnectionEvent::ConnectionOpenInit(connection_id) => Event::new( + EventType::Custom("ConnectionOpenInit".to_string()), + vec![("connection_id".to_string(), connection_id.to_string())], ), - ConnectionEvent::ConnectionUpdated(client_id) => Event::new( - EventType::Custom("ConnectionUpdated".to_string()), - vec![("connection_id".to_string(), client_id.to_string())], + ConnectionEvent::ConnectionOpenTry(connection_id) => Event::new( + EventType::Custom("ConnectionOpenTry".to_string()), + vec![("connection_id".to_string(), connection_id.to_string())], ), } } @@ -196,8 +196,7 @@ store, or to mock one. ```rust pub trait ConnectionReader { - fn connection_type(&self, connection_id: &ConnectionId) -> Option; - fn connection_state(&self, connection_id: &ConnectionId) -> Option; + fn connection_end(&self, connection_id: &ConnectionId) -> Option; } ``` @@ -205,28 +204,19 @@ A production implementation of this `Reader` would hold references to both the p store at the current height where the message processor executes, but we omit the actual implementation as the store interfaces are yet to be defined, as is the general IBC top-level module machinery. -A mock implementation of the `ConnectionReader` trait could look as follows, given the `MockConnection` -definition provided in the previous section. +A mock implementation of the `ConnectionReader` trait could look as follows: ```rust struct MockConnectionReader { connection_id: ConnectionId, - connection_type: Option, - connection_state: Option, + connection_end: Option, + client_reader: MockClientReader, } impl ConnectionReader for MockConnectionReader { - fn connection_type(&self, connection_id: &ConnectionId) -> Option { + fn connection_end(&self, connection_id: &ConnectionId) -> Option { if connection_id == &self.connection_id { - self.connection_type.clone() - } else { - None - } - } - - fn connection_state(&self, connection_id: &ConnectionId) -> Option { - if connection_id == &self.connection_id { - self.connection_state + self.connection_end.clone() } else { None } @@ -243,22 +233,16 @@ The same considerations w.r.t. to coupling and unit-testing apply here as well. ```rust pub trait ConnectionKeeper { - fn store_client_type( + fn store_connection( &mut self, client_id: ConnectionId, client_type: ConnectionType, ) -> Result<(), Error>; - fn store_client_state( - &mut self, - client_id: ConnectionId, - client_state: ConnectionState, - ) -> Result<(), Error>; - - fn store_consensus_state( + fn add_connection_to_client( &mut self, - client_id: ConnectionId, - consensus_state: ConsensusState, + client_id: ClientId, + connection_id: ConnectionId, ) -> Result<(), Error>; } ``` @@ -276,9 +260,10 @@ be defined in `ibc_modules::ics02_client::handler::create_client`. Each message processor must define a datatype which represent the message it can process. ```rust -pub struct MsgCreateConnection { - pub connection_id: ConnectionId, - pub connection_type: ConnectionType, +pub struct MsgConnectionOpenInit { + connection_id: ConnectionId, + client_id: ClientId, + counterparty: Counterparty, } ``` @@ -296,9 +281,9 @@ a `HandlerOutput`, where `T` is a concrete datatype and `E` is an error ty all potential errors yielded by the message processors of the current submodule. ```rust -pub struct CreateConnectionResult { - client_id: ConnectionId, - client_type: ConnectionType, +pub struct ConnectionMsgProcessingResult { + connection_id: ConnectionId, + connection_end: ConnectionEnd, } ``` @@ -311,32 +296,31 @@ the corresponding section. ```rust pub fn process( reader: &dyn ConnectionReader, - msg: MsgCreateConnection, -) -> HandlerResult -where - CD: ClientDef, + msg: MsgConnectionOpenInit, +) -> HandlerResult { let mut output = HandlerOutput::builder(); - let MsgCreateConnection { connection_id, connection_type, } = msg; + let MsgConnectionOpenInit { connection_id, client_id, counterparty, } = msg; - if reader.connection_state(&connection_id).is_some() { + if reader.connection_end(&connection_id).is_some() { return Err(Kind::ConnectionAlreadyExists(connection_id).into()); } output.log("success: no connection state found"); - if reader.client_type(&connection_id).is_some() { - return Err(Kind::ConnectionAlreadyExists(connection_id).into()); + if reader.client_reader.client_state(&client_id).is_none() { + return Err(Kind::ClientForConnectionMissing(client_id).into()); } - output.log("success: no connection type found"); + output.log("success: client found"); - output.emit(ConnectionEvent::ConnectionCreated(connection_id.clone())); + output.emit(ConnectionEvent::ConnectionOpenInit(connection_id.clone())); - Ok(output.with_result(CreateConnectionResult { + Ok(output.with_result(ConnectionMsgProcessingResult { connection_id, - connection_state, + client_id, + counterparty, })) } ``` @@ -353,13 +337,11 @@ Below is given an implementation of the `keep` function for the "Create Connecti ```rust pub fn keep( keeper: &mut dyn ConnectionKeeper, - result: CreateConnectionResult, + result: ConnectionMsgProcessingResult, ) -> Result<(), Error> -where - CD: ClientDef, { - keeper.store_connection_state(result.connection_id.clone(), result.connection_state)?; - keeper.store_connection_type(result.connection_id, result.connection_type)?; + keeper.store_connection(result.connection_id.clone(), result.connection_end)?; + keeper.add_connection_to_client(result.client_id, result.connection_id)?; Ok(()) } @@ -379,37 +361,32 @@ function defined in the previous section. To this end, the submodule should define an enumeration of all messages, in order for the top-level submodule dispatcher to forward them to the appropriate processor. -Such a definition for the ICS 002 Connection submodule is given below. +Such a definition for the ICS 003 Connection submodule is given below. ```rust pub enum ConnectionMsg { - CreateConnection(MsgCreateConnection), - UpdateConnection(UpdateConnectionMsg), + ConnectionOpenInit(MsgConnectionOpenInit), + ConnectionOpenTry(MsgConnectionOpenTry), + ... } ``` - -Because the messages mention chain-specific datatypes, the whole enumeration must be parametrized by -the type of chain, bounded by the `ClientDef` trait defined in an earlier section. -Other submodules may not need the generality and do away with the type parameter and trait -bound altogether. - The actual implementation of a submodule dispatcher is quite straightforward and unlikely to vary -much in substance between submodules. We give an implementation for the ICS 002 Connection module below. +much in substance between submodules. We give an implementation for the ICS 003 Connection module below. ```rust -pub fn dispatch(ctx: &mut Ctx, msg: ConnectionMsg) -> Result, Error> +pub fn dispatch(ctx: &mut Ctx, msg: Msg) -> Result, Error> where Ctx: ConnectionReader + ConnectionKeeper, { match msg { - ConnectionMsg::CreateConnection(msg) => { + Msg::ConnectionOpenInit(msg) => { let HandlerOutput { result, log, events, - } = create_connection::process(ctx, msg)?; + } = connection_open_init::process(ctx, msg)?; - create_connection::keep(ctx, result)?; + connection::keep(ctx, result)?; Ok(HandlerOutput::builder() .with_log(log) @@ -417,7 +394,7 @@ where .with_result(())) } - ConnectionMsg::UpdateConnection(msg) => // omitted + Msg::ConnectionOpenTry(msg) => // omitted } } ``` @@ -587,7 +564,7 @@ pub struct MsgUpdateClient { The `Keeper` and `Reader` traits are defined for any client: -``` +```rust pub trait ClientReader { fn client_type(&self, client_id: &ClientId) -> Option; fn client_state(&self, client_id: &ClientId) -> Option; From b5716167b3a2b52f25ba0f95e8cf386b1e18e180 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Wed, 19 Aug 2020 19:27:03 +0200 Subject: [PATCH 28/28] Added Tendermint to the new "Any*" enums Added client state to MsgCreateAnyClient, result, keeper, etc Test for concrete tendermint MsgCreateClient Updated TM client and consensus states, MsgCreateClient Extract client and consensus states from MsgCreateClient for TM Extract consensus state from MsgUpdateClient for TM --- modules/src/ics02_client/client_def.rs | 25 +-- modules/src/ics02_client/handler.rs | 7 +- .../src/ics02_client/handler/create_client.rs | 89 ++++++++-- .../src/ics02_client/handler/update_client.rs | 156 +++++++----------- modules/src/ics02_client/mocks.rs | 5 +- modules/src/ics02_client/msgs.rs | 6 +- modules/src/ics02_client/state.rs | 4 +- modules/src/ics07_tendermint/client_def.rs | 13 ++ modules/src/ics07_tendermint/client_state.rs | 68 ++++---- .../src/ics07_tendermint/consensus_state.rs | 9 +- modules/src/ics07_tendermint/header.rs | 31 +++- modules/src/ics07_tendermint/mod.rs | 3 +- .../ics07_tendermint/msgs/create_client.rs | 73 ++++---- .../ics07_tendermint/msgs/update_client.rs | 21 +-- modules/src/ics23_commitment/mod.rs | 6 + modules/src/lib.rs | 2 +- 16 files changed, 285 insertions(+), 233 deletions(-) create mode 100644 modules/src/ics07_tendermint/client_def.rs diff --git a/modules/src/ics02_client/client_def.rs b/modules/src/ics02_client/client_def.rs index ebb5c8a206..592281dbc0 100644 --- a/modules/src/ics02_client/client_def.rs +++ b/modules/src/ics02_client/client_def.rs @@ -4,11 +4,11 @@ use crate::ics02_client::client_type::ClientType; use crate::ics02_client::header::Header; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics23_commitment::CommitmentRoot; -use crate::ics24_host::identifier::ClientId; use crate::Height; use crate::ics02_client::mocks; use crate::ics07_tendermint as tendermint; +use crate::ics07_tendermint::client_def::TendermintClient; pub trait ClientDef: Clone { type Header: Header; @@ -24,17 +24,17 @@ pub enum AnyHeader { } impl Header for AnyHeader { - fn height(&self) -> Height { + fn client_type(&self) -> ClientType { match self { - Self::Mock(header) => header.height(), - Self::Tendermint(header) => header.height(), + Self::Mock(header) => header.client_type(), + Self::Tendermint(header) => header.client_type(), } } - fn client_type(&self) -> ClientType { + fn height(&self) -> Height { match self { - Self::Mock(header) => header.client_type(), - Self::Tendermint(header) => header.client_type(), + Self::Mock(header) => header.height(), + Self::Tendermint(header) => header.height(), } } } @@ -42,10 +42,11 @@ impl Header for AnyHeader { #[derive(Clone, Debug, PartialEq)] pub enum AnyClientState { Mock(mocks::MockClientState), + Tendermint(crate::ics07_tendermint::client_state::ClientState), } impl ClientState for AnyClientState { - fn client_id(&self) -> ClientId { + fn chain_id(&self) -> String { todo!() } @@ -54,7 +55,10 @@ impl ClientState for AnyClientState { } fn get_latest_height(&self) -> Height { - todo!() + match self { + AnyClientState::Tendermint(tm_state) => tm_state.get_latest_height(), + AnyClientState::Mock(mock_state) => mock_state.get_latest_height(), + } } fn is_frozen(&self) -> bool { @@ -72,6 +76,7 @@ impl ClientState for AnyClientState { #[derive(Clone, Debug, PartialEq)] pub enum AnyConsensusState { Mock(mocks::MockConsensusState), + Tendermint(crate::ics07_tendermint::consensus_state::ConsensusState), } impl ConsensusState for AnyConsensusState { @@ -95,6 +100,7 @@ impl ConsensusState for AnyConsensusState { #[derive(Clone, Debug, PartialEq, Eq)] pub enum AnyClient { Mock(mocks::MockClient), + Tendermint(TendermintClient), } impl ClientDef for AnyClient { @@ -102,4 +108,3 @@ impl ClientDef for AnyClient { type ClientState = AnyClientState; type ConsensusState = AnyConsensusState; } - diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs index d19583619a..cb80a63919 100644 --- a/modules/src/ics02_client/handler.rs +++ b/modules/src/ics02_client/handler.rs @@ -4,7 +4,7 @@ use crate::handler::{Event, EventType, HandlerOutput}; use crate::ics02_client::client_def::{AnyClient, AnyClientState, AnyConsensusState, ClientDef}; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::Error; -use crate::ics02_client::msgs::{MsgCreateClient, MsgUpdateClient}; +use crate::ics02_client::msgs::{MsgCreateAnyClient, MsgUpdateAnyClient}; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; @@ -61,8 +61,8 @@ impl From for Event { } pub enum ClientMsg { - CreateClient(MsgCreateClient), - UpdateClient(MsgUpdateClient), + CreateClient(MsgCreateAnyClient), + UpdateClient(MsgUpdateAnyClient), } pub fn dispatch(ctx: &mut Ctx, msg: ClientMsg) -> Result, Error> @@ -100,4 +100,3 @@ where } } } - diff --git a/modules/src/ics02_client/handler/create_client.rs b/modules/src/ics02_client/handler/create_client.rs index eca75c87d9..45f19200b9 100644 --- a/modules/src/ics02_client/handler/create_client.rs +++ b/modules/src/ics02_client/handler/create_client.rs @@ -5,26 +5,29 @@ use crate::ics02_client::client_def::{AnyClient, ClientDef}; use crate::ics02_client::client_type::ClientType; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; -use crate::ics02_client::msgs::MsgCreateClient; +use crate::ics02_client::msgs::MsgCreateAnyClient; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; +use std::time::Duration; #[derive(Debug)] pub struct CreateClientResult { client_id: ClientId, client_type: ClientType, client_state: CD::ClientState, + consensus_state: CD::ConsensusState, } pub fn process( ctx: &dyn ClientReader, - msg: MsgCreateClient, + msg: MsgCreateAnyClient, ) -> HandlerResult, Error> { let mut output = HandlerOutput::builder(); - let MsgCreateClient { + let MsgCreateAnyClient { client_id, client_type, + client_state, consensus_state, } = msg; @@ -40,14 +43,13 @@ pub fn process( output.log("success: no client type found"); - let client_state = todo!(); // CD::new_client_state(&consensus_state); - output.emit(ClientEvent::ClientCreated(client_id.clone())); Ok(output.with_result(CreateClientResult { client_id, client_type, client_state, + consensus_state, })) } @@ -55,8 +57,9 @@ pub fn keep( keeper: &mut dyn ClientKeeper, result: CreateClientResult, ) -> Result<(), Error> { + keeper.store_client_type(result.client_id.clone(), result.client_type)?; keeper.store_client_state(result.client_id.clone(), result.client_state)?; - keeper.store_client_type(result.client_id, result.client_type)?; + keeper.store_consensus_state(result.client_id, result.consensus_state)?; Ok(()) } @@ -67,8 +70,12 @@ mod tests { use crate::ics02_client::header::Header; use crate::ics02_client::mocks::*; use crate::ics02_client::state::{ClientState, ConsensusState}; + use crate::ics07_tendermint::client_def::TendermintClient; + use crate::ics07_tendermint::header::test_util::get_dummy_header; + use crate::ics07_tendermint::msgs::create_client::MsgCreateClient; use crate::ics23_commitment::CommitmentRoot; use crate::Height; + use std::str::FromStr; use thiserror::Error; #[test] @@ -82,9 +89,10 @@ mod tests { consensus_state: None, }; - let msg = MsgCreateClient { + let msg = MsgCreateAnyClient { client_id, client_type: ClientType::Tendermint, + client_state: MockClientState(42).into(), consensus_state: MockConsensusState(42).into(), }; @@ -97,10 +105,6 @@ mod tests { log, }) => { assert_eq!(result.client_type, ClientType::Tendermint); - // assert_eq!( - // result.client_state.as_ref().0, - // msg.consensus_state.as_ref().0 - // ); assert_eq!( events, vec![ClientEvent::ClientCreated(msg.client_id).into()] @@ -130,9 +134,10 @@ mod tests { consensus_state: None, }; - let msg = MsgCreateClient { + let msg = MsgCreateAnyClient { client_id, client_type: ClientType::Tendermint, + client_state: MockClientState(42).into(), consensus_state: MockConsensusState(42).into(), }; @@ -156,9 +161,10 @@ mod tests { consensus_state: None, }; - let msg = MsgCreateClient { + let msg = MsgCreateAnyClient { client_id, client_type: ClientType::Tendermint, + client_state: MockClientState(42).into(), consensus_state: MockConsensusState(42).into(), }; @@ -170,5 +176,60 @@ mod tests { panic!("expected an error"); } } -} + #[test] + fn test_tm_create_client_ok() { + use tendermint::account::Id as AccountId; + + let client_id: ClientId = "tendermint".parse().unwrap(); + let reader = MockClientReader { + client_id: client_id.clone(), + client_type: None, + client_state: None, + consensus_state: None, + }; + + let ics_msg = MsgCreateClient { + client_id, + header: get_dummy_header(), + trusting_period: Duration::from_secs(64000), + unbonding_period: Duration::from_secs(128000), + max_clock_drift: Duration::from_millis(3000), + signer: AccountId::from_str("7C2BB42A8BE69791EC763E51F5A49BCD41E82237").unwrap(), + }; + + //let msg = ics_msg.pre_process(); + let msg = MsgCreateAnyClient { + client_id: ics_msg.client_id().clone(), + client_type: ics_msg.client_type(), + client_state: ics_msg.client_state(), + consensus_state: ics_msg.consensus_state(), + }; + + let output = process(&reader, msg.clone()); + + match output { + Ok(HandlerOutput { + result, + events, + log, + }) => { + assert_eq!(result.client_type, ClientType::Tendermint); + assert_eq!( + events, + vec![ClientEvent::ClientCreated(msg.client_id).into()] + ); + assert_eq!( + log, + vec![ + "success: no client state found".to_string(), + "success: no client type found".to_string() + ] + ); + } + Err(err) => { + panic!("unexpected error: {}", err); + } + } + } +} diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs index 10a23f35ba..d548bfed9e 100644 --- a/modules/src/ics02_client/handler/update_client.rs +++ b/modules/src/ics02_client/handler/update_client.rs @@ -4,7 +4,7 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::ics02_client::client_def::{AnyClient, ClientDef}; use crate::ics02_client::error::{Error, Kind}; use crate::ics02_client::handler::{ClientEvent, ClientKeeper, ClientReader}; -use crate::ics02_client::msgs::MsgUpdateClient; +use crate::ics02_client::msgs::MsgUpdateAnyClient; use crate::ics02_client::state::{ClientState, ConsensusState}; use crate::ics24_host::identifier::ClientId; @@ -17,11 +17,11 @@ pub struct UpdateClientResult { pub fn process( ctx: &dyn ClientReader, - msg: MsgUpdateClient, + msg: MsgUpdateAnyClient, ) -> HandlerResult, Error> { let mut output = HandlerOutput::builder(); - let MsgUpdateClient { client_id, header } = msg; + let MsgUpdateAnyClient { client_id, header } = msg; let client_type = ctx .client_type(&client_id) @@ -36,14 +36,19 @@ pub fn process( .consensus_state(&client_id, latest_height) .ok_or_else(|| Kind::ConsensusStateNotFound(client_id.clone(), latest_height))?; - // CD::check_validity_and_update_state(&mut client_state, &consensus_state, &header).unwrap(); // FIXME + // Use client_state to validate the new header against the latest consensus_state. + // This function will return the new client_state (its latest_height changed) and a + // consensus_state obtained from header. These will be later persisted by the keeper. + // FIXME + // (new_client_state, new_consensus_state) = + // CD::check_validity_and_update_state(client_state, consensus_state, &header)?; output.emit(ClientEvent::ClientUpdated(client_id.clone())); Ok(output.with_result(UpdateClientResult { client_id, - client_state, - consensus_state, + client_state, // new_client_state + consensus_state, // new_consensus_state })) } @@ -57,96 +62,49 @@ pub fn keep( Ok(()) } -// #[cfg(test)] -// mod tests { -// use super::*; -// use crate::ics02_client::client_type::ClientType; -// use crate::ics02_client::header::Header; -// use crate::ics02_client::mocks::*; -// use crate::ics02_client::state::{ClientState, ConsensusState}; -// use crate::ics23_commitment::CommitmentRoot; -// use crate::Height; -// use thiserror::Error; - -// #[test] -// fn test_update_client_ok() { -// let mock = MockClientReader { -// client_id: "mockclient".parse().unwrap(), -// client_type: None, -// client_state: None, -// consensus_state: None, -// }; - -// let msg = MsgUpdateClient { -// client_id: "mockclient".parse().unwrap(), -// header: Box::new(MockHeader(1)), -// }; - -// let output = process(&mock, msg.clone()); - -// match output { -// Ok(HandlerOutput { -// result: _, -// events, -// log, -// }) => { -// // assert_eq!(result.client_state, MockClientState(0)); -// assert_eq!( -// events, -// vec![ClientEvent::ClientUpdated(msg.client_id).into()] -// ); -// assert!(log.is_empty()); -// } -// Err(err) => { -// panic!("unexpected error: {}", err); -// } -// } -// } - -// #[test] -// fn test_update_client_existing_client_type() { -// let mock = MockClientReader { -// client_id: "mockclient".parse().unwrap(), -// client_type: Some(ClientType::Tendermint), -// client_state: None, -// consensus_state: None, -// }; - -// let msg = MsgUpdateClient { -// client_id: "mockclient".parse().unwrap(), -// header: Box::new(MockHeader(1)), -// }; - -// let output = process(&mock, msg.clone()); - -// if let Err(err) = output { -// assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); -// } else { -// panic!("expected an error"); -// } -// } - -// #[test] -// fn test_update_client_existing_client_state() { -// let mock = MockClientReader { -// client_id: "mockclient".parse().unwrap(), -// client_type: None, -// client_state: Some(MockClientState(11).into()), -// consensus_state: None, -// }; - -// #[allow(unreachable_code)] -// let msg = MsgUpdateClient { -// client_id: "mockclient".parse().unwrap(), -// header: MockHeader(42).into(), -// }; - -// let output = process(&mock, msg.clone()); - -// if let Err(err) = output { -// assert_eq!(err.kind(), &Kind::ClientAlreadyExists(msg.client_id)); -// } else { -// panic!("expected an error"); -// } -// } -// } +#[cfg(test)] +mod tests { + use super::*; + use crate::ics02_client::client_type::ClientType; + use crate::ics02_client::header::Header; + use crate::ics02_client::mocks::*; + use crate::ics02_client::state::{ClientState, ConsensusState}; + use crate::ics23_commitment::CommitmentRoot; + use crate::Height; + use thiserror::Error; + + #[test] + fn test_update_client_ok() { + let mock = MockClientReader { + client_id: "mockclient".parse().unwrap(), + client_type: Some(ClientType::Tendermint), + client_state: MockClientState(42).into(), + consensus_state: MockConsensusState(42).into(), + }; + + let msg = MsgUpdateAnyClient { + client_id: "mockclient".parse().unwrap(), + header: MockHeader(46).into(), + }; + + let output = process(&mock, msg.clone()); + + match output { + Ok(HandlerOutput { + result: _, + events, + log, + }) => { + // assert_eq!(result.client_state, MockClientState(0)); + assert_eq!( + events, + vec![ClientEvent::ClientUpdated(msg.client_id).into()] + ); + assert!(log.is_empty()); + } + Err(err) => { + panic!("unexpected error: {}", err); + } + } + } +} diff --git a/modules/src/ics02_client/mocks.rs b/modules/src/ics02_client/mocks.rs index 68b482f0e2..1900fc379f 100644 --- a/modules/src/ics02_client/mocks.rs +++ b/modules/src/ics02_client/mocks.rs @@ -43,7 +43,7 @@ impl From for AnyClientState { } impl ClientState for MockClientState { - fn client_id(&self) -> ClientId { + fn chain_id(&self) -> String { todo!() } @@ -52,7 +52,7 @@ impl ClientState for MockClientState { } fn get_latest_height(&self) -> Height { - todo!() + Height::from(self.0 as u64) } fn is_frozen(&self) -> bool { @@ -183,4 +183,3 @@ impl ClientKeeper for MockClientKeeper { todo!() } } - diff --git a/modules/src/ics02_client/msgs.rs b/modules/src/ics02_client/msgs.rs index 3d844511d3..6ba0bda9be 100644 --- a/modules/src/ics02_client/msgs.rs +++ b/modules/src/ics02_client/msgs.rs @@ -10,16 +10,16 @@ use crate::ics24_host::identifier::ClientId; /// A type of message that triggers the creation of a new on-chain (IBC) client. #[derive(Clone, Debug)] -pub struct MsgCreateClient { +pub struct MsgCreateAnyClient { pub client_id: ClientId, pub client_type: ClientType, + pub client_state: CD::ClientState, pub consensus_state: CD::ConsensusState, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. #[derive(Clone, Debug)] -pub struct MsgUpdateClient { +pub struct MsgUpdateAnyClient { pub client_id: ClientId, pub header: CD::Header, } - diff --git a/modules/src/ics02_client/state.rs b/modules/src/ics02_client/state.rs index 033fb50e9e..eede4f6ec1 100644 --- a/modules/src/ics02_client/state.rs +++ b/modules/src/ics02_client/state.rs @@ -1,7 +1,5 @@ use super::client_type::ClientType; - use crate::ics23_commitment::CommitmentRoot; -use crate::ics24_host::identifier::ClientId; use crate::Height; #[dyn_clonable::clonable] @@ -22,7 +20,7 @@ pub trait ConsensusState: Clone + std::fmt::Debug { #[dyn_clonable::clonable] pub trait ClientState: Clone + std::fmt::Debug { /// Client ID of this state - fn client_id(&self) -> ClientId; + fn chain_id(&self) -> String; /// Type of client associated with this state (eg. Tendermint) fn client_type(&self) -> ClientType; diff --git a/modules/src/ics07_tendermint/client_def.rs b/modules/src/ics07_tendermint/client_def.rs new file mode 100644 index 0000000000..c7611315be --- /dev/null +++ b/modules/src/ics07_tendermint/client_def.rs @@ -0,0 +1,13 @@ +use crate::ics02_client::client_def::ClientDef; +use crate::ics07_tendermint::client_state::ClientState; +use crate::ics07_tendermint::consensus_state::ConsensusState; +use crate::ics07_tendermint::header::Header; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TendermintClient; + +impl ClientDef for TendermintClient { + type Header = Header; + type ClientState = ClientState; + type ConsensusState = ConsensusState; +} diff --git a/modules/src/ics07_tendermint/client_state.rs b/modules/src/ics07_tendermint/client_state.rs index c4d2fe133c..e223867dea 100644 --- a/modules/src/ics07_tendermint/client_state.rs +++ b/modules/src/ics07_tendermint/client_state.rs @@ -1,30 +1,34 @@ use crate::ics02_client::client_type::ClientType; use crate::ics07_tendermint::consensus_state::ConsensusState; use crate::ics07_tendermint::error::{Error, Kind}; -use crate::ics07_tendermint::header::Header; use crate::ics23_commitment::CommitmentRoot; -use crate::ics24_host::identifier::ClientId; use serde_derive::{Deserialize, Serialize}; use std::time::Duration; -use tendermint::lite::Header as liteHeader; +use tendermint::block::Height; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ClientState { - pub id: ClientId, + pub chain_id: String, + // pub trust_level: TrustLevel, pub trusting_period: Duration, pub unbonding_period: Duration, + pub max_clock_drift: Duration, + pub latest_height: crate::Height, pub frozen_height: crate::Height, - pub latest_header: Header, + //pub proof_specs: Specs } impl ClientState { pub fn new( - id: String, + chain_id: String, + // trust_level: TrustLevel, trusting_period: Duration, unbonding_period: Duration, - latest_header: Header, + max_clock_drift: Duration, + latest_height: crate::Height, frozen_height: crate::Height, + // proof_specs: Specs ) -> Result { // Basic validation of trusting period and unbonding period: each should be non-zero. if trusting_period <= Duration::new(0, 0) { @@ -44,23 +48,27 @@ impl ClientState { } // Basic validation for the frozen_height parameter. - if frozen_height != 0 { + if frozen_height != Height(0) { return Err(Kind::ValidationError .context("ClientState cannot be frozen at creation time") .into()); } - // Initially, no validation is needed for the `latest_header`. This has to be validated - // upon updating a client (see `update_client.rs` and fn - // `ClientState::verify_client_consensus_state`). + // Basic validation for the frozen_height parameter. + if latest_height <= Height(0) { + return Err(Kind::ValidationError + .context("ClientState latest height cannot be smaller than zero") + .into()); + } Ok(Self { // TODO: Consider adding a specific 'IdentifierError' Kind, akin to the one in ICS04. - id: id.parse().map_err(|e| Kind::ValidationError.context(e))?, + chain_id, trusting_period, unbonding_period, - latest_header, + max_clock_drift, frozen_height, + latest_height, }) } } @@ -72,8 +80,8 @@ impl From for ClientState { } impl crate::ics02_client::state::ClientState for ClientState { - fn client_id(&self) -> ClientId { - self.id.clone() + fn chain_id(&self) -> String { + self.chain_id.clone() } fn client_type(&self) -> ClientType { @@ -81,12 +89,12 @@ impl crate::ics02_client::state::ClientState for ClientState { } fn get_latest_height(&self) -> crate::Height { - self.latest_header.signed_header.header.height() + self.latest_height } fn is_frozen(&self) -> bool { // If 'frozen_height' is set to a non-zero value, then the client state is frozen. - self.frozen_height != 0 + self.frozen_height != Height(0) } fn verify_client_consensus_state( @@ -100,10 +108,9 @@ impl crate::ics02_client::state::ClientState for ClientState { #[cfg(test)] mod tests { use crate::ics07_tendermint::client_state::ClientState; - use crate::ics07_tendermint::header::test_util::get_dummy_header; - use crate::ics07_tendermint::header::Header; use crate::test::test_serialization_roundtrip; use std::time::Duration; + use tendermint::block::Height; use tendermint_rpc::endpoint::abci_query::AbciQuery; #[test] @@ -128,17 +135,19 @@ mod tests { id: String, trusting_period: Duration, unbonding_period: Duration, - latest_header: Header, + max_clock_drift: Duration, + latest_height: crate::Height, frozen_height: crate::Height, } // Define a "default" set of parameters to reuse throughout these tests. let default_params: ClientStateParams = ClientStateParams { - id: "abcdefghijkl".to_string(), + id: "thisisthechainid".to_string(), trusting_period: Duration::from_secs(64000), unbonding_period: Duration::from_secs(128000), - latest_header: get_dummy_header(), - frozen_height: 0, + max_clock_drift: Duration::from_millis(3000), + latest_height: Height(10), + frozen_height: Height(0), }; struct Test { @@ -153,18 +162,10 @@ mod tests { params: default_params.clone(), want_pass: true, }, - Test { - name: "Invalid client id".to_string(), - params: ClientStateParams { - id: "9000".to_string(), - ..default_params.clone() - }, - want_pass: false, - }, Test { name: "Invalid frozen height parameter (should be 0)".to_string(), params: ClientStateParams { - frozen_height: 1, + frozen_height: Height(1), ..default_params.clone() }, want_pass: false, @@ -205,7 +206,8 @@ mod tests { p.id, p.trusting_period, p.unbonding_period, - p.latest_header, + p.max_clock_drift, + p.latest_height, p.frozen_height, ); diff --git a/modules/src/ics07_tendermint/consensus_state.rs b/modules/src/ics07_tendermint/consensus_state.rs index a8fb21d2b8..1b6d0ab8bb 100644 --- a/modules/src/ics07_tendermint/consensus_state.rs +++ b/modules/src/ics07_tendermint/consensus_state.rs @@ -2,13 +2,14 @@ use crate::ics02_client::client_type::ClientType; use crate::ics23_commitment::CommitmentRoot; use serde_derive::{Deserialize, Serialize}; +use tendermint::Hash; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ConsensusState { - pub root: CommitmentRoot, pub height: crate::Height, pub timestamp: tendermint::time::Time, - pub validator_set: tendermint::validator::Set, + pub root: CommitmentRoot, + pub next_validators_hash: Hash, } impl ConsensusState { @@ -16,13 +17,13 @@ impl ConsensusState { root: CommitmentRoot, height: crate::Height, timestamp: tendermint::time::Time, - validator_set: tendermint::validator::Set, + next_validators_hash: Hash, ) -> Self { Self { root, height, timestamp, - validator_set, + next_validators_hash, } } } diff --git a/modules/src/ics07_tendermint/header.rs b/modules/src/ics07_tendermint/header.rs index 695e2beb3d..d86d13de67 100644 --- a/modules/src/ics07_tendermint/header.rs +++ b/modules/src/ics07_tendermint/header.rs @@ -5,14 +5,28 @@ use tendermint::block::signed_header::SignedHeader; use tendermint::validator::Set as ValidatorSet; use crate::ics02_client::client_type::ClientType; +use crate::ics07_tendermint::consensus_state::ConsensusState; +use crate::ics23_commitment::CommitmentRoot; use crate::Height; /// Tendermint consensus header #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Header { - pub signed_header: SignedHeader, - pub validator_set: ValidatorSet, - pub next_validator_set: ValidatorSet, + pub signed_header: SignedHeader, // contains the commitment root + pub validator_set: ValidatorSet, // the validator set that signed Header + pub trusted_height: Height, // the height of a trusted header seen by client less than or equal to Header + pub trusted_validator_set: ValidatorSet, // the last trusted validator set at trusted height +} + +impl Header { + pub(crate) fn consensus_state(&self) -> ConsensusState { + ConsensusState { + height: self.signed_header.header.height, + timestamp: self.signed_header.header.time, + root: CommitmentRoot::from_bytes(&self.signed_header.header.app_hash), + next_validators_hash: self.signed_header.header.next_validators_hash, + } + } } impl crate::ics02_client::header::Header for Header { @@ -21,9 +35,12 @@ impl crate::ics02_client::header::Header for Header { } fn height(&self) -> Height { - use tendermint::lite::types::Header; - self.signed_header.header.height() + self.signed_header.header.height } + + // fn consensus_state(&self) -> &dyn crate::ics02_client::state::ConsensusState { + // &self.consensus_state() + // } } #[cfg(test)] @@ -31,6 +48,7 @@ pub mod test_util { use crate::ics07_tendermint::header::Header; use subtle_encoding::hex; use tendermint::block::signed_header::SignedHeader; + use tendermint::block::Height; use tendermint::validator::Info as ValidatorInfo; use tendermint::validator::Set as ValidatorSet; use tendermint::{vote, PublicKey}; @@ -64,7 +82,8 @@ pub mod test_util { Header { signed_header: shdr, validator_set: vs.clone(), - next_validator_set: vs, + trusted_height: Height(9), + trusted_validator_set: vs, } } } diff --git a/modules/src/ics07_tendermint/mod.rs b/modules/src/ics07_tendermint/mod.rs index 76391f8d36..c2b0fc503e 100644 --- a/modules/src/ics07_tendermint/mod.rs +++ b/modules/src/ics07_tendermint/mod.rs @@ -1,7 +1,8 @@ //! ICS 07: Tendermint Client +pub mod client_def; pub mod client_state; pub mod consensus_state; pub mod error; pub mod header; -// pub mod msgs; +pub mod msgs; diff --git a/modules/src/ics07_tendermint/msgs/create_client.rs b/modules/src/ics07_tendermint/msgs/create_client.rs index 19b56ed84f..94489ef169 100644 --- a/modules/src/ics07_tendermint/msgs/create_client.rs +++ b/modules/src/ics07_tendermint/msgs/create_client.rs @@ -1,12 +1,12 @@ -use crate::ics02_client::client_type::ClientType; -use crate::ics07_tendermint::consensus_state::ConsensusState; use crate::ics07_tendermint::header::Header; -use crate::ics23_commitment::CommitmentRoot; use crate::ics24_host::identifier::ClientId; use crate::tx_msg::Msg; use std::time::Duration; +use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState}; +use crate::ics02_client::client_type::ClientType; +use crate::ics07_tendermint::client_state::ClientState; use serde_derive::{Deserialize, Serialize}; use tendermint::account::Id as AccountId; @@ -14,36 +14,58 @@ pub const TYPE_MSG_CREATE_CLIENT: &str = "create_client"; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct MsgCreateClient { - client_id: ClientId, - header: Header, - trusting_period: Duration, - bonding_period: Duration, - signer: AccountId, + pub client_id: ClientId, + pub header: Header, + // trust_level: Fraction, + pub trusting_period: Duration, + pub unbonding_period: Duration, + pub max_clock_drift: Duration, + // proof_specs: ProofSpecs, + pub signer: AccountId, } impl MsgCreateClient { pub fn new( client_id: ClientId, header: Header, + // trust_level: Fraction, trusting_period: Duration, - bonding_period: Duration, + unbonding_period: Duration, + max_clock_drift: Duration, + // proof_specs: ProofSpecs, signer: AccountId, ) -> Self { Self { client_id, header, trusting_period, - bonding_period, + unbonding_period, + max_clock_drift, signer, } } - fn get_client_id(&self) -> &ClientId { + pub(crate) fn client_id(&self) -> &ClientId { &self.client_id } - fn get_header(&self) -> &Header { - &self.header + pub(crate) fn client_type(&self) -> ClientType { + ClientType::Tendermint + } + + pub(crate) fn consensus_state(&self) -> AnyConsensusState { + AnyConsensusState::Tendermint(self.header.consensus_state()) + } + + pub(crate) fn client_state(&self) -> AnyClientState { + AnyClientState::Tendermint(ClientState { + chain_id: self.header.signed_header.header.chain_id.to_string(), + trusting_period: self.trusting_period, + unbonding_period: self.unbonding_period, + max_clock_drift: self.max_clock_drift, + latest_height: self.header.signed_header.header.height, + frozen_height: 0.into(), + }) } } @@ -71,28 +93,3 @@ impl Msg for MsgCreateClient { vec![self.signer] } } - -impl crate::ics02_client::msgs::MsgCreateClient for MsgCreateClient { - type ConsensusState = ConsensusState; - - fn client_id(&self) -> &ClientId { - &self.client_id - } - - fn client_type(&self) -> ClientType { - ClientType::Tendermint - } - - fn consensus_state(&self) -> Self::ConsensusState { - let root = CommitmentRoot; // TODO - let header = &self.header.signed_header.header; - - ConsensusState::new( - root, - header.height.into(), - header.time, - self.header.validator_set.clone(), - ) - } -} - diff --git a/modules/src/ics07_tendermint/msgs/update_client.rs b/modules/src/ics07_tendermint/msgs/update_client.rs index 5b5cb1350a..597e4d88f0 100644 --- a/modules/src/ics07_tendermint/msgs/update_client.rs +++ b/modules/src/ics07_tendermint/msgs/update_client.rs @@ -2,6 +2,7 @@ use crate::ics07_tendermint::header::Header; use crate::ics24_host::identifier::ClientId; use crate::tx_msg::Msg; +use crate::ics07_tendermint::consensus_state::ConsensusState; use serde_derive::{Deserialize, Serialize}; use tendermint::account::Id as AccountId; @@ -23,13 +24,17 @@ impl MsgUpdateClient { } } - fn get_client_id(&self) -> &ClientId { + fn client_id(&self) -> &ClientId { &self.client_id } - fn get_header(&self) -> &Header { + fn header(&self) -> &Header { &self.header } + + fn consensus_state(&self) -> ConsensusState { + self.header.consensus_state() + } } impl Msg for MsgUpdateClient { @@ -56,15 +61,3 @@ impl Msg for MsgUpdateClient { vec![self.signer] } } - -impl crate::ics02_client::msgs::MsgUpdateClient for MsgUpdateClient { - type Header = Header; - - fn client_id(&self) -> &ClientId { - &self.client_id - } - - fn header(&self) -> &Self::Header { - &self.header - } -} diff --git a/modules/src/ics23_commitment/mod.rs b/modules/src/ics23_commitment/mod.rs index 23251942ff..ced4f91e13 100644 --- a/modules/src/ics23_commitment/mod.rs +++ b/modules/src/ics23_commitment/mod.rs @@ -5,6 +5,12 @@ use tendermint::merkle::proof::Proof; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct CommitmentRoot; +impl CommitmentRoot { + pub fn from_bytes(_bytes: &[u8]) -> Self { + // TODO + CommitmentRoot {} + } +} #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct CommitmentPath; diff --git a/modules/src/lib.rs b/modules/src/lib.rs index 88020d6976..618dda4c8e 100644 --- a/modules/src/lib.rs +++ b/modules/src/lib.rs @@ -34,7 +34,7 @@ pub mod try_from_raw; pub mod tx_msg; /// Height of a block, same as in `tendermint` crate -pub type Height = tendermint::lite::Height; +pub type Height = tendermint::block::Height; #[cfg(test)] mod test;