chore(ibc-vm): return error in VerifyClientMessage

Signed-off-by: aeryz <[email protected]>

lib/ibc-vm-rs/src/lib.rs

@@ -44,8 +44,8 @@ pub enum IbcError {
     UnexpectedAction,
 
     // TODO(aeryz): this needs context
-    #[error("client message verification failed")]
-    ClientMessageVerificationFailed,
+    #[error("client message verification failed: {0}")]
+    ClientMessageVerificationFailed(String),
 
     #[error("connection ({0}) not found")]
     ConnectionNotFound(String),
@@ -193,7 +193,7 @@ pub enum IbcResponse {
         valid: bool,
     },
     VerifyClientMessage {
-        valid: bool,
+        error: Option<String>,
     },
     CheckForMisbehaviour {
         misbehaviour_found: bool,

lib/ibc-vm-rs/src/states/client_state.rs

@@ -67,14 +67,16 @@ impl<T: IbcHost> Runnable<T> for UpdateClient {
                     client_id,
                     client_msg,
                 },
-                &[IbcResponse::Status { status }, IbcResponse::VerifyClientMessage { valid }, IbcResponse::CheckForMisbehaviour { misbehaviour_found }],
+                &[IbcResponse::Status { status }, IbcResponse::VerifyClientMessage { error }, IbcResponse::CheckForMisbehaviour { misbehaviour_found }],
             ) => {
                 if *status != Status::Active {
                     return Err(IbcError::NotActive(client_id, *status).into());
                 }
-                if !valid {
-                    return Err(IbcError::ClientMessageVerificationFailed.into());
+
+                if let Some(error) = error {
+                    return Err(IbcError::ClientMessageVerificationFailed(error.clone()).into());
                 }
+
                 if *misbehaviour_found {
                     Either::Left((
                         Self::UpdatedStateOnMisbehaviour {

lib/near-verifier/src/error.rs

@@ -1,6 +1,5 @@
 use near_account_id::AccountId;
 use near_primitives_core::hash::CryptoHash;
-use unionlabs::near::raw_state_proof::RawStateProof;
 
 #[derive(thiserror::Error, Debug, PartialEq, Eq, Clone)]
 pub enum Error {

lib/unionlabs/src/uint.rs

@@ -8,7 +8,6 @@ use core::{
     str::FromStr,
 };
 
-use borsh::BorshSerialize;
 use serde::{Deserialize, Serialize};
 use serde_utils::HEX_ENCODING_PREFIX;
 

light-clients/cometbls/near/Cargo.toml

@@ -10,7 +10,6 @@ crate-type = ["cdylib"]
 
 [dependencies]
 borsh                     = { workspace = true, features = ["derive"] }
-cometbls-groth16-verifier = { workspace = true }
 hex-literal               = { workspace = true }
 hex.workspace             = true
 ibc-vm-rs                 = { workspace = true }

light-clients/cometbls/near/src/contract.rs

@@ -4,7 +4,6 @@ use near_sdk::{
     borsh::{BorshDeserialize, BorshSerialize},
     env, near_bindgen,
     store::LookupMap,
-    PanicOnDefault,
 };
 use unionlabs::{
     encoding::{DecodeAs, EncodeAs as _, Proto},
@@ -96,7 +95,10 @@ impl Contract {
                     ),
                 },
                 IbcQuery::VerifyClientMessage(msg) => IbcResponse::VerifyClientMessage {
-                    valid: self.verify_client_message(&client_id, msg),
+                    error: self
+                        .verify_client_message(&client_id, msg)
+                        .map_err(|e| e.to_string())
+                        .err(),
                 },
                 IbcQuery::CheckForMisbehaviour(msg) => IbcResponse::CheckForMisbehaviour {
                     misbehaviour_found: self.check_for_misbehaviour(msg),
@@ -106,7 +108,7 @@ impl Contract {
             .collect()
     }
 
-    pub fn status(&self) -> Status {
+    fn status(&self) -> Status {
         Status::Active
     }
 
@@ -126,7 +128,7 @@ impl Contract {
         path: MerklePath,
         value: Vec<u8>,
         // TODO(aeryz): make this a proper error type this is stupid
-    ) -> bool {
+    ) -> Result<(), Error> {
         let Ok(consensus_state) = self
             .consensus_states
             .get(client_id)
@@ -158,72 +160,71 @@ impl Contract {
     }
 
     // TODO(aeryz): client_msg can be Misbehaviour or Header
-    fn verify_client_message(&self, client_id: &str, client_msg: Vec<u8>) -> bool {
-        let header = Header::decode_as::<Proto>(&client_msg).unwrap();
-        let client_state = self.client_states.get(client_id).unwrap();
-        let consensus_states = self.consensus_states.get(client_id).unwrap();
-        let consensus_state = consensus_states.get(&header.trusted_height).unwrap();
+    fn verify_client_message(&self, client_id: &str, client_msg: Vec<u8>) -> Result<(), Error> {
+        let header = Header::decode_as::<Proto>(&client_msg)?;
+        let client_state = self
+            .client_states
+            .get(client_id)
+            .ok_or(Error::ClientNotFound(client_id.to_string()))?;
+        let consensus_states = self
+            .consensus_states
+            .get(client_id)
+            .ok_or(Error::ClientNotFound(client_id.to_string()))?;
+
+        let consensus_state = consensus_states
+            .get(&header.trusted_height)
+            .ok_or(Error::ConsensusStateNotFound(header.trusted_height))?;
 
         // SAFETY: height is bound to be 0..i64::MAX which makes it within the bounds of u64
         let untrusted_height_number = header.signed_header.height.inner() as u64;
         let trusted_height_number = header.trusted_height.revision_height;
 
         if untrusted_height_number <= trusted_height_number {
-            // return Err(InvalidHeaderError::SignedHeaderHeightMustBeMoreRecent {
-            //     signed_height: untrusted_height_number,
-            //     trusted_height: trusted_height_number,
-            // }
-            // .into());
-            return false;
+            return Err(Error::SignedHeaderHeightMustBeMoreRecent {
+                signed_height: untrusted_height_number,
+                trusted_height: trusted_height_number,
+            });
         }
 
         let trusted_timestamp = consensus_state.timestamp;
         // Normalized to nanoseconds to follow tendermint convention
         let untrusted_timestamp = header.signed_header.time.as_unix_nanos();
 
         if untrusted_timestamp <= trusted_timestamp {
-            // return Err(InvalidHeaderError::SignedHeaderTimestampMustBeMoreRecent {
-            //     signed_timestamp: untrusted_timestamp,
-            //     trusted_timestamp,
-            // }
-            // .into());
-            return false;
+            return Err(Error::SignedHeaderTimestampMustBeMoreRecent {
+                signed_timestamp: untrusted_timestamp,
+                trusted_timestamp,
+            });
         }
 
         if is_client_expired(
             untrusted_timestamp,
             client_state.trusting_period,
             env::block_timestamp(),
         ) {
-            // return Err(InvalidHeaderError::HeaderExpired(consensus_state.data.timestamp).into());
-            return false;
+            return Err(Error::HeaderExpired(consensus_state.timestamp));
         }
 
         let max_clock_drift = env::block_timestamp()
             .checked_add(client_state.max_clock_drift)
-            .unwrap();
-        // .ok_or(Error::MathOverflow)?;
+            .ok_or(Error::MathOverflow)?;
 
         if untrusted_timestamp >= max_clock_drift {
-            // return Err(InvalidHeaderError::SignedHeaderCannotExceedMaxClockDrift {
-            //     signed_timestamp: untrusted_timestamp,
-            //     max_clock_drift,
-            // }
-            // .into());
-            return false;
+            return Err(Error::SignedHeaderCannotExceedMaxClockDrift {
+                signed_timestamp: untrusted_timestamp,
+                max_clock_drift,
+            });
         }
 
         let trusted_validators_hash = consensus_state.next_validators_hash;
 
         if untrusted_height_number == trusted_height_number + 1
             && header.signed_header.validators_hash != trusted_validators_hash
         {
-            // return Err(InvalidHeaderError::InvalidValidatorsHash {
-            //     expected: trusted_validators_hash,
-            //     actual: header.signed_header.validators_hash,
-            // }
-            // .into());
-            return false;
+            return Err(Error::InvalidValidatorsHash {
+                expected: trusted_validators_hash,
+                actual: header.signed_header.validators_hash,
+            });
         }
 
         verify_zkp(
@@ -232,13 +233,10 @@ impl Contract {
             &header.signed_header,
             header.zero_knowledge_proof,
         )
-        .unwrap();
-
-        true
     }
 
     #[allow(unused)]
-    pub fn check_for_misbehaviour(&self, client_msg: Vec<u8>) -> bool {
+    fn check_for_misbehaviour(&self, client_msg: Vec<u8>) -> bool {
         false
     }
 

light-clients/cometbls/near/src/error.rs

@@ -1,9 +1,11 @@
 use unionlabs::{
     encoding::{DecodeErrorOf, Proto},
+    hash::H256,
     ibc::{
         core::{client::height::Height, commitment::merkle_proof::MerkleProof},
-        lightclients::cometbls,
+        lightclients::cometbls::{self, header::TryFromHeaderError},
     },
+    TryFromProtoBytesError,
 };
 
 #[derive(thiserror::Error, Debug, PartialEq)]
@@ -20,12 +22,12 @@ pub enum Error {
     #[error("unable to decode client state")]
     ClientStateDecode(#[source] DecodeErrorOf<Proto, cometbls::client_state::ClientState>),
 
+    #[error("client not found")]
+    ClientNotFound(String),
+
     #[error("Client state not found")]
     ClientStateNotFound,
 
-    #[error("Invalid ZKP: {0:?}")]
-    InvalidZKP(cometbls_groth16_verifier::Error),
-
     #[error("Consensus state not found for {0}")]
     ConsensusStateNotFound(Height),
 
@@ -49,4 +51,33 @@ pub enum Error {
 
     #[error("invalid timestamp")]
     InvalidTimestamp,
+
+    #[error(
+        "trusted validators hash ({expected:?}) does not match the given header's hash ({actual})"
+    )]
+    InvalidValidatorsHash { expected: H256, actual: H256 },
+
+    #[error("signed header ({signed_timestamp}) exceeds the max clock drift ({max_clock_drift})")]
+    SignedHeaderCannotExceedMaxClockDrift {
+        signed_timestamp: u64,
+        max_clock_drift: u64,
+    },
+
+    #[error("header with timestamp ({0}) is expired")]
+    HeaderExpired(u64),
+
+    #[error("signed header with timestamp ({signed_timestamp}) must be more recent than the trusted one ({trusted_timestamp})")]
+    SignedHeaderTimestampMustBeMoreRecent {
+        signed_timestamp: u64,
+        trusted_timestamp: u64,
+    },
+
+    #[error("signed header with height ({signed_height}) must be more recent than the trusted one ({trusted_height})")]
+    SignedHeaderHeightMustBeMoreRecent {
+        signed_height: u64,
+        trusted_height: u64,
+    },
+
+    #[error("header decode failed {0}")]
+    HeaderDecode(#[from] TryFromProtoBytesError<TryFromHeaderError>),
 }

light-clients/near/near/src/contract.rs

@@ -80,7 +80,7 @@ impl Contract {
                     ),
                 },
                 IbcQuery::VerifyClientMessage(msg) => IbcResponse::VerifyClientMessage {
-                    valid: self.verify_client_message(msg),
+                    error: self.verify_client_message(msg),
                 },
                 IbcQuery::CheckForMisbehaviour(msg) => IbcResponse::CheckForMisbehaviour {
                     misbehaviour_found: self.check_for_misbehaviour(msg),