fix(abl-token): block wallets from sending, not just receiving - #672
Merged
dev-jodee merged 4 commits intoAug 12, 2026
Merged
Conversation
get_extra_account_metas() only ever configured one extra account for the transfer hook's Execute call, resolved from the destination token account's owner. tx_hook() never saw the sender's ABWallet record, so a wallet marked allowed: false could still send its full balance out to any unlisted destination in every mint mode (Allow/Block/Threshold) - exactly the case the WalletBlocked error and admin UI's "Blocked" badge imply is prevented. Add a second extra account for the source token account's owner and extend the decision matrix so an explicitly blocked wallet is rejected on either side, while leaving Allow/Threshold mode's documented "who can receive" semantics untouched. The decision logic is pulled into a standalone decide() function with unit tests covering both sides of the block check plus the existing mode semantics, since this program has no integration-test harness that exercises tx_hook via a real hooked transfer. Also fixes the frontend's manual extra-account construction in useSendTokens() to push both the source and destination ab_wallet PDAs in the same order the program now expects.
Contributor
Greptile SummaryThe PR now checks both source and destination wallet status during Token-2022 transfers and provides a permissionless migration for existing one-entry extra-account metadata lists.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the migration instruction addresses incompatible legacy metadata accounts without depending on a retained transfer-hook authority. Important Files Changed
Reviews (4): Last reviewed commit: "fix(#672): make resize_meta_list permiss..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… new layout Greptile review on PR solana-foundation#672 flagged that extra_metas_account is a fixed-size PDA created once by init_mint/attach_to_mint: mints created before the sender-side check was added are left with the old, undersized (one-entry) account, so their transfers start failing the hook's account-count check after the program upgrades - the fix has no effect for them without a migration path. Add resize_meta_list, which reallocates extra_metas_account to the current get_meta_list_size() and rewrites its contents via ExtraAccountMetaList::update (not ::init, which only handles the account's first-ever write). Authorization reuses attach_to_mint's existing pattern: a no-op transfer_hook_update CPI back to Token-2022, which only succeeds if the caller is the mint's actual transfer-hook authority - so this instruction needs no authority table of its own. Tested against a litesvm account manually seeded with the old one-entry TLV layout to confirm the migration produces byte-identical output to a fresh init_mint, plus the idempotent (already-current-size) case and rejection of a non-authority caller.
Contributor
Author
|
Hey @dev-jodee — bumping this for review when you get a chance. CI is green and Greptile came back clean (5/5, no blocking findings). Thanks! |
dev-jodee
requested changes
Aug 11, 2026
dev-jodee
left a comment
dev-jodee
left a comment
Collaborator
There was a problem hiding this comment.
don't think you ran cargo fmt as well
| use super::*; | ||
|
|
||
| #[test] | ||
| fn source_blocked_is_always_rejected() { |
Collaborator
There was a problem hiding this comment.
dont think the ci is running those test, might need
cargo test -p abl-token
|
|
||
| pub fn get_meta_list_size() -> Result<usize> { | ||
| Ok(ExtraAccountMetaList::size_of(1).map_err(|_| ProgramError::InvalidArgument)?) | ||
| Ok(ExtraAccountMetaList::size_of(2).map_err(|_| ProgramError::InvalidArgument)?) |
Collaborator
There was a problem hiding this comment.
can we have a const for that magic number
| } | ||
|
|
||
| #[test] | ||
| fn test() { |
Collaborator
There was a problem hiding this comment.
can we have a better test name
…, magic number, naming, formatting 1. The tx_hook.rs unit tests and the litesvm integration test (tests/test.rs) were never actually running in CI. `anchor test` only executes Anchor.toml's [scripts] test command, and that command was mocha-only - no workflow anywhere calls `cargo test` for anchor projects (confirmed by grepping every workflow; only the native/pinocchio/asm workflows do that). Wired `cargo test -p abl-token` into the project's own [scripts] test command, since `anchor/` has its own local Cargo workspace - this doesn't touch shared CI infra, it's picked up automatically the next time CI (or anyone) runs `anchor test` for this project. Verified the full composite command runs all 13 Rust tests plus the existing TS stub. 2. Replaced the magic number in utils.rs's `ExtraAccountMetaList::size_of(2)` with a named NUM_EXTRA_ACCOUNTS constant. 3. Renamed the generically-named `fn test()` in tests/test.rs to `init_config_and_init_mint_succeed`, describing what it actually exercises. 4. Ran `cargo fmt -p abl-token` as requested - this also reformatted change_mode.rs, init_config.rs, and init_mint.rs, which weren't touched by this PR but weren't cargo-fmt'd either (whitespace only, verified via `cargo check` + `cargo test` before and after).
Contributor
Author
|
Thanks — all addressed in `3a7a44b`:
|
Comment on lines
+50
to
+56
| let tx_hook_accs = TransferHookUpdate { | ||
| token_program_id: self.token_program.to_account_info(), | ||
| mint: self.mint.to_account_info(), | ||
| authority: self.payer.to_account_info(), | ||
| }; | ||
| let ctx = CpiContext::new(self.token_program.key(), tx_hook_accs); | ||
| transfer_hook_update(ctx, Some(crate::ID_CONST))?; |
Contributor
There was a problem hiding this comment.
If an existing mint revoked its transfer-hook authority or configured an authority that cannot sign directly, resize_meta_list always fails at transfer_hook_update before rewriting the deterministic program-owned metadata account. The mint therefore remains on the incompatible one-entry layout, causing every transfer through the upgraded hook to fail with no recovery path.
Knowledge Base Used: Tokens Directory Overview
…Greptile review The migration path required the mint's current transfer-hook authority to sign (via a transfer_hook_update CPI used purely as an auth check). Any mint whose authority was revoked or otherwise made unavailable - a legitimate, common pattern for locking in a configuration - would be permanently stuck on the old, undersized metadata list with no way to migrate, since there would be no one left who could sign. The content resize_meta_list writes is fully determined by the mint's pubkey and this program's own fixed extra-account list, so nothing about it actually needs gating behind a signature. Removed the CPI entirely and replaced it with a direct, read-only check that the mint's TransferHook extension already points at this program - proving the call is meaningful without requiring anyone to sign for it. Added a new ABListError::MintNotUsingThisHook variant (appended, not inserted, so existing error codes keep their numbers) and three regression tests: the permissionless case (called by a signer with no relationship to the mint - this is the one that fails against the old, authority-gated code and passes after), the existing idempotent-resize case, and rejection of a mint whose transfer hook points elsewhere.
Contributor
Author
|
@dev-jodee Good catch, fixed in `851211f`: The migration was gated on the mint's transfer-hook authority signing a CPI back into Token-2022 - but that meant any mint whose authority was ever revoked (a legitimate pattern for locking in a config) would be permanently stuck on the old metadata layout, since there'd be no one left who could sign. Removed the CPI entirely. The content this instruction writes doesn't depend on who calls it - it's fully determined by the mint's pubkey and the program's own fixed extra-account list - so there was nothing that actually needed a signature. Replaced it with a direct, read-only check that the mint's Added a |
dev-jodee
approved these changes
Aug 12, 2026
dev-jodee
pushed a commit
that referenced
this pull request
Aug 19, 2026
* feat(allow-block-list-token): migrate frontend to @solana/kit Moves the webapp off @solana/web3.js + @solana/wallet-adapter-react onto @solana/kit + @solana/connector, matching the sibling kit examples (nft-meta-data-pointer, world-cup). Wallet connection goes through @solana/connector/react, RPC calls use kit's typed createSolanaRpc, and program interaction goes through a Codama-generated Kit-native client built from the Anchor IDL (scripts/generate-client.ts) instead of the @anchor-lang/core Program wrapper. Rebased the migration onto origin/main's already-merged abl-token fix (#672) rather than the stale program this branch forked from, since that fix changes tx_hook's client-facing account layout: transfers now need both the sender's and receiver's ab_wallet PDA (source first, then destination), not just the receiver's. useSendTokens resolves and appends both, in the order the program's get_extra_account_metas() expects. Verified: pnpm typecheck/build/lint/format:check all pass, anchor test passes (9 unit + 5 litesvm + 1 mocha, including the source-blocked regression test), no web3.js/wallet-adapter imports remain in src/, and anchor/ has zero diff from origin/main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(allow-block-list-token): ignore generated IDL in root prettier check The root `pnpm run check` script runs prettier from the repo root, which only reads the root .prettierignore, not the app-level one - so the app-level ignore added for idl/abl_token.json (a raw copy of the anchor build output, regenerated on every `pnpm run generate-client`) had no effect on CI's root-level check. Mirrors the existing games/gacha/pinocchio/idl/ entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(allow-block-list-token): address review feedback on kit migration - remove_wallet.rs: declare ab_wallet's PDA seeds (seeds = [AB_WALLET_SEED, wallet.key()]) instead of requiring the caller to pre-derive and pass the PDA directly, so getRemoveWalletInstructionAsync({ authority, wallet }) resolves it the same way getInitWalletInstructionAsync already does. Also aligns config's seeds with the CONFIG_SEED constant, matching init_wallet.rs. Regenerated the IDL/client and simplified the two frontend callers (removeWallet, processBatchWallets) accordingly. - Bump @solana/kit and @solana/program-client-core to ^7.1.0 (both the app's own deps and the generated client's peerDependencies, via a new dependencyVersions option on the codama renderVisitor call) and @solana-program/token-2022 to ^0.15.0. - cluster-data-access.tsx: drop useClusterRpc/deriveWebsocketUrl in favor of @solana/connector's useSolanaClient across every consumer, and fix addCluster's endpoint validation, which silently accepted any string - createSolanaRpc doesn't parse its endpoint eagerly despite a comment claiming otherwise. new URL(endpoint) is the actual check. - use-send-instruction.ts: adopt @solana/connector's useTransactionPreparer for blockhash + simulation-derived compute unit limit, sourcing rpc/rpcSubscriptions for the send-and-confirm step from useSolanaClient instead of the removed custom hook. (client.sendAndConfirmTransaction, suggested in review, doesn't actually exist in the installed - and latest published - @solana/connector@0.2.6, despite one JSDoc example; kept sendAndConfirmTransactionFactory from kit for that step.) - account-data-access.tsx: useSendTokens now resolves the transfer-hook's extra accounts via @solana-program/token-2022's getTransferCheckedWithTransferHookInstructionAsync (reads the mint's on-chain extra-account-metas list) instead of hardcoding this program's ab_wallet PDA convention client-side. useRequestAirdrop now uses kit's airdropFactory, which confirms the airdrop instead of returning immediately after requesting it. useTransferSol now checks signer.address against the viewed account instead of silently signing with a possibly-different connected wallet than the page's address. (useGetBalance/useGetTokenAccounts/useGetSignatures stay on a cluster-scoped RPC call, not connector's useBalance/useTokens/ useTransactions - those hooks are scoped to the connected wallet only and don't take an address, so they can't back the generic /account/[address] page, which needs to read arbitrary addresses.) - abl-token-data-access.tsx: fixed transferHookAuthority being set to mintAuthority instead of the form's own transferHookAuthority field (a legacy bug predating this migration). mintTo now uses getMintToATAInstructionPlanAsync + flattenInstructionPlan instead of manually assembling the create-ATA and mint-to instructions. - Added anchor/tests/basic.test.ts: LiteSVM-backed tests exercising the generated Kit client directly (init_config, init_wallet, the new seeds-based remove_wallet, and an authority-mismatch rejection case). This project's `anchor test` has no local-validator step to test against - its Anchor.toml [scripts] test command fully replaces Anchor's normal build+validator+deploy flow - so a real RPC connection isn't available; LiteSVM gives the TS client something real to run against without one. The old placeholder test never actually exercised anything. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(allow-block-list-token): correct program id resolution and hook account resolution The Codama-generated instruction builders resolve their default PDAs through findConfigPda()/findAbWalletPda() with no program-address argument, so those helpers fall back to the IDL's declare_id even when the instruction itself is built for a different program. programIdForCluster pointed devnet and testnet at 6z68wfurCMYkZG51s1Et9BJEd9nJGUusjHXNt4dGbNNF, which made every write path send a PDA derived from the local program id to a different program, while getConfig (the one call site that passed the override to the PDA helper) read the correct account. That address is a system-owned wallet copied from the legacy-next-tailwind-basic template, not a deployment of this program, so the override is dropped and the generated ABL_TOKEN_PROGRAM_ADDRESS is used throughout. - useSendTokens: getTransferCheckedWithTransferHookInstructionAsync resolves the hook's extra accounts from an AccountData seed over the destination token account, which it reads over RPC. A first-ever transfer to a recipient with no associated token account therefore threw during instruction construction, because the idempotent create-ATA instruction sat in the same unsent transaction. Create the ATA in its own transaction when it is missing, and restore the preconditions the migration dropped: an explicit error when the mint has no transfer hook, and one when its extra-account-metas account does not exist (both cases otherwise degrade to a bare transferChecked that fails on chain). - Replace the `enabled` + isLoading pairs with isPending. A disabled TanStack query reports isLoading false with no data, so ClusterChecker, AccountBalanceCheck and AblTokenProgram rendered their error states on first paint, before the connector client existed. - Serialize the program account with a bigint replacer. kit types lamports and space as bigint, and the BigInt.prototype.toJSON patch lives in layout.tsx, a server component, so the browser bundle never receives it. - Classify program errors by the codes in src/generated/errors instead of matching Anchor variant names in log strings. - useTransferSol: stop swallowing send failures, and report through the transaction toasts. ModalSend renders only on the connected wallet's own account page, matching the signer check. - Give initWallet and removeWallet distinct mutation keys (both used 'change-mode'), and invalidate get-ab-wallets after each write to the list. - Derive the connector's localnet cluster from the endpoint host rather than the cluster being named 'local', and key AppProvider on the endpoint, so a custom cluster is no longer advertised to the wallet as devnet. - generate-client: prefer anchor/target/idl/abl_token.json when present and fall back to the committed idl/. The client hardcodes the program id from the IDL, so a keys-synced local build otherwise left the webapp pointing at an address the deploy never created. - Delete anchor/src/abl-token-exports.ts and anchor/src/index.ts, unreachable since the @project/anchor alias was removed, and with them the last @anchor-lang/core and @solana/web3.js imports; drop both dependencies. - Drop the unused useHasTransferHookEnabled, take LAMPORTS_PER_SOL and lamportsToSol from @solana/connector instead of redefining them, and correct the .prettierignore note about how idl/ is produced. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: amilz <85324096+amilz@users.noreply.github.com>
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.
Summary
allow-block-list-token's transfer hook only ever checked the destination wallet's allow/block status. A wallet explicitly blocked viainit_wallet(ABWallet { allowed: false }) could still send its entire balance out through an ordinaryTransferCheckedto any unlisted destination — in every mint mode (Allow/Block/Threshold), since the sender'sABWalletrecord was never read at all.This directly contradicts the
WalletBlockederror name and the "Blocked" badge shown in the admin UI, and defeats the standard real-world reason to build a block-list token (e.g. sanctions compliance: stopping the sanctioned party from moving funds out).Root cause:
utils.rs'sget_extra_account_metas()configured exactly one extra account for the hook'sExecutecall, seeded fromaccount_index: 2(the destination token account).tx_hook.rshad asource_token_accountfield on the accounts struct but never read it — the decision logic never had access to the sender's status in any mode.Fix
utils.rs— add a second extra account seeded from the source token account's owner (account_index: 0), so Token-2022 resolves and appends both the source's and destination'sab_walletPDAs to everyExecuteCPI.tx_hook.rs— addsource_ab_walletto theTxHookaccounts struct (ordered to matchget_extra_account_metas()), and extend the decision logic so a wallet with an explicitallowed: falserecord is rejected regardless of which side of the transfer it's on. Allow/Threshold mode's documented "who may receive" semantics (see this example's README) are left untouched — only the block-list's own "blocked" concept, whose name and UI already imply full blocking, becomes bidirectional.decide(mint_mode, source_mode, destination_mode, amount)function with unit tests, since this program has no integration-test harness that exercisestx_hookvia a real hooked transfer (tests/test.rsonly coversinit_config/init_mint).src/components/account/account-data-access.tsx— the demo frontend'suseSendTokens()manually constructs the hook's extra accounts rather than relying on Token-2022's standard client-side auto-resolution. Updated it to push both the source and destinationab_walletPDAs, in the same order the program now expects, so the demo UI stays consistent with the hardened program. This is a demo-consistency fix, not itself security-critical — any standards-conformant Token-2022 client picks up the corrected on-chainExtraAccountMetaListautomatically. Not verified via live browser/wallet testing (no validator available in this environment); it's a direct structural mirror of the pre-existing destination-account push, keyed on the sender's own public key instead.Out of scope
block-list/pinocchio(a separate, unrelated example) has a self-documented limitation in its own README where setting up theExtraAccountMetaListwhile the block list is still empty locks in "0 extra accounts" untilsetup-extra-metasis manually re-run after the first block. Lower severity (requires operator error, not silently wrong-by-design) and a different program — worth a follow-up PR, not bundled here.Test plan
cargo check—abl-tokencompiles clean with the new account and function signature.cargo test --lib— 9/9 pass, including the newdecide()unit tests covering both sides of the block check and the existing Allow/Block/Threshold semantics.source_blocked_is_always_rejectedfails against it, then confirmed all 9 tests pass again against the fix.anchor build— IDL regenerates with exactly 7 accounts fortx_hook, in the order Token-2022 will actually append them.pnpm exec tsc --noEmiton the frontend — clean, no type errors from theaccount-data-access.tsxchange.pnpm run lint/pnpm run format:check— clean (pre-existing warnings/diffs in untouched files left as-is).