fix(contracts): embed nullifiers vector in Circle state to prevent archival replays (#254)

contracts/sharibo/src/lib.rs

@@ -136,6 +136,10 @@ pub struct Circle {
     /// Funding is unshielded (addresses are already public), so storing
     /// them here imposes no additional privacy loss — see issue #82.
     pub contributors: Vec<Address>,
+    /// Nullifier hashes used in successful claims for this circle.
+    /// Embedded inside the Circle persistent entry so they inherit the
+    /// continuously-extended TTL lifecycle of the circle itself (issue #254).
+    pub nullifiers: Vec<Fr>,
     /// True once `cancel_circle` has been called; prevents any further
     /// `fund` or `claim` calls so the circle is permanently closed.
     pub cancelled: bool,
@@ -323,6 +327,7 @@ impl Contract {
             pot: 0,
             vk,
             contributors: Vec::new(&env),
+            nullifiers: Vec::new(&env),
             cancelled: false,
             round_deadline_ledgers,
             round_started_ledger,
@@ -509,8 +514,7 @@ impl Contract {
         }
 
         // 3. this nullifier must not have claimed before (any round, this circle)
-        let nullifier_key = DataKey::Nullifier(circle_id, nullifier_hash.clone());
-        if env.storage().persistent().has(&nullifier_key) {
+        if circle.nullifiers.contains(&nullifier_hash) {
             panic_with_error!(&env, Error::AlreadyClaimed);
         }
 
@@ -533,18 +537,14 @@ impl Contract {
         }
 
         // effects
-        env.storage().persistent().set(&nullifier_key, &true);
-        env.storage()
-            .persistent()
-            .extend_ttl(&nullifier_key, LEDGER_THRESHOLD, LEDGER_EXTEND_TO);
-
         let token_client = token::Client::new(&env, &circle.token);
         token_client.transfer(&env.current_contract_address(), &recipient, &circle.pot);
 
         circle.pot = 0;
         circle.round += 1;
         circle.contributors = Vec::new(&env);
         circle.round_started_ledger = env.ledger().sequence();
+        circle.nullifiers.push_back(nullifier_hash);
         env.storage().persistent().set(&key, &circle);
         env.storage()
             .persistent()
@@ -681,9 +681,12 @@ impl Contract {
     /// `true` if the nullifier has ever been used in a successful claim for
     /// this circle (any round); the associated identity cannot claim again.
     pub fn has_claimed(env: Env, circle_id: u64, nullifier_hash: Fr) -> bool {
-        env.storage()
-            .persistent()
-            .has(&DataKey::Nullifier(circle_id, nullifier_hash))
+        let key = DataKey::Circle(circle_id);
+        if let Some(circle) = env.storage().persistent().get::<_, Circle>(&key) {
+            circle.nullifiers.contains(&nullifier_hash)
+        } else {
+            false
+        }
     }
 
     /// Step 1 of two-step admin transfer: the current admin nominates a

contracts/sharibo/src/test.rs

@@ -1308,6 +1308,52 @@ fn instance_ttl_extended_after_create_fund_claim() {
     assert_eq!(circle.round, 1, "claim should have advanced round to 1");
 }
 
+#[test]
+fn nullifier_fence_survives_ttl_expiry() {
+    // Regression test for issue #254:
+    // Nullifier storage entries embedded inside the Circle struct inherit the
+    // Circle's continuously-extended TTL. When the ledger advances past the
+    // initial extend_to period, re-extending the Circle entry ensures that
+    // stored nullifiers are preserved and cannot be bypassed.
+    let s = setup(5, 100);
+    let client = ContractClient::new(&s.env, &s.client_id);
+
+    for m in s.members.iter() {
+        client.fund(&s.circle_id, m);
+    }
+
+    let recipient = Address::generate(&s.env);
+    let nullifier_hash = real_nullifier_hash(&s.env);
+    let external_nullifier = real_external_nullifier_round0(&s.env);
+    let proof = real_valid_proof(&s.env);
+
+    client.claim(
+        &s.circle_id,
+        &recipient,
+        &nullifier_hash,
+        &external_nullifier,
+        &proof,
+    );
+
+    assert!(client.has_claimed(&s.circle_id, &nullifier_hash));
+
+    // Advance the ledger sequence past LEDGER_THRESHOLD.
+    s.env.ledger().with_mut(|l| {
+        l.sequence_number += LEDGER_THRESHOLD + 10;
+        l.timestamp += u64::from(LEDGER_THRESHOLD + 10) * 5;
+    });
+
+    // Re-funding for round 1 extends the Circle entry TTL.
+    let token_admin_client = token::StellarAssetClient::new(&s.env, &s.token);
+    for m in s.members.iter() {
+        token_admin_client.mint(m, &s.contribution);
+        client.fund(&s.circle_id, m);
+    }
+
+    // Verify nullifier fence is still intact after ledger advancement and Circle TTL extension.
+    assert!(client.has_claimed(&s.circle_id, &nullifier_hash));
+}
+
 // ---- Proptest: apply_fee invariants ----
 //
 // Two sub-suites:

docs/adr/004-storage-archival.md

@@ -0,0 +1,50 @@
+# ADR 004: Storage Archival and Nullifier Lifetime
+
+- **Status:** Accepted
+- **Date:** 2026-08-31
+- **Context:** Design for issue #254 (Nullifier entries can be archived — a claim could be replayed after TTL expiry).
+
+## Context
+
+In Soroban, persistent storage entries are archived when their TTL lapses.
+
+Previously, the double-claim fence in `Contract::claim` relied on standalone persistent storage entries:
+```rust
+let nullifier_key = DataKey::Nullifier(circle_id, nullifier_hash.clone());
+if env.storage().persistent().has(&nullifier_key) {
+    panic_with_error!(&env, Error::AlreadyClaimed);
+}
+```
+
+Soroban persistent entries have their TTL extended at write time to `LEDGER_EXTEND_TO = 500_000` ledgers (~1 month at 5s/ledger). Nullifiers are write-once per claim and were never accessed or extended again.
+
+In contrast, the `Circle` entry (`DataKey::Circle(circle_id)`) is continuously re-extended on every `fund` and `claim` call. If a circle remains active or is extended, but individual `DataKey::Nullifier` entries lapse and are archived, `env.storage().persistent().has(&nullifier_key)` would return `false`. This opened a vulnerability where a member could replay a previously used claim/nullifier in a subsequent round after its TTL expired.
+
+## Options Analyzed
+
+### Option (a) — Re-extend every circle's nullifier TTLs on each claim
+- On every claim, iterate through all previously stored nullifiers for the circle and extend their TTLs.
+- **Drawback**: Unbounded storage lookups and CPU/TTL extension work as the number of claims grows; fails to scale.
+
+### Option (b) — Store nullifiers as a bounded `Vec<Fr>` inside the `Circle` struct (Chosen)
+- Add `pub nullifiers: Vec<Fr>` to `pub struct Circle`.
+- Read and check `circle.nullifiers.contains(&nullifier_hash)` directly within `claim` and `has_claimed`.
+- Store nullifiers directly inside `Circle` at `DataKey::Circle(circle_id)`.
+- **Advantages**:
+  - Nullifiers inherit the `Circle` entry's continuously-extended TTL lifecycle. As long as the `Circle` entry is live or re-extended, all nullifiers registered for that circle remain live.
+  - For a fixed-size ROSCA, the number of nullifiers in a circle is bounded by design (`size` members per cycle).
+  - In-memory `Vec::contains()` on a small `soroban_sdk::Vec` is O(N) where N ≤ circle size, which is cheap and fits comfortably within CPU instruction limits.
+
+### Option (c) — Declare circles time-bounded
+- Require circles to complete within the 500,000 ledger TTL window and accept replay risks for expired circles.
+- **Drawback**: Leaves residual vulnerability if a circle spans more than ~1 month.
+
+## Decision
+
+Adopt **Option (b)**. Nullifiers are embedded within `Circle.nullifiers`.
+
+## Consequences
+
+- Nullifiers never archive independently of the `Circle` entry.
+- Double-claim fences survive arbitrary ledger advancement as long as the circle exists.
+- State size per `Circle` increases by `32 * N` bytes, naturally bounded by circle size.

docs/threat-model.md

@@ -37,13 +37,14 @@ Scope: `contracts/sharibo/src/lib.rs`, `circuits/membership.template.circom`, `p
 
 ### 2. No double claim — one payout per nullifier
 
-**Mechanism.** `nullifierHash = Poseidon(identityNullifier, externalNullifier)` is emitted as a circuit output (`circuits/membership.template.circom:113-116`). The contract records it under `DataKey::Nullifier(circle_id, nullifier_hash)` and rejects any claim that reuses one (`lib.rs:213-217`, marked used at `:231`), independent of the pairing check.
+**Mechanism.** `nullifierHash = Poseidon(identityNullifier, externalNullifier)` is emitted as a circuit output (`circuits/membership.template.circom:113-116`). The contract records it in `Circle.nullifiers` (`lib.rs`) and rejects any claim that reuses one, independent of the pairing check. Storing nullifiers inside the `Circle` persistent entry ensures nullifiers share the circle's continuously-extended TTL lifecycle and cannot be archived independently (see `docs/adr/004-storage-archival.md`).
 
-**Tests.** `contracts/sharibo/src/test.rs:284` (`second_claim_with_same_nullifier_reverts`), `:548` (`has_claimed_false_before_true_after`).
+**Tests.** `contracts/sharibo/src/test.rs` (`second_claim_with_same_nullifier_reverts`, `has_claimed_false_before_true_after`, `nullifier_fence_survives_ttl_expiry`).
 
 **Limits.**
-- `recipient` is a plain contract argument (`lib.rs:186`), not a circuit input — it is never bound to the proof (compare the circuit's signal list, `circuits/membership.template.circom:79-96`, which has no recipient signal at all; `circuits/test/membership.test.js:182` documents that the circuit does not bind the proof to any single expected value beyond what the verifier separately checks). Concretely: `(nullifier_hash, external_nullifier, proof)` is a valid claim for *any* `recipient`. If those values become visible before the original claim transaction is finalized — a careless or malicious relayer, or another party observing the pending transaction — anyone can resubmit the same tuple with a different `recipient` and redirect the payout. This is a payout-hijacking risk, not a privacy break: it costs the original claimant their payout, it does not deanonymize them. It's the same class of risk Tornado-Cash-style relayer designs address by adding the recipient (and a relayer fee) as a public input the circuit itself commits to; Sharibo does not do this today.
-- In the demo, `claim` is always submitted through the admin's client (`scripts/e2e.ts:201`, `app/src/App.tsx:424,464`), which is exactly the delivery path where the above risk would surface first — the admin (or whoever holds that key) sees the proof and recipient before broadcasting and is a required relayer for the demo flow, even though `claim` itself has no `require_auth` call (`lib.rs:183-249`) and would accept submission from any account.
+- `recipient` is a plain contract argument (`lib.rs`), not a circuit input — it is never bound to the proof (compare the circuit's signal list, `circuits/membership.template.circom:79-96`, which has no recipient signal at all; `circuits/test/membership.test.js:182` documents that the circuit does not bind the proof to any single expected value beyond what the verifier separately checks). Concretely: `(nullifier_hash, external_nullifier, proof)` is a valid claim for *any* `recipient`. If those values become visible before the original claim transaction is finalized — a careless or malicious relayer, or another party observing the pending transaction — anyone can resubmit the same tuple with a different `recipient` and redirect the payout. This is a payout-hijacking risk, not a privacy break: it costs the original claimant their payout, it does not deanonymize them. It's the same class of risk Tornado-Cash-style relayer designs address by adding the recipient (and a relayer fee) as a public input the circuit itself commits to; Sharibo does not do this today.
+- In the demo, `claim` is always submitted through the admin's client (`scripts/e2e.ts:201`, `app/src/App.tsx:424,464`), which is exactly the delivery path where the above risk would surface first — the admin (or whoever holds that key) sees the proof and recipient before broadcasting and is a required relayer for the demo flow, even though `claim` itself has no `require_auth` call (`lib.rs`) and would accept submission from any account.
+- Storage footprint per circle grows by `32 * N` bytes, bounded by the circle's size `N`.
 
 ### 3. Round binding — a proof for round *N* cannot be used in round *N+1*