From a33c51341aa74ce6511ac04dbf9e2bf154c2048d Mon Sep 17 00:00:00 2001 From: David Salami <31099392+Wizdave97@users.noreply.github.com> Date: Thu, 20 Apr 2023 15:13:52 +0100 Subject: [PATCH] Prioritize batch messaging (#30) * batch messaging * update docs * let router process messages individually * nit --- src/consensus_client.rs | 10 ++++---- src/handlers.rs | 20 +++++---------- src/handlers/request.rs | 12 +++------ src/handlers/response.rs | 33 +++++++++++------------- src/handlers/timeout.rs | 54 +++++++++++++++++++--------------------- src/messaging.rs | 18 +++++++------- src/router.rs | 52 +++++++++++++++++++++++++++----------- 7 files changed, 101 insertions(+), 98 deletions(-) diff --git a/src/consensus_client.rs b/src/consensus_client.rs index 3dd224ef6..e65616b05 100644 --- a/src/consensus_client.rs +++ b/src/consensus_client.rs @@ -92,7 +92,7 @@ pub trait ConsensusClient { /// Return unbonding period fn unbonding_period(&self) -> Duration; - /// Verify the merkle mountain range membership of proof of a request/response. + /// Verify the merkle mountain range membership proof of a batch of requests/responses. fn verify_membership( &self, host: &dyn ISMPHost, @@ -101,17 +101,17 @@ pub trait ConsensusClient { proof: &Proof, ) -> Result<(), Error>; - /// Transform the request/response into it's key in the state trie. - fn state_trie_key(&self, request: RequestResponse) -> Vec; + /// Transform the requests/responses into their equivalent key in the state trie. + fn state_trie_key(&self, request: RequestResponse) -> Vec>; /// Verify the state of proof of some arbitrary data. Should return the verified data fn verify_state_proof( &self, host: &dyn ISMPHost, - key: Vec, + keys: Vec>, root: StateCommitment, proof: &Proof, - ) -> Result>, Error>; + ) -> Result>>, Error>; /// Decode trusted state and check if consensus client is frozen fn is_frozen(&self, trusted_consensus_state: &[u8]) -> Result<(), Error>; diff --git a/src/handlers.rs b/src/handlers.rs index b8271a4e9..067268f70 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -18,10 +18,11 @@ use crate::{ consensus_client::{ConsensusClient, ConsensusClientId, StateMachineHeight}, error::Error, - host::{ISMPHost, StateMachine}, + host::ISMPHost, messaging::{Message, Proof}, + router::DispatchResult, }; -use alloc::{boxed::Box, collections::BTreeSet}; +use alloc::{boxed::Box, collections::BTreeSet, vec::Vec}; mod consensus; mod request; @@ -40,22 +41,13 @@ pub struct ConsensusClientCreatedResult { pub consensus_client_id: ConsensusClientId, } -pub struct RequestResponseResult { - /// Destination chain for request or response - pub dest_chain: StateMachine, - /// Source chain for request or response - pub source_chain: StateMachine, - /// Request nonce - pub nonce: u64, -} - /// Result returned when ismp messages are handled successfully pub enum MessageResult { ConsensusMessage(ConsensusUpdateResult), - Request(RequestResponseResult), - Response(RequestResponseResult), + Request(Vec), + Response(Vec), ConsensusClientCreated(ConsensusClientCreatedResult), - Timeout(RequestResponseResult), + Timeout(Vec), } /// This function serves as an entry point to handle the message types provided by the ISMP protocol diff --git a/src/handlers/request.rs b/src/handlers/request.rs index 29f989904..c77c3bb41 100644 --- a/src/handlers/request.rs +++ b/src/handlers/request.rs @@ -17,7 +17,7 @@ use crate::{ error::Error, - handlers::{validate_state_machine, MessageResult, RequestResponseResult}, + handlers::{validate_state_machine, MessageResult}, host::ISMPHost, messaging::RequestMessage, router::RequestResponse, @@ -33,20 +33,14 @@ where let state = host.state_machine_commitment(msg.proof.height)?; consensus_client.verify_membership( host, - RequestResponse::Request(msg.request.clone()), + RequestResponse::Request(msg.requests.clone()), state, &msg.proof, )?; let router = host.ismp_router(); - let result = RequestResponseResult { - dest_chain: msg.request.dest_chain(), - source_chain: msg.request.source_chain(), - nonce: msg.request.nonce(), - }; - - router.dispatch(msg.request)?; + let result = msg.requests.into_iter().map(|request| router.dispatch(request)).collect(); Ok(MessageResult::Request(result)) } diff --git a/src/handlers/response.rs b/src/handlers/response.rs index b52710b00..d843e8811 100644 --- a/src/handlers/response.rs +++ b/src/handlers/response.rs @@ -17,7 +17,7 @@ use crate::{ error::Error, - handlers::{validate_state_machine, MessageResult, RequestResponseResult}, + handlers::{validate_state_machine, MessageResult}, host::ISMPHost, messaging::ResponseMessage, router::RequestResponse, @@ -30,35 +30,32 @@ where H: ISMPHost, { let consensus_client = validate_state_machine(host, &msg.proof)?; - // For a response to be valid a request commitment must be present in storage - let commitment = host.request_commitment(&msg.response.request)?; - - if commitment != hash_request::(&msg.response.request) { - return Err(Error::RequestCommitmentNotFound { - nonce: msg.response.request.nonce(), - source: msg.response.request.source_chain(), - dest: msg.response.request.dest_chain(), - }) + for response in &msg.responses { + // For a response to be valid a request commitment must be present in storage + let commitment = host.request_commitment(&response.request)?; + + if commitment != hash_request::(&response.request) { + return Err(Error::RequestCommitmentNotFound { + nonce: response.request.nonce(), + source: response.request.source_chain(), + dest: response.request.dest_chain(), + }) + } } let state = host.state_machine_commitment(msg.proof.height)?; // Verify membership proof consensus_client.verify_membership( host, - RequestResponse::Response(msg.response.clone()), + RequestResponse::Response(msg.responses.clone()), state, &msg.proof, )?; let router = host.ismp_router(); - let result = RequestResponseResult { - dest_chain: msg.response.request.source_chain(), - source_chain: msg.response.request.dest_chain(), - nonce: msg.response.request.nonce(), - }; - - router.write_response(msg.response)?; + let result = + msg.responses.into_iter().map(|response| router.write_response(response)).collect(); Ok(MessageResult::Response(result)) } diff --git a/src/handlers/timeout.rs b/src/handlers/timeout.rs index 4a530b716..c11fe0cd1 100644 --- a/src/handlers/timeout.rs +++ b/src/handlers/timeout.rs @@ -17,7 +17,7 @@ use crate::{ error::Error, - handlers::{validate_state_machine, MessageResult, RequestResponseResult}, + handlers::{validate_state_machine, MessageResult}, host::ISMPHost, messaging::TimeoutMessage, router::RequestResponse, @@ -30,42 +30,38 @@ where H: ISMPHost, { let consensus_client = validate_state_machine(host, &msg.timeout_proof)?; - let commitment = host.request_commitment(&msg.request)?; - if commitment != hash_request::(&msg.request) { - return Err(Error::RequestCommitmentNotFound { - nonce: msg.request.nonce(), - source: msg.request.source_chain(), - dest: msg.request.dest_chain(), - }) - } - let state = host.state_machine_commitment(msg.timeout_proof.height)?; - if !msg.request.timed_out(state.timestamp()) { - Err(Error::RequestTimeoutNotElapsed { - nonce: msg.request.nonce(), - source: msg.request.source_chain(), - dest: msg.request.dest_chain(), - timeout_timestamp: msg.request.timeout(), - state_machine_time: state.timestamp(), - })? + for request in &msg.requests { + let commitment = host.request_commitment(request)?; + if commitment != hash_request::(request) { + return Err(Error::RequestCommitmentNotFound { + nonce: request.nonce(), + source: request.source_chain(), + dest: request.dest_chain(), + }) + } + + if !request.timed_out(state.timestamp()) { + Err(Error::RequestTimeoutNotElapsed { + nonce: request.nonce(), + source: request.source_chain(), + dest: request.dest_chain(), + timeout_timestamp: request.timeout(), + state_machine_time: state.timestamp(), + })? + } } - let key = consensus_client.state_trie_key(RequestResponse::Request(msg.request.clone())); + let key = consensus_client.state_trie_key(RequestResponse::Request(msg.requests.clone())); - let request = consensus_client.verify_state_proof(host, key, state, &msg.timeout_proof)?; + let requests = consensus_client.verify_state_proof(host, key, state, &msg.timeout_proof)?; - if request.is_some() { - Err(Error::ImplementationSpecific("Request not timed out".into()))? + if requests.into_iter().any(|val| val.is_some()) { + Err(Error::ImplementationSpecific("Some Requests not timed out".into()))? } - let result = RequestResponseResult { - dest_chain: msg.request.source_chain(), - source_chain: msg.request.dest_chain(), - nonce: msg.request.nonce(), - }; - let router = host.ismp_router(); - router.dispatch_timeout(msg.request)?; + let result = msg.requests.into_iter().map(|request| router.dispatch_timeout(request)).collect(); Ok(MessageResult::Timeout(result)) } diff --git a/src/messaging.rs b/src/messaging.rs index 6bcd39efe..3f7be74df 100644 --- a/src/messaging.rs +++ b/src/messaging.rs @@ -42,25 +42,25 @@ pub struct CreateConsensusClient { #[derive(Debug, Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)] pub struct RequestMessage { - /// Request from source chain - pub request: Request, - /// Membership proof for this request + /// Requests from source chain + pub requests: Vec, + /// Membership batch proof for these requests pub proof: Proof, } #[derive(Debug, Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)] pub struct ResponseMessage { - /// Response from sink chain - pub response: Response, - /// Membership proof for this response + /// Responses from sink chain + pub responses: Vec, + /// Membership batch proof for these responses pub proof: Proof, } #[derive(Debug, Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)] pub struct TimeoutMessage { - /// Request timeout request - pub request: Request, - /// Non membership state Proof for the timeout + /// Request timeouts + pub requests: Vec, + /// Non membership batch proof for these requests pub timeout_proof: Proof, } diff --git a/src/router.rs b/src/router.rs index 1e6c8f5b4..f28bbce34 100644 --- a/src/router.rs +++ b/src/router.rs @@ -15,7 +15,8 @@ //! ISMPRouter definition -use crate::{consensus_client::StateMachineHeight, error::Error, host::StateMachine, prelude::Vec}; +use crate::{consensus_client::StateMachineHeight, host::StateMachine, prelude::Vec}; +use alloc::string::String; use codec::{Decode, Encode}; use core::time::Duration; @@ -141,20 +142,43 @@ pub type GetResponse = Vec<(Vec, Vec)>; /// Convenience enum for membership verification. pub enum RequestResponse { - Request(Request), - Response(Response), + Request(Vec), + Response(Vec), } -pub trait ISMPRouter { - /// Dispatch a request from a module to the ISMP router. - /// If request source chain is the host, it should be committed in state as a sha256 hash - fn dispatch(&self, request: Request) -> Result<(), Error>; - - /// Dispatch a request timeout from a module to the ISMP router. - /// If request source chain is the host, it should be committed in state as a sha256 hash - fn dispatch_timeout(&self, request: Request) -> Result<(), Error>; +#[derive(Debug, PartialEq, Eq)] +pub enum DispatchResult { + Error { + /// Descriptive error message + msg: String, + /// Request nonce + nonce: u64, + /// Source chain for request or response + source: StateMachine, + /// Destination chain for request or response + dest: StateMachine, + }, + Success { + /// Destination chain for request or response + dest_chain: StateMachine, + /// Source chain for request or response + source_chain: StateMachine, + /// Request nonce + nonce: u64, + }, +} - /// Provide a response to a previously received request. - /// If response source chain is the host, it should be committed in state as a sha256 hash - fn write_response(&self, response: Response) -> Result<(), Error>; +pub trait ISMPRouter { + /// Dispatch some requests to the ISMP router. + /// For outgoing requests, they should be committed in state as a keccak256 hash + /// For incoming requests, they should be dispatched to destination modules + fn dispatch(&self, request: Request) -> DispatchResult; + + /// Dispatch request timeouts to the router which should dispatch them to modules + fn dispatch_timeout(&self, request: Request) -> DispatchResult; + + /// Dispatch some responses to the ISMP router. + /// For outgoing responses, the router should commit them to host state as a keccak256 hash + /// For incoming responses, they should be dispatched to destination modules + fn write_response(&self, response: Response) -> DispatchResult; }