fix: corrected the sync-protocol implementation on next sync committee updates (#406)

Author: srdtrk

e2e/interchaintestv8/wasm/cw_ics08_wasm_eth.wasm.gz

@@ -128,6 +128,9 @@ pub enum EthereumIBCError {
     #[error("expected next sync committee to be known and stored in state")]
     NextSyncCommitteeUnknown,
 
+    #[error("unexpected next sync committee in the update")]
+    UnexpectedNextSyncCommittee,
+
     #[error("bls aggregate error: {0}")]
     BlsAggregateError(String),
 

packages/ethereum/ethereum-light-client/src/error.rs

@@ -187,7 +187,8 @@ pub fn validate_light_client_update<V: BlsVerify>(
     let signature_period =
         client_state.compute_sync_committee_period_at_slot(update.signature_slot);
 
-    if trusted_consensus_state.next_sync_committee().is_some() {
+    let is_next_sync_committee_known = trusted_consensus_state.next_sync_committee().is_some();
+    if is_next_sync_committee_known {
         ensure!(
             signature_period == stored_period || signature_period == stored_period + 1,
             EthereumIBCError::InvalidSignaturePeriodWhenNextSyncCommitteeExists {
@@ -209,16 +210,18 @@ pub fn validate_light_client_update<V: BlsVerify>(
     let update_attested_period =
         client_state.compute_sync_committee_period_at_slot(update_attested_slot);
 
+    let is_next_sync_committee_update = update.next_sync_committee_branch.is_some();
+    let update_has_next_sync_committee = !is_next_sync_committee_known
+        && is_next_sync_committee_update
+        && update_attested_period == stored_period;
     // There are two options to do a light client update:
     // 1. We are updating the header with a newer one.
     // 2. We haven't set the next sync committee yet and we can use any attested header within the same
     // signature period to set the next sync committee. This means that the stored header could be larger.
     // The light client implementation needs to take care of it.
     ensure!(
         update_attested_slot > trusted_consensus_state.finalized_slot()
-            || (update_attested_period == stored_period
-                && update.next_sync_committee.is_some()
-                && trusted_consensus_state.next_sync_committee().is_none()),
+            || update_has_next_sync_committee,
         EthereumIBCError::IrrelevantUpdate {
             update_attested_slot,
             trusted_finalized_slot: trusted_consensus_state.finalized_slot(),
@@ -251,31 +254,46 @@ pub fn validate_light_client_update<V: BlsVerify>(
 
     // Verify that if the update contains the next sync committee, and the signature periods do match,
     // next sync committees match too.
-    if let (Some(next_sync_committee), Some(stored_next_sync_committee)) = (
-        &update.next_sync_committee,
-        trusted_consensus_state.next_sync_committee(),
-    ) {
-        if update_attested_period == stored_period {
+    if is_next_sync_committee_update {
+        if update_attested_period == stored_period && is_next_sync_committee_known {
             ensure!(
-                next_sync_committee == stored_next_sync_committee,
+                update.next_sync_committee.as_ref().unwrap()
+                    == trusted_consensus_state.next_sync_committee().unwrap(),
                 EthereumIBCError::NextSyncCommitteeMismatch {
-                    expected: stored_next_sync_committee.aggregate_pubkey,
-                    found: next_sync_committee.aggregate_pubkey,
+                    expected: trusted_consensus_state
+                        .next_sync_committee()
+                        .unwrap()
+                        .aggregate_pubkey,
+                    found: update
+                        .next_sync_committee
+                        .as_ref()
+                        .unwrap()
+                        .aggregate_pubkey,
                 }
             );
         }
+
         // This validates the given next sync committee against the attested header's state root.
         validate_merkle_branch(
-            next_sync_committee.tree_hash_root(),
-            update.next_sync_committee_branch.unwrap_or_default().into(),
+            update
+                .next_sync_committee
+                .as_ref()
+                .unwrap()
+                .tree_hash_root(),
+            update.next_sync_committee_branch.unwrap().into(),
             NEXT_SYNC_COMMITTEE_BRANCH_DEPTH,
             get_subtree_index(next_sync_committee_gindex_at_slot(
                 client_state,
-                update.finalized_header.beacon.slot,
+                update.attested_header.beacon.slot,
             )?),
             update.attested_header.beacon.state_root,
         )
         .map_err(|e| EthereumIBCError::ValidateNextSyncCommitteeFailed(Box::new(e)))?;
+    } else {
+        ensure!(
+            update.next_sync_committee.is_none(),
+            EthereumIBCError::UnexpectedNextSyncCommittee
+        );
     }
 
     // Verify sync committee aggregate signature