Fix code inconsistencies across various programs#12
Merged
mikemaccana merged 8 commits intoMay 11, 2026
Merged
Conversation
…cient balance, remove unused InvalidMint Three related issues in the token-swap program: - Migrate token::transfer to token::transfer_checked. transfer_checked passes the mint and decimals through the CPI, which guards callers from decimal-mismatch bugs and is the modern recommended path. - In deposit_liquidity and swap_exact_tokens_for_tokens, fail with the new TutorialError::InsufficientBalance instead of silently clamping the requested amount to the user's available balance. The silent clamp broke slippage protection for callers building on top of this program: they expected their input amount to be the amount actually used, and their min_output_amount was computed against that input. Clamping let trades succeed on worse terms than the caller asked for. - Remove the unused TutorialError::InvalidMint variant. Mint mismatches are already handled by Anchor's has_one = mint_a / has_one = mint_b constraints on the pool, so emitting a custom error from program code would never trigger. Dead error variants are confusing for readers.
…ative) Issues 4 and 5 from the 2026-04-22 audit: - Unknown instruction discriminants previously logged a message and returned Ok(()) in both basics/counter/native and basics/counter/pinocchio. Return ProgramError::InvalidInstructionData instead so callers actually see a failure. Also reject empty instruction data explicitly rather than panicking via split_at(1). - basics/counter/native had no owner check on the counter account, so the program would happily decode any 8 bytes of data as a Counter even when the account belonged to a different program. Add an owner check (counter_account.owner == program_id) and return IncorrectProgramId on mismatch. Tests: - Add test_unknown_instruction_fails for both native and pinocchio variants. - Add test_wrong_owner_fails for native. - Tighten the existing happy-path test to actually assert the transaction succeeded (it previously discarded the result via `let _ = ...is_ok();`). - Refactor program_bytes into a module-level PROGRAM_SO const to avoid repeating the include_bytes! path.
…legum id The bubblegum_program account in cnft-burn::BurnCnft and in cnft-vault's Withdraw / WithdrawTwo was declared as UncheckedAccount with no constraint on its address. Each program already defines a MPL_BUBBLEGUM_ID constant and the account is forwarded into a CPI, so a caller could substitute a different account and steer the cross-program invocation somewhere unexpected. Add #[account(address = MPL_BUBBLEGUM_ID)] to the relevant fields. The mpl- bubblegum crate is not in scope as a dependency in this workspace, so the address constraint with the existing constant is the lowest-touch fix - it preserves the UncheckedAccount shape used by the rest of the example.
…t_b rent to maker Two related issues in the escrow anchor program: - Add a cancel_offer instruction (issue 7). Without it, an abandoned offer would leave the maker's token-A locked in the vault forever and the offer account's rent unrecoverable. cancel_offer requires the maker's signature, returns vault tokens to the maker, closes the vault, and closes the offer account (rent refunded to the maker via close = maker). - Shift the rent burden for the maker's token-B ATA from the taker to the maker (issue 8). It used to be init_if_needed on the take_offer side with payer = taker, meaning the taker paid the maker's rent. Move the init_if_needed onto make_offer with payer = maker, and treat the account as an existing (mut) ATA on take_offer. The party who chose to open the offer now pays its rent. Tests: - Update existing make_offer and take_offer tests to pass the new maker_token_account_b on the make side. - Add test_cancel_offer (maker cancels and gets tokens + rent back). - Add test_cancel_offer_rejects_non_maker (the has_one = maker + seed-based PDA constraints reject a non-maker signer).
…e_default_account_state with mint_authority The update_default_account_state CPI was signed by `payer` but the freeze authority on the mint is `mint_authority` (set as Some(mint_authority.key) in initialize_mint just above). The CPI only succeeded when the caller happened to pass the same pubkey for both the payer and the mint authority slots. Anyone passing a separate payer would have been silently rejected. Fix by signing the update with mint_authority (the actual freeze authority recorded on the mint). The requirement that the freeze authority sign is now explicit in the code rather than implicit through identical pubkeys.
…ervice The carnival example used to log a 'sorry' message and return Ok(()) for every refusal branch (not enough tickets, rider too short), and error.rs was empty. Callers and tests could not tell a successful ride/game/snack from a refusal, and the existing tests batched a list of mixed valid and invalid attempts through .unwrap() - they were 'passing' precisely because rejections silently succeeded. Changes: - Add a CarnivalError enum with NotEnoughTickets and RiderTooShort. - Return CarnivalError variants from the three handlers on the refusal branches (eat_food, get_on_ride, play_game). - Rewrite test_carnival.rs to test the success path and each rejection path separately, asserting is_err() on the rejection paths.
…ag unaudited session-keys crate near declare_id This example depends on the third-party `session-keys` crate, which has not been independently audited. Anyone reading the example landing on declare_id! should be told that up front, so add an explicit warning comment right above declare_id! explaining that this is for education only and pointing at what to do before considering it for production.
This was referenced May 11, 2026
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
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.
Fixes the 11 code inconsistencies flagged in the 2026-04-22 audit across various program examples. One commit per logically-grouped fix.
Fixes
TutorialError::InvalidMintwas defined but never emitted (Anchorhas_onealready handles the mint-mismatch check upstream). Remove the dead variant. (66d2dee)token::transfertotoken::transfer_checkedacrossdeposit_liquidity,swap_exact_tokens_for_tokens, andwithdraw_liquidityso the mint and decimals are validated on every transfer. (66d2dee)InsufficientBalanceerror when the requested amount exceeds the available balance. (66d2dee)Ok(()). Both variants now returnProgramError::InvalidInstructionData. (2cf93df)account.owner == program_id) that returnsProgramError::IncorrectProgramIdon mismatch. (2cf93df)bubblegum_programwas anUncheckedAccountwith no program id check. Pin it via#[account(address = MPL_BUBBLEGUM_ID)](the constant already lives in the crate). (f90ac8e)cancel_offerinstruction: maker-signed, returns vault tokens to the maker, closes the vault, closes the offer account (rent refunded to maker viaclose = maker). (b0da596)init_if_neededonmaker_token_account_bon the take side meant the taker paid the maker's rent. Move the init tomake_offerwithpayer = maker, treat as plainmutontake_offer. (b0da596)update_default_account_statewas signed bypayer, but the freeze authority recorded on the mint ismint_authority. The CPI only worked by coincidence when the caller used the same account for both. Sign withmint_authorityto make the requirement explicit. (6a1c095)error.rswas empty and every refusal branch in the carnival handlers logged a message and returnedOk(()). Tests batched valid + invalid attempts through.unwrap()and "passed" because rejections silently succeeded. Add aCarnivalErrorenum (NotEnoughTickets,RiderTooShort), return real errors from the refusal branches, and split the tests into one-success-or-one-rejection cases that actually assertis_err()on rejections. (d2f0320)session-keyscrate is used without any warning. Add a comment block abovedeclare_id!explicitly flagging it as educational only and pointing at what to harden before any production use. (88f0380)Verification
For every program touched I ran the local LiteSVM /
node:testsuites and confirmed they pass before pushing:tokens/token-swap-cargo test: 4 passed.basics/counter/native-pnpm build-and-test: 3 passed (existing 2 + newtest_unknown_instruction_failsandtest_wrong_owner_fails).basics/counter/pinocchio-pnpm build-and-test: 2 passed (existing + newtest_unknown_instruction_fails).compression/cnft-burn,compression/cnft-vault-cargo check(no Rust test suite in repo).tokens/escrow/anchor-cargo test: 4 passed (test_make_offer,test_take_offer, newtest_cancel_offer, newtest_cancel_offer_rejects_non_maker).tokens/token-extensions/default-account-state/native-pnpm build-and-test: 1 passed (existing TS suite still green).basics/repository-layout/anchor-cargo test: 8 passed (success + rejection cases per instruction).tokens/token-extensions/nft-meta-data-pointer/anchor-example- comment-only; pre-existing build failure of thesession-keyscrate is unchanged (not introduced by this PR).Note
Medium Risk
Medium risk: changes touch on-chain token/CPI flows (escrow, token-swap, cNFT CPI) and alter failure semantics, which can impact client expectations and fund movement if mis-specified.
Overview
Improves correctness and safety of several Solana program examples by turning previously silent/ambiguous behaviors into explicit failures and tightening CPI/account validation.
In
basics/counter(native + pinocchio), empty/unknown instruction data now returnsInvalidInstructionData, native increment additionally enforcescounter_account.owner == program_id, and tests now assert success/failure cases (including unknown discriminants and wrong-owner accounts).In the Anchor
carnivalexample, introducesCarnivalErrorand changes ride/game/food “refusal” branches to return real errors instead ofOk(()), with tests split to explicitly expect either success or rejection.Hardens compressed NFT CPI examples by constraining
bubblegum_programtoMPL_BUBBLEGUM_IDincnft-burnandcnft-vaultwithdraw instructions.Extends the Anchor
escrowexample with a maker-signedcancel_offerthat returns vault funds and closesvault/offer, and shifts initialization of the maker’s token-B ATA tomake_offer(so the maker—not the taker—pays rent); tests updated accordingly.In token examples, fixes
default-account-stateto signupdate_default_account_statewithmint_authority, adds a warning comment about unauditedsession-keysusage, and updatestoken-swapto remove deadInvalidMint, fail fast withInsufficientBalance(no balance clamping), and usetransfer_checkedfor mint/decimal validation in deposit/swap/withdraw.Reviewed by Cursor Bugbot for commit 26e37b0. Bugbot is set up for automated code reviews on this repo. Configure here.