Addresses the following review comments from @Arjentix:
- "I think there are too many internal implementation information
exposed here. This structure is used by our users, program devs. And
they should not care about distinction between private or public pda
or different masks"
(on ChainedCall.pda_seeds, same feedback repeated on Claim::Pda)
I rewrote both docstrings to drop internal details (visibility masks,
per-form derivation names, npk handling). Program devs see only that
they emit a seed and the `AccountId` is derived from
`(program_id, seed)` regardless of whether the account is public or
private.
- "Let's reflect the new nuance in the name"
(on compute_authorized_pdas returning public-form derivations only)
I renamed the function to `compute_public_authorized_pdas`. After
the PR #446 rework the function only returns public-form
derivations, the private-form authorization lives in the circuit
guest. Updated the call site in nssa/src/validated_state_diff.rs
and the two unit tests.
Addresses the following review comment:
- "I think this should be a constructor `AccountId::for_private_pda`.
Consider also removing the existing `impl From<(ProgramId, Seed)> for
AccountId` for public pdas in favor of a `AccountId::for_public_pda`
to have a unified way of constructing pdas"
I replaced `impl From<(&ProgramId, &PdaSeed)> for AccountId` with
`AccountId::for_public_pda(program_id: &ProgramId, seed: &PdaSeed) ->
Self` and replaced the free function `private_pda_account_id(...)`
with `AccountId::for_private_pda(program_id: &ProgramId, seed:
&PdaSeed, npk: &NullifierPublicKey) -> Self`. Both live in an inherent
`impl AccountId` block in nssa/core/src/program.rs next to the PDA
derivation logic. Migrated all call sites across nssa/core,
nssa/src/state.rs, nssa/src/validated_state_diff.rs,
program_methods/guest/src/bin/privacy_preserving_circuit.rs,
programs/amm/core, programs/associated_token_account/core, the example
tail-call binary, and the ATA tutorial doc. Test function names that
referenced the old free function were also renamed
(private_pda_account_id_* to for_private_pda_*).
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).
