fix(security): require the canonical ledger for backing withdrawals

[Bayyan16]

Summary

Fixes #433 by making the backing-domain ledger a mandatory, canonical account for WithdrawBackingBucket (tag 50).

The withdrawal path now fails closed unless account index 6 is the writable, program-owned PDA derived from:

["lp_backing_ledger", market, domain_le]

Once validated, the handler always synchronizes the ledger, enforces the recorded-principal ceiling, decrements total_principal_atoms, increments total_principal_withdrawn_atoms, and writes the ledger back after the engine mutation.

Root cause

handle_withdraw_backing_bucket previously loaded the trailing ledger with accounts.get(6). Every ledger-dependent safety property was then conditional on that Option:

• the amount <= ledger.total_principal_atoms consistency check;
• the principal decrement;
• the cumulative withdrawn-principal increment; and
• the ledger write-back.

An authorized backing-bucket authority could therefore omit the account, withdraw real backing from the engine bucket and token vault, and leave the wrapper ledger unchanged. Because LP-vault NAV and redemption accounting consume that ledger, the result was persistent phantom principal and a permanently inconsistent backing domain.

Merely requiring an arbitrary program-owned ledger would not fully close the invariant: a caller could substitute a fresh zeroed account and leave the ledger used by LP-vault accounting untouched. This PR therefore applies both hardening requirements from the issue: the account is mandatory and its address is pinned to the existing per-market/per-domain PDA derivation used by the LP-vault paths.

Implementation

Production handler

• Replaces accounts.get(6) with account(accounts, 6)?.
• Requires the ledger to be writable and owned by the Percolator program.
• Derives the expected backing ledger PDA from (program_id, market, domain) and rejects any other key.
• Removes the optional ledger branches so synchronization, principal validation, counter updates, and write-back are unconditional.
• Preserves the instruction tag, encoded payload, and existing account order; only the trailing account contract is tightened.
• Performs all account identity checks before borrowing or mutating market state.

Regression coverage

• Adds a compiled-BPF/LiteSVM regression proving that:
  • omitting account 6 is rejected;
  • substituting another correctly-sized, program-owned ledger is rejected;
  • both rejected transactions leave market bytes, canonical ledger bytes, vault balance, and destination balance unchanged; and
  • the canonical path succeeds and updates principal from 100 to 60, withdrawn principal from 0 to 40, and token balances consistently.
• Extends the native wrapper test to cover omission, substitution, atomic rejection, and successful counter synchronization.
• Updates CU and differential fixtures to use the canonical ledger PDA and to carry the same ledger through top-up and withdrawal operations.

Security properties after this change

For every successful tag-50 withdrawal:

1. exactly one ledger identity is accepted for a (market, domain) pair;
2. the ledger is writable and program-owned;
3. the withdrawal cannot exceed recorded principal;
4. the engine bucket and ledger principal decrease in the same instruction;
5. cumulative withdrawn principal increases by the same amount; and
6. Solana transaction atomicity rolls back all state and token changes if any subsequent operation fails.

Compatibility and migration notes

This is an intentional fail-closed account-contract change. Existing clients must append the canonical backing-domain ledger at account index 6 when constructing tag 50.

A domain funded historically without its canonical ledger cannot use tag 50 to have a fresh ledger silently adopt the bucket balance: a new ledger starts with zero recorded principal, so the principal ceiling rejects the withdrawal. This is the safe behavior because automatically adopting the engine bucket could legitimize an already-divergent ledger. If deployed state already contains a divergence, it should be handled through a separately reviewed reconciliation/migration path rather than bypassing the invariant in this withdrawal handler.

TopUpBackingBucket remains unchanged because #433 identifies its no-ledger mode as secondary hardening: omission on a deposit does not itself remove backing. Expanding that instruction's account contract is intentionally kept out of this security fix.

Validation

Validation was performed from a clean sibling layout against the latest percolator engine and the repository's deployed program pins:

• matcher: d4d4f1c40efb37cbcf44e42b99a46d49c32c4dca
• NFT: f18da240677c390adb74285d03b2bbdc9e1dfef1
• stake: 474079f968d9f8aeb3ba85e611ef2e1eaf4353e4

Commands and results:

• cargo build-sbf --no-default-features --tools-version v1.54
  • PASS; fresh percolator_prog.so produced with platform-tools v1.54.
• cargo test --test v16_cu v16_bpf_withdraw_backing_bucket_requires_canonical_ledger --no-default-features -- --nocapture
  • PASS: 1 passed, 0 failed.
• cargo test --test v16_cu --no-default-features --no-fail-fast
  • 99 passed, 1 failed.
  • The sole failure is the pre-existing and allowlisted v16_bpf_batch_trade_cpi_tail_fanout_budget_rejects_oversized_product CU finding documented in tests/KNOWN_FAILING.txt.
• cargo test --no-fail-fast
  • 570 passed, 35 failed across all test binaries.
  • The unique failing set matches all 35 entries in tests/KNOWN_FAILING.txt exactly: no new failures and no stale allowlist entries.
• cargo clippy --all-targets --no-default-features
  • PASS (exit 0); existing repository warnings remain unchanged.
• git diff --check origin/main...HEAD
  • PASS.

Kani is not installed in the local validation environment, so cargo kani --tests was not executed locally. The change does not modify engine arithmetic or Kani harnesses; the new host and compiled-BPF regressions directly exercise the affected wrapper account contract and state transition.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4172e50e-4b2a-4d0c-ac5d-dfa9cb24ff75

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6cdfb19e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Create the canonical ledger before requiring it for withdrawals

For any ordinary backing domain that has not used the LP-vault deposit path, this makes every principal withdrawal impossible: handle_top_up_backing_bucket still accepts deposits without a ledger at src/v16_program.rs:10301-10318, and the only repository path that creates this PDA is the LP-vault-specific deposit flow at src/v16_program.rs:14726-14762. A caller cannot independently create a program-owned PDA, while a zeroed ledger would record zero principal and fail the new ceiling check. Thus both historical and newly accepted ledgerless top-ups become stranded; the program needs a generic canonical-ledger initialization/migration path, or must create and require the ledger when accepting every top-up.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Migrate ledger ownership when rotating backing authority

After an initialized domain's backing authority is legitimately rotated through UpdateAssetAuthority, this passes the new authority into read_or_new_backing_domain_ledger, but that function rejects whenever the ledger's stored authority is the previous key (src/v16_program.rs:10103-10109). No rotation path updates the canonical ledger, so the new authorized holder cannot withdraw or perform a ledger-backed top-up; if the old key was retired, the principal is permanently stranded. The authority rotation must migrate the canonical ledger atomically or ledger validation must accommodate the authorized rotation.

Useful? React with 👍 / 👎.