Fixes

cosmwasm/union-ibc/light-clients/ethereum/src/client.rs

@@ -12,7 +12,7 @@ use ethereum_sync_protocol::{
 use evm_storage_verifier::{
     verify_account_storage_root, verify_storage_absence, verify_storage_proof,
 };
-use union_ibc_light_client::IbcClientCtx;
+use union_ibc_light_client::{IbcClientCtx, IbcClientError};
 use union_ibc_msg::lightclient::Status;
 use unionlabs::{
     encoding::Bincode, ensure, ethereum::ibc_commitment_key, hash::H256,
@@ -46,7 +46,7 @@ impl union_ibc_light_client::IbcClient for EthereumLightClient {
         key: Vec<u8>,
         storage_proof: Self::StorageProof,
         value: Vec<u8>,
-    ) -> Result<(), union_ibc_light_client::IbcClientError<Self>> {
+    ) -> Result<(), IbcClientError<Self>> {
         let consensus_state = ctx.read_self_consensus_state(height)?;
         Ok(verify_membership(
             key,
@@ -61,7 +61,7 @@ impl union_ibc_light_client::IbcClient for EthereumLightClient {
         height: u64,
         key: Vec<u8>,
         storage_proof: Self::StorageProof,
-    ) -> Result<(), union_ibc_light_client::IbcClientError<Self>> {
+    ) -> Result<(), IbcClientError<Self>> {
         let consensus_state = ctx.read_self_consensus_state(height)?;
         Ok(verify_non_membership(
             key,
@@ -89,41 +89,45 @@ impl union_ibc_light_client::IbcClient for EthereumLightClient {
     fn verify_creation(
         _client_state: &Self::ClientState,
         _consensus_state: &Self::ConsensusState,
-    ) -> Result<(), union_ibc_light_client::IbcClientError<Self>> {
+    ) -> Result<(), IbcClientError<Self>> {
         Ok(())
     }
 
     fn verify_header(
         ctx: IbcClientCtx<Self>,
         header: Header,
-    ) -> Result<
-        (u64, Self::ClientState, Self::ConsensusState),
-        union_ibc_light_client::IbcClientError<Self>,
-    > {
+    ) -> Result<(u64, Self::ClientState, Self::ConsensusState), IbcClientError<Self>> {
         let client_state = ctx.read_self_client_state()?;
         let consensus_state = ctx.read_self_consensus_state(header.trusted_height.height())?;
-        if client_state.chain_spec == PresetBaseKind::Minimal {
-            verify_header::<Minimal>(&ctx, client_state, consensus_state, header)
-        } else {
-            verify_header::<Mainnet>(&ctx, client_state, consensus_state, header)
+
+        match client_state.chain_spec {
+            PresetBaseKind::Minimal => {
+                verify_header::<Minimal>(&ctx, client_state, consensus_state, header)
+            }
+            PresetBaseKind::Mainnet => {
+                verify_header::<Mainnet>(&ctx, client_state, consensus_state, header)
+            }
         }
         .map_err(Into::into)
     }
 
     fn misbehaviour(
         ctx: IbcClientCtx<Self>,
         misbehaviour: Self::Misbehaviour,
-    ) -> Result<Self::ClientState, union_ibc_light_client::IbcClientError<Self>> {
+    ) -> Result<Self::ClientState, IbcClientError<Self>> {
         let consensus_state =
             ctx.read_self_consensus_state(misbehaviour.trusted_height.height())?;
 
         let mut client_state = ctx.read_self_client_state()?;
 
-        if client_state.chain_spec == PresetBaseKind::Minimal {
-            verify_misbehaviour::<Minimal>(&ctx, &client_state, consensus_state, misbehaviour)?
-        } else {
-            verify_misbehaviour::<Mainnet>(&ctx, &client_state, consensus_state, misbehaviour)?
-        };
+        match client_state.chain_spec {
+            PresetBaseKind::Minimal => {
+                verify_misbehaviour::<Minimal>(&ctx, &client_state, consensus_state, misbehaviour)?
+            }
+            PresetBaseKind::Mainnet => {
+                verify_misbehaviour::<Mainnet>(&ctx, &client_state, consensus_state, misbehaviour)?
+            }
+        }
 
         client_state.frozen_height = Height::new(1);
 
@@ -185,7 +189,6 @@ pub fn verify_non_membership(
 pub fn check_commitment_key(path: H256, key: U256) -> Result<(), Error> {
     let expected_commitment_key = ibc_commitment_key(path);
 
-    // Data MUST be stored to the commitment path that is defined in ICS23.
     if expected_commitment_key != key {
         Err(Error::InvalidCommitmentKey {
             expected: expected_commitment_key,
@@ -211,7 +214,7 @@ pub fn verify_header<C: ChainSpec>(
     let (current_sync_committee, next_sync_committee) =
         header.consensus_update.currently_trusted_sync_committee();
 
-    validate_light_client_update::<C, VerificationContext>(
+    validate_light_client_update::<C, _>(
         &header.consensus_update.clone().into(),
         current_sync_committee,
         next_sync_committee,

cosmwasm/union-ibc/light-clients/ethereum/src/lib.rs

@@ -3,3 +3,6 @@ pub mod client;
 pub mod contract;
 pub mod errors;
 pub mod verification;
+
+#[cfg(test)]
+pub mod tests;

lib/ethereum-sync-protocol/src/lib.rs

@@ -71,21 +71,19 @@ pub fn validate_light_client_update<C: ChainSpec, V: BlsVerify>(
     fork_parameters: &ForkParameters,
     bls_verifier: V,
 ) -> Result<(), Error> {
-    // Verify sync committee has sufficient participants
+    // verify that the sync committee has sufficient participants
     let sync_aggregate = &update.sync_aggregate;
-    let set_bits = sync_aggregate
-        .sync_committee_bits
-        .iter()
-        .map(|i| *i as usize)
-        .sum::<usize>();
+    let set_bits = BytesBitIterator::new(&sync_aggregate.sync_committee_bits)
+        .filter(|included| *included)
+        .count();
     ensure(
         set_bits >= C::MIN_SYNC_COMMITTEE_PARTICIPANTS::USIZE,
         Error::InsufficientSyncCommitteeParticipants(set_bits),
     )?;
 
     is_valid_light_client_header::<C>(fork_parameters, &update.attested_header)?;
 
-    // Verify update does not skip a sync committee period
+    // verify that the update does not skip a sync committee period
     let update_attested_slot = update.attested_header.beacon.slot;
     let update_finalized_slot = update.finalized_header.beacon.slot;
 
@@ -139,7 +137,7 @@ pub fn validate_light_client_update<C: ChainSpec, V: BlsVerify>(
         )?;
     }
 
-    // Verify update is relevant
+    // verify that the update is relevant
     let update_attested_period = compute_sync_committee_period_at_slot::<C>(update_attested_slot);
 
     // There are two options to do a light client update:
@@ -166,11 +164,10 @@ pub fn validate_light_client_update<C: ChainSpec, V: BlsVerify>(
     // to match the finalized checkpoint root saved in the state of `attested_header`.
     // NOTE(aeryz): We always expect to get `finalized_header` and it's embedded into the type definition.
     is_valid_light_client_header::<C>(fork_parameters, &update.finalized_header)?;
-    let finalized_root = update.finalized_header.beacon.tree_hash_root();
 
     // This confirms that the `finalized_header` is really finalized.
     validate_merkle_branch(
-        &finalized_root.into(),
+        &update.finalized_header.beacon.tree_hash_root().into(),
         &update.finality_branch,
         floorlog2(FINALIZED_ROOT_INDEX),
         get_subtree_index(FINALIZED_ROOT_INDEX),
@@ -213,7 +210,7 @@ pub fn validate_light_client_update<C: ChainSpec, V: BlsVerify>(
 
     // It's not mandatory for all of the members of the sync committee to participate. So we are extracting the
     // public keys of the ones who participated.
-    let participant_pubkeys = BytesBitIterator::new(&&*update.sync_aggregate.sync_committee_bits)
+    let participant_pubkeys = BytesBitIterator::new(&sync_aggregate.sync_committee_bits)
         .zip(sync_committee.pubkeys.iter())
         .filter_map(|(included, pubkey)| if included { Some(pubkey) } else { None })
         .collect::<Vec<_>>();

lib/unionlabs/src/hash.rs

@@ -57,7 +57,7 @@ impl<'a> BytesBitIterator<'a> {
         // debug_assert_eq!(self.hash_bytes.len(), Hash::LENGTH); // invariant
         // debug_assert_lt!(index, Hash::LENGTH_IN_BITS); // assumed precondition
         let pos = index / 8;
-        let bit = 7 - index % 8;
+        let bit = index % 8;
         (self.bz[pos] >> bit) & 1 != 0
     }
 }

voyager/modules/proof/ethereum/src/main.rs

@@ -13,13 +13,14 @@ use jsonrpsee::{
 };
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use tracing::instrument;
+use tracing::{debug, instrument};
 use unionlabs::{
     ethereum::ibc_commitment_key, hash::H160, ibc::core::client::height::Height, uint::U256,
     ErrorReporter,
 };
 use voyager_message::{
     core::ChainId,
+    into_value,
     module::{ProofModuleInfo, ProofModuleServer},
     ProofModule,
 };
@@ -78,7 +79,7 @@ impl Module {
 
 #[async_trait]
 impl ProofModuleServer<IbcUnion> for Module {
-    #[instrument(skip_all, fields(chain_id = %self.chain_id))]
+    #[instrument(skip_all, fields(chain_id = %self.chain_id, %at, ?path))]
     async fn query_ibc_proof(
         &self,
         _: &Extensions,
@@ -87,6 +88,11 @@ impl ProofModuleServer<IbcUnion> for Module {
     ) -> RpcResult<Value> {
         let location = ibc_commitment_key(path.key());
 
+        debug!(
+            "querying proof for slot {location} for IBC handler contract {}",
+            self.ibc_handler_address
+        );
+
         let execution_height = at.height();
 
         let proof = self
@@ -122,6 +128,6 @@ impl ProofModuleServer<IbcUnion> for Module {
                 .collect(),
         };
 
-        Ok(serde_json::to_value(proof).expect("serialization is infallible; qed;"))
+        Ok(into_value(proof))
     }
 }