[LOW] ReconcileBurnedNft abandons the NFT mint and ExtraAccountMetaList PDA — ~0.0068 SOL per NFT, unrecoverable (the #102 leak, on the path added later by #138)

[0x-SquidSol]

Summary

ReconcileBurnedNft (tag 7) returns the PositionNft PDA's rent to the recorded last holder but abandons the NFT mint and the ExtraAccountMetaList PDA. Once it has run, neither can ever be reclaimed by anyone.

#102 closed exactly this leak — for the two burn instructions that existed at the time. BurnPositionNft and EmergencyBurn both call close_extra_metas (processor.rs:968, :1198) and close the mint under the mint_auth PDA. ReconcileBurnedNft was added later by #138 and takes seven accounts:

0 nft_pda   1 nft_mint   2 portfolio   3 mint_auth
4 nft_registry   5 percolator_prog   6 last_holder

There is no extra_metas slot and no token-program slot, so it structurally cannot close either account.

Why it is permanent

Reconcile closes nft_pda (processor.rs:1329-1333), and every path that could reclaim the two accounts requires nft_pda to still be program-owned and to hold a valid PositionNftV16:

• BurnPositionNft / EmergencyBurn are the only callers of close_extra_metas and the only signers of a mint close. Both also require verify_holder_ata_account with amount == 1 — but Reconcile only runs when supply is already 0, so neither can ever run again.
• RepairExtraMetas requires nft_pda.owner == program_id (processor.rs:1534).

Both are dead the moment Reconcile returns.

Cost

Rent-exempt minimums, (128 + len) * 3480 * 2:

Account	Length	Stranded
ExtraAccountMetaList PDA	261	2,707,440 lamports
NFT mint (post-realloc)	458	4,078,560 lamports
Total		6,786,000 lamports ≈ 0.0068 SOL per reconciled NFT

No security impact — nothing is stolen and no position is at risk. It is an unrecoverable loss of the holder's own rent, on the recovery path that exists to make them whole after an out-of-band burn.

Proof of concept

tests/poc_reconcile_rent_leak.rs, against main at 215842e. Host-side, driving the real processor::process.

Reproduction note: cargo test does not build as shipped — the solana-sdk dev-dependency pulls openssl-sys, which needs a system OpenSSL. Neither it nor proptest is referenced anywhere in src/; removing both from [dev-dependencies] makes the suite build. Results are the 51 pre-existing tests plus these 4.

//! PoC: `ReconcileBurnedNft` closes the PositionNft PDA but abandons the NFT
//! mint and the ExtraAccountMetaList PDA, and once it has run neither can ever
//! be reclaimed.
//!
//! `#102` closed exactly this leak — for the two burn instructions that existed
//! at the time, `BurnPositionNft` and `EmergencyBurn`, both of which now call
//! `close_extra_metas` and close the mint. `ReconcileBurnedNft` (tag 7) was
//! added later by `#138` and takes only seven accounts: `nft_pda`, `nft_mint`,
//! `portfolio`, `mint_auth`, `nft_registry`, `percolator_prog`, `last_holder`.
//! There is no `extra_metas` slot and no token-program slot, so it structurally
//! cannot close either account.
//!
//! The leak is permanent, not merely deferred. Reconcile closes `nft_pda`, and
//! every path that could reclaim the two accounts requires `nft_pda` to still be
//! program-owned and to hold a valid `PositionNftV16`:
//!   * `BurnPositionNft` / `EmergencyBurn` — the only callers of
//!     `close_extra_metas` and the only signers of a mint close — additionally
//!     require the holder's ATA to hold `amount == 1`, but supply is already 0.
//!   * `RepairExtraMetas` requires `nft_pda.owner == program_id`.
//! Both are dead the moment Reconcile returns.

use bytemuck::Zeroable;
use percolator_nft::{
    cpi_v16::{derive_nft_registry, PERCOLATOR_MAINNET},
    instruction::{TAG_EMERGENCY_BURN, TAG_RECONCILE_BURNED_NFT, TAG_REPAIR_EXTRA_METAS},
    processor,
    slab_types_v16 as sl,
    state_v16::{
        mint_authority_pda, position_nft_pda, PositionNftV16, POSITION_NFT_V16_MAGIC,
        POSITION_NFT_V16_VERSION,
    },
    token2022::{get_associated_token_address, TOKEN_2022_PROGRAM_ID},
    transfer_hook::extra_account_metas_pda,
};
use solana_program::{account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey};

const PROG: Pubkey = Pubkey::new_from_array([9u8; 32]);
const ALICE: Pubkey = Pubkey::new_from_array([0xA1; 32]);
const PORTFOLIO: Pubkey = Pubkey::new_from_array([0x50; 32]);
const MARKET_GROUP: Pubkey = Pubkey::new_from_array([0x60; 32]);
const NFT_MINT: Pubkey = Pubkey::new_from_array([0x11; 32]);
const ASSET_INDEX: u32 = 7;
const MARKET_ID: u64 = 42;

/// Rent-exempt minimums, `(128 + len) * 3480 * 2`.
const PDA_RENT: u64 = 2_275_920; // 199 bytes
const METAS_RENT: u64 = 2_707_440; // 261 bytes
const MINT_RENT: u64 = 4_078_560; // 458 bytes, the mint's post-realloc length

fn leak<T>(v: T) -> &'static mut T {
    Box::leak(Box::new(v))
}

