test: repair the reconcile fixtures broken by #182 — main's CI is red - #186
Merged
Merged
Conversation
…CI is red CI on main has failed since `fix(nft): ReconcileBurnedNft reclaims mint and ExtraAccountMetaList rent (dcccrypto#182)` merged; the four commits before it are green. dcccrypto#182 changed ReconcileBurnedNft's account contract and two older PoC fixtures were not updated with it. Both are mine, from dcccrypto#179 and dcccrypto#175. Three changes, all in test fixtures -- no production code: 1. Two accounts added. dcccrypto#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 dcccrypto#184 item 4.1 and it is fixed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 31 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
main's CI has been red for two days and this fixes it. Small, test-only, no production changes — please take this ahead of my other open PRs.What broke
CI on
mainhas failed sincefix(nft): ReconcileBurnedNft reclaims mint and ExtraAccountMetaList rent (#182)merged on 2026-08-31. 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 — this is my regression to fix.Three changes, all fixtures
extra_metas(7) andtoken_program(8) required rather than optional, so the mint and ExtraAccountMetaList rent can be reclaimed whilenft_pdais still alive. The fixtures passed 7 and failedNotEnoughAccountKeys.nft_mintis now writable. Reconcile closes it to reclaim rent and rejects a read-only one — which surfaced asInvalidAccountDataonce the count was right.last_holderalongside thenft_pdarent, soPDA_RENTbecamePDA_RENT + PDA_RENT. NamedRECONCILE_PAYOUTwith the reason rather than repeating the arithmetic at three call sites.These are fixtures catching up to a deliberate behaviour change, not being relaxed: each now asserts strictly more than before — two extra accounts pinned, writability pinned, a larger payout pinned.
Why it went unnoticed for two days
cargo testcannot build as shipped.solana-sdkandproptestare declared in[dev-dependencies], referenced nowhere insrc/ortests/, andsolana-sdkdrags inopenssl-sys, which fails without a system OpenSSL. So the suite aborts before compiling a single test on any machine without one — nobody running it locally would have seen this, and CI was the only signal.That is #184 item 4.1. It is fixed in my follow-up hardening PR rather than here, to keep this one reviewable at a glance.
Verification
Both feature configurations, zero failures:
cargo test --lib --tests— 54 lib + all four PoC binaries greencargo test --lib --tests --features devnet— green(Run locally with the unused dev-deps temporarily stripped, since that is the only way the suite builds today; CI has OpenSSL and will run them as-is.)
🤖 Generated with Claude Code