diff --git a/README.md b/README.md index b523325..f7cc44e 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,9 @@ percolator-nft (this program) ## v17 Layout Support The NFT program mirrors the converged v17 portfolio layout (`PortfolioAccountV16Account`, -9227 bytes) to read position state directly without CPI. Struct offsets are validated at -compile-time with size and offset assertions. +9227 bytes) to read position state directly without CPI. Struct field offsets are verified +for internal consistency via compile-time `const_assert!` macros; alignment with the live +engine layout requires runtime validation against real on-chain data (deferred, see #110H). ## Transfer Hook diff --git a/src/cpi_v16.rs b/src/cpi_v16.rs index 7bd54b6..4946b48 100644 --- a/src/cpi_v16.rs +++ b/src/cpi_v16.rs @@ -10,8 +10,7 @@ //! The decision functions take an already-decoded `&PortfolioAccountV16Account` //! (+ `&PositionNftV16` where relevant) so they are pure and exhaustively //! unit-testable WITHOUT a Solana runtime. The on-chain handlers do account -//! plumbing + the (unchanged) Token-2022 CPIs around these functions; the -//! end-to-end path is verified by the A.5 LiteSVM suite. +//! plumbing + the (unchanged) Token-2022 CPIs around these functions. //! //! ## v16 slot-reuse anchor = `market_id` (per-asset-slot incarnation id) //! diff --git a/src/instruction.rs b/src/instruction.rs index 7c1c47c..9ebfe41 100644 --- a/src/instruction.rs +++ b/src/instruction.rs @@ -112,16 +112,20 @@ pub const TAG_EMERGENCY_BURN: u8 = 5; /// Tag 6: RepairExtraAccountMetas /// -/// Rewrite the ExtraAccountMetaList PDA data for an existing NFT mint so -/// its flags match the current processor's `build_extra_account_metas` -/// output — most importantly, marking the portfolio account writable. +/// Rewrite the ExtraAccountMetaList PDA data for an existing NFT mint so its +/// flags match the current processor's `EXTRA_META_ENTRY_FLAGS`. /// -/// Historical mints produced an ExtraAccountMetaList where the portfolio was -/// declared read-only. That was wrong — the transfer hook CPIs into -/// percolator-prog with `TransferPortfolioOwnership` (tag 72), which mutates -/// `owner` in the portfolio. Without portfolio writable, the CPI fails with -/// `writable privilege escalated` and every transfer bounces. Burn + remint -/// is not a workaround: burn requires the position already be closed. +/// Current flags (see EXTRA_META_ENTRY_FLAGS in processor.rs): +/// [5] PositionNft PDA — WRITABLE. #105 removed the f_snap_at_mint write, but +/// #152/#153 added a new one: the hook writes `nft_state.last_holder` on +/// every genuine Token-2022 transfer, so this must stay writable or such a +/// transfer fails. +/// [6] Portfolio — read-only. The hook performs no invoke/invoke_signed +/// and never mutably borrows it; the pre-#105 B-3 ownership CPI that once +/// required write access moved to mint/burn. +/// +/// Historical mints may carry either shape; `RepairExtraMetas` rewrites them to +/// the current one. /// /// Permissionless by design. The only data written to the PDA is /// deterministic from the on-chain state of `nft_mint` + its `nft_pda` diff --git a/src/processor.rs b/src/processor.rs index 09d5b0f..a8e3442 100644 --- a/src/processor.rs +++ b/src/processor.rs @@ -241,6 +241,30 @@ fn registry_registers_program(data: &[u8], program_id: &Pubkey) -> bool { } } +/// `(is_signer, is_writable)` for the seven ExtraAccountMetaList entries, which the +/// Token-2022 transfer hook receives at account indices 5..=11. +/// +/// Shared by the mint path and by RepairExtraMetas so the two can never disagree. +/// RepairExtraMetas is PERMISSIONLESS, so a divergence would let any caller rewrite a +/// live NFT's metas into a shape the hook cannot use. +/// +/// Entry 5 (PositionNft PDA) must stay writable: #105 removed the f_snap_at_mint write, +/// but #152/#153 added `nft_state.last_holder = new_owner` in process_transfer_hook +/// (transfer_hook.rs:542-545), which runs on every genuine Token-2022 transfer. A +/// read-only meta there makes each such TransferChecked fail. +/// +/// Entry 6 (Portfolio) is read-only: the hook performs no invoke/invoke_signed and never +/// mutably borrows `portfolio` — it only reads it to check the NFT PDA binding. +pub(crate) const EXTRA_META_ENTRY_FLAGS: [(bool, bool); 7] = [ + (false, true), // 5: PositionNft PDA — writable (hook writes last_holder) + (false, false), // 6: Portfolio account — read-only (hook only reads) + (false, false), // 7: Percolator program — read-only + (false, false), // 8: Mint authority PDA — read-only + (false, false), // 9: Instructions sysvar — read-only + (false, false), // 10: NFT program (self) — read-only + (false, false), // 11: NFT registry PDA — read-only +]; + fn process_mint_position_nft( program_id: &Pubkey, accounts: &[AccountInfo], @@ -585,8 +609,8 @@ fn process_mint_position_nft( // Atomic ExtraAccountMetaList PDA initialization // // TLV layout (7 entries): - // [5] PositionNft PDA — writable (hook updates f_snap_at_mint) - // [6] Portfolio account — WRITABLE (B-3 CPI mutates portfolio.owner) + // [5] PositionNft PDA — WRITABLE (hook records last_holder, #152/#153) + // [6] Portfolio account — read-only (hook only reads it; B-3 CPI moved to mint/burn per #105) // [7] Percolator program — read-only (from portfolio.owner, allowlist-verified) // [8] Mint authority PDA — read-only // [9] Instructions sysvar — read-only @@ -622,7 +646,10 @@ fn process_mint_position_nft( } const EXTRA_META_ENTRY_LEN: usize = 35; - const EXTRA_META_COUNT: usize = 7; + // Derived from the shared flags table, never re-typed: EXTRA_META_ENTRY_FLAGS + // is what actually decides the entries, so a count declared independently + // could silently disagree with it. + const EXTRA_META_COUNT: usize = EXTRA_META_ENTRY_FLAGS.len(); const EXTRA_METAS_ACCOUNT_LEN: usize = 8 /* TLV type */ + 4 /* TLV length */ + 4 /* entry count */ + EXTRA_META_ENTRY_LEN * EXTRA_META_COUNT; @@ -668,22 +695,20 @@ fn process_mint_position_nft( data[8..12].copy_from_slice(&tlv_value_len.to_le_bytes()); data[12..16].copy_from_slice(&(EXTRA_META_COUNT as u32).to_le_bytes()); - let entries: [(Pubkey, bool, bool); EXTRA_META_COUNT] = [ - // 5: PositionNft PDA — writable (hook updates f_snap_at_mint on transfer) - (*nft_pda.key, false, true), - // 6: Portfolio account — WRITABLE (B-3 CPI mutates portfolio.owner) - (*portfolio.key, false, true), - // 7: Percolator program — read-only, from verified portfolio.owner - (percolator_prog_id, false, false), - // 8: Mint authority PDA — read-only - (*mint_auth.key, false, false), - // 9: Instructions sysvar — read-only - (sysvar_instructions::id(), false, false), - // 10: NFT program (self) — read-only - (*program_id, false, false), - // 11: Per-market NFT registry PDA — read-only, derived under wrapper_program_id - (registry_pda, false, false), + let keys: [Pubkey; EXTRA_META_COUNT] = [ + *nft_pda.key, + *portfolio.key, + percolator_prog_id, + *mint_auth.key, + sysvar_instructions::id(), + *program_id, + registry_pda, ]; + // Flags come from the shared table so mint and RepairExtraMetas cannot diverge. + let entries: [(Pubkey, bool, bool); EXTRA_META_COUNT] = core::array::from_fn(|i| { + let (is_signer, is_writable) = EXTRA_META_ENTRY_FLAGS[i]; + (keys[i], is_signer, is_writable) + }); for (i, (key, is_signer, is_writable)) in entries.iter().enumerate() { let off = 16 + i * EXTRA_META_ENTRY_LEN; @@ -1056,22 +1081,51 @@ fn process_emergency_burn(program_id: &Pubkey, accounts: &[AccountInfo]) -> Prog let portfolio_gone = portfolio.data_is_empty() || portfolio.lamports() == 0; // ── Check emergency burn eligibility (position flat / no active leg) ── - if !portfolio_gone { + if portfolio_gone { + msg!("EmergencyBurn: bound portfolio already closed/reclaimed by the core — skipping eligibility + unwrap (#131)"); + } else { cpi_v16::verify_portfolio_program(portfolio)?; let portfolio_data = portfolio.try_borrow_data()?; - let p = slab_types_v16::decode_portfolio(&portfolio_data) - .map_err(cpi_v16::map_decode_err)?; - // #110C: apply the same provenance check that MintPositionNft (line 304) - // enforces. Without this, a portfolio with a mismatched portfolio_account_id - // passes decode_portfolio at emergency-burn even though it would be rejected at mint. - if p.provenance_header.portfolio_account_id != portfolio.key.to_bytes() { - msg!("EmergencyBurn: portfolio_account_id mismatch (#110C)"); - return Err(NftError::InvalidNftPda.into()); + match slab_types_v16::decode_portfolio(&portfolio_data) { + Ok(p) => { + // #110C: apply the same provenance check that MintPositionNft (line 304) + // enforces. Without this, a portfolio with a mismatched portfolio_account_id + // passes decode_portfolio at emergency-burn even though it would be rejected at mint. + if p.provenance_header.portfolio_account_id != portfolio.key.to_bytes() { + msg!("EmergencyBurn: portfolio_account_id mismatch (#110C)"); + return Err(NftError::InvalidNftPda.into()); + } + cpi_v16::emergency_burn_ok(p, &nft_state_copy).map_err(ProgramError::from)?; + } + // #110B, NARROWED: only a genuine layout migration may bypass the + // eligibility gate. + // + // Skipping the gate lets a position be emergency-burned without proving it + // is flat, and this falls through to UnwrapEscrowedPortfolio, which is not + // itself leg-gated — so the set of errors that reach it must be exactly the + // set this fallback was justified by ("a future layout migration changed + // magic/version"), not every decode failure. + // + // Admitted: BadVersion / BadAccountVersion / BadLayoutDiscriminator — the + // account IS ours (wrapper-owned, verified above by verify_portfolio_program) + // and is a shape this build predates. + // + // Rejected: TooShort, BadMagic, BadKind, Cast, OwnerMismatch — on a + // wrapper-owned account these mean corruption or a wrong account, not a + // migration. OwnerMismatch in particular is a violated engine invariant and + // must never widen burn eligibility. + Err( + e @ (slab_types_v16::PortfolioDecodeError::BadVersion + | slab_types_v16::PortfolioDecodeError::BadAccountVersion + | slab_types_v16::PortfolioDecodeError::BadLayoutDiscriminator), + ) => { + msg!( + "EmergencyBurn: portfolio present but from an unknown layout version ({:?}) — skipping eligibility, proceeding to unwrap (#110B)", + e + ); + } + Err(e) => return Err(cpi_v16::map_decode_err(e)), } - cpi_v16::emergency_burn_ok(p, &nft_state_copy) - .map_err(ProgramError::from)?; - } else { - msg!("EmergencyBurn: bound portfolio already closed/reclaimed by the core — skipping eligibility + unwrap (#131)"); } // ── Verify holder owns the NFT via the canonical Token-2022 ATA ── @@ -1467,7 +1521,8 @@ fn process_repair_extra_metas( } const EXTRA_META_ENTRY_LEN: usize = 35; - const EXTRA_META_COUNT: usize = 7; + // Derived from the shared flags table — see the note at the mint path. + const EXTRA_META_COUNT: usize = EXTRA_META_ENTRY_FLAGS.len(); const HEADER_LEN: usize = 16; const EXTRA_METAS_ACCOUNT_LEN: usize = HEADER_LEN + EXTRA_META_ENTRY_LEN * EXTRA_META_COUNT; @@ -1494,15 +1549,20 @@ fn process_repair_extra_metas( data[8..12].copy_from_slice(&tlv_value_len.to_le_bytes()); data[12..16].copy_from_slice(&(EXTRA_META_COUNT as u32).to_le_bytes()); - let entries: [(Pubkey, bool, bool); EXTRA_META_COUNT] = [ - (*nft_pda.key, false, true), // 5: PositionNft PDA — writable - (*portfolio.key, false, true), // 6: Portfolio account — WRITABLE (B-3 CPI) - (percolator_prog_id, false, false), // 7: Percolator program — read-only - (*mint_auth.key, false, false), // 8: Mint authority PDA — read-only - (sysvar_instructions::id(), false, false), // 9: Instructions sysvar — read-only - (*program_id, false, false), // 10: NFT program (self) — read-only - (registry_pda, false, false), // 11: Per-market NFT registry PDA — read-only + let keys: [Pubkey; EXTRA_META_COUNT] = [ + *nft_pda.key, + *portfolio.key, + percolator_prog_id, + *mint_auth.key, + sysvar_instructions::id(), + *program_id, + registry_pda, ]; + // Same shared table as the mint path — see EXTRA_META_ENTRY_FLAGS. + let entries: [(Pubkey, bool, bool); EXTRA_META_COUNT] = core::array::from_fn(|i| { + let (is_signer, is_writable) = EXTRA_META_ENTRY_FLAGS[i]; + (keys[i], is_signer, is_writable) + }); for (i, (key, is_signer, is_writable)) in entries.iter().enumerate() { let off = HEADER_LEN + i * EXTRA_META_ENTRY_LEN; data[off] = 0; @@ -1512,7 +1572,7 @@ fn process_repair_extra_metas( } msg!( - "RepairExtraMetas: rewrote ExtraAccountMetaList for mint {} (portfolio now writable)", + "RepairExtraMetas: rewrote ExtraAccountMetaList for mint {} (nft_pda writable, portfolio read-only)", nft_mint.key ); Ok(()) @@ -1747,3 +1807,56 @@ fn rent_recipient_guard_accepts_writable_holder() { assert!(result.is_ok()); } + +#[cfg(test)] +mod extra_meta_flag_tests { + use super::*; + + /// Entry 5 is the PositionNft PDA and the transfer hook WRITES it + /// (`nft_state.last_holder = new_owner`, transfer_hook.rs:542-545, gated on + /// `is_genuine_token2022_transfer`). Flipping it read-only makes every genuine + /// Token-2022 TransferChecked fail, and because RepairExtraMetas is permissionless + /// that is a free brick-any-NFT vector. This pins the flag so the regression cannot + /// return silently — the 34 pre-existing tests all passed with it read-only. + #[test] + fn position_nft_pda_meta_entry_is_writable() { + assert_eq!( + EXTRA_META_ENTRY_FLAGS[0], + (false, true), + "entry 5 (PositionNft PDA) must be non-signer and WRITABLE: the transfer hook \ + writes last_holder to it on every genuine Token-2022 transfer" + ); + } + + /// Entry 6 is the Portfolio account. The hook performs no invoke/invoke_signed and + /// never mutably borrows it, so read-only is correct and narrows the write lock. + #[test] + fn portfolio_meta_entry_is_read_only() { + assert_eq!( + EXTRA_META_ENTRY_FLAGS[1], + (false, false), + "entry 6 (Portfolio) is read-only: the hook only reads it to check the NFT \ + PDA binding" + ); + } + + /// Everything from entry 7 on is a program/sysvar/PDA the hook only reads. + #[test] + fn remaining_meta_entries_are_read_only_non_signers() { + for (i, flags) in EXTRA_META_ENTRY_FLAGS.iter().enumerate().skip(2) { + assert_eq!( + *flags, + (false, false), + "entry {} must be a read-only non-signer", + i + 5 + ); + } + } + + /// No entry is ever a signer — the hook is invoked by Token-2022, which cannot + /// produce signatures for these accounts. + #[test] + fn no_meta_entry_is_a_signer() { + assert!(EXTRA_META_ENTRY_FLAGS.iter().all(|(s, _)| !*s)); + } +} diff --git a/src/slab_types_v16.rs b/src/slab_types_v16.rs index 3a85c3e..9644fc3 100644 --- a/src/slab_types_v16.rs +++ b/src/slab_types_v16.rs @@ -39,9 +39,9 @@ //! //! ## Compile-time guards //! -//! `const_assert!` on every sub-struct size + key field offsets fail to compile -//! if the vendored layout drifts from the engine. The LiteSVM integration test -//! (cross-cut phase) is the runtime ground truth. +//! `const_assert!` on every sub-struct size + key field offsets verify internal +//! consistency of the mirror struct. They do NOT verify alignment with the live +//! engine's layout — that requires runtime validation against real on-chain data. #![allow(dead_code)] // wired into cpi.rs / processor.rs diff --git a/src/transfer_hook.rs b/src/transfer_hook.rs index 95de242..0fadd0f 100644 --- a/src/transfer_hook.rs +++ b/src/transfer_hook.rs @@ -281,7 +281,7 @@ pub fn process_execute( let _source_authority = next_account_info(accounts_iter)?; // 3: source authority (unused per spec) let extra_metas = next_account_info(accounts_iter)?; // 4: ExtraAccountMetaList PDA let nft_pda = next_account_info(accounts_iter)?; // 5: PositionNft PDA (writable) - let portfolio = next_account_info(accounts_iter)?; // 6: Portfolio account (writable) + let portfolio = next_account_info(accounts_iter)?; // 6: Portfolio account (read-only — hook only reads it) let percolator_prog = next_account_info(accounts_iter)?; // 7: Percolator program let mint_auth = next_account_info(accounts_iter)?; // 8: Mint authority PDA let sysvar_ix = next_account_info(accounts_iter)?; // 9: Instructions sysvar