feat(lightclients): clean up errors

feat(lightclients): clean up more errors feat(lightclients): clean up more errors feat(lightclients): clean up more errors
unionlabs · Apr 29, 2024 · 007a241 · 007a241
1 parent bc79201
commit 007a241
Show file tree

Hide file tree

Showing 73 changed files with 880 additions and 996 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lib/chain-utils/src/ethereum.rs b/lib/chain-utils/src/ethereum.rs
@@ -11,7 +11,7 @@ use contracts::{
         GetConnectionCall, GetConnectionReturn, GetConsensusStateCall, GetConsensusStateReturn,
         GetHashedPacketAcknowledgementCommitmentCall,
         GetHashedPacketAcknowledgementCommitmentReturn, GetHashedPacketCommitmentCall,
-        GetHashedPacketCommitmentReturn, HasPacketReceiptCall, HasPacketReceiptReturn, IBCHandler,
+        GetHashedPacketCommitmentReturn, HasPacketReceiptCall, IBCHandler,
         IbcCoreConnectionV1ConnectionEndData, NextClientSequenceCall, NextConnectionSequenceCall,
         OwnershipTransferredFilter,
     },

diff --git a/lib/ethereum-verifier/src/error.rs b/lib/ethereum-verifier/src/error.rs
@@ -1,8 +1,16 @@
 use milagro_bls::AmclError;
 use trie_db::TrieError;
-use unionlabs::{bls::BlsPublicKey, hash::H256};
+use unionlabs::{
+    bls::{BlsPublicKey, BlsSignature},
+    hash::H256,
+};
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, thiserror::Error)]
+#[error("invalid merkle branch \
+    (leaf: {leaf}, branch: [{branch}], \
+    depth: {depth}, index: {index}, root: {root})",
+    branch = .branch.iter().map(ToString::to_string).collect::<Vec<_>>().join(", ")
+)]
 pub struct InvalidMerkleBranch {
     pub leaf: H256,
     pub branch: Vec<H256>,
@@ -11,11 +19,19 @@ pub struct InvalidMerkleBranch {
     pub root: H256,
 }
 
-#[derive(Debug, PartialEq, thiserror::Error)]
+#[derive(Debug, PartialEq, thiserror::Error, Clone)]
+#[error("signature cannot be verified (public_keys: {public_keys:?}, msg: {msg}, signature: {signature})", msg = serde_utils::to_hex(.msg))]
+pub struct InvalidSignature {
+    pub public_keys: Vec<BlsPublicKey>,
+    pub msg: Vec<u8>,
+    pub signature: BlsSignature,
+}
+
+#[derive(Debug, PartialEq, thiserror::Error, Clone)]
 pub enum Error {
-    #[error("invalid merkle branch ({0:?})")]
-    InvalidMerkleBranch(InvalidMerkleBranch),
-    #[error("invalid chain conversion")]
+    #[error(transparent)]
+    InvalidMerkleBranch(#[from] InvalidMerkleBranch),
+    #[error("invalid chain version")]
     InvalidChainVersion,
     #[error("crypto error")]
     Crypto,
@@ -48,54 +64,47 @@ pub enum Error {
         stored_period: u64,
     },
     #[error(
-        "next sync committee ({got}) does not match with the one in the current state ({expected})"
+        "next sync committee ({found}) does not match with the one in the current state ({expected})"
     )]
     NextSyncCommitteeMismatch {
         expected: BlsPublicKey,
-        got: BlsPublicKey,
+        found: BlsPublicKey,
     },
     #[error("insufficient number of sync committee participants ({0})")]
     InsufficientSyncCommitteeParticipants(usize),
     #[error("bls error ({0:?})")]
     Bls(AmclError),
-    #[error("proof is invalid due to value mismatch expected: {e}, actual: {a}", e = serde_utils::to_hex(expected), a = serde_utils::to_hex(actual))]
+    #[error(
+        "proof is invalid due to value mismatch, expected: {expected}, actual: {actual}",
+        expected = serde_utils::to_hex(expected),
+        actual = serde_utils::to_hex(actual)
+    )]
     ValueMismatch { expected: Vec<u8>, actual: Vec<u8> },
     #[error("proof is invalid due to missing value: {v}", v = serde_utils::to_hex(value))]
     ValueMissing { value: Vec<u8> },
     #[error("trie error ({0:?})")]
     Trie(Box<TrieError<primitive_types::H256, rlp::DecoderError>>),
     #[error("rlp decoding failed ({0:?})")]
