[HIGH] ACCOUNTS_NFT_BURN / ACCOUNTS_NFT_EMERGENCY_BURN mark the holder non-writable — every burn built from these templates is rejected with InvalidAccountData

[0x-SquidSol]

Summary

ACCOUNTS_NFT_BURN and ACCOUNTS_NFT_EMERGENCY_BURN mark account 0 — the NFT holder — as "s": signer, not writable. The program requires it to be signer and writable, because the holder is the rent recipient for every account those instructions close.

Any consumer building a BurnPositionNft or EmergencyBurn instruction from these templates via buildNftAccountMetas produces isWritable: false for the holder, and the deployed program rejects the transaction with InvalidAccountData.

This is a current break against the deployed programs, not a latent one, and it is independent of the Reconcile ABI change discussed in dcccrypto/percolator-nft#182.

Evidence

The program requires writable. percolator-nft/src/processor.rs:

fn require_writable_rent_recipient(holder: &AccountInfo) -> ProgramResult {   // :765
    if !holder.is_writable {
         return Err(ProgramError::InvalidAccountData);
    }
    Ok(())
}

called at :825 (BurnPositionNft) and :1000 (EmergencyBurn). The holder is also the AccountMeta::new(destination, false) of three closes — the ATA close, the mint close, and the direct PDA lamport credit.

The program's own ABI table says so. percolator-nft/src/instruction.rs:44 and :99, identically:

///   0. `[signer, writable]`  NFT holder (rent recipient)

The SDK disagrees. src/abi/nft.ts:200 and :218:

export const ACCOUNTS_NFT_BURN: AccountMeta[] = [
  "s", "w", "w", "w", "w", "r", "r", "w", "r", "r",
];
export const ACCOUNTS_NFT_EMERGENCY_BURN: AccountMeta[] = [
  "s", "w", "w", "w", "w", "r", "r", "w", "r", "r",
];

buildNftAccountMetas (src/abi/nft.ts:143-156) maps "s" to {isSigner: true, isWritable: false} — so the holder goes on the wire read-only.

Why it went unnoticed

Nothing in this repo consumes these templates: a grep across src/, test/ and playground/ finds no call site of buildNftAccountMetas and no consumer of any ACCOUNTS_NFT_* array. They are used only by external consumers, so the repo's own tests never exercised them.

The existing drift tests also could not have caught it — they assert the shorthand string codes, and the defect only becomes visible once those codes are turned into {isSigner, isWritable} booleans.

This is the same failure mode the long comment above buildNftAccountMetas already documents: the earlier bug there was passing these arrays to the wrong builder, which silently produced isSigner: undefined / isWritable: undefined and was described as failing with "InvalidAccountData at ~2.4k CU, before any CPI". Same symptom, different cause.

Proof of concept

Added to test/drift-check.test.ts, which already tracks this surface. The tests round-trip each template through the real builder and assert the {isSigner, isWritable} pairs the runtime actually sees, rather than the string codes:

× BurnPositionNft: the holder is the rent recipient, so it must be signer AND writable
    AssertionError: expected [ true, false ] to deeply equal [ true, true ]

× EmergencyBurn: same holder requirement (processor.rs:1000)
    AssertionError: expected [ true, false ] to deeply equal [ true, true ]

Fix

Change account 0 to "sw" in both templates. One character each.

I have a PR ready with the fix plus the round-trip tests above, which also cover MintPositionNft and the count-mismatch throw. Testing the booleans rather than the codes is the part that matters — it is what makes this class of defect visible, and it would have caught the historical wrong-builder bug too.

[dcccrypto]

CONFIRMED. Fixed in #377.

ACCOUNTS_NFT_BURN / ACCOUNTS_NFT_EMERGENCY_BURN marked the holder non-writable, but the holder is the rent recipient on burn — closing the account credits lamports to it, which requires a writable meta. Any client building these instructions from the SDK's own account lists produced transactions that could not succeed.

This is the SDK-vs-program drift class: the constant was internally consistent and simply disagreed with the program. Verified against the deployed NFT program rather than the SDK's own types, per the rule that the SDK is never its own evidence for an ABI question.