fn acct(
    key: Pubkey,
    owner: Pubkey,
    data: Vec<u8>,
    lamports: u64,
    writable: bool,
    signer: bool,
) -> AccountInfo<'static> {
    AccountInfo::new(
        leak(key),
        signer,
        writable,
        leak(lamports),
        Box::leak(data.into_boxed_slice()),
        leak(owner),
        false,
        0,
    )
}

fn token_account(mint: &Pubkey, owner: &Pubkey, amount: u64) -> Vec<u8> {
    let mut d = vec![0u8; 165];
    d[0..32].copy_from_slice(mint.as_ref());
    d[32..64].copy_from_slice(owner.as_ref());
    d[64..72].copy_from_slice(&amount.to_le_bytes());
    d[108] = 1;
    d
}

fn mint_account(supply: u64) -> Vec<u8> {
    let mut d = vec![0u8; 200];
    d[36..44].copy_from_slice(&supply.to_le_bytes());
    d[45] = 1;
    d
}

fn portfolio_buf() -> Vec<u8> {
    let (mint_auth, _) = mint_authority_pda(&PROG);
    let mut a: sl::PortfolioAccountV16Account = Zeroable::zeroed();
    a.provenance_header.market_group_id = MARKET_GROUP.to_bytes();
    a.provenance_header.portfolio_account_id = PORTFOLIO.to_bytes();
    a.provenance_header.owner = mint_auth.to_bytes();
    a.provenance_header.version = sl::V16PodU16::new(sl::V16_ACCOUNT_VERSION);
    a.provenance_header.layout_discriminator = sl::V16PodU16::new(sl::V16_LAYOUT_DISCRIMINATOR);
    a.owner = mint_auth.to_bytes();
    // Terminal: no active leg — the EmergencyBurn-eligible shape.
    let mut buf = vec![0u8; sl::HEADER_LEN + sl::EXPECTED_PORTFOLIO_ACCOUNT_SIZE];
    buf[0..8].copy_from_slice(&sl::MAGIC.to_le_bytes());
    buf[8..10].copy_from_slice(&sl::VERSION.to_le_bytes());
    buf[10] = sl::KIND_PORTFOLIO;
    buf[sl::HEADER_LEN..].copy_from_slice(bytemuck::bytes_of(&a));
    buf
}

fn nft_pda_buf(bump: u8) -> Vec<u8> {
    let mut s: PositionNftV16 = Zeroable::zeroed();
    s.magic = sl::V16PodU64::new(POSITION_NFT_V16_MAGIC);
    s.version = POSITION_NFT_V16_VERSION;
    s.bump = bump;
    s.portfolio_account = PORTFOLIO.to_bytes();
    s.nft_mint = NFT_MINT.to_bytes();
    s.asset_index = sl::V16PodU32::new(ASSET_INDEX);
    s.market_id_at_mint = sl::V16PodU64::new(MARKET_ID);
    s.last_holder = ALICE.to_bytes();
    bytemuck::bytes_of(&s).to_vec()
}

/// The three accounts whose fate this test is about, shared across a scenario so
/// their post-state can be inspected.
struct Fixture {
    nft_pda: AccountInfo<'static>,
    nft_mint: AccountInfo<'static>,
    extra_metas: AccountInfo<'static>,
}

fn fixture(mint_supply: u64) -> Fixture {
    let (nft_pda_key, bump) = position_nft_pda(&PORTFOLIO, MARKET_ID, &PROG);
    let (metas, _) = extra_account_metas_pda(&NFT_MINT, &PROG);
    Fixture {
        nft_pda: acct(nft_pda_key, PROG, nft_pda_buf(bump), PDA_RENT, true, false),
        nft_mint: acct(
            NFT_MINT,
            TOKEN_2022_PROGRAM_ID,
            mint_account(mint_supply),
            MINT_RENT,
            true,
            false,
        ),
        extra_metas: acct(metas, PROG, vec![7u8; 261], METAS_RENT, true, false),
    }
}

