fix(tranche): price senior deposits against the senior sub-pool by 0x-SquidSol · Pull Request #135 · dcccrypto/percolator-stake

0x-SquidSol · 2026-06-17T15:16:38Z

Summary

Fixes the senior LP deposit/withdraw pricing asymmetry reported in #134.

With tranches enabled, process_deposit minted senior LP at the global pool price, while senior withdrawals redeem at the senior sub-pool price. After the junior tranche absorbs an insurance loss (total_flushed > total_returned), the global per-LP price drops below the senior per-LP price, so an unprivileged user could mint senior LP cheap (global) and redeem dear (senior), extracting value from existing senior LP holders and the junior first-loss buffer. The junior deposit path was already sub-pool-priced; the senior deposit path was the missed twin.

Change

math::calc_senior_lp_for_deposit(senior_total_lp, senior_balance, amount) — senior-side mirror of calc_junior_lp_for_deposit (delegates to calc_lp_for_deposit, inheriting round-DOWN/pool-favoring semantics and the orphaned-value guards).
process_deposit branches on tranche_enabled(): senior deposits now price against the senior sub-pool (senior_total_lp() / senior_balance()) — the same basis calc_senior_collateral_for_withdraw redeems against — while non-tranche pools keep global pricing. A senior_total_lp() == 0 first-depositor 1:1 bootstrap prevents bricking the senior tranche when junior deposited first and a fee/loss left orphaned senior value with zero senior LP.

All four tranche legs now price against their own sub-pool (or global when tranches are off): senior deposit ↔ senior withdraw and junior deposit ↔ junior withdraw — closing the asymmetry in both the loss direction (theft) and the fee direction (overpay).

Safety / blast radius

No state-layout change, no instruction/ABI change, no new error variants. Senior accounting stays fully derived (senior_total_lp = total_lp_supply − junior_total_lp; senior_balance = total_pool_value − effective_junior_balance); the deposit only changes how many LP are minted and still bumps only the global counters — no senior setters needed.
Both legs round DOWN (pool-favoring), so a senior deposit→withdraw round-trip cannot profit.
The 1:1 bootstrap triggers only when no senior LP exists, so it cannot dilute any existing senior holder.

Tests

tests/poc_senior_deposit_mispricing.rs: global_pricing_was_exploitable (documents the original bug, ≈ +25%), senior_subpool_pricing_prevents_extraction (fix → 0 profit, incumbent senior left whole), bootstrap_first_senior_deposit_does_not_brick.
src/math.rs: unit tests for calc_senior_lp_for_deposit (first-deposit 1:1, pro-rata, orphaned-value reject, round-trip-no-profit-after-loss).
cargo build-sbf (deployment target) green; fix reproduced and then neutralized against the compiled lib.

Refs #134

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

Bug Fixes
- Corrected senior-tranche deposit minting to use senior sub-pool pricing when tranches are enabled, preventing deposit/withdrawal mispricing.
- Improved handling of senior-edge cases, including first-senior bootstrapping and protection against orphaned senior value (zero senior supply with nonzero senior balance).
Tests
- Added regression coverage for exploitable mispricing, non-extractable pricing behavior, pro-rata/non-1:1 ratios, and orphaned/first-deposit scenarios.
- Added round-trip checks to ensure senior deposits/withdrawals remain non-profitable after junior-absorbed losses.

