diff --git a/app/src/commands/service.rs b/app/src/commands/service.rs index 22f0c76c..0df0c80c 100644 --- a/app/src/commands/service.rs +++ b/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(); diff --git a/modules/enclave-api/src/enclave.rs b/modules/enclave-api/src/enclave.rs index 5431d183..60e6f0a5 100644 --- a/modules/enclave-api/src/enclave.rs +++ b/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: CommitStoreAccessor { /// 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: CommitStoreAccessor { 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) } } - diff --git a/modules/service/src/ecall_pool.rs b/modules/service/src/ecall_pool.rs index bfb71d79..4667d56b 100644 --- a/modules/service/src/ecall_pool.rs +++ b/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::::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); diff --git a/modules/service/src/elc.rs b/modules/service/src/elc.rs index 0d98d989..232f2301 100644 --- a/modules/service/src/elc.rs +++ b/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}")) })?; diff --git a/modules/service/src/speculative/scheduler.rs b/modules/service/src/speculative/scheduler.rs index 6e5103e5..3d275321 100644 --- a/modules/service/src/speculative/scheduler.rs +++ b/modules/service/src/speculative/scheduler.rs @@ -21,7 +21,7 @@ pub(crate) struct StreamingSpeculativeBatchResult { } pub(crate) enum StreamingSpeculativeBatchInput { - Unit(ResidentSpeculativeUpdateClientRequest), + Unit(Box), 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( 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, diff --git a/modules/service/src/speculative/service.rs b/modules/service/src/speculative/service.rs index fe9a3756..bc7f99a5 100644 --- a/modules/service/src/speculative/service.rs +++ b/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::::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::::new("test-home", enclave, 1); @@ -936,9 +999,12 @@ 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(), @@ -946,6 +1012,8 @@ mod tests { } .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)