fn run_reconcile(f: &Fixture) -> Result<(), ProgramError> {
    let (mint_auth, _) = mint_authority_pda(&PROG);
    let (registry, _) = derive_nft_registry(&PERCOLATOR_MAINNET, &MARKET_GROUP);
    let accounts = vec![
        f.nft_pda.clone(),
        f.nft_mint.clone(),
        acct(PORTFOLIO, PERCOLATOR_MAINNET, portfolio_buf(), 0, true, false),
        acct(mint_auth, Pubkey::default(), vec![], 0, false, false),
        acct(registry, PERCOLATOR_MAINNET, vec![], 0, false, false),
        acct(PERCOLATOR_MAINNET, Pubkey::default(), vec![], 0, false, false),
        acct(ALICE, Pubkey::default(), vec![], 0, true, false),
    ];
    processor::process(&PROG, &accounts, &[TAG_RECONCILE_BURNED_NFT])
}

/// EmergencyBurn is the only other path that closes BOTH the mint and the
/// extra-metas PDA. Run it against the post-reconcile state.
fn run_emergency_burn(f: &Fixture) -> Result<(), ProgramError> {
    let (mint_auth, _) = mint_authority_pda(&PROG);
    let (registry, _) = derive_nft_registry(&PERCOLATOR_MAINNET, &MARKET_GROUP);
    let accounts = vec![
        acct(ALICE, Pubkey::default(), vec![], 0, true, true),
        f.nft_pda.clone(),
        f.nft_mint.clone(),
        acct(
            get_associated_token_address(&ALICE, &NFT_MINT),
            TOKEN_2022_PROGRAM_ID,
            token_account(&NFT_MINT, &ALICE, 1),
            0,
            true,
            false,
        ),
        acct(PORTFOLIO, PERCOLATOR_MAINNET, portfolio_buf(), 0, true, false),
        acct(mint_auth, Pubkey::default(), vec![], 0, false, false),
        acct(TOKEN_2022_PROGRAM_ID, Pubkey::default(), vec![], 0, false, false),
        f.extra_metas.clone(),
        acct(registry, PERCOLATOR_MAINNET, vec![], 0, false, false),
        acct(PERCOLATOR_MAINNET, Pubkey::default(), vec![], 0, false, false),
    ];
    processor::process(&PROG, &accounts, &[TAG_EMERGENCY_BURN])
}

fn run_repair_extra_metas(f: &Fixture) -> Result<(), ProgramError> {
    let (mint_auth, _) = mint_authority_pda(&PROG);
    let accounts = vec![
        acct(ALICE, Pubkey::default(), vec![], 1_000_000_000, true, true),
        f.nft_pda.clone(),
        f.nft_mint.clone(),
        f.extra_metas.clone(),
        acct(PORTFOLIO, PERCOLATOR_MAINNET, portfolio_buf(), 0, false, false),
        acct(mint_auth, Pubkey::default(), vec![], 0, false, false),
        acct(Pubkey::default(), Pubkey::default(), vec![], 0, false, false),
        acct(PERCOLATOR_MAINNET, Pubkey::default(), vec![], 0, false, false),
    ];
    processor::process(&PROG, &accounts, &[TAG_REPAIR_EXTRA_METAS])
}

// -- 1. the leak -------------------------------------------------------------

#[test]
fn reconcile_closes_the_pda_but_abandons_the_mint_and_extra_metas() {
    let f = fixture(0); // supply 0 — the out-of-band-burn state Reconcile handles
    assert!(run_reconcile(&f).is_ok(), "reconcile must succeed");

    assert_eq!(
        **f.nft_pda.lamports.borrow(),
        0,
        "the PositionNft PDA rent IS returned to the last holder",
    );
    assert_eq!(
        **f.nft_mint.lamports.borrow(),
        MINT_RENT,
        "...but the mint keeps its rent — Reconcile has no slot to close it",
    );
    assert_eq!(
        **f.extra_metas.lamports.borrow(),
        METAS_RENT,
        "...and so does the ExtraAccountMetaList PDA",
    );
    assert!(
        !f.extra_metas.data.borrow().iter().all(|b| *b == 0),
        "extra_metas is untouched, still carrying its meta list",
    );
}

#[test]
fn the_abandoned_rent_is_what_102_valued_plus_the_mint() {
    // Recorded so the cost is explicit rather than implied.
    assert_eq!(METAS_RENT + MINT_RENT, 6_786_000); // ≈ 0.00679 SOL per reconciled NFT
}

