Skip to content

feat!: Add new path for pda seed input#486

Open
schouhy wants to merge 1 commit into
mainfrom
schouhy/private-pdas-as-external-input
Open

feat!: Add new path for pda seed input#486
schouhy wants to merge 1 commit into
mainfrom
schouhy/private-pdas-as-external-input

Conversation

@schouhy
Copy link
Copy Markdown
Collaborator

@schouhy schouhy commented May 19, 2026

🎯 Purpose

Private PDAs previously could only be used through a program that used Claim::Pda or pda_seeds to establish the binding between the PDA account_id and the npk. This meant you couldn't call auth_transfer directly to fund a private PDA, you always needed a wrapper. This PR removes that limitation.

⚙️ Approach

  • Add an optional seed: Option<(PdaSeed, ProgramId)> field to the PrivatePdaInit and PrivatePdaUpdate variants of InputAccountIdentity. When the seed is provided, the circuit verifies AccountId::for_private_pda(pda_program_id, seed, npk, identifier) == pre_state.account_id directly, establishing the binding without needing a Claim::Pda or caller pda_seeds. It also verifies is_authoried is set to false.
  • Add binding logic in execution_state.rs for when an external seed is supplied
  • Remove the authorization check on private pda claiming in the ppt circuit. Authorization checks only remains on public account claiming.
  • Rename pda_fund_spend_proxy to pda_spend_proxy. The fund path is now handled directly via auth_transfer.
  • Rewrite fund_private_pda in integration tests to use auth_transfer directly with the external seed

🧪 How to Test

RISC0_DEV_MODE=1 cargo test

🔗 Dependencies

None

🔜 Future Work

Update wallet commands to use the external seed path when appropiate.

📋 PR Completion Checklist

Mark only completed items. A complete PR should have all boxes ticked.

  • Complete PR description
  • Implement the core functionality
  • Add/update tests
  • Add/update documentation and inline comments

@schouhy schouhy force-pushed the schouhy/private-pdas-as-external-input branch from f518be7 to e4a7196 Compare May 20, 2026 01:32
@schouhy schouhy marked this pull request as ready for review May 20, 2026 03:16
Copy link
Copy Markdown
Collaborator

@Pravdyvy Pravdyvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Collaborator

@moudyellaz moudyellaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks, left some minor comments casue i struggled a bit.

Comment thread wallet/src/privacy_preserving_tx.rs
@moudyellaz
Copy link
Copy Markdown
Collaborator

One more thing on commit hygiene: the PR title carries feat!: but none of the individual commits (b37e3ca8...e4a71961) use the Conventional Commits ! marker or a BREAKING CHANGE: footer. If the merge is a squash-merge that lifts the PR title, we'll be fine on the changelog side, but if any of these land as-is (e.g., on a rebase merge), the breaking nature gets lost.

Could you add a BREAKING CHANGE: footer to b37e3ca8 (the type-shape change) and mark it with !, and the same on eb5f54d7 for the rename? That way the convention is honored regardless of merge strategy.

Comment thread nssa/core/src/circuit_io.rs
"Cannot claim unauthorized private PDA {pre_account_id}"
);
}
Claim::Authorized => {}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why we need this change...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, now you can claim other users pda-derived accounts, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to remove the authorization constraint on claiming private pdas because otherwise there will be no way for bob to send some funds to alice in a (newly initialized) private pda for which bob doesn't know the nsk. It's the same reason we don't impose authorization on claiming a regular private account. The surface area for griefing attacks is reduced compared to public pdas because private ones depend on the npk and an identifier
.
But yes, you could claim other users pdas for a specific npk and identifier.

@schouhy schouhy force-pushed the schouhy/private-pdas-as-external-input branch 6 times, most recently from 92be397 to 51e5076 Compare May 21, 2026 14:17
BREAKING CHANGE: add identity variants to the circuit and change semantics for `Claim::Authorized` for private PDAs
@schouhy schouhy force-pushed the schouhy/private-pdas-as-external-input branch from 51e5076 to 3c6d623 Compare May 21, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants