STR-3155: bind snark account updates to specific seqnos

Cargo.toml

@@ -496,6 +496,15 @@ ssz_types = { git = "https://github.com/alpenlabs/ssz-gen", tag = "v0.15.0" }
 tree_hash = { git = "https://github.com/alpenlabs/ssz-gen", tag = "v0.15.0" }
 tree_hash_derive = { git = "https://github.com/alpenlabs/ssz-gen", tag = "v0.15.0" }
 
+# Disable debug_assertions in bitreq to dodge a known concurrency race in its
+# `next_request_id >= readable_request_id` invariant, which fires under
+# concurrent bitcoind RPC traffic in coverage/debug builds. Tracked upstream at
+# https://github.com/rust-bitcoin/corepc/issues/583 (PR #584 unmerged).
+#
+# TODO(STR-3554): remove this when PR merged
+[profile.dev.package.bitreq]
+debug-assertions = false
+
 # This is needed for custom build of SP1
 [profile.release.build-override]
 opt-level = 3

bin/alpen-client/src/prover/spec_acct.rs

@@ -29,7 +29,7 @@ use strata_paas::{ProofSpec, ProverError as PaasError, ProverResult, ReceiptStor
 use strata_proofimpl_alpen_acct::{EeAcctProgram, EeAcctProofInput};
 use strata_snark_acct_runtime::{Coinput, IInnerState, PrivateInput as UpdatePrivateInput};
 use strata_snark_acct_types::{
-    OutputMessage, OutputTransfer, ProofState, UpdateOutputs, UpdateProofPubParams,
+    OutputMessage, OutputTransfer, ProofState, Seqno, UpdateOutputs, UpdateProofPubParams,
 };
 
 use super::ChunkTask;
@@ -372,7 +372,12 @@ impl ProofSpec for AcctSpec {
                 )),
             })?;
 
