Rlp decoding fix & Sync committee rotation fixes (#48)

parachain/modules/consensus/sync-committee/primitives/src/types.rs

@@ -94,6 +94,8 @@ pub struct VerifierState {
     pub current_sync_committee: SyncCommittee<SYNC_COMMITTEE_SIZE>,
     /// Committee for the next sync period
     pub next_sync_committee: SyncCommittee<SYNC_COMMITTEE_SIZE>,
+    /// state_period
+    pub state_period: u64,
 }
 
 /// Finalized header proof

parachain/modules/consensus/sync-committee/primitives/src/util.rs

@@ -13,7 +13,7 @@ use ssz_rs::prelude::*;
 
 /// Returns true if sync committee update is required
 pub fn should_have_sync_committee_update(state_period: u64, signature_period: u64) -> bool {
-    state_period != signature_period
+    signature_period == state_period + 1
 }
 
 /// Return the sync committee period at the given ``epoch``

parachain/modules/consensus/sync-committee/prover/src/lib.rs

@@ -229,7 +229,7 @@ impl SyncCommitteeProver {
     /// we use `head`
     pub async fn fetch_light_client_update(
         &self,
-        client_state: VerifierState,
+        mut client_state: VerifierState,
         finality_checkpoint: Checkpoint,
         latest_block_id: Option<&str>,
         debug_target: &str,
@@ -251,8 +251,7 @@ impl SyncCommitteeProver {
         };
         let mut block = self.fetch_block(&get_block_id(latest_root)).await?;
         let min_signatures = (2 * SYNC_COMMITTEE_SIZE) / 3;
-        let state_period =
-            compute_sync_committee_period_at_slot(client_state.finalized_header.slot);
+        let state_period = client_state.state_period;
         loop {
             // Some checks on the epoch finalized by the signature block
             let parent_root = block.parent_root;
@@ -298,8 +297,13 @@ impl SyncCommitteeProver {
         let execution_payload_proof = prove_execution_payload(&mut finalized_state)?;
 
         let signature_period = compute_sync_committee_period_at_slot(block.slot);
+        let client_state_next_sync_committee_root =
+            client_state.next_sync_committee.hash_tree_root()?;
+        let attested_state_current_sync_committee_root =
+            attested_state.current_sync_committee.hash_tree_root()?;
         let sync_committee_update =
-            if should_have_sync_committee_update(state_period, signature_period) {
+            // We must make sure we switch the sync comittee only when the finalized header has changed sync committees
+            if should_have_sync_committee_update(state_period, signature_period) && client_state_next_sync_committee_root == attested_state_current_sync_committee_root {
                 let sync_committee_proof = prove_sync_committee_update(&mut attested_state)?;
                 Some(SyncCommitteeUpdate {
                     next_sync_committee: attested_state.next_sync_committee,

parachain/modules/consensus/sync-committee/prover/src/test.rs

@@ -261,6 +261,7 @@ async fn test_client_sync() {
         latest_finalized_epoch: compute_epoch_at_slot(block_header.slot),
         current_sync_committee: state.current_sync_committee,
         next_sync_committee: state.next_sync_committee,
+        state_period: 810,
     };
 
     let mut next_period = start_period + 1;
@@ -299,6 +300,7 @@ async fn test_sync_committee_hand_offs() {
         latest_finalized_epoch: compute_epoch_at_slot(block_header.slot),
         current_sync_committee: state.current_sync_committee,
         next_sync_committee: state.next_sync_committee,
+        state_period: 805,
     };
 
     // Verify an update from state_period + 1
@@ -326,7 +328,7 @@ async fn test_sync_committee_hand_offs() {
         .unwrap();
     assert!(update.sync_committee_update.is_some());
     client_state = verify_sync_committee_attestation(client_state, update).unwrap();
-
+    assert_eq!(client_state.state_period, state_period + 1);
     // Verify block in the current state_period
     let latest_block_id = {
         let slot = ((signature_period * EPOCHS_PER_SYNC_COMMITTEE_PERIOD) * SLOTS_PER_EPOCH) +
@@ -384,6 +386,7 @@ async fn test_prover() {
         latest_finalized_epoch: compute_epoch_at_slot(block_header.slot),
         current_sync_committee: state.current_sync_committee,
         next_sync_committee: state.next_sync_committee,
+        state_period: compute_sync_committee_period_at_slot(block_header.slot),
     };
 
     let mut count = 0;
@@ -425,8 +428,8 @@ async fn test_prover() {
                 );
 
                 count += 1;
-                // For CI purposes we test finalization of 3 epochs
-                if count == 4 {
+                // For CI purposes we test finalization of 2 epochs
+                if count == 2 {
                     break
                 }
             },

parachain/modules/consensus/sync-committee/verifier/src/lib.rs

@@ -59,7 +59,7 @@ pub fn verify_sync_committee_attestation(
         ))?
     }
 
-    let state_period = compute_sync_committee_period_at_slot(trusted_state.finalized_header.slot);
+    let state_period = trusted_state.state_period;
     let update_signature_period = compute_sync_committee_period_at_slot(update.signature_slot);
     if !(state_period..=state_period + 1).contains(&update_signature_period) {
         Err(Error::InvalidUpdate("State period does not contain signature period".into()))?
@@ -192,6 +192,7 @@ pub fn verify_sync_committee_attestation(
                 latest_finalized_epoch: update.finality_proof.epoch,
                 current_sync_committee: trusted_state.next_sync_committee,
                 next_sync_committee: sync_committee_update.next_sync_committee,
+                state_period: state_period + 1,
             }
         } else {
             Err(Error::InvalidUpdate("Expected sync committee update to be present".into()))?

parachain/modules/ismp/sync-committee/src/arbitrum.rs

@@ -249,11 +249,13 @@ pub fn verify_arbitrum_payload<H: IsmpHost + Send + Sync>(
         _ => Err(Error::MembershipProofVerificationFailed("Value not found in proof".to_string()))?,
     };
 
-    let proof_value = <B256 as Decodable>::decode(&mut &*proof_value).map_err(|_| {
-        Error::ImplementationSpecific(format!("Error decoding state hash {:?}", &proof_value))
-    })?;
+    let proof_value = <alloy_primitives::U256 as Decodable>::decode(&mut &*proof_value)
+        .map_err(|_| {
+            Error::ImplementationSpecific(format!("Error decoding state hash {:?}", &proof_value))
+        })?
+        .to_be_bytes::<32>();
 
-    if proof_value.0 != state_hash.0 {
+    if proof_value != state_hash.0 {
         Err(Error::MembershipProofVerificationFailed(
             "State hash from proof does not match calculated state hash".to_string(),
         ))?

parachain/modules/ismp/sync-committee/src/optimism.rs

@@ -19,7 +19,6 @@ use crate::{
     utils::{derive_array_item_key, get_contract_storage_root, get_value_from_proof, to_bytes_32},
 };
 use alloc::{format, string::ToString};
-use alloy_primitives::{Bytes, B256};
 use alloy_rlp::Decodable;
 use ethabi::ethereum_types::{H160, H256, U128, U256};
 use ismp::{
@@ -84,14 +83,16 @@ pub fn verify_optimism_payload<H: IsmpHost + Send + Sync>(
         _ => Err(Error::MembershipProofVerificationFailed("Value not found in proof".to_string()))?,
     };
 
-    let proof_value = <B256 as Decodable>::decode(&mut &*proof_value).map_err(|_| {
-        Error::ImplementationSpecific(format!(
-            "Error decoding contract account from key {:?}",
-            &proof_value
-        ))
-    })?;
+    let proof_value = <alloy_primitives::U256 as Decodable>::decode(&mut &*proof_value)
+        .map_err(|_| {
+            Error::ImplementationSpecific(format!(
+                "Error decoding output root from {:?}",
+                &proof_value
+            ))
+        })?
+        .to_be_bytes::<32>();
 
-    if proof_value.0 != output_root.0 {
+    if proof_value != output_root.0 {
         return Err(Error::MembershipProofVerificationFailed(
             "Invalid optimism output root proof".to_string(),
         ))
@@ -109,15 +110,15 @@ pub fn verify_optimism_payload<H: IsmpHost + Send + Sync>(
         _ => Err(Error::MembershipProofVerificationFailed("Value not found in proof".to_string()))?,
     };
 
-    let block_and_timestamp = <Bytes as Decodable>::decode(&mut &*block_and_timestamp)
-        .map_err(|_| {
-            Error::ImplementationSpecific(format!(
-                "Error decoding block and timestamp from{:?}",
-                &block_and_timestamp
-            ))
-        })?
-        .0
-        .to_vec();
+    let block_and_timestamp =
+        <alloy_primitives::U256 as Decodable>::decode(&mut &*block_and_timestamp)
+            .map_err(|_| {
+                Error::ImplementationSpecific(format!(
+                    "Error decoding block and timestamp from{:?}",
+                    &block_and_timestamp
+                ))
+            })?
+            .to_be_bytes::<32>();
 
     let block_and_timestamp = U256::from_big_endian(&block_and_timestamp);
     // Timestamp is contained in the first two u64 values

parachain/modules/ismp/sync-committee/src/utils.rs

@@ -119,7 +119,7 @@ pub fn get_contract_storage_root<H: IsmpHost + Send + Sync>(
 
     let contract_account = <Account as Decodable>::decode(&mut &*result).map_err(|_| {
         Error::ImplementationSpecific(format!(
-            "Error decoding contract account from key {:?}",
+            "Error decoding contract account from value {:?}",
             &result
         ))
     })?;
@@ -172,7 +172,7 @@ pub(super) fn get_values_from_proof<H: IsmpHost + Send + Sync>(
     Ok(values)
 }
 
-pub(super) fn get_value_from_proof<H: IsmpHost + Send + Sync>(
+pub fn get_value_from_proof<H: IsmpHost + Send + Sync>(
     key: Vec<u8>,
     root: H256,
     proof: Vec<Vec<u8>>,

parachain/runtime/src/lib.rs

@@ -206,7 +206,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("hyperbridge"),
     impl_name: create_runtime_str!("hyperbridge"),
     authoring_version: 1,
-    spec_version: 107,
+    spec_version: 108,
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 1,