From 75a7fd888e6d9cc54035dd0b439868dca9c12710 Mon Sep 17 00:00:00 2001 From: 0X-SquidSol Date: Wed, 2 Sep 2026 10:26:29 -0400 Subject: [PATCH] =?UTF-8?q?test:=20repair=20the=20reconcile=20fixtures=20b?= =?UTF-8?q?roken=20by=20#182=20=E2=80=94=20main's=20CI=20is=20red?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on main has failed since `fix(nft): ReconcileBurnedNft reclaims mint and ExtraAccountMetaList rent (#182)` merged; the four commits before it are green. #182 changed ReconcileBurnedNft's account contract and two older PoC fixtures were not updated with it. Both are mine, from #179 and #175. Three changes, all in test fixtures -- no production code: 1. Two accounts added. #182 made `extra_metas` (7) and `token_program` (8) REQUIRED rather than optional, so the mint and ExtraAccountMetaList rent can be reclaimed while nft_pda is still alive. The fixtures passed 7 accounts and failed with NotEnoughAccountKeys. 2. `nft_mint` is now writable. Reconcile closes it to reclaim its rent and rejects a read-only one, which surfaced as InvalidAccountData once the account count was right. 3. The expected payout doubled. Reconcile now sweeps the ExtraAccountMetaList rent to `last_holder` alongside the nft_pda rent, so the fixtures' PDA_RENT expectation became PDA_RENT + PDA_RENT. Named as RECONCILE_PAYOUT with the reason, rather than repeating the arithmetic at three call sites. These are the fixtures catching up to a deliberate behaviour change, not a relaxation: each one now asserts MORE than it did (two extra accounts pinned, writability pinned, a larger payout pinned). Worth noting how this went unnoticed for two days: `cargo test` cannot build as shipped, because `solana-sdk` and `proptest` are declared as dev-dependencies, used nowhere, and drag in openssl-sys. So nobody running the suite locally would have seen it -- only CI, which nobody was watching. That is #184 item 4.1 and it is fixed separately. Co-Authored-By: Claude Opus 5 --- tests/poc_pda_revival_brick.rs | 13 +++++++++++-- tests/poc_stale_last_holder.rs | 27 +++++++++++++++++++++------ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/tests/poc_pda_revival_brick.rs b/tests/poc_pda_revival_brick.rs index d79e464..f210f3a 100644 --- a/tests/poc_pda_revival_brick.rs +++ b/tests/poc_pda_revival_brick.rs @@ -138,17 +138,26 @@ fn run_reconcile() -> (Result<(), ProgramError>, AccountInfo<'static>) { let (mint_auth, _) = mint_authority_pda(&PROG); let (nft_pda_key, bump) = position_nft_pda(&PORTFOLIO, MARKET_ID, &PROG); let (registry, _) = derive_nft_registry(&PERCOLATOR_MAINNET, &MARKET_GROUP); + let (extra_metas, _) = extra_account_metas_pda(&NFT_MINT, &PROG); let nft_pda = acct(nft_pda_key, PROG, live_nft_pda_buf(bump), PDA_RENT, true, false); let accounts = vec![ nft_pda.clone(), - acct(NFT_MINT, TOKEN_2022_PROGRAM_ID, mint_account(0), 0, false, false), + // #182: the mint must be WRITABLE now — reconcile closes it to reclaim + // its rent, and refuses a read-only one. + acct(NFT_MINT, TOKEN_2022_PROGRAM_ID, mint_account(0), 0, true, false), acct(PORTFOLIO, PERCOLATOR_MAINNET, portfolio_buf(mint_auth.to_bytes()), 0, true, false), acct(mint_auth, Pubkey::default(), vec![], 0, false, false), acct(registry, PERCOLATOR_MAINNET, registry_buf(), 0, false, false), acct(PERCOLATOR_MAINNET, Pubkey::default(), vec![], 0, false, false), - acct(OWNER, Pubkey::default(), vec![], 0, true, false), // last_holder + acct(OWNER, Pubkey::default(), vec![], 0, true, false), // 6 last_holder + // #182 made these two REQUIRED rather than optional, so that the mint + // and ExtraAccountMetaList rent can be reclaimed while nft_pda is still + // alive. This fixture predates that change (#179) and was not updated + // when it landed, which is what turned main's CI red. + acct(extra_metas, PROG, vec![0u8; 8], PDA_RENT, true, false), // 7 (writable, closed) + acct(TOKEN_2022_PROGRAM_ID, Pubkey::default(), vec![], 0, false, false), // 8 ]; // Permissionless: assert it, do not merely assume it. diff --git a/tests/poc_stale_last_holder.rs b/tests/poc_stale_last_holder.rs index 2da9e25..8871d00 100644 --- a/tests/poc_stale_last_holder.rs +++ b/tests/poc_stale_last_holder.rs @@ -278,6 +278,14 @@ fn run_hook_inner( /// Drive `ReconcileBurnedNft` (tag 7) with `recipient` supplied as account 6. /// Returns the result plus the recipient's lamports afterwards. +/// What a successful reconcile now pays the recorded holder. +/// +/// #182 made reconcile reclaim the ExtraAccountMetaList rent as well as the +/// nft_pda rent, both to `last_holder`. The fixture funds each PDA with +/// PDA_RENT, so the payout is two of them. (The mint in this fixture holds 0 +/// lamports, so closing it contributes nothing here.) +const RECONCILE_PAYOUT: u64 = PDA_RENT + PDA_RENT; + fn run_reconcile( last_holder_in_state: [u8; 32], recipient: Pubkey, @@ -285,18 +293,25 @@ fn run_reconcile( let (mint_auth, _) = mint_authority_pda(&PROG); let (nft_pda_key, bump) = position_nft_pda(&PORTFOLIO, MARKET_ID, &PROG); let (registry, _) = derive_nft_registry(&PERCOLATOR_MAINNET, &MARKET_GROUP); + let (extra_metas, _) = extra_account_metas_pda(&NFT_MINT, &PROG); let recipient_ai = acct(recipient, Pubkey::default(), vec![], 0, true); let accounts = vec![ acct(nft_pda_key, PROG, nft_pda_buf(bump, last_holder_in_state), PDA_RENT, true), - // supply == 0: the NFT really was burned out of band - acct(NFT_MINT, TOKEN_2022_PROGRAM_ID, mint_account(0), 0, false), + // supply == 0: the NFT really was burned out of band. + // #182: writable, because reconcile now closes the mint to reclaim rent. + acct(NFT_MINT, TOKEN_2022_PROGRAM_ID, mint_account(0), 0, true), acct(PORTFOLIO, PERCOLATOR_MAINNET, portfolio_buf(mint_auth.to_bytes()), 0, true), acct(mint_auth, Pubkey::default(), vec![], 0, false), acct(registry, PERCOLATOR_MAINNET, vec![], 0, false), acct(PERCOLATOR_MAINNET, Pubkey::default(), vec![], 0, false), - recipient_ai.clone(), + recipient_ai.clone(), // 6 last_holder + // #182 made these REQUIRED: the mint and ExtraAccountMetaList rent can + // only be reclaimed while nft_pda is still alive, so reconcile is the + // one place it can happen. This fixture predates that change. + acct(extra_metas, PROG, vec![0u8; 8], PDA_RENT, true), // 7 (writable, closed) + acct(TOKEN_2022_PROGRAM_ID, Pubkey::default(), vec![], 0, false), // 8 ]; let r = processor::process(&PROG, &accounts, &[TAG_RECONCILE_BURNED_NFT]); @@ -362,7 +377,7 @@ fn seller_is_paid_the_escrowed_portfolio_and_the_rent() { let (r, alice_lamports) = run_reconcile(ALICE.to_bytes(), ALICE); assert!(r.is_ok(), "reconcile to the stale seller succeeds: {r:?}"); assert_eq!( - alice_lamports, PDA_RENT, + alice_lamports, RECONCILE_PAYOUT, "PDA rent swept to the seller (the unwrap CPI likewise names her as new owner)", ); } @@ -374,7 +389,7 @@ fn control_a_correctly_recorded_buyer_can_reconcile() { // processor.rs:1275 as the sole reason Bob was refused above. let (r, bob_lamports) = run_reconcile(BOB.to_bytes(), BOB); assert!(r.is_ok(), "with the buyer correctly recorded, he recovers: {r:?}"); - assert_eq!(bob_lamports, PDA_RENT); + assert_eq!(bob_lamports, RECONCILE_PAYOUT); } // -- 5. the fix --------------------------------------------------------------- @@ -403,7 +418,7 @@ fn buyer_recorded_by_the_fix_can_reconcile_and_seller_cannot() { let (bob_r, bob_lamports) = run_reconcile(recorded, BOB); assert!(bob_r.is_ok(), "the real owner recovers his own position: {bob_r:?}"); - assert_eq!(bob_lamports, PDA_RENT); + assert_eq!(bob_lamports, RECONCILE_PAYOUT); let (alice_r, _) = run_reconcile(recorded, ALICE); assert!(