+        let seq_no = batch.update_seq_no().ok_or_else(|| {
+            PaasError::TransientFailure(format!("batch {batch_id} has no assigned update seq_no"))
+        })?;
+
         let pub_params = UpdateProofPubParams::new(
+            Seqno::new(seq_no),
             cur_state,
             new_state,
             messages,

bin/prover-perf/src/programs/alpen_acct.rs

@@ -20,7 +20,7 @@ use strata_evm_ee::EvmExecutionEnvironment;
 use strata_proofimpl_alpen_acct::{EeAcctProgram, EeAcctProofInput};
 use strata_proofimpl_alpen_chunk::EeChunkProgram;
 use strata_snark_acct_runtime::{IInnerState, PrivateInput as UpdatePrivateInput};
-use strata_snark_acct_types::{LedgerRefs, ProofState, UpdateOutputs, UpdateProofPubParams};
+use strata_snark_acct_types::{LedgerRefs, ProofState, Seqno, UpdateOutputs, UpdateProofPubParams};
 use tracing::info;
 use zkaleido::{ExecutionSummary, ZkVmHost, ZkVmProgram};
 
@@ -77,6 +77,7 @@ fn prepare_input() -> EeAcctProofInput {
     let extra_data_bytes = encode_to_vec(&extra_data).expect("encode extra data");
 
     let pub_params = UpdateProofPubParams::new(
+        Seqno::zero(),
         ProofState::new(pre_root, 0),
         ProofState::new(post_root, 0),
         vec![],

bin/strata-test-cli/src/mock_ee/withdrawal.rs

@@ -11,7 +11,8 @@ use strata_msg_fmt::{Msg, OwnedMsg};
 use strata_ol_msg_types::{WithdrawalMsgData, WITHDRAWAL_MSG_TYPE_ID};
 use strata_ol_stf::BRIDGE_GATEWAY_ACCT_ID;
 use strata_snark_acct_types::{
-    LedgerRefs, OutputMessage, ProofState, UpdateOperationData, UpdateOutputs, UpdateProofPubParams,
+    LedgerRefs, OutputMessage, ProofState, Seqno, UpdateOperationData, UpdateOutputs,
+    UpdateProofPubParams,
 };
 
 /// Deterministic BIP-340 signing key whose verifying key matches the
@@ -69,7 +70,7 @@ pub(crate) fn build_snark_withdrawal_json(
     //
     // The mock withdrawal does not advance the snark account's inner state
     // or inbox index, so `cur_state == new_state == proof_state`.
-    let claim_ssz = sign_claim_ssz(&proof_state, &proof_state, &outputs);
+    let claim_ssz = sign_claim_ssz(Seqno::new(seq_no), &proof_state, &proof_state, &outputs);
     let signature = bip340_test_sign(&claim_ssz);
     let update_proof_hex = hex::encode(signature);
 
@@ -98,11 +99,13 @@ pub(crate) fn build_snark_withdrawal_json(
 /// Reconstructs the `UpdateProofPubParams` claim the OL builds in
 /// `snark_acct_sys::compute_update_claim` and returns its SSZ encoding.
 fn sign_claim_ssz(
+    seq_no: Seqno,
     cur_state: &ProofState,
     new_state: &ProofState,
     outputs: &UpdateOutputs,
 ) -> Vec<u8> {
     let pub_params = UpdateProofPubParams::new(
+        seq_no,
         cur_state.clone(),
         new_state.clone(),
         Vec::<MessageEntry>::new(),
@@ -313,7 +316,7 @@ mod tests {
         let msg_payload = MsgPayload::new(BitcoinAmount::from_sat(100_000_000), owned_msg.to_vec());
         let output_message = OutputMessage::new(BRIDGE_GATEWAY_ACCT_ID, msg_payload);
         let outputs = UpdateOutputs::new(vec![], vec![output_message]);
-        let claim_ssz = sign_claim_ssz(&proof_state, &proof_state, &outputs);
+        let claim_ssz = sign_claim_ssz(Seqno::new(5), &proof_state, &proof_state, &outputs);
 
         let proof_hex = json["payload"]["update_proof"].as_str().unwrap();
         let proof_bytes = hex::decode(proof_hex).unwrap();

crates/ee-acct-runtime/tests/common/mod.rs

@@ -20,10 +20,7 @@ use strata_snark_acct_runtime::{
     ArchivedPrivateInput as ArchivedSnarkPrivateInput, Coinput, IInnerState, InputMessage,
     PrivateInput as SnarkPrivateInput, ProgramResult, SnarkAccountProgram,
 };
-use strata_snark_acct_types::{
-    ProofState, SnarkAccountState, UpdateManifest, UpdateOperationData, UpdateOutputs,
-    UpdateProofPubParams,
-};
+use strata_snark_acct_types::*;
 
 /// Serializes an [`EePrivateInput`] and a [`SnarkPrivateInput`] with rkyv, then
 /// calls `f` with the archived references.
@@ -130,6 +127,7 @@ pub fn verify_update(
     let post_root = post_state.compute_state_root();
 
     let pub_params = UpdateProofPubParams::new(
+        Seqno::new(operation.seq_no()),
         ProofState::new(pre_root, 0),
         ProofState::new(post_root, operation.processed_messages().len() as u64),
         operation.processed_messages().to_vec(),
@@ -286,6 +284,7 @@ pub(crate) fn verify_with_chunks(
     let post_root = post_state.compute_state_root();
 
     let pub_params = UpdateProofPubParams::new(
+        Seqno::new(operation.seq_no()),
         ProofState::new(pre_root, 0),
         ProofState::new(post_root, operation.processed_messages().len() as u64),
         operation.processed_messages().to_vec(),

crates/proof-impl/alpen-acct/src/program.rs

@@ -125,7 +125,9 @@ mod tests {
     use strata_identifiers::Hash;
     use strata_predicate::{PredicateKey, PredicateTypeId};
     use strata_snark_acct_runtime::{IInnerState, PrivateInput as UpdatePrivateInput};
-    use strata_snark_acct_types::{LedgerRefs, ProofState, UpdateOutputs, UpdateProofPubParams};
+    use strata_snark_acct_types::{
+        LedgerRefs, ProofState, Seqno, UpdateOutputs, UpdateProofPubParams,
+    };
 
     use super::*;
 
@@ -151,6 +153,7 @@ mod tests {
 
         // With zero chunks and no state change, pre == post state root.
         let pub_params = UpdateProofPubParams::new(
+            Seqno::zero(),
             ProofState::new(state_root, 0),
             ProofState::new(state_root, 0),
             vec![],

crates/snark-acct-runtime/src/update_builder.rs

@@ -377,6 +377,7 @@ impl<P: SnarkAccountProgramVerification> FinalizedUpdate<P> {
         let coinputs = self.coinputs.into_iter().map(Coinput::new).collect();
 
         let pub_params = strata_snark_acct_types::UpdateProofPubParams::new(
+            self.snark_state.seq_no(),
             ProofState::new(self.pre_state.compute_state_root(), pre_inbox_idx),
             new_proof_state,
             self.messages,

crates/snark-acct-sys/src/verification.rs

@@ -168,6 +168,7 @@ fn compute_update_claim(
     let outputs = effects_to_update_outputs(update.effects());
 
     let pub_params = UpdateProofPubParams::new(
+        update.seq_no(),
         cur_state,
         update.new_proof_state().clone(),
         update.processed_messages().to_vec(),

crates/snark-acct-types/src/proof_interface.rs

@@ -3,12 +3,13 @@
 use strata_acct_types::MessageEntry;
 
 use crate::{
-    LedgerRefs, ProofState, UpdateOutputs,
+    LedgerRefs, ProofState, Seqno, UpdateOutputs,
     ssz_generated::ssz::proof_interface::UpdateProofPubParams,
 };
 
 impl UpdateProofPubParams {
     pub fn new(
+        seq_no: Seqno,
         cur_state: ProofState,
         new_state: ProofState,
         message_inputs: Vec<MessageEntry>,
@@ -17,6 +18,7 @@ impl UpdateProofPubParams {
         extra_data: Vec<u8>,
     ) -> Self {
         Self {
+            seq_no: *seq_no.inner(),
             cur_state,
             new_state,
             message_inputs: message_inputs
@@ -30,6 +32,10 @@ impl UpdateProofPubParams {
         }
     }
 
+    pub fn seq_no(&self) -> Seqno {
+        Seqno::new(self.seq_no)
+    }
+
     pub fn cur_state(&self) -> ProofState {
         self.cur_state.clone()
     }
@@ -58,11 +64,12 @@ impl UpdateProofPubParams {
 #[cfg(test)]
 mod tests {
     use proptest::prelude::*;
+    use ssz::Encode as _;
     use strata_acct_types::{AccountId, BitcoinAmount, MsgPayload};
     use strata_test_utils_ssz::ssz_proptest;
 
     use super::*;
-    use crate::{AccumulatorClaim, OutputMessage, OutputTransfer};
+    use crate::{AccumulatorClaim, OutputMessage, OutputTransfer, Seqno};
 
     fn proof_state_strategy() -> impl Strategy<Value = ProofState> {
         (any::<[u8; 32]>(), any::<u64>()).prop_map(|(inner_state, next_idx)| ProofState {
@@ -140,6 +147,7 @@ mod tests {
 
     fn update_proof_pub_params_strategy() -> impl Strategy<Value = UpdateProofPubParams> {
         (
+            any::<u64>(),
             proof_state_strategy(),
             proof_state_strategy(),
             prop::collection::vec(message_entry_strategy(), 0..3),
@@ -148,8 +156,17 @@ mod tests {
             prop::collection::vec(any::<u8>(), 0..32),
         )
             .prop_map(
-                |(cur_state, new_state, message_inputs, ledger_refs, outputs, extra_data)| {
+                |(
+                    seq_no,
+                    cur_state,
+                    new_state,
+                    message_inputs,
+                    ledger_refs,
+                    outputs,
+                    extra_data,
+                )| {
                     UpdateProofPubParams {
+                        seq_no,
                         cur_state,
                         new_state,
                         message_inputs: message_inputs
@@ -166,4 +183,32 @@ mod tests {
     }
 
     ssz_proptest!(UpdateProofPubParams, update_proof_pub_params_strategy());
+
+    /// Regression test for the Zellic replay finding: two
+    /// `UpdateProofPubParams` that are otherwise identical but built for
+    /// different `seq_no` values must produce different SSZ encodings.
+    /// `compute_update_claim` (in `strata-snark-acct-sys`) hashes this SSZ
+    /// encoding into the proof claim, so this property is what prevents an
+    /// older proof from being replayed at a later `seq_no`.
+    #[test]
+    fn pub_params_encoding_binds_seq_no() {
+        let proof_state = ProofState::new([0u8; 32].into(), 0);
+        let make = |seq_no: u64| {
+            UpdateProofPubParams::new(
+                Seqno::new(seq_no),
+                proof_state.clone(),
+                proof_state.clone(),
+                Vec::new(),
+                LedgerRefs::new_empty(),
+                UpdateOutputs::new_empty(),
+                Vec::new(),
+            )
+            .as_ssz_bytes()
+        };
+        assert_ne!(
+            make(7),
+            make(8),
+            "claim must differ across seq_no to prevent proof replay"
+        );
+    }
 }

crates/snark-acct-types/ssz/proof_interface.ssz

@@ -9,6 +9,11 @@ MAX_MESSAGE_ENTRIES = 2 << 10
 ### Public params that we provide as the claim the proof must prove the relate
 ### to each other correctly.
 class UpdateProofPubParams(Container):
+    ### Sequence number this update is authorized under.  Binding this into
+    ### the claim prevents replaying the same proof under a different
+    ### replay-protection counter.
+    seq_no: uint64
+
     ### Current state we're extending.
     cur_state: state.ProofState