From 5e7264d39bbc6e94f2b45b0965d7b596c08ae7d0 Mon Sep 17 00:00:00 2001 From: Naohiro Yoshida Date: Wed, 3 Jul 2024 18:23:53 +0900 Subject: [PATCH] Include Client's Input when an error occurs. (#51) change error Signed-off-by: Naohiro Yoshida --- light-client/src/client.rs | 190 +++++++++++++++++++++++++++------ light-client/src/commitment.rs | 7 +- light-client/src/errors.rs | 101 ++++++++++++++++-- 3 files changed, 254 insertions(+), 44 deletions(-) diff --git a/light-client/src/client.rs b/light-client/src/client.rs index 07ae57e..c7fa8a0 100644 --- a/light-client/src/client.rs +++ b/light-client/src/client.rs @@ -18,7 +18,7 @@ use crate::commitment::{ calculate_ibc_commitment_storage_key, decode_eip1184_rlp_proof, verify_proof, }; use crate::consensus_state::ConsensusState; -use crate::errors::Error; +use crate::errors::{ClientError, Error}; use crate::header::Header; use crate::message::ClientMessage; @@ -38,16 +38,125 @@ impl LightClient for ParliaLightClient { client_id: &ClientId, ) -> Result { let any_client_state = ctx.client_state(client_id)?; - let client_state = ClientState::try_from(any_client_state)?; + let client_state = + ClientState::try_from(any_client_state).map_err(|e| ClientError::LatestHeight { + cause: e, + client_id: client_id.clone(), + })?; Ok(client_state.latest_height) } fn create_client( &self, - _ctx: &dyn HostClientReader, + ctx: &dyn HostClientReader, any_client_state: Any, any_consensus_state: Any, ) -> Result { + InnerLightClient + .create_client(ctx, any_client_state.clone(), any_consensus_state.clone()) + .map_err(|e| { + ClientError::CreateClient { + cause: e, + client_state: any_client_state, + consensus_sate: any_consensus_state, + } + .into() + }) + } + + fn update_client( + &self, + ctx: &dyn HostClientReader, + client_id: ClientId, + any_message: Any, + ) -> Result { + InnerLightClient + .update_client(ctx, client_id.clone(), any_message.clone()) + .map_err(|e| { + ClientError::UpdateClient { + cause: e, + client_id, + message: any_message, + } + .into() + }) + } + + fn verify_membership( + &self, + ctx: &dyn HostClientReader, + client_id: ClientId, + prefix: CommitmentPrefix, + path: String, + value: Vec, + proof_height: Height, + proof: Vec, + ) -> Result { + InnerLightClient + .verify_membership( + ctx, + client_id.clone(), + prefix.clone(), + path.clone(), + value.clone(), + proof_height, + proof.clone(), + ) + .map_err(|e| { + ClientError::VerifyMembership { + cause: e, + client_id, + prefix, + path, + value, + proof_height, + proof, + } + .into() + }) + } + + fn verify_non_membership( + &self, + ctx: &dyn HostClientReader, + client_id: ClientId, + prefix: CommitmentPrefix, + path: String, + proof_height: Height, + proof: Vec, + ) -> Result { + InnerLightClient + .verify_non_membership( + ctx, + client_id.clone(), + prefix.clone(), + path.clone(), + proof_height, + proof.clone(), + ) + .map_err(|e| { + ClientError::VerifyNonMembership { + cause: e, + client_id, + prefix, + path, + proof_height, + proof, + } + .into() + }) + } +} + +struct InnerLightClient; + +impl InnerLightClient { + fn create_client( + &self, + _ctx: &dyn HostClientReader, + any_client_state: Any, + any_consensus_state: Any, + ) -> Result { let client_state = ClientState::try_from(any_client_state.clone())?; let consensus_state = ConsensusState::try_from(any_consensus_state)?; @@ -77,7 +186,7 @@ impl LightClient for ParliaLightClient { ctx: &dyn HostClientReader, client_id: ClientId, any_message: Any, - ) -> Result { + ) -> Result { match ClientMessage::try_from(any_message.clone())? { ClientMessage::Header(header) => Ok(self.update_state(ctx, client_id, header)?.into()), ClientMessage::Misbehaviour(misbehavior) => { @@ -96,6 +205,7 @@ impl LightClient for ParliaLightClient { } } + #[allow(clippy::too_many_arguments)] fn verify_membership( &self, ctx: &dyn HostClientReader, @@ -105,7 +215,7 @@ impl LightClient for ParliaLightClient { value: Vec, proof_height: Height, proof: Vec, - ) -> Result { + ) -> Result { let value = keccak_256(&value); let state_id = self.verify_commitment( ctx, @@ -136,33 +246,33 @@ impl LightClient for ParliaLightClient { path: String, proof_height: Height, proof: Vec, - ) -> Result { + ) -> Result { let state_id = self.verify_commitment(ctx, client_id, &prefix, &path, None, &proof_height, proof)?; Ok(VerifyNonMembershipResult { message: VerifyMembershipProxyMessage::new(prefix, path, None, proof_height, state_id), }) } -} -impl ParliaLightClient { pub fn update_state( &self, ctx: &dyn HostClientReader, client_id: ClientId, header: Header, - ) -> Result { + ) -> Result { //Ensure header can be verified. let height = header.height(); let timestamp = header.timestamp()?; let trusted_height = header.trusted_height(); - let any_client_state = ctx.client_state(&client_id)?; - let any_consensus_state = ctx.consensus_state(&client_id, &trusted_height)?; + let any_client_state = ctx.client_state(&client_id).map_err(Error::LCPError)?; + let any_consensus_state = ctx + .consensus_state(&client_id, &trusted_height) + .map_err(Error::LCPError)?; //Ensure client is not frozen let client_state = ClientState::try_from(any_client_state)?; if client_state.frozen { - return Err(Error::ClientFrozen(client_id).into()); + return Err(Error::ClientFrozen(client_id)); } // Create new state and ensure header is valid @@ -207,16 +317,18 @@ impl ParliaLightClient { ctx: &dyn HostClientReader, client_id: ClientId, misbehaviour: Misbehaviour, - ) -> Result<(ClientState, Vec, ValidationContext), LightClientError> { - let any_client_state = ctx.client_state(&client_id)?; - let any_consensus_state1 = - ctx.consensus_state(&client_id, &misbehaviour.header_1.trusted_height())?; - let any_consensus_state2 = - ctx.consensus_state(&client_id, &misbehaviour.header_2.trusted_height())?; + ) -> Result<(ClientState, Vec, ValidationContext), Error> { + let any_client_state = ctx.client_state(&client_id).map_err(Error::LCPError)?; + let any_consensus_state1 = ctx + .consensus_state(&client_id, &misbehaviour.header_1.trusted_height()) + .map_err(Error::LCPError)?; + let any_consensus_state2 = ctx + .consensus_state(&client_id, &misbehaviour.header_2.trusted_height()) + .map_err(Error::LCPError)?; let client_state = ClientState::try_from(any_client_state)?; if client_state.frozen { - return Err(Error::ClientFrozen(client_id).into()); + return Err(Error::ClientFrozen(client_id)); } let trusted_consensus_state1 = ConsensusState::try_from(any_consensus_state1)?; @@ -266,20 +378,24 @@ impl ParliaLightClient { value: Option>, proof_height: &Height, storage_proof_rlp: Vec, - ) -> Result { - let client_state = ClientState::try_from(ctx.client_state(&client_id)?)?; + ) -> Result { + let client_state = + ClientState::try_from(ctx.client_state(&client_id).map_err(Error::LCPError)?)?; if client_state.frozen { - return Err(Error::ClientFrozen(client_id).into()); + return Err(Error::ClientFrozen(client_id)); } let proof_height = *proof_height; if client_state.latest_height < proof_height { - return Err( - Error::UnexpectedProofHeight(proof_height, client_state.latest_height).into(), - ); + return Err(Error::UnexpectedProofHeight( + proof_height, + client_state.latest_height, + )); } - let consensus_state = - ConsensusState::try_from(ctx.consensus_state(&client_id, &proof_height)?)?; + let consensus_state = ConsensusState::try_from( + ctx.consensus_state(&client_id, &proof_height) + .map_err(Error::LCPError)?, + )?; let storage_root = consensus_state.state_root; let storage_proof = decode_eip1184_rlp_proof(&storage_proof_rlp)?; verify_proof( @@ -299,11 +415,13 @@ impl ParliaLightClient { client_id: &ClientId, client_state: &ClientState, heights: Vec, - ) -> Result, LightClientError> { + ) -> Result, Error> { let mut prev_states = Vec::new(); for height in heights { - let consensus_state: ConsensusState = - ctx.consensus_state(client_id, &height)?.try_into()?; + let consensus_state: ConsensusState = ctx + .consensus_state(client_id, &height) + .map_err(Error::LCPError)? + .try_into()?; prev_states.push(PrevState { height, state_id: gen_state_id(client_state.clone(), consensus_state)?, @@ -316,10 +434,12 @@ impl ParliaLightClient { fn gen_state_id( client_state: ClientState, consensus_state: ConsensusState, -) -> Result { +) -> Result { let client_state = Any::try_from(client_state.canonicalize())?; let consensus_state = Any::try_from(consensus_state.canonicalize())?; - gen_state_id_from_any(&client_state, &consensus_state).map_err(LightClientError::commitment) + gen_state_id_from_any(&client_state, &consensus_state) + .map_err(LightClientError::commitment) + .map_err(Error::LCPError) } #[cfg(test)] @@ -412,7 +532,7 @@ mod test { .client_state .clone() .ok_or_else(|| light_client::Error::client_state_not_found(client_id.clone()))?; - Ok(Any::try_from(cs)?) + Ok(Any::try_from(cs).unwrap()) } fn consensus_state( @@ -427,7 +547,7 @@ mod test { light_client::Error::consensus_state_not_found(client_id.clone(), *height) })? .clone(); - Ok(Any::try_from(state)?) + Ok(Any::try_from(state).unwrap()) } } @@ -792,7 +912,7 @@ mod test { true, ) .unwrap_err(); - let expected = format!("{:?}", err).contains(" ClientFrozen: xx-parlia-0"); + let expected = format!("{:?}", err).contains("ClientFrozen: xx-parlia-0"); assert!(expected, "{}", err); } diff --git a/light-client/src/commitment.rs b/light-client/src/commitment.rs index 20b50f3..dd378be 100644 --- a/light-client/src/commitment.rs +++ b/light-client/src/commitment.rs @@ -67,11 +67,12 @@ pub fn verify_proof( Ok(()) } -fn verify(root: &Hash, proof: &[Vec], key: &[u8]) -> Result>, Error> { +fn verify(raw_root: &Hash, proof: &[Vec], key: &[u8]) -> Result>, Error> { let db = StorageProof::new(proof.to_vec()).into_memory_db::(); - let root: primitive_types::H256 = root.into(); + let root: primitive_types::H256 = raw_root.into(); let trie = TrieDBBuilder::>::new(&db, &root).build(); - trie.get(&keccak_256(key)).map_err(Error::TrieError) + trie.get(&keccak_256(key)) + .map_err(|e| Error::TrieError(e, *raw_root, proof.to_vec(), key.to_vec())) } fn trim_left_zero(value: &[u8]) -> &[u8] { diff --git a/light-client/src/errors.rs b/light-client/src/errors.rs index 895f97b..51ffd19 100644 --- a/light-client/src/errors.rs +++ b/light-client/src/errors.rs @@ -3,8 +3,8 @@ use alloc::vec::Vec; use core::fmt::Formatter; use k256::ecdsa::signature; -use light_client::commitments::Error as CommitmentError; -use light_client::types::{ClientId, Height, Time, TimeError}; +use light_client::commitments::{CommitmentPrefix, Error as CommitmentError}; +use light_client::types::{Any, ClientId, Height, Time, TimeError}; use trie_db::TrieError; use crate::misc::{Address, BlockNumber, Hash}; @@ -111,7 +111,11 @@ pub enum Error { UnexpectedClientId(String), UnexpectedDifferentHeight(Height, Height), UnexpectedSameBlockHash(Height), - TrieError(BoxedTrieError), + + TrieError(BoxedTrieError, Hash, Vec>, Vec), + + // Framework + LCPError(light_client::Error), } impl core::fmt::Display for Error { @@ -207,8 +211,8 @@ impl core::fmt::Display for Error { e1, e2, e3, e4, e5 ) } - Error::TrieError(e1) => { - write!(f, "TrieError : {:?}", e1) + Error::TrieError(e1, e2, e3, e4) => { + write!(f, "TrieError : {:?} {:?} {:?} {:?}", e1, e2, e3, e4) } Error::InvalidProofFormatError(e1) => { write!(f, "InvalidProofFormatError : {:?}", e1) @@ -327,8 +331,93 @@ impl core::fmt::Display for Error { Error::LCPCommitmentError(e1) => { write!(f, "LCPCommitmentError : {}", e1) } + Error::LCPError(e1) => { + write!(f, "LCPError: {}", e1) + } } } } -impl light_client::LightClientSpecificError for Error {} +#[derive(Debug)] +pub enum ClientError { + LatestHeight { + cause: Error, + client_id: ClientId, + }, + CreateClient { + cause: Error, + client_state: Any, + consensus_sate: Any, + }, + UpdateClient { + cause: Error, + client_id: ClientId, + message: Any, + }, + VerifyMembership { + cause: Error, + client_id: ClientId, + prefix: CommitmentPrefix, + path: String, + value: Vec, + proof_height: Height, + proof: Vec, + }, + VerifyNonMembership { + cause: Error, + client_id: ClientId, + prefix: CommitmentPrefix, + path: String, + proof_height: Height, + proof: Vec, + }, +} + +impl core::fmt::Display for ClientError { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + match self { + ClientError::LatestHeight { + cause, + client_id + } => write!( + f, + "LatestHeight: cause={}\nclient_id={}", + cause, client_id + ), + ClientError::CreateClient {cause, client_state, consensus_sate} => write!( + f, + "CreateClient: cause={}\nclient_state={:?}\nconsensus_state={:?}", + cause, client_state, consensus_sate + ), + ClientError::UpdateClient{cause, client_id, message} => write!( + f, + "CreateClient: cause={}\nclient_id={:?}\nmessage={:?}", + cause, client_id, message + ), + ClientError::VerifyMembership { + cause, client_id, + prefix, + path, + value, + proof_height, + proof + } => write!( + f, + "VerifyMembership: cause={}\nclient_id={:?}\nprefix={:?}\npath={:?}\nvalue={:?}\nproof_height={:?}\nproof={:?}", + cause, client_id, prefix, path, value, proof_height, proof + ), + ClientError::VerifyNonMembership { + cause, client_id, + prefix, + path, + proof_height, + proof + } => write!( + f, + "VerifyNonMembership: cause={}\nclient_id={:?}\nprefix={:?}\npath={:?}\nproof_height={:?}\nproof={:?}", + cause, client_id, prefix, path, proof_height, proof + ), + } + } +} +impl light_client::LightClientSpecificError for ClientError {}