docs: split miscoupled private-PDA test docs and clean phrasing

Addresses the following review comments:

- "Isn't two_mask_3_claims_under_same_seed_are_rejected already checking
  that there's a mechanism protecting against this exploit scenario?"
  The doc block at nssa/src/state.rs:2488-2504 mixes three paragraphs,
  one about reuse, one TODO about wallet side input, one exploit pin,
  all attached to two_mask_3_claims_under_same_seed_are_rejected. The
  reuse test below it had no doc at all. I split as follows: the
  exploit-pin paragraph stays on two_mask_3_claims_..., the reuse
  paragraph moves to a fresh docstring on
  mask_3_reuse_across_txs_currently_unsupported.

- "I don't understand this. I think this should fail because ... the
  input pre_state which is marked with is_authorized=true will make
  things fail."
  The reuse test's new docstring cites the actual reject site, the
  post-loop private_pda_bound_positions assertion in
  privacy_preserving_circuit.rs:185-192. At top level the Entry::Vacant
  arm accepts is_authorized=true unconditionally, the rejection comes
  from the bound-positions check firing because noop emits no Claim::Pda
  and there is no caller ChainedCall.pda_seeds.

- "let's dont have this TODO as part of the doc"
  The block is moved out into regular // comments immediately above
  mask_3_reuse_across_txs_currently_unsupported.

- "let's not add implementation details to docs"
  In caller_pda_seeds_authorize_mask_3_private_pda_for_callee's
  docstring, I dropped the parenthetical "(Occupied branch)" and the
  trailing sentence about which validate_and_sync_states code path gets
  exercised.

- "what does \`Claim::Pda(seed)\` / \`pda_seeds\` mean?"
  I rewrote the pda_family_binding docstring at
  privacy_preserving_circuit.rs:33-39: replaced the ambiguous
  "Claim::PrivatePda and ChainedCall's private seeds into plain
  Claim::Pda(seed) / pda_seeds" phrase with "a Claim::Pda(seed) in a
  program's post_state or a caller's ChainedCall.pda_seeds entry".

- Suggestion on nssa/src/validated_state_diff.rs:226 rewriting
  "The public-execution path only sees mask-0 accounts" to
  "The public-execution path only sees public accounts".
  Applied: "The public-execution path only sees public accounts".

- Clarification requested on the private_pda_bound_positions field:
  I expanded the docstring at privacy_preserving_circuit.rs:26-31 to
  state that binding is an idempotent property, not an event, and to
  enumerate the two proof paths that populate it (a Claim::Pda on a
  mask-3 pre_state, or a caller's pda_seeds matching under the private
  derivation).
This commit is contained in:
Moudy 2026-04-21 00:37:06 +02:00
parent 9e4e546c9e
commit 34f8b6cac8
3 changed files with 37 additions and 29 deletions

View File

