Verify speculative base against stored state_id only

app/src/commands/service.rs

@@ -71,11 +71,8 @@ impl ServiceCmd {
             Self::Start(cmd) => {
                 let addr = cmd.address.parse()?;
                 let enclave_parallelism = cmd.max_enclave_concurrency.max(1);
-                let enclave = enclave_loader.load(
-                    opts,
-                    cmd.enclave.path.as_ref(),
-                    cmd.enclave.is_debug(),
-                )?;
+                let enclave =
+                    enclave_loader.load(opts, cmd.enclave.path.as_ref(), cmd.enclave.is_debug())?;
                 let metadata = enclave.metadata()?;
                 let mrenclave = metadata.mrenclave().to_hex_string();
                 let mut rb = Builder::new_multi_thread();

modules/enclave-api/src/enclave.rs

@@ -1,5 +1,4 @@
 use crate::errors::{Error, Result};
-use commitments::gen_state_id_from_any;
 use keymanager::EnclaveKeyManager;
 use lcp_types::{store_key, Any, EnclaveMetadata, Height};
 use sgx_types::{sgx_enclave_id_t, SgxResult};
@@ -137,9 +136,9 @@ pub trait HostStoreTxManager<S: CommitStore>: CommitStoreAccessor<S> {
     /// The explicit base client state must match the latest canonical
     /// client_state. This prevents an old, historically valid base state from
     /// overwriting a newer latest-only client_state. The caller-supplied
-    /// `(client_state, consensus_state)` pair must also re-derive the
-    /// height-indexed state ID previously stored by a successful
-    /// serial/speculative update.
+    /// `prev_state_id` (observed in-enclave by the first speculative unit)
+    /// must also match the height-indexed state ID previously stored by a
+    /// successful create/serial/speculative update.
     fn apply_write_set_with_expected_base(
         &self,
         update_key: UpdateKey,
@@ -235,15 +234,14 @@ pub trait HostStoreTxManager<S: CommitStore>: CommitStoreAccessor<S> {
                 prev_height.revision_height()
             ))
         })?;
-        let expected_prev_state_id = gen_state_id_from_any(client_state, consensus_state)?.to_vec();
-        if expected_prev_state_id.as_slice() != prev_state_id {
-            return Err(Error::invalid_argument(format!(
-                "speculative base state_id does not match client_state/consensus_state: client_id={} height={}-{}",
-                client_id,
-                prev_height.revision_number(),
-                prev_height.revision_height()
-            )));
-        }
+        // Do not recompute the state ID from the supplied raw Anys here: light
+        // clients derive state IDs from a canonicalized client state (e.g.
+        // latest_height/frozen reset), and that canonicalization is
+        // ELC-specific and only available inside the enclave. The supplied
+        // base bytes are already pinned to the canonical store by the two
+        // checks above, and the stored state_id below was written by the
+        // in-enclave light client for exactly those bytes, so comparing the
+        // observed prev_state_id against the stored state_id closes the chain.
         let state_id_key = store_key::state_id_bytes(client_id, prev_height);
         let stored_state_id = self.use_mut_store(|store| store.tx_get(tx_id, &state_id_key))?;
         if stored_state_id.as_deref() != Some(prev_state_id) {
@@ -278,4 +276,3 @@ where
         store.deref_mut().apply(f)
     }
 }
-

modules/service/src/ecall_pool.rs

@@ -60,9 +60,7 @@ impl EcallPool {
         let job: Job = Box::new(move || {
             let _ = tx.send(f());
         });
-        sender
-            .send(job)
-            .expect("ECALL pool worker channel closed");
+        sender.send(job).expect("ECALL pool worker channel closed");
         rx.recv()
             .expect("ECALL pool worker terminated before producing a result")
     }
