fix(nft): hand closed PDAs back to the System program (#178)

[0x-SquidSol]

Closes #178.

Problem

Every close path zeroed the account and drained its lamports but never handed it back to the System program — no resize(0), no assign. The runtime reaps zero-lamport accounts only at the end of a transaction, so a later instruction in the same transaction crediting lamports back left the account alive: still program-owned, still full length, all zero.

For the PositionNft PDA that is a permanent deadlock. MintPositionNft gates on !nft_pda.data_is_empty(), so a 199-byte zeroed account reads as "already minted" forever; and every handler that could clear it runs verify_position_nft, which rejects magic == 0. ReconcileBurnedNft requires no signer at all, so any third party could do it for the price of one rent-exempt balance:

ix0: ReconcileBurnedNft
ix1: System::transfer(attacker -> nft_pda, 2_275_920)

Because market_id is per-asset-slot rather than per-position-instance (state_v16.rs), the PDA address is stable across close-and-reopen — so this destroys a (portfolio, asset-slot) pair's ability to ever be wrapped again, not merely the current position.

Worth noting that a comment in RepairExtraMetas already reasoned about this and relied on the assumption it breaks: "close_extra_metas zeroes its lamports and data without reassigning the owner, but the runtime reaps zero-lamport accounts at end of transaction, so on any LATER transaction it loads as System-owned and empty." That holds only if the account is actually reaped.

Fix

All four close sites — BurnPositionNft, EmergencyBurn, ReconcileBurnedNft and close_extra_metas — route through one shared helper so they cannot diverge:

fn release_closed_account_to_system(account: &AccountInfo) -> ProgramResult {
    { let mut data = account.try_borrow_mut_data()?; data.fill(0); }
    if cfg!(target_os = "solana") { account.resize(0)?; }
    account.assign(&solana_program::system_program::id());
    Ok(())
}

Both halves are required, and I checked each: assign alone leaves the account full-length so data_is_empty() still reads false and the mint gate still bricks; resize alone leaves it program-owned, so #129's system_instruction::allocate rejects it with AccountAlreadyInUse. Together they produce System-owned + empty, which is exactly what the #129 transfer→allocate→assign creation path consumes.

Why the resize is cfg-guarded, and why as cfg! rather than #[cfg]

AccountInfo::resize writes the new length through data_ptr.offset(-8) and reads original_data_len() through key_ptr.offset(-4). Both are valid only for an AccountInfo the runtime serialized, so it must not run against a host-built fixture — doing so corrupts the heap. This is not theoretical: an unguarded version failed the existing suite with STATUS_HEAP_CORRUPTION while every individual test still printed ok.

That part follows precedent — SPL Token-2022 splits delete_account on exactly this cfg, and percolator-prog guards its own realloc(0) the same way.

The runtime cfg! rather than the #[cfg] attribute is a deliberate divergence: CI runs cargo test --lib --tests on a host target only, so under the attribute the resize line would never be compiled, type-checked or linted anywhere — and the resize is the half that actually removes the brick. cfg! folds to a constant false off-target, so it still never executes host-side, while keeping the call under the compiler. Verified by introducing a deliberate typo: the host build now fails, where the attribute form accepted it.

Verification

• 5 new tests in tests/poc_pda_revival_brick.rs: the post-close state, plus a mint A/B using Rent::get() (processor.rs) as a sentinel — it is the first syscall and sits after the data_is_empty() gate, so UnsupportedSysvar means every pre-syscall check passed. Three are controls: a properly-reaped PDA passes the same gate, a program-owned full-length PDA is still rejected (pinning that the fix is in the close path, not a loosened mint gate), and a live NFT still blocks re-mint.
• The permissionless claim is asserted, not assumed — the fixture asserts no account carries is_signer before invoking.
• 51 pre-existing tests pass; cargo clippy --all-targets -- -D warnings clean; cargo build-sbf --tools-version v1.52 succeeds.

Known coverage gap, stated plainly: because the resize cannot execute off-target, no test exercises it. reconcile_hands_the_pda_back_to_the_system_program observes only the assign, and a_revived_closed_pda_no_longer_bricks_the_mint_slot hand-constructs the System-owned empty account rather than obtaining it from a close — so the two halves are joined by reasoning rather than by a test. The right closure is a litesvm case that loads the real .so, runs ReconcileBurnedNft, asserts owner == system_program && data.len() == 0, then performs the revival and a successful mint. percolator-prog and percolator-stake both already use litesvm; percolator-nft's [dev-dependencies] is currently empty, so adding it is a larger change than belongs in this PR. Happy to follow up.

Notes

• The RepairExtraMetas comment that depended on end-of-transaction reaping is updated; that property is now structural. The change also makes both of its gates fire earlier and for the right reason.
• One test fixture shared a single &program_id as both an account's owner slot and the program_id argument. AccountInfo::assign writes through that reference, so the local would silently become the system program id. Given its own storage.
• Deliberately not included: ReconcileBurnedNft never closes extra_metas at all, so its rent (~0.00207 SOL) is unreclaimable after a reconcile — a pre-existing leak, unchanged here, and more visible now that nft_pda hands itself back on that path. Worth its own issue.
• ReconcileBurnedNft being permissionless is what turns this from a footgun into a griefing vector, but it is defensible on its own terms (a recovery crank whose recipient is pinned to the recorded last_holder), so the fix is in the close paths rather than a new signer requirement.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Closed NFT accounts are now fully cleared and returned to the System program immediately.
  • Fixed an issue where revived, closed accounts could block minting for the associated slot.
  • Preserved safeguards preventing duplicate minting when an account or NFT is still active.

• Tests

  • Added coverage for account closure, revival, validation, and re-minting scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ba58e640-006d-48e8-a69d-333c254d8657

📥 Commits

Reviewing files that changed from the base of the PR and between 215842e and 5cddff9.

📒 Files selected for processing (2)

• src/processor.rs
• tests/poc_pda_revival_brick.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

NFT PDA close behavior

Layer / File(s)	Summary
Shared close helper and call-site integration src/processor.rs	A shared helper clears account data, resizes accounts on Solana, and assigns System ownership. NFT and extra-metadata close paths now use the helper. Unit coverage verifies the updated owner handling.
PDA lifecycle test harness tests/poc_pda_revival_brick.rs	Integration fixtures and helpers invoke reconciliation and mint processing with constructed account sets.
Close and mint regression coverage tests/poc_pda_revival_brick.rs	Tests verify System ownership after close, revived PDA behavior, zeroed PDA rejection, and continued rejection of full-length or live NFTs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🔵 Low · up to 5cddf

Closed PDAs are now cleared, resized to zero, and returned to the System program so they can be recreated instead of remaining permanently unusable. The PR is mergeable with owner awareness that the on-chain resize, revival, and remint path still needs a runtime-backed regression test or explicit follow-up.

Suggested reviewers: dcccrypto

Sequence Diagram(s)

sequenceDiagram
  participant ReconcileBurnedNft
  participant processor
  participant nft_pda
  participant SystemProgram
  ReconcileBurnedNft->>processor: process reconciliation
  processor->>nft_pda: clear and resize account data
  processor->>SystemProgram: assign account ownership
  processor-->>ReconcileBurnedNft: return close result

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: closed NFT PDAs are reassigned to the System program.
Linked Issues check	✅ Passed	The PR addresses issue #178. All three PositionNft close paths use a shared helper that clears data, resizes the account to zero on Solana, and assigns System ownership. The resulting account state pr…
Out of Scope Changes check	✅ Passed	The changes remain within scope. The shared helper, close_extra_metas update, comments, and regression tests all support consistent account-closing semantics and the linked issue objectives.

Full details: Linked Issues check

Explanation

The PR addresses issue #178. All three PositionNft close paths use a shared helper that clears data, resizes the account to zero on Solana, and assigns System ownership. The resulting account state prevents same-transaction PDA revival and supports reminting.

• Fix all pre-merge checks with AI

✨ 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.}