// -- 2. and it is unrecoverable ----------------------------------------------

#[test]
fn after_reconcile_no_path_can_reclaim_them() {
    let f = fixture(0);
    assert!(run_reconcile(&f).is_ok());

    // EmergencyBurn is the only other instruction that closes BOTH accounts.
    let eb = run_emergency_burn(&f);
    assert!(
        eb.is_err(),
        "EmergencyBurn must be dead once nft_pda is closed, got {eb:?}",
    );

    // RepairExtraMetas is permissionless but equally gated on nft_pda.
    let rp = run_repair_extra_metas(&f);
    assert!(
        rp.is_err(),
        "RepairExtraMetas must be dead too, got {rp:?}",
    );

    // The lamports are still sitting there, now unreachable.
    assert_eq!(**f.nft_mint.lamports.borrow(), MINT_RENT);
    assert_eq!(**f.extra_metas.lamports.borrow(), METAS_RENT);
}

// -- 3. control: the burn paths DO reclaim both ------------------------------

#[test]
fn control_emergency_burn_reclaims_both_when_the_pda_is_alive() {
    // Same accounts, but Reconcile has NOT run — so nft_pda is still live and
    // EmergencyBurn closes the mint and the metas. This isolates "Reconcile
    // omitted them" from "these accounts are never closeable".
    let f = fixture(1); // a live NFT: supply 1, holder holds it
    let r = run_emergency_burn(&f);
    assert!(r.is_ok(), "EmergencyBurn on a live NFT must succeed: {r:?}");
    assert_eq!(
        **f.extra_metas.lamports.borrow(),
        0,
        "EmergencyBurn DOES reclaim the extra-metas rent (#102)",
    );
}

running 4 tests
test the_abandoned_rent_is_what_102_valued_plus_the_mint ... ok
test reconcile_closes_the_pda_but_abandons_the_mint_and_extra_metas ... ok
test control_emergency_burn_reclaims_both_when_the_pda_is_alive ... ok
test after_reconcile_no_path_can_reclaim_them ... ok

control_emergency_burn_reclaims_both_when_the_pda_is_alive is the one that carries the argument: the identical accounts, with Reconcile simply not run, let EmergencyBurn close the metas PDA and return its rent. So the accounts are perfectly closeable — Reconcile just has no way to reach them. after_reconcile_no_path_can_reclaim_them then shows both recovery routes fail once Reconcile has closed nft_pda, with the lamports still sitting there unreachable.

Suggested fix

Extend Reconcile to close both, mirroring what EmergencyBurn already does, and pay the rent to last_holder_ai exactly as the PDA rent already is.

Worth doing this as optional trailing accounts rather than required ones, so it is not an ABI break: process_reconcile_burned_nft reads its accounts positionally and ignores extras today, so accounts.get(7) / accounts.get(8) for extra_metas and the token program lets existing 7-account callers keep working while a crank that supplies them recovers the rent. Reconcile is permissionless, and the recipient is already pinned to the recorded last_holder, so admitting the two extra accounts grants no new authority — the close targets are both re-derived from nft_mint, which is itself pinned to nft_state.nft_mint.

If an ABI break is acceptable, making them required is simpler and guarantees the rent is always recovered rather than only when the cranker bothers.

[0x-SquidSol]

Correction: the stranded amount is 7,676,880 lamports, not 6,786,000 — and the accounts are dead earlier than I said

Two corrections to the report above, both found in review. Neither weakens it.

1. I understated the loss by 13%. I took the mint's length as 458 bytes on the belief that Token-2022 shrinks it after writing metadata. It never does — there is no realloc of the mint anywhere in the crate. process_mint_position_nft allocates

final_size = mint_space + metadata_tlv_size + 128
           =    338     +      120          + 128   = 586

and funds rent.minimum_balance(586). The Token-2022 metadata Initialize writes into the existing TLV region rather than resizing, which is precisely why the #115 comment sits next to that allocation. So the 128 bytes of slack are funded and never refunded.

Account	Length	Stranded
ExtraAccountMetaList PDA	261	2,707,440
NFT mint	586 (not 458)	4,969,440 (not 4,078,560)
Total		7,676,880 ≈ 0.0077 SOL