@@ -144,9 +142,9 @@ mod tests {
         // Verifies the "1 thread = 1 TCS forever" property under BIND policy:
         // the set of OS thread ids that execute jobs is bounded by pool size.
         let pool = Arc::new(EcallPool::new(3));
-        let observed = Arc::new(std::sync::Mutex::new(
-            std::collections::HashSet::<thread::ThreadId>::new(),
-        ));
+        let observed = Arc::new(std::sync::Mutex::new(std::collections::HashSet::<
+            thread::ThreadId,
+        >::new()));
         let mut handles = Vec::new();
         for _ in 0..30 {
             let pool = Arc::clone(&pool);

modules/service/src/elc.rs

@@ -202,7 +202,10 @@ where
             let header_memory = header_memory_budget.reserve_for_chunk(&chunk_msg).await?;
             if let Some(unit) = decoder.push_chunk(chunk_msg.chunk, header_memory)? {
                 units += 1;
-                if tx.send(StreamingSpeculativeBatchInput::Unit(unit)).is_err() {
+                if tx
+                    .send(StreamingSpeculativeBatchInput::Unit(Box::new(unit)))
+                    .is_err()
+                {
                     let result = scheduler.await.map_err(|e| {
                         Status::aborted(format!("speculative batch worker failed: {e}"))
                     })?;

modules/service/src/speculative/scheduler.rs

@@ -21,7 +21,7 @@ pub(crate) struct StreamingSpeculativeBatchResult {
 }
 
 pub(crate) enum StreamingSpeculativeBatchInput {
-    Unit(ResidentSpeculativeUpdateClientRequest),
+    Unit(Box<ResidentSpeculativeUpdateClientRequest>),
     Complete,
 }
 
@@ -77,7 +77,7 @@ where
             if state.failure.is_some() {
                 break;
             }
-            if let Err(e) = state.enqueue(unit) {
+            if let Err(e) = state.enqueue(*unit) {
                 state.failure = Some(e);
                 shared.ready.notify_all();
                 shared.complete.notify_all();
@@ -271,9 +271,7 @@ fn streaming_speculative_worker<E, S>(
         let req_clone = req.request().clone();
         let result = speculative
             .with_speculative_request_permit(|| {
-                pool.run(move || {
-                    speculative_inner.speculative_update_client(&app_inner, req_clone)
-                })
+                pool.run(move || speculative_inner.speculative_update_client(&app_inner, req_clone))
             })
             .map_err(|e| SpeculativeBatchFailure {
                 kind: SpeculativeBatchFailureKind::SpeculativeExecutionFailed,

modules/service/src/speculative/service.rs

@@ -757,6 +757,69 @@ mod tests {
             .expect("stored consensus state and matching state_id should be accepted");
     }
 
+    // Regression test: light clients derive state IDs from a
+    // canonicalized client state (e.g. latest_height/frozen reset before
+    // hashing), so the stored/observed state ID is generally NOT equal to
+    // gen_state_id_from_any over the raw supplied Anys. The stitch must not
+    // recompute the state ID from the raw base; it must accept the batch as
+    // long as the stored state_id and the enclave-observed prev_state_id
+    // agree, even when both differ from the raw-Any hash.
+    #[test]
+    fn stitch_accepts_first_base_state_when_state_id_uses_canonicalized_form() {
+        let client_id = "07-tendermint-0";
+        let enclave = FakeEnclave::new(Duration::from_millis(1));
+        let app = AppService::<FakeEnclave, MemStore>::new("test-home", enclave, 1);
+        let service = SpeculativeService::new(1);
+        let mut req = with_explicit_base_state_payload(mk_req(
+            "unit-0000",
+            client_id,
+            Some(Height::new(0, 10)),
+            None,
+        ));
+        let prev_height = req.base_state.prev_height.expect("test base prev_height");
+        // Simulate an ELC whose canonicalized state_id differs from the hash
+        // of the raw supplied Anys.
+        let canonical_form_state_id = vec![7u8; 32];
+        assert_ne!(
+            canonical_form_state_id,
+            state_id_for_base_state(&req.base_state),
+            "test requires a state_id that differs from the raw-Any hash"
+        );
+        req.base_state.prev_state_id = Some(canonical_form_state_id.clone());
+        seed_canonical_base_state(&app, client_id, &req.base_state);
+        app.enclave.use_mut_store(|store| {
+            store.set(
+                lcp_types::store_key::state_id_bytes(client_id, &prev_height),
+                canonical_form_state_id.clone(),
+            );
+        });
+        let result = SpeculativeUpdateClientResult {
+            response: MsgUpdateClientResponse::default(),
+            write_set: WriteSet::default(),
+            base_state: req.base_state.clone(),
+            observed_transition: ObservedStateTransition {
+                prev_height: Some(prev_height),
+                prev_state_id: Some(canonical_form_state_id),
+                post_height: Height::new(0, 11),
+                post_state_id: vec![1; 32],
+            },
+        };
+
+        service
+            .stitch_speculative_update_client_batch(
+                &app,
+                SpeculativeUpdateClientBatch {
+                    client_id: client_id.to_string(),
+                    units: vec![req],
+                },
+                SpeculativeUpdateClientBatchResult {
+                    client_id: client_id.to_string(),
+                    units: vec![result],
+                },
+            )
+            .expect("canonicalized-form state_id matching the stored state_id should be accepted");
+    }
+
     #[test]
     fn stitch_rejects_first_base_state_when_prev_state_id_is_missing() {
         let client_id = "07-tendermint-0";
@@ -924,7 +987,7 @@ mod tests {
     }
 
     #[test]
-    fn stitch_rejects_first_base_state_when_client_state_does_not_match_state_id() {
+    fn stitch_rejects_first_base_state_when_client_state_does_not_match_stored_state_id() {
         let client_id = "07-tendermint-0";
         let enclave = FakeEnclave::new(Duration::from_millis(1));
         let app = AppService::<FakeEnclave, MemStore>::new("test-home", enclave, 1);
@@ -936,16 +999,21 @@ mod tests {
             None,
         ));
         let prev_height = req.base_state.prev_height.expect("test base prev_height");
-        let prev_state_id = state_id_for_base_state(&req.base_state);
+        // Seed the canonical store from the original base, then mutate the
+        // supplied client_state. The stored state_id keeps reflecting the
+        // original pair while the unit (executing from the supplied base)
+        // observes the mutated pair's state_id, so the stored-state_id check
+        // must reject the stitch.
         seed_canonical_base_state(&app, client_id, &req.base_state);
-        req.base_state.prev_state_id = Some(prev_state_id.clone());
         req.base_state.client_state = Some(
             Any {
                 type_url: "/ibc.mock.ClientState".to_string(),
                 value: vec![9],
             }
             .into(),
         );
+        let observed_prev_state_id = state_id_for_base_state(&req.base_state);
+        req.base_state.prev_state_id = Some(observed_prev_state_id.clone());
         set_canonical_client_state(
             &app,
             client_id,
@@ -960,7 +1028,7 @@ mod tests {
             base_state: req.base_state.clone(),
             observed_transition: ObservedStateTransition {
                 prev_height: Some(prev_height),
-                prev_state_id: Some(prev_state_id),
+                prev_state_id: Some(observed_prev_state_id),
                 post_height: Height::new(0, 11),
                 post_state_id: vec![1; 32],
             },
@@ -978,13 +1046,13 @@ mod tests {
                     units: vec![result],
                 },
             )
-            .expect_err("client_state inconsistent with state_id should be rejected");
+            .expect_err("client_state inconsistent with stored state_id should be rejected");
 
         assert_eq!(err.kind, SpeculativeBatchFailureKind::BaseStateMismatch);
         assert_eq!(err.unit_id.as_deref(), Some("unit-0000"));
         assert!(
             err.detail
-                .contains("speculative base state_id does not match client_state/consensus_state"),
+                .contains("stored speculative base state_id mismatch"),
             "unexpected error detail: {}",
             err.detail
         );
@@ -1088,9 +1156,9 @@ mod tests {
         });
         first_req.base_state.prev_state_id = Some(state_id_for_base_state(&first_req.base_state));
         seed_canonical_base_state(&app, client_id, &first_req.base_state);
-        tx.send(StreamingSpeculativeBatchInput::Unit(
+        tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
             ResidentSpeculativeUpdateClientRequest::unmetered(first_req),
-        ))
+        )))
         .expect("send first unit");
 
         for _ in 0..100 {
@@ -1104,7 +1172,7 @@ mod tests {
             "expected first unit to start before input stream closes"
         );
 
-        tx.send(StreamingSpeculativeBatchInput::Unit(
+        tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
             ResidentSpeculativeUpdateClientRequest::unmetered(with_explicit_base_state_payload(
                 SpeculativeUpdateClientRequest {
                     unit_id: "unit-0001".to_string(),
@@ -1133,7 +1201,7 @@ mod tests {
                     },
                 },
             )),
-        ))
+        )))
         .expect("send second unit");
         tx.send(StreamingSpeculativeBatchInput::Complete)
             .expect("send batch complete");
@@ -1175,9 +1243,9 @@ mod tests {
         req.update.signer = vec![0; 20];
         seed_canonical_base_state(&app, client_id, &req.base_state);
 
-        tx.send(StreamingSpeculativeBatchInput::Unit(
+        tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
             ResidentSpeculativeUpdateClientRequest::unmetered(req),
-        ))
+        )))
         .expect("send first unit");
         drop(tx);
 
@@ -1226,9 +1294,9 @@ mod tests {
         req.update.signer = vec![0; 20];
         seed_canonical_base_state(&app, client_id, &req.base_state);
 
-        tx.send(StreamingSpeculativeBatchInput::Unit(
+        tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
             ResidentSpeculativeUpdateClientRequest::unmetered(req),
-        ))
+        )))
         .expect("send first unit");
         tx.send(StreamingSpeculativeBatchInput::Complete)
             .expect("send batch complete");
@@ -1272,7 +1340,7 @@ mod tests {
             )
         });
 
-        tx.send(StreamingSpeculativeBatchInput::Unit(
+        tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
             ResidentSpeculativeUpdateClientRequest::unmetered(SpeculativeUpdateClientRequest {
                 unit_id: "unit-0000".to_string(),
                 update: MsgUpdateClient {
@@ -1291,7 +1359,7 @@ mod tests {
                     consensus_state: None,
                 },
             }),
-        ))
+        )))
         .expect("send first unit");
         drop(tx);
 
@@ -1365,9 +1433,9 @@ mod tests {
         }
         seed_canonical_base_state(&app, client_id, &requests[0].base_state);
         for req in requests {
-            tx.send(StreamingSpeculativeBatchInput::Unit(
+            tx.send(StreamingSpeculativeBatchInput::Unit(Box::new(
                 ResidentSpeculativeUpdateClientRequest::unmetered(req),
-            ))
+            )))
             .expect("send unit");
         }
         tx.send(StreamingSpeculativeBatchInput::Complete)