-    RlpDecode(rlp::DecoderError),
-    #[error("custom query error: ({0})")]
-    CustomError(String),
-    #[error("update header contains deneb specific informations")]
+    RlpDecode(#[from] rlp::DecoderError),
+    #[error("custom query error")]
+    CustomQuery(#[from] unionlabs::cosmwasm::wasm::union::custom_query::Error),
+    // boxed as this variant is significantly larger than the rest of the variants (due to the BlsSignature contained within)
+    #[error(transparent)]
+    InvalidSignature(Box<InvalidSignature>),
+    #[error("update header contains deneb specific information")]
     MustBeDeneb,
     #[error("finalized slot cannot be the genesis slot")]
     FinalizedSlotIsGenesis,
 }
 
-#[derive(Debug, thiserror::Error, PartialEq)]
-#[error("verify storage absence error: {0}")]
-pub struct VerifyStorageAbsenceError(#[from] pub Error);
-
-#[derive(Debug, thiserror::Error, PartialEq)]
-#[error("validate light client error: {0}")]
-pub struct ValidateLightClientError(#[from] pub Error);
-
-#[derive(Debug, thiserror::Error, PartialEq)]
-#[error("verify account storage root error: {0}")]
-pub struct VerifyAccountStorageRootError(#[from] pub Error);
-
-#[derive(Debug, thiserror::Error, PartialEq)]
-#[error("verify storage proof error: {0}")]
-pub struct VerifyStorageProofError(#[from] pub Error);
-
+// NOTE: Implemented here instead of via #[from] since AmclError doesn't implement std::error::Error
 impl From<AmclError> for Error {
     fn from(e: AmclError) -> Self {
         Error::Bls(e)
     }
 }
 
+// NOTE: Implemented here instead of via #[from] since Box<TrieError<primitive_types::H256, rlp::DecoderError>> doesn't implement std::error::Error
 impl From<Box<TrieError<primitive_types::H256, rlp::DecoderError>>> for Error {
     fn from(e: Box<TrieError<primitive_types::H256, rlp::DecoderError>>) -> Self {
         Error::Trie(e)

diff --git a/lib/ethereum-verifier/src/verify.rs b/lib/ethereum-verifier/src/verify.rs
@@ -31,8 +31,7 @@ use crate::{
         compute_domain, compute_epoch_at_slot, compute_fork_version, compute_signing_root,
         compute_sync_committee_period_at_slot, validate_merkle_branch,
     },
-    Error, LightClientContext, ValidateLightClientError, VerifyAccountStorageRootError,
-    VerifyStorageAbsenceError, VerifyStorageProofError,
+    Error, LightClientContext,
 };
 
 type MinSyncCommitteeParticipants<Ctx> =
@@ -71,7 +70,7 @@ pub fn validate_light_client_update<Ctx: LightClientContext, V: BlsVerify>(
     current_slot: u64,
     genesis_validators_root: H256,
     bls_verifier: V,
-) -> Result<(), ValidateLightClientError> {
+) -> Result<(), Error> {
     // Verify sync committee has sufficient participants
     let sync_aggregate = &update.sync_aggregate;
     ensure(
@@ -171,7 +170,7 @@ pub fn validate_light_client_update<Ctx: LightClientContext, V: BlsVerify>(
                 next_sync_committee == stored_next_sync_committee,
                 Error::NextSyncCommitteeMismatch {
                     expected: stored_next_sync_committee.aggregate_pubkey,
-                    got: next_sync_committee.aggregate_pubkey,
+                    found: next_sync_committee.aggregate_pubkey,
                 },
             )?;
         }
@@ -255,7 +254,7 @@ pub fn verify_account_storage_root(
     address: &H160,
     proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
     storage_root: &H256,
-) -> Result<(), VerifyAccountStorageRootError> {
+) -> Result<(), Error> {
     match get_node(root, address.as_ref(), proof)? {
         Some(account) => {
             let account = rlp::decode::<Account>(account.as_ref()).map_err(Error::RlpDecode)?;
@@ -287,7 +286,7 @@ pub fn verify_storage_proof(
     key: U256,
     expected_value: &[u8],
     proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
-) -> Result<(), VerifyStorageProofError> {
+) -> Result<(), Error> {
     match get_node(root, key.to_be_bytes(), proof)? {
         Some(value) if value == expected_value => Ok(()),
         Some(value) => Err(Error::ValueMismatch {
@@ -311,7 +310,7 @@ pub fn verify_storage_absence(
     root: H256,
     key: U256,
     proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
-) -> Result<bool, VerifyStorageAbsenceError> {
+) -> Result<bool, Error> {
     Ok(get_node(root, key.to_be_bytes(), proof)?.is_none())
 }
 
@@ -506,7 +505,6 @@ mod tests {
             INITIAL_DATA.genesis_validators_root,
             BlsVerifier,
         )
-        .map_err(|e| e.0)
     }
 
     #[test]
@@ -770,7 +768,7 @@ mod tests {
                 proof_value.as_ref(),
                 VALID_PROOF.storage_proof.proof.iter()
             ),
-            Err(VerifyStorageProofError(Error::ValueMismatch { .. }))
+            Err(Error::ValueMismatch { .. })
         ));
     }