dcccrypto#130) effective_junior_balance is used in every junior deposit (process_deposit_junior) for LP pricing and every junior withdrawal (process_withdraw) for collateral valuation. It composes the already-Kani-proven distribute_loss with a derivation of gross_senior from raw accounting fields. The wrapper composition itself — the net_loss == 0 short-circuit, the gross_senior derivation, and the final saturating_sub — had no formal verification, despite the historical BUG-6 fix where losses were previously double-applied on the junior side. Added 4 Kani proofs to tests/kani.rs: 1. proof_effective_junior_balance_no_panic Panic-freedom across arbitrary accounting states, including pathological cases (withdrawn > deposited, flushed > returned). 2. proof_effective_junior_balance_bounded_by_raw effective_junior_balance() <= junior_balance() for all inputs. Loss adjustment can only decrease, never inflate. Includes kani::cover! guard proving the loss-application branch is reachable beyond the trivial net_loss == 0 short-circuit. 3. proof_effective_junior_balance_no_loss_identity When total_flushed == total_returned (no outstanding loss), effective == junior_balance exactly. Pins the short-circuit against regression that would silently apply non-zero loss on lossless pools. 4. proof_effective_junior_tranche_conservation effective_junior + senior_balance == total_pool_value when both are well-defined. Regression guard in case senior_balance()'s definition ever changes away from being the tpv residual. All proofs use the established 1e9 bound for CBMC tractability, matching the style of the surrounding distribute_loss and distribute_fees proofs. All are inside the #[cfg(kani)] module — no effect on normal builds. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…to#129) .cargo/audit.toml ignores RUSTSEC-2022-0093 (ed25519-dalek 1.x) and RUSTSEC-2024-0344 (curve25519-dalek 3.x) with the rationale that these crates are dev-only and never reach the BPF binary. The dependency chain (solana-keypair -> ed25519-dalek-bip32 -> dalek 1.x) is reachable only via solana-sdk which lives under [dev-dependencies]. But no CI check mechanically verifies the "dev-only" claim. If a future dep change pulled either crate into the normal dep tree, the BPF binary would ship with known-vulnerable code and CI would say nothing — the ignore rules would silently become unsafe. Added .github/workflows/audit.yml with a bpf-tree-shake job that: 1. Runs cargo tree -e normal --no-default-features (excludes dev-dependencies and build-dependencies, matching the on-chain build's actual dep surface) 2. Greps the result for ed25519-dalek v1.x and curve25519-dalek v3.x 3. Fails if either appears, with a clear error pointing to the offending audit.toml ignore Verified locally: grep patterns are version-specific — curve25519-dalek v4.1.3 is legitimately present in the normal tree via solana-program, but only v3.x has the ignored CVE, so no false positive. Runs on PR, master push, weekly schedule (catches silent regressions). NOTE: the companion `cargo audit` step (tracked separately) is blocked by upstream cargo-audit 0.21.x lacking CVSS 4.0 parser support. Will be added when a fixed cargo-audit is released to crates.io. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

process_admin_set_tranche_config allowed the admin to change junior_fee_mult_bps at any time, even while junior LPs were deposited. Since process_accrue_fees reads the multiplier live (no per-epoch snapshot), a mid-life change immediately rewrites the fee split for every junior LP. Attack path: 1. Juniors deposit at multiplier M_1 2. Vault accumulates trading fees 3. Admin bumps multiplier to M_2 > M_1 4. Anyone calls AccrueFees — junior sub-pool captures outsized share 5. Admin (holding a junior position) withdraws at inflated value 6. Admin resets multiplier to M_1 to cover tracks The inverse also works: admin depresses the multiplier to silently reduce junior yield below what depositors were promised at deposit time. Fix: block any multiplier change when junior_total_lp() > 0. Idempotent re-writes (same value) still succeed so admin tooling can reapply config. Once all juniors withdraw (junior_total_lp back to 0), the multiplier is freely configurable for the next cohort. The "no disable" issue (set_tranche_enabled always hardcoded true) is left as-is — it's a design limitation, but because the current code cannot disable tranches, the related "disable mid-flight" attack surface that Agents A/C raised does not exist in practice. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…cccrypto#126) process_admin_set_insurance_policy forwarded a caller-supplied authority: Pubkey to wrapper Tag 22 without any validation. A malicious admin could set authority = their_wallet_key, causing the wrapper to record their wallet as the policy authority. They could then bypass the stake program entirely by calling the wrapper's WithdrawInsuranceLimited directly (Tag 23), signing as themselves. Funds extract to the attacker while pool.total_returned is NEVER incremented — process_admin_withdraw_insurance (the only path that updates total_returned) is skipped entirely. This silently desyncs total_pool_value() from the vault's actual balance. LP holders see no on-chain anomaly until they try to redeem against a pool whose accounting claims the funds are still in insurance, but the tokens are in the attacker's wallet. Fix: derive the expected vault_auth PDA and require the caller-supplied authority to equal it. This mirrors the exact check already present in process_admin_withdraw_insurance (lines 1418-1422). Non-breaking ABI change — the parameter is retained, just now validated. The vault_auth PDA is the ONLY signer the stake program can produce for WithdrawInsuranceLimited, so any other authority value breaks the program's own withdrawal path while enabling the bypass. There is no legitimate alternate value. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

