From 8f2d937a9d3e496072490054d83406a85abc0f6f Mon Sep 17 00:00:00 2001 From: Marvin Jones Date: Wed, 15 Jul 2026 14:06:20 -0400 Subject: [PATCH] test(privacy): fix AMM clock ownership bug, confirm 5 chained-call privacy tests, add 5 more MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: advance_clock() left the clock account DEFAULT_PROGRAM_ID-owned, which spel-framework's guest dispatcher (upstream in logos-co/spel) silently drops from a program's output as an unclaimed, non-default, default-owned account. That desynced the privacy circuit's account count from the caller-supplied identities, blocking every AMM chained-call privacy test with "Invalid account_identities length". Fix: own the fixture's clock account with a placeholder non-default ProgramId. Rewrote the 5 previously not-expressible Swap/AddLiquidity/RemoveLiquidity privacy tests to assert real success (transaction applied, resulting state and private commitments verified) now that they're unblocked. Added 5 more tests: Swap to a fresh PrivateUnauthorized/PrivateAuthorizedInit destination (both confirmed not-expressible, for two distinct reasons — guest ABI signer requirement vs. destination-must-already-exist), and NewDefinition with a private PrivateAuthorizedInit/PrivateUnauthorized initial LP holder (expressible for the former, not for the latter). Updated docs/findings.md and docs/privacy-test-matrix.md accordingly. --- docs/findings.md | 24 +- docs/privacy-test-matrix.md | 84 ++- programs/integration_tests/tests/amm.rs | 869 ++++++++++++++++++++++-- 3 files changed, 896 insertions(+), 81 deletions(-) diff --git a/docs/findings.md b/docs/findings.md index 7383046..042d433 100644 --- a/docs/findings.md +++ b/docs/findings.md @@ -78,16 +78,20 @@ In this task, we extend testing for LEZ programs to cover privacy features: | Function tested | Test name | Category | Description of objective | Result | |---|---|---|---|---| -| SwapExactInput | `amm_swap_a_to_b_private_user_holding_is_not_expressible` | REGULAR, CHAIN | Private `user_holding_a` deposit leg — confirms the circuit-level account-count bug also fires with a real private account (8 vs 7 accounts), not just the all-public control case | ❌ (confirmed not-expressible — circuit bug) | -| SwapExactOutput | `amm_swap_exact_output_private_user_holding_is_not_expressible` | REGULAR, CHAIN | Same confirmation for `SwapExactOutput` — identical account/chained-call shape to `SwapExactInput` (8 vs 7 accounts) | ❌ (confirmed not-expressible — circuit bug) | -| AddLiquidity | `amm_add_liquidity_private_lp_holding_is_not_expressible` | REGULAR, CHAIN | Private LP-output holding (`user_holding_lp`) — same circuit bug (10 vs 9 accounts) | ❌ (confirmed not-expressible — circuit bug) | -| AddLiquidity | `amm_add_liquidity_private_user_holdings_is_not_expressible` | REGULAR, CHAIN | Private deposit legs (`user_holding_a` + `user_holding_b`) — same circuit bug (10 vs 9 accounts) | ❌ (confirmed not-expressible — circuit bug) | -| RemoveLiquidity | `amm_remove_liquidity_private_lp_holding_is_not_expressible` | REGULAR, CHAIN | Private LP holding (the account that signs/burns to remove liquidity) — same circuit bug (10 vs 9 accounts) | ❌ (confirmed not-expressible — circuit bug) | -| RemoveLiquidity | `amm_remove_liquidity_private_new_user_holdings_is_not_expressible` | EXIST, CHAIN | Brand-new `PrivateUnauthorized` token A/B destinations — rejected by a separate, unrelated program-level precondition (destination must already exist) before the circuit bug is even reached | ❌ (confirmed not-expressible — different reason) | +| SwapExactInput | `amm_swap_a_to_b_private_user_holding` | REGULAR, CHAIN | Private `user_holding_a` deposit leg, through the Token + TWAP-oracle chained calls | ✅ | +| SwapExactOutput | `amm_swap_exact_output_private_user_holding` | REGULAR, CHAIN | Same coverage for `SwapExactOutput` | ✅ | +| AddLiquidity | `amm_add_liquidity_private_lp_holding` | REGULAR, CHAIN | Private LP-output holding (`user_holding_lp`) receives newly-minted LP on top of an existing private balance | ✅ | +| AddLiquidity | `amm_add_liquidity_private_user_holdings` | REGULAR, CHAIN | Private deposit legs (`user_holding_a` + `user_holding_b`), public LP recipient | ✅ | +| RemoveLiquidity | `amm_remove_liquidity_private_lp_holding` | REGULAR, CHAIN | Private LP holding (the account that signs/burns to remove liquidity) | ✅ | +| RemoveLiquidity | `amm_remove_liquidity_private_new_user_holdings_is_not_expressible` | EXIST, CHAIN | Brand-new `PrivateUnauthorized` token A/B destinations — rejected by a separate, unrelated program-level precondition (destination must already exist) | ❌ (confirmed not-expressible — different reason) | +| SwapExactInput | `amm_swap_a_to_b_private_unauthorized_destination_is_not_expressible` | EXIST, CHAIN | Swap paying out to a brand-new `PrivateUnauthorized` destination (`npk` only, no `nsk`) | ❌ (confirmed not-expressible — guest ABI requires both swap legs to be signers, which `PrivateUnauthorized` can never satisfy by construction) | +| SwapExactInput | `amm_swap_a_to_b_private_authorized_init_destination_is_not_expressible` | REGULAR, CHAIN | Swap paying out to a brand-new `PrivateAuthorizedInit` destination (owner self-initializes with its own `nsk`) | ❌ (confirmed not-expressible — same "destination must already exist" precondition as `RemoveLiquidity`) | +| NewDefinition | `amm_new_definition_private_initial_lp_holder` | REGULAR | Pool creation with a private `PrivateAuthorizedInit` initial LP holder | ✅ | +| NewDefinition | `amm_new_definition_private_unauthorized_lp_holder_is_not_expressible` | EXIST, REGULAR | Pool creation with a `PrivateUnauthorized` initial LP holder (`npk` only, no `nsk`) | ❌ (confirmed not-expressible — guest ABI requires `user_holding_lp` to be a signer, which `PrivateUnauthorized` can never satisfy; same shape as the `Swap` `PrivateUnauthorized` finding above) | ### Remarks -- `RemoveLiquidity` and `Swap`s may have issues with `PrivateUnauthorized` and `PrivateAuthorizedInit` that match issues detected in Stablecoin; e.g., explicitly requires `is_authorized = true` and non default accounts. -- `clock` account issue: clock is silent dropped during privacy executions. +- `Swap` and `Remove` rejects any uninitialized destination account; this is a AMM design choice, and not Token program requirement. +- AMM tests were initially blocked by a bug. ## ATA program @@ -156,4 +160,6 @@ ATA program offers limited usage with private accounts. Private accounts can be Privacy coverage for LEZ program tests is greatly improved from the added tests. Though, there are a few noticable gaps: - `PrivateUnauthorized` accounts can be blocked by programs with a check `is_authorized = true`. However, this issue can be avoided by defining `is_authorized = true` for account initialization with `PrivateUnauthorized` (e.g., no knowledge of `npk`). Account initialization cannot be used to maliciously alter a pre-existing account, and thus `is_authorized = true` would not offer any malicious path forward for the third-party initializing the account. - Privacy transactions have issues with chain calls in which multiple calls affect the same private account. This issue can be mitigated by adopting account diff paradigm instead of the current "account state replacement" that we currently use. -- AMM tests are blocked by issues with the clock account; bug in `spel-framework`. \ No newline at end of file + +Additional observation: +AMM's chained-call privacy tests were blocked by the clock account being `DEFAULT_PROGRAM_ID`-owned in the test fixture, which trips a `spel-framework` dispatcher bug (upstream in `logos-co/spel`, confirmed present through v0.6.0) that silently drops any default-owned, non-default, unclaimed account from a program's output. Fixed by giving the fixture's clock account a non-default owner; see the AMM section. The dispatcher bug itself remains open upstream. \ No newline at end of file diff --git a/docs/privacy-test-matrix.md b/docs/privacy-test-matrix.md index 861899d..e4d535f 100644 --- a/docs/privacy-test-matrix.md +++ b/docs/privacy-test-matrix.md @@ -542,13 +542,58 @@ arbitrary timestamp as a private witness and no check anywhere would catch it. W to the LEZ/SPEL maintainers independent of whether/when the AMM test-writing blocker itself gets prioritized. +**Upstream provenance checked (2026-07-15)**: cloned both `logos-co/spel` (upstream) and +`0x-r4bbit/spel` (the pinned fork) to check whether the filter was fork-introduced. It isn't. +`git log -S"is_default_owner"` on upstream `logos-co/spel` finds it added in PR #126, squash-merged +to upstream `main` as commit `1f51875` ("`SpelOutput::execute()` with auto-claim support"), still +present at upstream's current HEAD (`0cb7e09`, v0.6.0). Walking the fork's history back to its +merge point from `logos-co/release/v0.5.0` (commit `73fc462`) shows the filter already present +there too, before any of the fork's own commits. **This is an upstream `logos-co/spel` bug**, +inherited unchanged by the pinned fork — report it against the former, not the latter. + +### ✅ Fixed for these tests (2026-07-15) — test-fixture clock ownership, not a circuit workaround + +The immediate blocker for all five `CHAIN`-dimension AMM privacy tests below was that +`advance_clock` (`programs/integration_tests/tests/amm.rs`) constructed the clock account with +`..Account::default()`, leaving `program_owner == DEFAULT_PROGRAM_ID` and tripping the +`spel-framework` filter above on every AMM/TWAP call that echoes it back. The *real* production +clock account is owned by a dedicated clock program (`lez/system_accounts/src/lib.rs`'s +`clock_account()`: `program_owner: programs::clock().id()`), not `DEFAULT_PROGRAM_ID` — but that +constructor lives behind the gated `system-programs`/`artifacts` feature this test crate can't +reach, so `advance_clock` fabricates the account directly and never set an owner. + +Fix applied: `advance_clock` now sets `program_owner` to a placeholder non-default `ProgramId` +(`[42_u32; 8]`, a stand-in — not the real production clock program ID, which isn't reachable from +this crate). Nothing in AMM/TWAP-oracle logic checks the clock's `program_owner` value (only its +`account_id` against `CLOCK_01_PROGRAM_ACCOUNT_ID`), and `validate_execution`'s other rules +(no-ownership-change, no-unauthorized-data/balance-change) are satisfied trivially since clock is +echoed unchanged — so this is a safe, minimal, test-only fix. All five previously-blocked tests +were rewritten from `_is_not_expressible`/`expect_err` assertions to real success assertions +(rebuilding the `PrivacyPreservingTransaction`, applying it via +`transition_from_privacy_preserving_transaction`, and checking resulting public state + +commitments for the private accounts) and now pass. Full `amm.rs` suite: 40/40 passing. + +The `spel-framework` dispatcher filter itself is untouched and remains an open upstream bug +(confirmed to originate in `logos-co/spel`, not the pinned fork — see above) — it would still +silently drop any other `DEFAULT_PROGRAM_ID`-owned, non-default, unclaimed account threaded +through a chained call. Worth reporting upstream regardless of this fixture-level fix. + ### Existing -6 private tests out of 33 pre-existing public + 6 = 39. No test can yet demonstrate an -actually-working AMM privacy path — five exist purely to confirm the circuit bug also blocks -real private accounts (not just the all-public control case), and one -(`amm_remove_liquidity_private_new_user_holdings_is_not_expressible`) found a second, distinct, -earlier blocker specific to `RemoveLiquidity`. +10 private tests out of 34 pre-existing public + 10 = 44. Six demonstrate an actually-working AMM +privacy path end-to-end (private account touched, transaction applied, resulting public state and +private commitments verified): the five chained-call tests unblocked by the clock test-fixture fix +above, plus `amm_new_definition_private_initial_lp_holder` (pool creation with a fresh +`PrivateAuthorizedInit` LP holder — `new_definition.rs` explicitly permits this, unlike +`swap`/`remove`). Four are confirmed not-expressible, splitting into two distinct root causes: +`amm_remove_liquidity_private_new_user_holdings_is_not_expressible` and +`amm_swap_a_to_b_private_authorized_init_destination_is_not_expressible` both hit the same +"destination must already exist and be Token-Program-owned" precondition; +`amm_swap_a_to_b_private_unauthorized_destination_is_not_expressible` and +`amm_new_definition_private_unauthorized_lp_holder_is_not_expressible` both hit an earlier, more +fundamental guest-ABI signer requirement that a `PrivateUnauthorized` identity can never satisfy — +any account declared a required signer in the guest ABI structurally excludes `PrivateUnauthorized` +recipients, regardless of what the program's own internal precondition would otherwise allow. **Second finding, unrelated to the circuit bug (2026-07-13)**: `remove_liquidity` requires `user_holding_a`/`user_holding_b` to already exist and already be owned by the configured Token @@ -568,25 +613,28 @@ program-level precondition that predates privacy entirely, not a circuit artifac | Instruction | Dimension | Test | Priority | Depends on | Status | |---|---|---|---|---|---| -| SwapExactInput | `CHAIN` | `amm_swap_a_to_b_private_user_holding_is_not_expressible` | P1 | Token, TWAP oracle (public leg) | **Confirmed not-expressible** — private `user_holding_a`, fails identically to the all-public control (8 vs 7 accounts) | -| SwapExactOutput | `CHAIN` | `amm_swap_exact_output_private_user_holding_is_not_expressible` | P1 | Token, TWAP oracle (public leg) | **Confirmed not-expressible** — identical 8-account/chained-call shape to `SwapExactInput`, fails identically (8 vs 7 accounts) | -| AddLiquidity | `CHAIN` | `amm_add_liquidity_private_user_holdings_is_not_expressible` — private deposit legs (`user_holding_a`/`user_holding_b`) | P1 | Token, TWAP oracle (public leg) | **Confirmed not-expressible** — fails identically (10 vs 9 accounts) | -| AddLiquidity | BASE | `amm_add_liquidity_private_lp_holding_is_not_expressible` — private LP output holding | P1 | Token | **Confirmed not-expressible** — private `user_holding_lp`, fails identically (10 vs 9 accounts) | -| RemoveLiquidity | `CHAIN` | `amm_remove_liquidity_private_lp_holding_is_not_expressible` | P1 | Token, TWAP oracle (public leg) | **Confirmed not-expressible** — private `user_holding_lp`, fails identically (10 vs 9 accounts) | -| RemoveLiquidity | `EXIST` (negative) | `amm_remove_liquidity_private_new_user_holdings_is_not_expressible` — brand-new `PrivateUnauthorized` token A/B destinations | P1 | Token | **Confirmed not-expressible for a different reason** — AMM's own precondition requires the destination to already be owned by the Token Program; fails before the circuit bug is even reached | -| Swap / AddLiquidity | `EXIST` | `amm_swap_into_existing_private_holding` | P2 | Token | **Blocked** — see above | -| NewDefinition | BASE | `amm_new_definition_private_initial_lp_holder` | P2 | Token | **Blocked** — see above (also issues chained calls reusing `pool`-derived accounts; check on resolution) | -| Swap / AddLiquidity (vault) | `PDA` | `amm_swap_with_private_vault_pda` — predicted **not-expressible** per the ATA `PDA` finding (same `for_public_pda`-only root cause, confirmed in `amm_core`); write as a quick confirmation citing that finding, not a fresh investigation | P2 | Token | Not started (also behind the blocker above) | -| AddLiquidity / RemoveLiquidity | `GROUP` | `amm_group_owned_lp_holding` | P3 | Token, `key_protocol` | **Blocked** — see above | +| SwapExactInput | `CHAIN` | `amm_swap_a_to_b_private_user_holding` | P1 | Token, TWAP oracle (public leg) | ✅ **Expressible** — private `user_holding_a`, unblocked by the clock test-fixture fix | +| SwapExactOutput | `CHAIN` | `amm_swap_exact_output_private_user_holding` | P1 | Token, TWAP oracle (public leg) | ✅ **Expressible** — same fix | +| AddLiquidity | `CHAIN` | `amm_add_liquidity_private_user_holdings` — private deposit legs (`user_holding_a`/`user_holding_b`) | P1 | Token, TWAP oracle (public leg) | ✅ **Expressible** — same fix | +| AddLiquidity | BASE | `amm_add_liquidity_private_lp_holding` — private LP output holding | P1 | Token | ✅ **Expressible** — private `user_holding_lp`, same fix | +| RemoveLiquidity | `CHAIN` | `amm_remove_liquidity_private_lp_holding` | P1 | Token, TWAP oracle (public leg) | ✅ **Expressible** — private `user_holding_lp`, same fix | +| RemoveLiquidity | `EXIST` (negative) | `amm_remove_liquidity_private_new_user_holdings_is_not_expressible` — brand-new `PrivateUnauthorized` token A/B destinations | P1 | Token | **Confirmed not-expressible for a different reason** — AMM's own precondition requires the destination to already be owned by the Token Program; unrelated to the clock issue | +| Swap | `EXIST` (negative) | `amm_swap_a_to_b_private_unauthorized_destination_is_not_expressible` — brand-new `PrivateUnauthorized` recipient (`npk` only) | P1 | Token | **Confirmed not-expressible** — the guest ABI (`#[account(mut, signer)]` on both `user_holding_a`/`user_holding_b` in `methods/guest/src/bin/amm.rs`) requires every swap participant to be a signer; `PrivateUnauthorized` is authorized by nobody by construction, so it can never satisfy this, independent of the destination-must-exist issue below | +| Swap | `EXIST` (negative) | `amm_swap_a_to_b_private_authorized_init_destination_is_not_expressible` — brand-new `PrivateAuthorizedInit` recipient (self-initializes with its own `nsk`, satisfies the signer requirement) | P1 | Token | **Confirmed not-expressible for a different reason** — same "destination must already exist and be Token-Program-owned" precondition as `RemoveLiquidity`'s finding above (`swap.rs` asserts `user_holding_b.account.program_owner == token_program_id` unconditionally) | +| Swap / AddLiquidity | `EXIST` | `amm_swap_into_existing_private_holding` | P2 | Token | Not started — swap into an *already-existing* private destination (`PrivateAuthorizedUpdate`); distinct from the two fresh-destination rows above, which are both confirmed not-expressible | +| NewDefinition | BASE | `amm_new_definition_private_initial_lp_holder` | P2 | Token | ✅ **Expressible** — `new_definition.rs`'s own precondition on `user_holding_lp` (`account != Account::default() || is_authorized`) explicitly permits a fresh, authorized LP holder; confirmed working with a `PrivateAuthorizedInit` initial LP holder, unblocked by the clock fix above | +| NewDefinition | `EXIST` (negative), BASE | `amm_new_definition_private_unauthorized_lp_holder_is_not_expressible` — `PrivateUnauthorized` initial LP holder (`npk` only) | P2 | Token | **Confirmed not-expressible** — same guest-ABI signer requirement as the `Swap` `PrivateUnauthorized` finding above: `user_holding_lp` is `#[account(mut, signer)]` unconditionally, so it's rejected before `new_definition.rs`'s own "fresh LP holding requires user authorization" precondition is ever reached | +| Swap / AddLiquidity (vault) | `PDA` | `amm_swap_with_private_vault_pda` — predicted **not-expressible** per the ATA `PDA` finding (same `for_public_pda`-only root cause, confirmed in `amm_core`); write as a quick confirmation citing that finding, not a fresh investigation | P2 | Token | Not started — blocked by the separate PDA-formula finding, not the clock issue | +| AddLiquidity / RemoveLiquidity | `GROUP` | `amm_group_owned_lp_holding` | P3 | Token, `key_protocol` | Not started — no longer blocked by the clock issue (fixed above) | | Pool/Config (any) | `PDA` | `amm_attempt_private_pool_pda` — same predicted not-expressible outcome as above; low priority given the vault row already confirms the root cause for this program | P3 | Token | Not started | | Initialize, UpdateConfig, CreatePriceObservations, CreateOraclePriceAccount, SyncReserves | — | **Not planned** — admin/infra instructions over public protocol state; a private admin authority is legitimate but low value | — | — | Out of scope (for now) | Note: every Swap/AddLiquidity/RemoveLiquidity chains to *both* Token (transfers) and TWAP oracle (tick refresh) in one instruction — so every `CHAIN` row above is automatically also a "some legs private, some public" test. Call that out explicitly when the test is written, -not as an incidental detail. **All of these are currently blocked by the circuit-level issue -above, since it fires with zero private accounts involved — no privacy dimension can be tested -on any pool-mutating AMM instruction until it's resolved.** +not as an incidental detail. **The clock-account blocker above is fixed (2026-07-15) — the +remaining unwritten rows are open to pick up, except the `PDA` rows, which are separately +blocked by the `for_public_pda`-only formula finding.** --- diff --git a/programs/integration_tests/tests/amm.rs b/programs/integration_tests/tests/amm.rs index 6fd9c53..b827a37 100644 --- a/programs/integration_tests/tests/amm.rs +++ b/programs/integration_tests/tests/amm.rs @@ -10,11 +10,16 @@ use amm_core::{ MINIMUM_LIQUIDITY, }; use clock_core::{ClockAccountData, CLOCK_01_PROGRAM_ACCOUNT_ID}; -use integration_tests::{private_authorized_update_identity, private_unauthorized_identity}; +use integration_tests::{ + private_authorized_init_identity, private_authorized_update_identity, + private_unauthorized_identity, +}; use nssa::{ error::LeeError, execute_and_prove, - privacy_preserving_transaction::circuit::ProgramWithDependencies, + privacy_preserving_transaction::{ + circuit::ProgramWithDependencies, Message, PrivacyPreservingTransaction, WitnessSet, + }, program::Program, program_deployment_transaction::{self, ProgramDeploymentTransaction}, public_transaction, PrivateKey, PublicKey, PublicTransaction, V03State, @@ -1738,19 +1743,16 @@ fn amm_create_price_observations_without_current_tick_account_fails() { /// Advances the canonical 1-block clock to `timestamp` by writing the clock account directly into /// state. `RecordTick` reads this account (`CLOCK_01_PROGRAM_ACCOUNT_ID`), so the TWAP tests use it /// to simulate the passage of time between observations. -/// -/// rc6 moved the clock program out of `nssa` into the separate system-programs crate (gated behind -/// the guest-building `artifacts` feature), so the clock can no longer be ticked by submitting a -/// real clock transaction here. Instead we set the account state directly via -/// `force_insert_account`, matching how the upstream rc6 state-machine tests seed accounts. #[cfg(test)] fn advance_clock(state: &mut V03State, timestamp: u64) { + let clock_id: nssa_core::program::ProgramId = [42_u32; 8]; let data = ClockAccountData { block_id: 0, timestamp, } .to_bytes(); let clock_account = Account { + program_owner: clock_id, data: Data::try_from(data).expect("clock account data fits"), ..Account::default() }; @@ -3124,7 +3126,7 @@ fn amm_with_deps() -> ProgramWithDependencies { } #[test] -fn amm_swap_a_to_b_private_user_holding_is_not_expressible() { +fn amm_swap_a_to_b_private_user_holding() { let mut state = state_for_amm_tests(); let user_a_nsk = PrivateKeys::user_a_nsk(); @@ -3185,7 +3187,7 @@ fn amm_swap_a_to_b_private_user_holding_is_not_expressible() { deadline: u64::MAX, }; - let result = execute_and_prove( + let (output, proof) = execute_and_prove( vec![ config_pre, pool_pre, @@ -3208,22 +3210,239 @@ fn amm_swap_a_to_b_private_user_holding_is_not_expressible() { InputAccountIdentity::Public, ], &amm_with_deps(), + ) + .expect("SwapExactInput with a private user holding must succeed now that the clock account is properly owned"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::user_b(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![current_nonce(&state, Ids::user_b())], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[&Keys::user_b()]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + assert_eq!( + state.get_account_by_id(Ids::pool_definition()), + Accounts::pool_definition_swap_2() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_a()), + Accounts::vault_a_swap_2() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_b()), + Accounts::vault_b_swap_2() + ); + assert_eq!( + state.get_account_by_id(Ids::user_b()), + Accounts::user_b_holding_swap_2() + ); + + let user_a_nonce_after = + Nonce::private_account_nonce_init(&user_a_id).private_account_nonce_increment(&user_a_nsk); + let new_user_a_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_a_definition(), + balance: Balances::user_a_swap_2(), + }), + nonce: user_a_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_a_id, &new_user_a_account)) + .is_some()); +} + +/// Swap that pays out to `PrivateUnauthorized` is prohibited: both swap legs must be signers, +/// which `PrivateUnauthorized` (no `nsk`) can never be. +#[test] +fn amm_swap_a_to_b_private_unauthorized_destination_is_not_expressible() { + let state = state_for_amm_tests(); + + let user_b_npk = PrivateKeys::user_b_npk(); + let user_b_vpk = PrivateKeys::user_b_vpk(); + let user_b_id = PrivateKeys::user_b_id(); + + let config_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::config()), false, Ids::config()); + let pool_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::pool_definition()), + false, + Ids::pool_definition(), + ); + let vault_a_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_a()), + false, + Ids::vault_a(), + ); + let vault_b_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_b()), + false, + Ids::vault_b(), + ); + let user_a_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_a()), true, Ids::user_a()); + let user_b_pre = AccountWithMetadata::new(Account::default(), false, user_b_id); + let current_tick_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::current_tick_account()), + false, + Ids::current_tick_account(), + ); + let clock_pre = AccountWithMetadata::new( + state.get_account_by_id(CLOCK_01_PROGRAM_ACCOUNT_ID), + false, + CLOCK_01_PROGRAM_ACCOUNT_ID, + ); + + let instruction = amm_core::Instruction::SwapExactInput { + swap_amount_in: Balances::swap_amount_in(), + min_amount_out: Balances::swap_min_out(), + token_definition_id_in: Ids::token_a_definition(), + deadline: u64::MAX, + }; + + let result = execute_and_prove( + vec![ + config_pre, + pool_pre, + vault_a_pre, + vault_b_pre, + user_a_pre, + user_b_pre, + current_tick_pre, + clock_pre, + ], + Program::serialize_instruction(instruction).unwrap(), + vec![ + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + private_unauthorized_identity(user_b_npk, &user_b_vpk, 0), + InputAccountIdentity::Public, + InputAccountIdentity::Public, + ], + &amm_with_deps(), ); let err = result.expect_err( - "SwapExactInput must be rejected by the privacy-preserving circuit: the same \ - 'Invalid account_identities length' bug confirmed with an all-public control case \ - also fires with a real private user holding", + "SwapExactInput must be rejected: user_holding_b must be a signer per the guest ABI, \ + which a PrivateUnauthorized identity (no nsk, unauthorized by construction) can never \ + satisfy", ); let message = format!("{err:?}"); assert!( - message.contains("Invalid account_identities length"), - "expected the known circuit-level length-mismatch bug, got a different error: {message}" + message.contains("must be a signer"), + "expected the guest ABI's signer requirement on user_holding_b, got a different \ + error: {message}" + ); +} + +/// Swap that pays out to `PrivateAuthorizedInit` is prohibited. Payment +/// is only permitted to already initialized accounts. +#[test] +fn amm_swap_a_to_b_private_authorized_init_destination_is_not_expressible() { + let state = state_for_amm_tests(); + + let user_b_nsk = PrivateKeys::user_b_nsk(); + let user_b_vpk = PrivateKeys::user_b_vpk(); + let user_b_id = PrivateKeys::user_b_id(); + + let config_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::config()), false, Ids::config()); + let pool_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::pool_definition()), + false, + Ids::pool_definition(), + ); + let vault_a_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_a()), + false, + Ids::vault_a(), + ); + let vault_b_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_b()), + false, + Ids::vault_b(), + ); + let user_a_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_a()), true, Ids::user_a()); + let user_b_pre = AccountWithMetadata::new(Account::default(), true, user_b_id); + let current_tick_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::current_tick_account()), + false, + Ids::current_tick_account(), + ); + let clock_pre = AccountWithMetadata::new( + state.get_account_by_id(CLOCK_01_PROGRAM_ACCOUNT_ID), + false, + CLOCK_01_PROGRAM_ACCOUNT_ID, + ); + + let instruction = amm_core::Instruction::SwapExactInput { + swap_amount_in: Balances::swap_amount_in(), + min_amount_out: Balances::swap_min_out(), + token_definition_id_in: Ids::token_a_definition(), + deadline: u64::MAX, + }; + + let result = execute_and_prove( + vec![ + config_pre, + pool_pre, + vault_a_pre, + vault_b_pre, + user_a_pre, + user_b_pre, + current_tick_pre, + clock_pre, + ], + Program::serialize_instruction(instruction).unwrap(), + vec![ + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + private_authorized_init_identity(user_b_nsk, &user_b_vpk, 0), + InputAccountIdentity::Public, + InputAccountIdentity::Public, + ], + &amm_with_deps(), + ); + + let err = result.expect_err( + "SwapExactInput must be rejected by the AMM program itself: user_holding_b must already \ + be initialized and owned by the configured Token Program before any chained call or the \ + privacy-preserving circuit is ever reached", + ); + let message = format!("{err:?}"); + assert!( + message.contains("User Token B holding must be owned by the configured Token Program"), + "expected the AMM program's own initialized-destination precondition, got a different \ + error: {message}" ); } #[test] -fn amm_swap_exact_output_private_user_holding_is_not_expressible() { +fn amm_swap_exact_output_private_user_holding() { let mut state = state_for_amm_tests(); let user_a_nsk = PrivateKeys::user_a_nsk(); @@ -3284,7 +3503,7 @@ fn amm_swap_exact_output_private_user_holding_is_not_expressible() { deadline: u64::MAX, }; - let result = execute_and_prove( + let (output, proof) = execute_and_prove( vec![ config_pre, pool_pre, @@ -3307,32 +3526,105 @@ fn amm_swap_exact_output_private_user_holding_is_not_expressible() { InputAccountIdentity::Public, ], &amm_with_deps(), + ) + .expect("SwapExactOutput with a private user holding must succeed now that the clock account is properly owned"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::user_b(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![current_nonce(&state, Ids::user_b())], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[&Keys::user_b()]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + // exact_amount_out = 200 (token B), max_amount_in = 1_000 (token A cap), 30 bps fee, against + // the fresh pool's initial reserves (5_000 A / 2_500 B): + // effective_in_min = ceil(5_000 * 200 / (2_500 - 200)) = ceil(1_000_000 / 2_300) = 435 + // deposit_amount = ceil(435 * 10_000 / (10_000 - 30)) = ceil(4_350_000 / 9_970) = 437 + let deposit_amount = 437_u128; + let withdraw_amount = Balances::swap_min_out(); + + let pool = pool_definition(&state.get_account_by_id(Ids::pool_definition())); + assert_eq!(pool.reserve_a, Balances::vault_a_init() + deposit_amount); + assert_eq!(pool.reserve_b, Balances::vault_b_init() - withdraw_amount); + match TokenHolding::try_from(&state.get_account_by_id(Ids::vault_a()).data) + .expect("valid holding") + { + TokenHolding::Fungible { balance, .. } => { + assert_eq!(balance, Balances::vault_a_init() + deposit_amount); + } + TokenHolding::NftMaster { .. } | TokenHolding::NftPrintedCopy { .. } => { + panic!("expected Fungible vault holding") + } + } + match TokenHolding::try_from(&state.get_account_by_id(Ids::vault_b()).data) + .expect("valid holding") + { + TokenHolding::Fungible { balance, .. } => { + assert_eq!(balance, Balances::vault_b_init() - withdraw_amount); + } + TokenHolding::NftMaster { .. } | TokenHolding::NftPrintedCopy { .. } => { + panic!("expected Fungible vault holding") + } + } + assert_eq!( + state.get_account_by_id(Ids::user_b()), + Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_b_definition(), + balance: Balances::user_b_init() + withdraw_amount, + }), + nonce: Nonce(1), + } ); - let err = result.expect_err( - "SwapExactOutput must be rejected by the privacy-preserving circuit: the same \ - 'Invalid account_identities length' bug also fires with a real private user holding", - ); - let message = format!("{err:?}"); - assert!( - message.contains("Invalid account_identities length"), - "expected the known circuit-level length-mismatch bug, got a different error: {message}" - ); + let user_a_nonce_after = + Nonce::private_account_nonce_init(&user_a_id).private_account_nonce_increment(&user_a_nsk); + let new_user_a_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_a_definition(), + balance: Balances::user_a_init() - deposit_amount, + }), + nonce: user_a_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_a_id, &new_user_a_account)) + .is_some()); } #[test] -fn amm_add_liquidity_private_lp_holding_is_not_expressible() { +fn amm_add_liquidity_private_lp_holding() { let mut state = state_for_amm_tests(); let user_lp_nsk = PrivateKeys::user_lp_nsk(); let user_lp_vpk = PrivateKeys::user_lp_vpk(); let user_lp_id = PrivateKeys::user_lp_id(); + let user_lp_initial_balance = 500_u128; let user_lp_account = Account { program_owner: Ids::token_program(), balance: 0, data: Data::from(&TokenHolding::Fungible { definition_id: Ids::token_lp_definition(), - balance: 500, + balance: user_lp_initial_balance, }), nonce: Nonce::private_account_nonce_init(&user_lp_id), }; @@ -3389,7 +3681,7 @@ fn amm_add_liquidity_private_lp_holding_is_not_expressible() { deadline: u64::MAX, }; - let result = execute_and_prove( + let (output, proof) = execute_and_prove( vec![ config_pre, pool_pre, @@ -3416,21 +3708,82 @@ fn amm_add_liquidity_private_lp_holding_is_not_expressible() { InputAccountIdentity::Public, ], &amm_with_deps(), + ) + .expect("AddLiquidity with a private LP holding must succeed now that the clock account is properly owned"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::token_lp_definition(), + Ids::user_a(), + Ids::user_b(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![ + current_nonce(&state, Ids::user_a()), + current_nonce(&state, Ids::user_b()), + ], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[&Keys::user_a(), &Keys::user_b()]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + assert_eq!( + state.get_account_by_id(Ids::pool_definition()), + Accounts::pool_definition_add() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_a()), + Accounts::vault_a_add() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_b()), + Accounts::vault_b_add() + ); + assert_eq!( + state.get_account_by_id(Ids::token_lp_definition()), + Accounts::token_lp_definition_add() + ); + assert_eq!( + state.get_account_by_id(Ids::user_a()), + Accounts::user_a_holding_add() + ); + assert_eq!( + state.get_account_by_id(Ids::user_b()), + Accounts::user_b_holding_add() ); - let err = result.expect_err( - "AddLiquidity must be rejected by the privacy-preserving circuit: the same \ - 'Invalid account_identities length' bug also fires with a real private LP holding", - ); - let message = format!("{err:?}"); - assert!( - message.contains("Invalid account_identities length"), - "expected the known circuit-level length-mismatch bug, got a different error: {message}" - ); + // Minted LP = post-add total supply - pre-add total supply, independent of who holds it. + let minted_lp = Balances::token_lp_supply_add() - Balances::pool_lp_supply_init(); + let user_lp_nonce_after = Nonce::private_account_nonce_init(&user_lp_id) + .private_account_nonce_increment(&user_lp_nsk); + let new_user_lp_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_lp_definition(), + balance: user_lp_initial_balance + minted_lp, + }), + nonce: user_lp_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_lp_id, &new_user_lp_account)) + .is_some()); } #[test] -fn amm_remove_liquidity_private_lp_holding_is_not_expressible() { +fn amm_remove_liquidity_private_lp_holding() { let mut state = state_for_amm_tests(); let user_lp_nsk = PrivateKeys::user_lp_nsk(); @@ -3498,7 +3851,7 @@ fn amm_remove_liquidity_private_lp_holding_is_not_expressible() { deadline: u64::MAX, }; - let result = execute_and_prove( + let (output, proof) = execute_and_prove( vec![ config_pre, pool_pre, @@ -3525,17 +3878,74 @@ fn amm_remove_liquidity_private_lp_holding_is_not_expressible() { InputAccountIdentity::Public, ], &amm_with_deps(), + ) + .expect("RemoveLiquidity with a private LP holding must succeed now that the clock account is properly owned"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::token_lp_definition(), + Ids::user_a(), + Ids::user_b(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + assert_eq!( + state.get_account_by_id(Ids::pool_definition()), + Accounts::pool_definition_remove() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_a()), + Accounts::vault_a_remove() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_b()), + Accounts::vault_b_remove() + ); + assert_eq!( + state.get_account_by_id(Ids::token_lp_definition()), + Accounts::token_lp_definition_remove() + ); + assert_eq!( + state.get_account_by_id(Ids::user_a()), + Accounts::user_a_holding_remove() + ); + assert_eq!( + state.get_account_by_id(Ids::user_b()), + Accounts::user_b_holding_remove() ); - let err = result.expect_err( - "RemoveLiquidity must be rejected by the privacy-preserving circuit: the same \ - 'Invalid account_identities length' bug also fires with a real private LP holding", - ); - let message = format!("{err:?}"); - assert!( - message.contains("Invalid account_identities length"), - "expected the known circuit-level length-mismatch bug, got a different error: {message}" - ); + // user_lp burned its entire private balance (remove_liquidity_amount == its full holding). + let user_lp_nonce_after = Nonce::private_account_nonce_init(&user_lp_id) + .private_account_nonce_increment(&user_lp_nsk); + let new_user_lp_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_lp_definition(), + balance: 0, + }), + nonce: user_lp_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_lp_id, &new_user_lp_account)) + .is_some()); } #[test] @@ -3639,7 +4049,7 @@ fn amm_remove_liquidity_private_new_user_holdings_is_not_expressible() { } #[test] -fn amm_add_liquidity_private_user_holdings_is_not_expressible() { +fn amm_add_liquidity_private_user_holdings() { let mut state = state_for_amm_tests(); let user_a_nsk = PrivateKeys::user_a_nsk(); @@ -3732,7 +4142,7 @@ fn amm_add_liquidity_private_user_holdings_is_not_expressible() { deadline: u64::MAX, }; - let result = execute_and_prove( + let (output, proof) = execute_and_prove( vec![ config_pre, pool_pre, @@ -3759,15 +4169,366 @@ fn amm_add_liquidity_private_user_holdings_is_not_expressible() { InputAccountIdentity::Public, ], &amm_with_deps(), + ) + .expect("AddLiquidity with private deposit holdings must succeed now that the clock account is properly owned"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::token_lp_definition(), + Ids::user_lp(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + assert_eq!( + state.get_account_by_id(Ids::pool_definition()), + Accounts::pool_definition_add() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_a()), + Accounts::vault_a_add() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_b()), + Accounts::vault_b_add() + ); + assert_eq!( + state.get_account_by_id(Ids::token_lp_definition()), + Accounts::token_lp_definition_add() + ); + // user_lp is public and already seeded by state_for_amm_tests() (Accounts::user_lp_holding(), + // balance Balances::user_lp_init()); it just receives the same minted LP as the all-public + // amm_add_liquidity test, landing on the same post-state. + assert_eq!( + state.get_account_by_id(Ids::user_lp()), + Accounts::user_lp_holding_add() + ); + + let user_a_nonce_after = + Nonce::private_account_nonce_init(&user_a_id).private_account_nonce_increment(&user_a_nsk); + let new_user_a_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_a_definition(), + balance: Balances::user_a_init() - Balances::add_max_a(), + }), + nonce: user_a_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_a_id, &new_user_a_account)) + .is_some()); + + let user_b_nonce_after = + Nonce::private_account_nonce_init(&user_b_id).private_account_nonce_increment(&user_b_nsk); + let new_user_b_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_b_definition(), + balance: Balances::user_b_init() - Balances::add_max_b(), + }), + nonce: user_b_nonce_after, + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_b_id, &new_user_b_account)) + .is_some()); +} + +/// Initialized pool mints LP tokens to `PrivateAuthorizedInit`. +#[test] +fn amm_new_definition_private_initial_lp_holder() { + let mut state = state_for_amm_tests_with_new_def(); + state.force_insert_account(Ids::vault_a(), Accounts::vault_a_reinitializable()); + state.force_insert_account(Ids::vault_b(), Accounts::vault_b_reinitializable()); + + let user_lp_nsk = PrivateKeys::user_lp_nsk(); + let user_lp_vpk = PrivateKeys::user_lp_vpk(); + let user_lp_id = PrivateKeys::user_lp_id(); + + let config_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::config()), false, Ids::config()); + let pool_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::pool_definition()), + false, + Ids::pool_definition(), + ); + let vault_a_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_a()), + false, + Ids::vault_a(), + ); + let vault_b_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_b()), + false, + Ids::vault_b(), + ); + let token_lp_definition_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::token_lp_definition()), + false, + Ids::token_lp_definition(), + ); + let lp_lock_holding_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::lp_lock_holding()), + false, + Ids::lp_lock_holding(), + ); + let user_a_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_a()), true, Ids::user_a()); + let user_b_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_b()), true, Ids::user_b()); + let user_lp_pre = AccountWithMetadata::new(Account::default(), true, user_lp_id); + let current_tick_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::current_tick_account()), + false, + Ids::current_tick_account(), + ); + let clock_pre = AccountWithMetadata::new( + state.get_account_by_id(CLOCK_01_PROGRAM_ACCOUNT_ID), + false, + CLOCK_01_PROGRAM_ACCOUNT_ID, + ); + + let instruction = amm_core::Instruction::NewDefinition { + token_a_amount: Balances::vault_a_init(), + token_b_amount: Balances::vault_b_init(), + fees: Balances::fee_tier(), + deadline: u64::MAX, + }; + + let (output, proof) = execute_and_prove( + vec![ + config_pre, + pool_pre, + vault_a_pre, + vault_b_pre, + token_lp_definition_pre, + lp_lock_holding_pre, + user_a_pre, + user_b_pre, + user_lp_pre, + current_tick_pre, + clock_pre, + ], + Program::serialize_instruction(instruction).unwrap(), + vec![ + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + private_authorized_init_identity(user_lp_nsk, &user_lp_vpk, 0), + InputAccountIdentity::Public, + InputAccountIdentity::Public, + ], + &amm_with_deps(), + ) + .expect("NewDefinition with a private initial LP holder must succeed: the program's own precondition allows a fresh, authorized user_holding_lp"); + + let message = Message::try_from_circuit_output( + vec![ + Ids::config(), + Ids::pool_definition(), + Ids::vault_a(), + Ids::vault_b(), + Ids::token_lp_definition(), + Ids::lp_lock_holding(), + Ids::user_a(), + Ids::user_b(), + Ids::current_tick_account(), + CLOCK_01_PROGRAM_ACCOUNT_ID, + ], + vec![ + current_nonce(&state, Ids::user_a()), + current_nonce(&state, Ids::user_b()), + ], + output, + ) + .unwrap(); + let witness_set = WitnessSet::for_message(&message, proof, &[&Keys::user_a(), &Keys::user_b()]); + state + .transition_from_privacy_preserving_transaction( + &PrivacyPreservingTransaction::new(message, witness_set), + 0, + 0, + ) + .unwrap(); + + assert_eq!( + state.get_account_by_id(Ids::pool_definition()), + Accounts::pool_definition_new_init() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_a()), + Accounts::vault_a_init() + ); + assert_eq!( + state.get_account_by_id(Ids::vault_b()), + Accounts::vault_b_init() + ); + assert_eq!( + state.get_account_by_id(Ids::token_lp_definition()), + Accounts::token_lp_definition_new_init() + ); + assert_eq!( + state.get_account_by_id(Ids::lp_lock_holding()), + Accounts::lp_lock_holding_new_init() + ); + assert_eq!( + state.get_account_by_id(Ids::user_a()), + Accounts::user_a_holding_new_init() + ); + assert_eq!( + state.get_account_by_id(Ids::user_b()), + Accounts::user_b_holding_new_init() + ); + + let current_tick = state.get_account_by_id(Ids::current_tick_account()); + assert_eq!(current_tick.program_owner, Ids::twap_oracle_program()); + let tick_account = twap_oracle_core::CurrentTickAccount::try_from(¤t_tick.data) + .expect("current tick account must hold a valid CurrentTickAccount"); + let expected_tick = twap_oracle_core::price_to_tick(amm_core::spot_price_q64_64( + Balances::vault_a_init(), + Balances::vault_b_init(), + )); + assert_eq!(tick_account.tick, expected_tick); + + // A fresh PrivateAuthorizedInit account's nonce starts at private_account_nonce_init, not + // incremented — this is its first-ever commitment, not an update to an existing one. + let new_user_lp_account = Account { + program_owner: Ids::token_program(), + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: Ids::token_lp_definition(), + balance: Balances::lp_user_init(), + }), + nonce: Nonce::private_account_nonce_init(&user_lp_id), + }; + assert!(state + .get_proof_for_commitment(&Commitment::new(&user_lp_id, &new_user_lp_account)) + .is_some()); +} + +/// Initialized pool cannot mint LP tokens to `PrivateUnauthorized`. +#[test] +fn amm_new_definition_private_unauthorized_lp_holder_is_not_expressible() { + let mut state = state_for_amm_tests_with_new_def(); + state.force_insert_account(Ids::vault_a(), Accounts::vault_a_reinitializable()); + state.force_insert_account(Ids::vault_b(), Accounts::vault_b_reinitializable()); + + let user_lp_npk = PrivateKeys::user_lp_npk(); + let user_lp_vpk = PrivateKeys::user_lp_vpk(); + let user_lp_id = PrivateKeys::user_lp_id(); + + let config_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::config()), false, Ids::config()); + let pool_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::pool_definition()), + false, + Ids::pool_definition(), + ); + let vault_a_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_a()), + false, + Ids::vault_a(), + ); + let vault_b_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::vault_b()), + false, + Ids::vault_b(), + ); + let token_lp_definition_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::token_lp_definition()), + false, + Ids::token_lp_definition(), + ); + let lp_lock_holding_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::lp_lock_holding()), + false, + Ids::lp_lock_holding(), + ); + let user_a_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_a()), true, Ids::user_a()); + let user_b_pre = + AccountWithMetadata::new(state.get_account_by_id(Ids::user_b()), true, Ids::user_b()); + let user_lp_pre = AccountWithMetadata::new(Account::default(), false, user_lp_id); + let current_tick_pre = AccountWithMetadata::new( + state.get_account_by_id(Ids::current_tick_account()), + false, + Ids::current_tick_account(), + ); + let clock_pre = AccountWithMetadata::new( + state.get_account_by_id(CLOCK_01_PROGRAM_ACCOUNT_ID), + false, + CLOCK_01_PROGRAM_ACCOUNT_ID, + ); + + let instruction = amm_core::Instruction::NewDefinition { + token_a_amount: Balances::vault_a_init(), + token_b_amount: Balances::vault_b_init(), + fees: Balances::fee_tier(), + deadline: u64::MAX, + }; + + let result = execute_and_prove( + vec![ + config_pre, + pool_pre, + vault_a_pre, + vault_b_pre, + token_lp_definition_pre, + lp_lock_holding_pre, + user_a_pre, + user_b_pre, + user_lp_pre, + current_tick_pre, + clock_pre, + ], + Program::serialize_instruction(instruction).unwrap(), + vec![ + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + InputAccountIdentity::Public, + private_unauthorized_identity(user_lp_npk, &user_lp_vpk, 0), + InputAccountIdentity::Public, + InputAccountIdentity::Public, + ], + &amm_with_deps(), ); let err = result.expect_err( - "AddLiquidity must be rejected by the privacy-preserving circuit: the same \ - 'Invalid account_identities length' bug also fires with real private deposit holdings", + "NewDefinition must be rejected: user_holding_lp must be a signer per the guest ABI, \ + which a PrivateUnauthorized identity (no nsk, unauthorized by construction) can never \ + satisfy", ); let message = format!("{err:?}"); assert!( - message.contains("Invalid account_identities length"), - "expected the known circuit-level length-mismatch bug, got a different error: {message}" + message.contains("must be a signer"), + "expected the guest ABI's signer requirement on user_holding_lp, got a different \ + error: {message}" ); }