(A SHORT position's name is one byte longer, giving 587 bytes and 4,976,400.) The mistake was easy to miss because 128 + 458 = 586, so my figure looked like a correct computation — it was the right formula applied to the wrong length.

Worth separating out: the + 128 is dead weight in its own right. Those bytes are never used by the TLV, so every NFT over-funds its mint by 890,880 lamports. On the normal burn paths it comes back to the holder; on this path it was exactly the amount I missed.

2. The two accounts are unrecoverable from the out-of-band burn onward, not from Reconcile onward. I framed Reconcile as destroying recoverable value. It isn't — BurnPositionNft and EmergencyBurn are the only instructions that close the mint or call close_extra_metas, and both require verify_holder_ata_account with amount == 1. After an out-of-band burn the ATA holds 0, so both are already dead before Reconcile is ever considered, and RepairExtraMetas only ever pays rent in — it has no close path at all.

That makes the case stronger rather than weaker: Reconcile is not one of several chances, it is the only instruction that can still legally touch those accounts in that state, and it runs at most once.

Consequence for the fix. I had proposed adding the two accounts as optional trailing accounts to avoid an ABI break. That is the wrong call and I've changed it — they are required in the PR:

• Reconcile is permissionless, irreversible and one-shot. With an optional form, any caller running the seven-account version first — a stale client, a helpful third party, or a griefer paying one 5,000-lamport fee — destroys 7.68M lamports of someone else's rent permanently, with no second chance to notice.
• The compatibility it would preserve protects an empty set: tag 7 has no account list in instruction.rs, no row in the README instruction table, and there is no client SDK or IDL in the repo. A search across this repo and both sibling trees finds zero callers.

I also want to correct my own incentive reasoning: I described the cranker as getting nothing. In the dominant case the cranker is last_holder, who is recovering an escrowed position worth far more than the rent — so supplying two extra metas costs them nothing. The problem was never the incentive; it was that an opt-in on a one-shot irreversible instruction can be lost by whoever gets there first.

[0x-SquidSol]

Correction: there IS a first-party caller of tag 7 — percolator-sdk — and the required-accounts form in #183 breaks it

I wrote, both here and in #183:

The compatibility it would preserve protects an empty set: tag 7 had no account list in instruction.rs, no row in the README instruction table, and a search across this repo and both sibling trees finds no callers.

That search was scoped to percolator-nft, percolator and percolator-prog — the three repos I had cloned. It missed dcccrypto/percolator-sdk, which is a first-party TypeScript SDK and does call tag 7:

// percolator-sdk/src/abi/nft.ts
export function encodeNftReconcile(): Uint8Array { ... }   // tag 7

/**
 *   0. [writable]  PositionNft PDA (closed)
 *   1. []          NFT mint (Token-2022 — supply must be 0)
 *   ...
 *   6. [writable]  Recorded last-holder wallet (escrow + PDA-rent recipient)
 */
export const ACCOUNTS_NFT_RECONCILE: AccountMeta[] = [
  "w", "r", "w", "r", "r", "r", "w",
];

Exactly seven accounts, with the NFT mint marked read-only. #183 as it stands requires nine and requires the mint writable, so it breaks this on both counts.

What I'd suggest

The argument for required still holds and I don't think it should be abandoned: Reconcile is permissionless, irreversible and one-shot, so an optional form can be permanently defeated by whoever calls first — including a griefer paying a single fee to destroy the last holder's rent. That property is what makes an opt-in the wrong shape here, and it is unaffected by the existence of a caller.

What changes is that this is now a coordinated release rather than a free ABI break. Three options, in the order I'd rank them:

1. Ship both together. Land fix(nft): ReconcileBurnedNft reclaims the mint and ExtraAccountMetaList rent (#182) #183 and a matching percolator-sdk change — extend ACCOUNTS_NFT_RECONCILE to nine, flip account 1 to "w", and update the doc block. The SDK is first-party and in the same org, so lockstep is realistic. I'm happy to prepare that PR if useful.
2. Optional, with the arity hole closed. Accept 7 or 9 and hard-error on 8, so a malformed call fails loudly rather than silently destroying the rent. Weaker, because the 7-account path still wins any race, but it is non-breaking.
3. Defer fix(nft): ReconcileBurnedNft reclaims the mint and ExtraAccountMetaList rent (#182) #183 until the SDK change is scheduled.

I would not merge #183 alone. Note also that any deployed keeper or bot built against the published SDK carries the seven-account list, so the blast radius of merging in isolation is wider than the repo suggests.

One more thing found while looking

percolator-sdk/src/abi/nft.ts's module header documents the PositionNft PDA seeds as ["position_nft", portfolio_account, asset_index_u16_LE]. The code is correct — derivePositionNftPda uses market_id_u64_LE and cites #108 — so this is a stale comment rather than a bug, but it describes the pre-#108 scheme that #108 was filed to remove, and it sits at the top of the file a reader consults first.