With tranches enabled, process_deposit minted senior LP at the GLOBAL pool price while senior withdrawals redeem at the SENIOR sub-pool price. After the junior tranche absorbs an insurance loss (total_flushed > total_returned) the global per-LP price falls below the senior per-LP price, so an unprivileged user could mint senior LP cheap (global) and redeem dear (senior), extracting value from existing senior LP holders and the junior first-loss buffer. Price senior deposits via math::calc_senior_lp_for_deposit(senior_total_lp(), senior_balance(), amount) when tranche_enabled() — the same basis the senior withdraw path uses (calc_senior_collateral_for_withdraw) and symmetric with the junior deposit path. A senior_total_lp() == 0 first-depositor 1:1 bootstrap avoids bricking the senior tranche when junior deposited first and a fee/loss left orphaned senior value with zero senior LP. No state-layout change, no ABI change, no new error variants; senior accounting remains fully derived from the global counters. Adds tests/poc_senior_deposit_mispricing.rs (regression: old global pricing is exploitable; sub-pool pricing is not; bootstrap does not brick) and math unit tests for calc_senior_lp_for_deposit. Refs dcccrypto#134 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-17T15:16:52Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f9eb66cc-cef1-4983-9cc8-eebaaae250f0

📥 Commits

Reviewing files that changed from the base of the PR and between cfba95c and 151b12d.

📒 Files selected for processing (4)

src/math.rs
src/processor.rs
tests/poc_senior_bootstrap_orphan.rs
tests/poc_senior_deposit_mispricing.rs

🚧 Files skipped from review as they are similar to previous changes (2)

src/math.rs
tests/poc_senior_deposit_mispricing.rs

📝 Walkthrough

Walkthrough

Fixes a senior-tranche LP deposit mispricing vulnerability by introducing calc_senior_lp_for_deposit in src/math.rs that prices deposits against the senior sub-pool rather than the global pool. Updates process_deposit in src/processor.rs to branch on tranche_enabled() and use the new function. Adds comprehensive regression tests in two new files covering exploit reproduction, fixed-behavior validation, and orphaned-state edge cases.

Changes

Senior LP Deposit Mispricing Fix

Layer / File(s)	Summary
`calc_senior_lp_for_deposit` function and unit tests `src/math.rs`	Adds `calc_senior_lp_for_deposit` as a thin wrapper over `calc_lp_for_deposit` using senior sub-pool `(senior_total_lp, senior_balance)` parameters. Includes documentation on orphaned-value blocking and bootstrap semantics. Unit tests cover 1:1 first deposit, pro-rata minting, orphaned-value rejection, and no-profit round-trip after junior-absorbed loss.
Tranche-aware LP minting in `process_deposit` `src/processor.rs`	Replaces the unconditional `pool.calc_lp_for_deposit` call with a `tranche_enabled()` branch: senior deposits use `calc_senior_lp_for_deposit` with Option error handling; non-tranche deposits fall back to the original path. Bootstrap and orphaned-value guard behavior are delegated to the math function.
PoC exploit reproduction and edge-case regression tests `tests/poc_senior_deposit_mispricing.rs`, `tests/poc_senior_bootstrap_orphan.rs`	Two new test files validating the fix from multiple angles. `poc_senior_deposit_mispricing.rs` reproduces the original exploit with global pricing and confirms the fixed sub-pool pricing prevents extraction while preserving incumbent value. `poc_senior_bootstrap_orphan.rs` validates C9 orphaned-state invariants, confirms 1:1 bootstrap for first-depositor when senior_balance is 0, blocks deposits into orphaned pools, and ensures junior-first scenarios do not brick the first senior deposit.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Senior LP deposits priced against the global pool but redeemed against the senior sub-pool — unprivileged value extraction from senior LPs after a junior-absorbed loss #134: This PR directly implements the fix described in that issue — adding calc_senior_lp_for_deposit() and modifying process_deposit() to price senior deposits against the senior sub-pool instead of the global pool, preventing unprivileged value extraction.

