fix(stake): #211 — validate LP recipient ownership on deposit

[Morenikeoa]

Problem

process_deposit and process_deposit_junior validate that the collateral source (user_ata) is SPL-Token-owned, holds the correct mint, and is owned by the depositor signer. Neither function applies the equivalent check to user_lp_ata, the destination of the mint_to CPI that issues LP receipt tokens.

Impact

The StakeDeposit record (and its cooldown) is always derived from the depositor's signer key, but LP receipt tokens can be minted into a token account owned by a different wallet. That creates a receipt/record mismatch: the depositor's record tracks a position whose receipt they don't hold, and the actual LP holder can't withdraw because the deposit record isn't keyed to them. process_withdraw already enforces this exact invariant on its LP source (mint + signer-owner check) — deposit was the asymmetric, unfixed side. Not direct fund theft (the depositor supplies their own collateral and SPL mint_to still enforces the destination's mint), but a real stuck-receipt / griefing path.

Fix

Added validate_lp_recipient_account (SPL-Token-owned → correct pool.lp_mint → owned by user.key) mirroring the existing withdraw-side check, and call it in both process_deposit and process_deposit_junior right after the existing user_ata validation block, before any CPI.

Proof of Fix

• New PoC test poc_deposit_should_reject_lp_recipient_not_owned_by_depositor constructs a depositor with a valid user_ata, and a user_lp_ata with the correct lp_mint but owned by a third party, then calls Deposit directly. It now correctly rejects with Unauthorized/InvalidAccount.
• Verified the negative: with the new check temporarily removed, the same test fails — execution falls all the way through to the SPL invoke() CPI stub (InvalidArgument from the missing syscall, not a clean validation error), confirming the unpatched path would have proceeded with the transfer/mint instead of rejecting.
• Full cargo test --lib (128 tests) and integration/unit/cpi_tags/error_codes/struct_layout suites (67 tests) pass with no regressions.

Related

Closes #211

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Strengthened validation for deposit operations to ensure LP receipts are only minted to token accounts owned by the depositor, preventing accidental or malicious routing to unauthorized accounts.

• Tests

  • Added test coverage to verify deposits correctly reject LP recipient accounts with mismatched ownership.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9df3d8de-fabc-4fe0-9bea-a208576a6230

📥 Commits

Reviewing files that changed from the base of the PR and between 1e57344 and 9f07a77.

📒 Files selected for processing (1)

• src/processor.rs

---

📝 Walkthrough

Walkthrough

Adds a validate_lp_recipient_account helper to src/processor.rs that checks SPL-Token ownership, data length, LP mint match, and depositor ownership for user_lp_ata. Both process_deposit and process_deposit_junior now call this validator before minting LP tokens. A PoC test asserts rejection when the LP recipient account is owned by a third party.

Changes

LP Recipient Ownership Validation

Layer / File(s)	Summary
validate_lp_recipient_account helper src/processor.rs	New function validates that user_lp_ata is SPL-Token-owned, has at least ACCOUNT_LEN bytes, holds the pool's LP mint, and is owned by the depositor; returns StakeError::InvalidAccount, StakeError::InvalidMint, or StakeError::Unauthorized on each failure.
Wire validator into deposit paths src/processor.rs	process_deposit (line 673–678) and process_deposit_junior (line 2491–2494) each invoke validate_lp_recipient_account with user_lp_ata, &pool.lp_mint, and user.key before the pricing and mint_to CPI, closing the asymmetric gap relative to the existing withdraw-side check.
PoC test for mismatched LP recipient src/processor.rs	Adds a test that builds a Deposit scenario where user_lp_ata has the correct LP mint but is controlled by a third-party wallet, and asserts the processor returns Unauthorized or InvalidAccount before minting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• dcccrypto/percolator-stake#208: Also modifies deposit flows in src/processor.rs to add SPL-Token-program ownership checks and stake-discriminator hardening — directly adjacent token-account validation work in the same functions.
• dcccrypto/percolator-stake#30: Adds generic validation helpers and pre-validates pool_pda in process_deposit, the same pattern of centralizing account validation that this PR applies to the LP recipient account.

Poem

🐇 Hop hop, no sneaky swap!
The LP receipt must land in your lap,
Third-party wallets get a firm tap —
Unauthorized! Shut the gap.
Your tokens, your account, full stop. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description includes Problem, Impact, Fix, and Proof of Fix sections with comprehensive technical details, but is missing the standard template sections (Summary, How to test, Checklist).	Restructure the description to follow the repository template with Summary, How to test, Checklist, and Related sections; move current content into appropriate sections.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main security fix: validating LP recipient ownership on deposit, directly addressing issue #211.
Linked Issues check	✅ Passed	The PR implementation matches all coding requirements from issue #211: validates LP recipient account ownership (SPL-Token-owned, correct mint, correct owner), adds validation to both deposit paths, includes PoC test, and ensures alignment with withdraw-side checks.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #211: the new helper function and validation calls in deposit paths, plus the PoC test are entirely aligned with the linked objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[dcccrypto]

Superseded by the rebased version (your branch conflicted with #213/#216 in the test module). Your fix landed on main with full credit, @Morenikeoa — thank you. Closes the same #211 gap.