Token-2022: wrappers read mint_info, refuse armed hooks, report delivered amounts - #301
Merged
Conversation
Declares mint_info(bytes32) on IHelperProgram and ISplCached — the same selector twice, because a contract must read from the track it is already on and verify_call refuses a legacy cross-state read once a cached invoke has fired in the transaction. The return is deliberately shaped around the armed state rather than presence: hookProgram and feeBps read zero when the extension is absent or present and inert, so `hookProgram != 0` is the question a caller should ask. `extensions` carries presence separately, one bit per ExtensionType discriminant, for a caller that wants to refuse a family outright. The test asserts parity in both directions — the signature hashes to the const the program dispatches on, both interfaces compile to that same selector, and the five outputs are in the program's encoding order. Signature drift between Rust and Solidity is the documented top cause of silent dispatch failures, so it fails here rather than at a dispatcher that no longer recognises the call. Wires tests/token2022 into the CI test list; a test CI does not run is not a gate. 216 passing, up from 209.
Layer 3 of the four-layer propagation, which slice 1 skipped. Three call sites in the canonical example contract: the legacy read, the cached read a cached-track contract must use instead once a cached invoke has fired, and the fee predicate -- showing that feeBps gates whether to measure the delivered amount rather than being multiplied, since the real fee is capped by maximum_fee which mint_info deliberately does not carry. Compiles; the solidity suite is 223 passing.
…use an armed hook Slice 2 items 1 and 2. They land together because the guard uses the value the constructor now already holds. Both wrapper constructors take decimals from mint_info instead of parsing the mint with load_mint. decimals is the only mint fact either needs, so the Solidity mint parser leaves the wrapper path entirely: no length bound to guess, no TLV read in Solidity, and structure validated where it belongs, by SPL's unpack inside the program. Each reads on its own track -- legacy via HelperProgram, cached via SplCached -- because verify_call refuses a legacy cross-state read once a cached invoke has fired, so a cached contract reading the legacy home would fail mid-transaction. Three paths can bring a wrapper into existence, and all three now refuse an armed hook: the factory, and each constructor. The second constructor matters because the deploy scripts call it directly and the factory gate never sees them. The factory checks before deploying rather than relying on a failing CREATE, which would burn the caller's gas and revert without saying why. A present-but-unarmed hook is accepted, and the tests pin it. Its program_id is zero, no CPI is reached, and the mint transfers perfectly well -- refusing on presence would reject most real Token-2022 mints. The error names both the mint and the hook so a refusal is diagnosable. Tests assert the wiring that can silently regress: each path keys on the armed hook, each reads its own track, and neither wrapper parses mint bytes any more. Suite is 232 passing, up from 223.
…omputed Slice 2 item 5, and the last of it. Both wrappers emitted the caller-requested value verbatim, which is wrong the moment a transfer fee is armed: SPL credits the destination less than was asked for, so every downstream holder of that event saw a number that never arrived. Measured rather than computed, for a reason that is not stylistic. SPL caps the fee at maximum_fee, so ceil(amount * bps / 10_000) is wrong above the cap -- and mint_info deliberately does not carry maximum_fee, because adding it would only move SPL's arithmetic into Solidity. feeBps is therefore a predicate: it decides whether the extra balance reads are worth paying for, and legacy and zero-fee transfers pay nothing. Each wrapper reads on its own track. The self-transfer case flagged during the design pass turned out to be a real bug in the first cut of this change, not a hypothetical. Sending to yourself with an armed fee debits value and credits value minus fee, so the account nets MINUS the fee -- after minus before underflows, and Solidity 0.8 reverts. ERC-20 self-transfer must not revert. Measuring the loss instead gives the delivered amount in both directions, and it is one conditional. Tested through the repo's pure-logic mirror convention: both directions, the uncapped and capped fee showing why a bps computation would be wrong, and a guard that neither wrapper carries SPL's arithmetic. Suite is 237 passing, up from 232.
Slice 4. The token program is part of the ATA seeds, so an ATA for a Token-2022 mint sits at a different address than the same mint under legacy SPL Token. UserPda.ataForKey hardcoded the legacy program, so all three raw-pubkey call sites derived one address while HelperProgram.create_ata_for_key created another, and the transfer then targeted an account that never existed. That broke bridge-out for every Token-2022 mint, extension-free ones included -- a bug independent of any extension. Adds ataForKeyWithProgram beside ataForKey, mirroring how ataWithProgram already sits beside ata, and migrates all three callers to resolve the program from mint_info: bridgeOutToSolana, ensureRecipientAta, and RomeBridgeWithdraw. Today's bridged assets are legacy SPL, so this is behaviour-neutral now and correct when that changes. No program-aware batch variant. atas has no caller outside its own test wrapper, so building one would be surface with no consumer; it is documented as legacy-only instead and gains a variant when something needs it. This deviates from the plan, which said to fix both sites, and the reason is that only one of them is actually broken. UserPda.ata needed no change and its comment claimed otherwise -- it delegates to HelperProgram.ata, which resolves the program from the mint's owner in Rust. The comment is corrected and a test pins it, because the next reader would otherwise "fix" a working path. Byte-identity of the derivation is not asserted here: it runs through Rome's find_program_address precompile, which hardhat does not provide, for the same reason tests/cpi/UserPda.test.ts gates its integration cases on a live stack. That is funded-suite work. The wiring is asserted, which is what can silently regress. Suite is 240 passing, up from 237.
The previous commit added ataForKeyWithProgram and made three callers pass a program they had to look up themselves. That was the wrong precedent to copy: ataWithProgram exists as an escape hatch for callers that already know the program, is unused, and is documented as reserved. The working primary is ata(address,bytes32), which resolves the program from the mint's owner inside HelperProgram and asks nothing of its callers. So ataForKey now does the same. One function, correct for legacy SPL Token and Token-2022 alike, and callers just call it. That removes the two problems the sibling created: a legacy-only entry point left reachable in a shared library where a doc comment is the only thing discouraging its use, and three call sites each having to remember to resolve and thread a program. It becomes view rather than pure, which is why it was not written this way originally. That costs nothing here -- all three callers already mutate state, and the mint read moved from the caller into the helper rather than being added. ataWithProgram is untouched and stays as the explicit-program escape hatch; an over-broad edit in the previous commit had removed it, restored here. atas is still legacy-only with no caller outside its test wrapper, documented rather than given a per-mint resolve it would pay for N times. 241 passing.
_balance_of copied balanceOf's body verbatim, because balanceOf was external and so not callable internally. The legacy wrapper's is public, which is why that one just called it. Made the cached one public too and deleted the duplicate. Same class of mistake as the ataForKeyWithProgram sibling: a second entry point for something that already existed, instead of adjusting the existing one.
…ected Cleanup pass over the edits. parseMint was left orphaned by the wrapper change -- its only remaining caller is a bench probe -- but it still rejected any mint longer than 82 bytes, which is a latent copy of the bug this work exists to fix, waiting for its next caller. Fixed in place rather than left: the base layout it parses is byte-identical under Token-2022, with the account-type byte and TLV region trailing it, so the guard only has to reject SHORT data. load_mint now also checks the account is owned by a token program at all, which it previously read and discarded. The wrappers do not use either any more -- they take decimals from mint_info -- and the doc says so, so nobody reintroduces mint parsing on that path. Two comments in erc20spl.sol still described ataForKey as a "deterministic two-step" derivation with a hardcoded program. It now resolves the token program from the mint, so both are corrected to say why: the program is part of the ATA seeds, so assuming it derives an address create_ata_for_key never creates. Also simplified a guard I had written as `!= LEN && <= LEN`, which is just `< LEN`. 241 passing.
The admission tests asserted on source text. That catches a deleted check, but it would pass just as happily if the check were unreachable, or if a wrapper refused a present-but-*unarmed* hook — the failure that would reject most real Token-2022 mints. mint_info is the only precompile either constructor calls, so a stateless mock installed at the two precompile addresses with hardhat_setCode is enough to run them for real. It derives its answer from the mint id, which keeps it stateless under setCode (code is copied, storage is not). Presence is a separate byte from arming in that layout, deliberately. The first version derived the extension bitmap from the armed state and so could not express a present-but-inert hook — which meant a wrapper keying on presence passed every test. With them separated, that mutation fails two. The header comment claimed behaviour was the funded matrix's job because no armed-hook mint existed to point a run at. Both halves are now wrong: it is a committed fixture the CI validator loads at genesis, and the refusal against it is asserted on a live stack in the tests repo.
…eir source Three source-text assertions were carrying weight they could not hold. Track: "each wrapper reads mint_info on its own track" was a grep. Installing the mock at one precompile address at a time makes it behaviour — the wrapper whose home is absent must fail to construct. It matters because verify_call refuses a legacy cross-state read once a cached invoke has fired, so a cached wrapper reading the legacy home fails mid-transaction, on a path no local test reaches. Derivation: ata-derivation asserted source and deferred byte-identity to a funded run because hardhat has no find_program_address. Byte-identity against Solana does need a chain, but the property that actually broke bridge-out does not — an ATA's seeds include the token program, so two mints differing in nothing but the program they report must derive to different addresses. Mocking find_program_address as a hash of its inputs is enough to observe that. Hardcoding the program back, which is the original bug, fails the test. Factory: its gate was source-only. The add-existing-mint path needs no mint creation, so mocking mint_info runs it end to end — an armed hook reverts before any CREATE, an inert one registers a wrapper. Mutations verified for each: presence-instead-of-armed fails 2, cached-reads-legacy fails 2, hardcoded-program fails 1.
…using it add_spl_token_with_metadata exists to take a wrapper's name and symbol from the mint rather than from whoever registers it, but it looked only in the Metaplex account. A Token-2022 mint carries its identity inside the mint itself, under its own metadata authority — MetadataPointer alongside TokenMetadata — so for those mints the function reverted "Metadata does not exist" about a mint that plainly had metadata, in a place it did not look. Native is read first, because it is the source closest to the mint; Metaplex stays the fallback and is what legacy SPL mints use. Strictly more inputs now succeed, and the message when none is found says which condition failed. add_spl_token_no_metadata is untouched. Choosing your own label stays allowed: anyone can deploy their own factory and their own wrapper on any chain, and curation is what answers that — Rome's registry already declines to carry permissionless registrations. A gate here would be a restriction neither Solana nor Ethereum has. Symbol uniqueness is likewise untouched: that path already derived symbols from Metaplex, so this introduces no new class of collision. The TLV walk lives in SplTokenLib beside the mint parsing already there, bounds every length against the extension's own end, and treats absence as a value rather than an error. Asserted against the real ai16z mint bytes committed as a CI fixture, following tests/oracle/PythPullParser.test.ts. Not read: a MetadataPointer aimed at a separate account. Those fall through to the Metaplex attempt — today's behaviour — because the self-referential shape is what real mints use and is the one verified against actual bytes. Mutations: wrong extension type, wrong payload offset, and skipping the native read each fail a test.
Its input is a mint account — data someone else controls — and every length in it is attacker-chosen, so the parser has to survive a hostile one rather than trust it. Five cases: an entry claiming more bytes than the account holds, a zero-length entry of the type being looked for, a name length larger than its own entry, a buffer with no terminator, and truncated or empty accounts. The no-terminator case is the one worth having. Every entry advances the cursor by at least its 4-byte header, so a buffer of zero-length entries cannot loop forever — but that is a property of the code rather than of the format, and an unbounded loop over attacker data is the failure mode this parser most needs to not have.
anil-rome
marked this pull request as ready for review
July 25, 2026 13:45
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
1 participant
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.
Wrapper-side Token-2022 support. Companion to rome-evm-private #464, which must merge first — the
mint_infoselector these contracts call does not exist on master.One gate blocked every extension-bearing mint
parseMintrequired exactly 82 bytes. A Token-2022 mint with extensions is longer — the base layout is byte-identical and the extensions trail it — so every such mint was rejected before anything else was tried. Extension-free 2022 mints already worked.Solidity no longer parses mint bytes
Both wrappers take
decimalsfrommint_infoinstead of unpacking the mint themselves. That removes the second interpretation of the same bytes, which is what drifted from the program's in the first place.mint_info(bytes32)onIHelperProgramandISplCachedverify_callrefuses a legacy cross-state read once a cached invoke has fired, so a cached wrapper needs its own home. One interpreter behind both.maximum_fee, so bps arithmetic is wrong above the cap.feeBpsis a predicate, not an operand.ataForKeyresolves the program from the mintcreate_ata_for_keynever created — bridge-out was broken for every 2022 mintparseMintaccepts>= 82,load_mintchecks the ownerRegistration reads the mint's own identity
add_spl_token_with_metadataexists to take a wrapper's name from the mint rather than from whoever registers it — but it looked only in the Metaplex account. A Token-2022 mint carries its identity inside the mint, under its own metadata authority, so for those mints it reverted"Metadata does not exist"about a mint that plainly had metadata, in a place it did not look.Native is read first, Metaplex stays the fallback. Strictly more inputs succeed; nothing that worked stops working.
add_spl_token_no_metadatais untouched — choosing your own label stays allowed. Anyone can deploy their own factory and their own wrapper on any chain, and curation answers squatting: Rome's registry already declines to carry permissionless registrations. A gate here would be a restriction neither Solana nor Ethereum has. Symbol uniqueness is untouched too — that path already derived symbols from Metaplex, so this adds no new class of collision.The TLV walk lives in
SplTokenLibbeside the mint parsing already there. It bounds every length against the extension's own end and treats absence as a value rather than an error.Not read, deliberately
A
MetadataPointeraimed at a separate account falls through to the Metaplex attempt — today's behaviour. The self-referential shape is what real mints use and is the one verified against actual bytes. Nothing forces the two wrappers over one mint to agree on a name, because nothing should.Verification
hardhat compileclean · 263 passing (241 at the branch point).The admission and derivation tests execute rather than reading source. A stateless
mint_infomock installed withhardhat_setCoderuns the real constructors; presence is a separate byte from arming in its layout, so a wrapper keying on presence fails. Installing the mock at one precompile at a time turns "each wrapper reads its own track" into behaviour rather than a grep.Mutation-verified, each failing a test: presence-instead-of-armed (2), cached-reads-legacy (2), hardcoded program — the original
ataForKeybug (1), wrong extension type (1), wrong payload offset (1), skipping the native read (1).Red team on the TLV walk, whose input is data someone else controls: an entry claiming more bytes than the account holds, a zero-length entry of the type sought, a name length larger than its own entry, a buffer with no terminator, and truncated or empty accounts. The no-terminator case is the one worth having — an unbounded loop over attacker data is the failure mode this parser most needs to not have.
Parsing is asserted against the real ai16z mint bytes, committed as a CI fixture, following
tests/oracle/PythPullParser.test.ts.Residual
Wrapper
name()/symbol()are asserted once at registration and immutable. Neon live-reads Metaplex on every call, which cannot be squatted but makes the ERC-20 name mutable by the metadata authority and forbids both custom labels and metadata-less mints. Ours is the better trade; a UI wanting to show "the mint now says X" reads Solana, not a new field here.