@ -2417,10 +2417,9 @@ pub mod tests {
/// Happy path for the caller-seeds authorization of a mask-3 PDA. The delegator claims a
/// private PDA via `Claim::Pda(seed)`, then chains to a callee (`noop`) delegating the same
/// seed via `ChainedCall.pda_seeds`. In the callee's step, the `pre_state` is already in
/// `self.pre_states` (Occupied branch) and its authorization is established via the private
/// derivation `private_pda_account_id(delegator, seed, npk) == pre.account_id`. This exercises
/// the `authorized_via_private` path in `validate_and_sync_states`.
/// seed via `ChainedCall.pda_seeds`. In the callee's step, the `pre_state`'s authorization
/// is established via the private derivation
/// `private_pda_account_id(delegator, seed, npk) == pre.account_id`.
#[test]
fn caller_pda_seeds_authorize_mask_3_private_pda_for_callee() {
let delegator = Program::private_pda_delegator();
@ -2485,20 +2484,11 @@ pub mod tests {
assert!(matches!(result, Err(NssaError::CircuitProvingError(_))));
}
/// Pins the current limitation: a mask-3 PDA that was claimed in a previous transaction
/// cannot be re-used in a new transaction as-is, because this PR only binds wallet-supplied
/// npks via a fresh `Claim::Pda` or a caller's `ChainedCall.pda_seeds` — neither is present
/// when a program operates on an already-owned private PDA at top level. The circuit rejects.
///
/// TODO: a follow-up PR in the Private PDAs series needs to let the wallet supply a
/// `(seed, original_owner_program_id)` side input per mask-3 `pre_state` so the circuit
/// can re-verify `private_pda_account_id(owner, seed, npk) == pre.account_id` without a
/// claim.
/// Exploit-scenario pin. A single `(program_id, seed)` pair can derive a family of
/// `AccountId`s (one public PDA and one private PDA per distinct npk). Without the tx-wide
/// `AccountId`s, one public PDA and one private PDA per distinct npk. Without the tx-wide
/// family-binding check, a program could claim `PDA_alice` (mask-3, `alice_npk`) and
/// `PDA_bob` (mask-3, `bob_npk`) under the same seed in one transaction, and once reuse
/// is supported a later chained call could delegate both to a callee via
/// `PDA_bob` (mask-3, `bob_npk`) under the same seed in one transaction, and once reuse
/// is supported a later chained call could delegate both to a callee via
/// `pda_seeds: [S]` and mix balances across them. The binding check rejects the setup
/// here: after the first claim records `(program, seed) → PDA_alice`, the second claim
/// tries to record `(program, seed) → PDA_bob` and panics.
@ -2530,6 +2520,18 @@ pub mod tests {
assert!(matches!(result, Err(NssaError::CircuitProvingError(_))));
}
/// Pins the current limitation: a mask-3 PDA that was claimed in a previous transaction
/// cannot be re-used in a new transaction as-is. This PR only binds supplied npks via a
/// fresh `Claim::Pda` or a caller's `ChainedCall.pda_seeds`, neither is present when a
/// program operates on an already-owned private PDA at top level. The reject site is the
/// post-loop `private_pda_bound_positions` assertion in
/// `privacy_preserving_circuit.rs`: `noop` emits no `Claim::Pda` and there is no caller
/// `ChainedCall.pda_seeds`, so position 0 is never bound and the assertion fires.
//
// TODO: a follow-up PR in the Private PDAs series needs to let the wallet supply a
// `(seed, original_owner_program_id)` side input per mask-3 `pre_state` so the circuit
// can re-verify `private_pda_account_id(owner, seed, npk) == pre.account_id` without a
// claim.
#[test]
fn mask_3_reuse_across_txs_currently_unsupported() {
let program = Program::noop();

View File

@ -223,7 +223,7 @@ impl ValidatedStateDiff {
}
Claim::Pda(seed) => {
// The program can only claim accounts that correspond to the PDAs it is
// authorized to claim. The public-execution path only sees mask-0
// authorized to claim. The public-execution path only sees public
// accounts, so the public-PDA derivation is the correct formula here.
let pda = AccountId::from((&chained_call.program_id, &seed));
ensure!(

View File

@ -23,20 +23,26 @@ struct ExecutionState {
post_states: HashMap<AccountId, Account>,
block_validity_window: BlockValidityWindow,
timestamp_validity_window: TimestampValidityWindow,
/// Positions (in `pre_states`) of mask-3 accounts whose wallet-supplied npk has been bound
/// to their `AccountId` via a proven `private_pda_account_id(program_id, seed, npk)` check.
/// The binding happens when the circuit validates a `Claim::Pda(seed)` on that `pre_state`,
/// or when it authorizes that `pre_state` via a caller's `ChainedCall.pda_seeds`. After the
/// main loop, every mask-3 position must appear in this set; otherwise the npk is unbound
/// and the circuit rejects.
/// Positions (in `pre_states`) of mask-3 accounts whose supplied npk has been bound to
/// their `AccountId` via a proven `private_pda_account_id(program_id, seed, npk)` check.
/// Two proof paths populate this set:
/// 1. A `Claim::Pda(seed)` in a program's post_state on that `pre_state`.
/// 2. A caller's `ChainedCall.pda_seeds` entry matching that `pre_state` under the
/// private derivation.
/// Binding is an idempotent property, not an event: the same position can legitimately be
/// bound through both paths in the same tx (e.g. a program claims a private PDA and then
/// delegates it to a callee), and the set uses `contains`, not `assert!(insert)`. After
/// the main loop, every mask-3 position must appear in this set; otherwise the npk is
/// unbound and the circuit rejects.
mask3_bound_positions: HashSet<usize>,
/// Across the whole transaction, each `(program_id, seed)` pair may resolve to at most one
/// `AccountId`. A seed under a program can derive a family of accounts (one public PDA and
/// one private PDA per distinct npk), and unifying `Claim::PrivatePda` and `ChainedCall`'s
/// private seeds into plain `Claim::Pda(seed)` / `pda_seeds` would otherwise let a single
/// `pda_seeds: [S]` in a chained call authorize multiple family members at once. We record
/// every claim and caller-authorization resolution here and reject any mismatch, making the
/// rule: one `(program, seed)` → one account per tx.
/// `AccountId`. A seed under a program can derive a family of accounts, one public PDA and
/// one private PDA per distinct npk. Without this check, a single `pda_seeds: [S]` entry in
/// a chained call could authorize multiple family members at once (different npks under the
/// same seed) and let a callee mix balances across them. Every claim and every
/// caller-authorization resolution is recorded here, either as a new `(program, seed)` →
/// `AccountId` entry or as an equality check against the existing one, making the rule: one
/// `(program, seed)` → one account per tx.
pda_family_binding: HashMap<(ProgramId, PdaSeed), AccountId>,
}