Possibly related PRs

dcccrypto/percolator-stake#112: calc_senior_lp_for_deposit in this PR delegates to calc_lp_for_deposit, so #112's change to make calc_lp_for_deposit return None on invalid accounting directly affects the tranche-aware senior deposit minting behavior.

Poem

🐇 Hop hop, the price was wrong,
Senior LPs priced all along
Against the global pool — a trap!
Eve filled her pockets in the gap.
Now senior math sees senior ground,
No profit made, no loss unsound! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(tranche): price senior deposits against the senior sub-pool' clearly and specifically identifies the main change—fixing senior deposit pricing to use the senior sub-pool instead of the global pool.
Description check	✅ Passed	The description comprehensively covers the bug, solution, implementation details, and testing approach, and includes a checklist with required verification steps.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

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

…conditional bootstrap The senior deposit path special-cased `senior_total_lp() == 0` into an unconditional 1:1 mint, which bypassed the orphaned-value (C9) guard inside `calc_lp_for_deposit`. In the reachable state where all LP has exited (`total_lp_supply == 0`) but the pool still holds value (e.g. insurance was returned post-resolution), a tranche pool has `senior_total_lp() == 0` with `senior_balance() > 0`. The bootstrap minted 1 senior LP for a 1-token deposit against the whole orphaned balance, which the depositor could then withdraw in full at the senior sub-pool price — draining the orphan. Fix: always price senior deposits via `calc_senior_lp_for_deposit` (which delegates to `calc_lp_for_deposit`), with no `senior_total_lp() == 0` special case. It mints 1:1 only for a true first senior (`senior_balance == 0`) and returns `None` for orphaned senior value (`senior_balance > 0`), rejecting the deposit exactly as the non-tranche path does. A legitimate first senior always has `senior_balance == 0` (empty pool, or a junior-first pool where junior captures 100% of fees and absorbs 100% of loss because `gross_senior == 0`), so this does not brick the first senior deposit. Adds tests/poc_senior_bootstrap_orphan.rs: reproduces the orphan-theft under the old bootstrap, asserts the fix rejects it, and proves the first senior still mints 1:1 across empty / junior-only / junior-only-post-loss-and-recovery states. Updates the existing senior-mispricing regression test to the no-bootstrap helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

0x-SquidSol · 2026-06-17T18:33:13Z

Follow-up commit: close a C9-bypass introduced by the first cut of this fix

Adversarial re-review of this branch surfaced a defect in the senior deposit code added here, and 151b12d fixes it.

The defect

This PR's senior deposit path special-cased senior_total_lp() == 0 into an unconditional 1:1 mint:

let senior_lp = pool.senior_total_lp();
if senior_lp == 0 {
    amount            // <-- bypasses the orphaned-value (C9) guard
} else {
    calc_senior_lp_for_deposit(senior_lp, senior_bal, amount)...
}

That branch never consults the orphaned-value (C9) guard that lives inside calc_lp_for_deposit. The exact C9-reachable state — all LP withdrawn (total_lp_supply == 0) after insurance was returned (total_returned > 0, so total_pool_value() > 0) — gives a tranche pool senior_total_lp() == 0 with senior_balance() > 0. A 1-token deposit then mints 1 senior LP against the whole orphaned balance and redeems it at the senior sub-pool price:

orphan: total_deposited 1_000_000, total_withdrawn 500_000, total_flushed 500_000, total_returned 500_000
        => total_lp_supply 0, senior_balance 500_000
attacker: deposit 1 -> mint 1 LP (bootstrap) -> withdraw 1 LP -> 500_001 out  (net +500_000)

On the currently-merged code this is additionally gated by the market_resolved deposit check (total_returned > 0 implies a resolved market, and process_deposit rejects when market_resolved()), so it is latent rather than live — but it makes the C9 protection depend entirely on one unrelated control instead of the verified math.

The fix (`151b12d`)

Drop the special case; always price senior deposits via calc_senior_lp_for_deposit → calc_lp_for_deposit, which already encodes C9:

true first senior (senior_total_lp == 0 && senior_balance == 0) → mints 1:1
orphaned senior value (senior_total_lp == 0 && senior_balance > 0) → returns None → deposit rejected, same as the non-tranche path

This does not brick the first senior deposit: a legitimate first senior always has senior_balance == 0 — empty pool, or a junior-first pool where junior captures 100% of fees (senior weight 0) and absorbs 100% of loss (gross_senior == 0). Both proven and tested.

Verification

cargo build --lib and cargo build-sbf: clean.
New tests/poc_senior_bootstrap_orphan.rs: reproduces the orphan-theft under the old bootstrap, asserts the fix rejects it, and proves first-senior 1:1 across empty / junior-only / junior-only-post-loss-and-recovery states.
Existing poc_senior_deposit_mispricing.rs updated to the no-bootstrap helper; its exploit-reproduce-then-neutralize assertions still hold.

0x-SquidSol · 2026-06-17T18:42:24Z

Reachability update: live-exploitable (not merely latent) for markets with permissionless resolution enabled

My note above called this "latent, gated by the market_resolved deposit check." Tracing the gate against the monolith CPI target shows that's only true for the default config — it was live otherwise:

process_deposit blocks only on this program's local market_resolved() flag (processor.rs:452). That flag is written in exactly one place — process_admin_resolve_market (processor.rs:1404) — i.e. only when resolution goes through this program's admin path. Nothing syncs the monolith's resolved status into it.
The monolith exposes ResolvePermissionless (tag 29): anyone can resolve a stale market when its permissionless_resolve_stale_slots > 0 (disabled by default, but a documented "fallback exit" liveness feature). The stake program never sets or reads that field, so it cannot prevent a market it administers from enabling it.
A permissionless resolve therefore resolves the monolith market without setting this program's local flag. AdminWithdrawInsurance (requires a resolved market) then succeeds and credits total_returned, while process_deposit still sees market_resolved() == false and accepts deposits. (Withdrawals aren't gated on the flag either, so LPs can fully exit first → total_lp_supply == 0 orphan.)

Net: for a stake-administered market with permissionless resolution enabled, the old bootstrap was a live orphan-theft — gated only by an admin returning insurance to a fully-exited pool (a normal recovery action), with the final steal being unprivileged. Only default-config markets (permissionless_resolve_stale_slots == 0) kept it latent. This fix closes it unconditionally, independent of the market's resolution config — which is the point: the C9 invariant should not depend on an unrelated, externally-configurable gate.

dcccrypto · 2026-06-17T23:23:59Z

Integrated into main via #142 (the HIGH senior sub-pool deposit-pricing fix), with your commits cherry-picked and attribution preserved. Your branch was based on pre-v17 c3a617a, which auto-merged into the v17 CRITICAL-1 admin redesign incorrectly, so I re-applied it by hand against current v17 and verified (round-trip-no-profit PoC + proptest, build-sbf clean). Thanks — great find.

0x-SquidSol and others added 5 commits June 13, 2026 21:04

dcccrypto force-pushed the main branch from 1bee206 to bdd0539 Compare June 17, 2026 21:08

dcccrypto mentioned this pull request Jun 17, 2026

fix(stake): 5 verified v17 bounty findings — senior pricing (HIGH), JIT-fee snipe, distribute_fees, cooldown bound, tranche coverage #142

Merged

dcccrypto closed this Jun 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tranche): price senior deposits against the senior sub-pool - #135

fix(tranche): price senior deposits against the senior sub-pool#135
0x-SquidSol wants to merge 6 commits into
dcccrypto:mainfrom
0x-SquidSol:fix/senior-deposit-subpool-pricing

0x-SquidSol commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Uh oh!

0x-SquidSol commented Jun 17, 2026

Uh oh!

0x-SquidSol commented Jun 17, 2026

Uh oh!

dcccrypto commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

0x-SquidSol commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change

Safety / blast radius

Tests

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Uh oh!

0x-SquidSol commented Jun 17, 2026

Follow-up commit: close a C9-bypass introduced by the first cut of this fix

The defect

The fix (151b12d)

Verification

Uh oh!

0x-SquidSol commented Jun 17, 2026

Reachability update: live-exploitable (not merely latent) for markets with permissionless resolution enabled

Uh oh!

dcccrypto commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

0x-SquidSol commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

The fix (`151b12d`)