Repository navigation
feat(sdk): support tokens without the optional ERC-20 decimals() view via pool-declared and family-default decimals - #504
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CI Test Report✅ 4306/4306 tests passed (1088 suites) in 5m 56s SummaryCoverage report |
andrevmatos
requested changes
Oct 21, 2026
andrevmatos
left a comment
andrevmatos
left a comment
Collaborator
There was a problem hiding this comment.
Please address the two inline comments to minimize public interface changes and keep decimals resolution automatic.
| * @param tokenPool - Token pool address on this chain | ||
| * @returns The decimals the pool declares for its token | ||
| */ | ||
| getTokenPoolDecimals?(tokenPool: string): Promise<number> |
Collaborator
There was a problem hiding this comment.
Please expose the pool’s decimals as an optional decimals property from getTokenPoolConfig, instead of adding Chain.getTokenPoolDecimals. Decimals belong to the pool configuration. Keep the lookup and fallback logic internal, and update the callers to use the existing API. Pools without getTokenDecimals() should remain readable.
| * pools read, so destination preflights don't need the on-chain view either. | ||
| **/ | ||
| export async function parseTokenAmounts(source: Chain, transferTokens: readonly string[]) { | ||
| export async function parseTokenAmounts( |
Collaborator
There was a problem hiding this comment.
Please keep --transfer-tokens as token=amount and remove the new caller-supplied tokenAmounts[].extraData and destDecimals fields. Resolve decimals internally from the token, then its pool. For preflight conversions, use the chain family’s static decimals value as the final fallback. Keep pool-generated extraData automatic. Apply this to SDK callers too: getDestTokenAmount still reads source-token metadata directly when extraData is absent. Add tests for both sides and update the docs and recovery hints.
SyedAsadKazmi
changed the title
andrevmatos
reviewed
Oct 22, 2026
andrevmatos
left a comment
andrevmatos
left a comment
Collaborator
There was a problem hiding this comment.
Two follow-up requests on source amount parsing and RPC error handling.
Comment on lines
+754
to
+760
| const decimals = await resolveTokenDecimals(source, token, async () => { | ||
| if (!router) return undefined | ||
| const registry = await source.getTokenAdminRegistryFor(router).catch(() => undefined) | ||
| if (!registry) return undefined | ||
| return (await source.getRegistryTokenConfig(registry, token)).tokenPool | ||
| }) | ||
| return { token, amount: parseUnits(amount_, decimals) } |
Collaborator
There was a problem hiding this comment.
[P1] Keep family defaults out of source transfer amount parsing
Please require token- or pool-declared decimals before parseUnits; family defaults should apply only to preflights. I reproduced token=0.001 resolving to 1000n with a six-decimal pool, but 1000000000000000n after a registry timeout, because the resolver suppresses the error and falls back to 18. This changes the actual transfer amount by 10^12. Please propagate lookup failures here and add a regression test for this case.
| try { | ||
| decimals = await contract.decimals() | ||
| } catch (err) { | ||
| throw new CCIPTokenDecimalsUnavailableError(token, { |
Collaborator
There was a problem hiding this comment.
[P2] Preserve RPC failures instead of reporting missing decimals
This catch also converts RPC failures into CCIPTokenDecimalsUnavailableError. I reproduced a TIMEOUT becoming TOKEN_DECIMALS_UNAVAILABLE with isTransient: false. Please emit this typed error only for recognized unavailable-method or unusable-return cases, and preserve transport errors for retry or simulation-unavailable handling. Also avoid suppressing transport failures in the lazy pool resolver (resolveTokenDecimals), where they currently trigger the family-default fallback. Please add timeout regression tests for both paths.
andrevmatos
reviewed
Oct 22, 2026
andrevmatos
left a comment
andrevmatos
left a comment
Collaborator
There was a problem hiding this comment.
One further request on destination pool selection.
| .getTokenPoolRemotes(sourcePoolAddress, dest.network.chainSelector) | ||
| .catch(() => undefined) | ||
| const pools = remotes?.[dest.network.name]?.remotePools | ||
| if (pools?.length) return pools[pools.length - 1] |
Collaborator
There was a problem hiding this comment.
[P2] Use the registered destination pool for decimals resolution
Please resolve the pool through the destination TokenAdminRegistry when offRamp is available, as checkExecute does. Selecting the last source-configured remote pool does not ensure it matches the registered destination pool. I reproduced conversion using six decimals from the last remote pool while the registered pool declares eighteen; the registry is never queried. This makes conversion and execution use different decimals. Using the destination registry also removes the repeated source remote lookup and ordering assumption. Please add a regression test where the registered pool differs from the last remote pool.
SyedAsadKazmi
force-pushed
the
feat/token-decimals-fallbacks
branch
from
October 22, 2026 16:12
504dff5 to
d056a12
Compare
andrevmatos
reviewed
Oct 22, 2026
andrevmatos
left a comment
andrevmatos
left a comment
Collaborator
There was a problem hiding this comment.
Documentation corrections for recovery guidance and pool-generated message data.
| explicitly | ||
| ``` | ||
|
|
||
| `decimals()` is an **optional** view in ERC-20 — some tokens don't implement it. The CLI resolves the decimals automatically from the token pool's `getTokenDecimals()` (pool v1.5.1+); preflights additionally fall back to the chain family's default. For the transfer amount itself nothing is guessed — this error means neither the token nor its pool could declare them (e.g. a v1.5.0 pool). To declare them yourself, pass `tokenAmounts[].extraData` with `abi.encode(['uint256'], [decimals])` (the source-decimals declaration CCIP pools read). |
Collaborator
There was a problem hiding this comment.
Please remove the advice to supply tokenAmounts[].extraData as a decimals override. The CLI has no such option, and SDK send inputs (MessageInput.tokenAmounts) now accept only { token, amount }. Emitted messages and preflight inputs can carry pool data, but that is not a supported override for source amount parsing. Document automatic token/pool resolution and failure when neither declares decimals. Please align the SDK querying-data page, error reference, and recovery hints too.
|
|
||
| `decimals()` is an **optional** view in ERC-20 — some tokens don't implement it. When the token doesn't, the CLI resolves its decimals automatically from the token pool's `getTokenDecimals()` (pool v1.5.1+) — the same value the pool itself uses for its conversion. A plain `token=amount` just works; no decimals input is needed. If neither the token nor its pool can declare them, the send fails with `TOKEN_DECIMALS_UNAVAILABLE` rather than guessing — these decimals determine the actual transfer amount. | ||
|
|
||
| The source pool also declares its decimals in the message's `extraData` (the `destPoolData` it returns from `lockOrBurn`), so destination preflights never need the token's on-chain view either. The on-chain message itself ignores it. |
Collaborator
There was a problem hiding this comment.
Please correct the claim that the on-chain message ignores this data. Pool-generated destPoolData travels in the emitted message and reaches destination releaseOrMint as sourcePoolData; the destination pool uses it for decimals conversion. It is protocol data, not SDK-only metadata. Also qualify the claim that preflights never need the token view: extraData can declare source decimals, but preflights still resolve destination decimals.
andrevmatos
previously approved these changes
Oct 22, 2026
SyedAsadKazmi
force-pushed
the
feat/token-decimals-fallbacks
branch
from
October 23, 2026 17:18
47cf012 to
08d62dd
Compare
andrevmatos
approved these changes
Oct 23, 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.
Support tokens without the optional ERC-20
decimals()viewdecimals()is optional in ERC-20. Tokens that skip it used to crash the CLI with a raw provider error (Failed to call "decimals") and the SDK had no way to resolve their decimals.Decimals now resolve per side with the same fallback chain, all internal: the token's own view first, then the pool's
getTokenDecimals()(pool v1.5.1+, which is what the pool itself uses for conversion), then the chain family's static default for preflight conversions. For the actual transfer amount nothing is guessed: if neither the token nor its pool can declare them, the send fails withTOKEN_DECIMALS_UNAVAILABLE. In most cases a plaintoken=amountjust works now, no decimals input needed.SDK
EVMChain.getTokenInfothrows the new typedCCIPTokenDecimalsUnavailableError(TOKEN_DECIMALS_UNAVAILABLE, with recovery hint) for the recognized unavailable cases (a revert, or a return that can't decode) instead of the raw provider error. Transport failures (TIMEOUT, NETWORK_ERROR, ...) keep their own codes so retry handling still sees them.name()is best effort since it's optional too.getTokenPoolConfigexposes the pool's declared decimals as a new optionaldecimalsfield (v1.5.1+ pools). v1.5.0 pools predate the getter and stay readable without it; a transport failure on the getter propagates instead of silently reading as "no decimals".resolveTokenDecimalshelper holds the fallback chain in one place: token view, then pool-declared decimals, then the family's static default (ChainStatic.decimals, e.g. 18 on EVM). The family default is preflight-only;familyDefault: falseopts out where the decimals determine the actual transfer amount. The pool is resolved lazily: a resolver with nothing to resolve from returns undefined and falls through, a lookup failure propagates.estimateReceiveExecution,getRequiredCCVs, bothcheckExecutelayers) andgetDestTokenAmountuse it on both sides.getDestTokenAmountresolves the dest pool through the destination TokenAdminRegistry via the offRamp, the same waycheckExecutedoes, so conversion and execution use the same pool. BothEVMChain.estimateReceiveExecutionoverloads (messageId lookup and direct) pass the offRamp through, so direct estimation converts via the registered pool too. It no longer reads source-token metadata directly whenextraDatais absent. Pool-generatedextraData(the source pool'sdestPoolDatafromlockOrBurn) stays the automatic path.tokenAmountsentries keep their original shape.CLI
send --transfer-tokensstays plaintoken=amount. Source decimals resolve token, then source pool (via the router's registry), all insideparseTokenAmounts. No family default here: these decimals set the actual transfer amount, so a lookup failure (e.g. a registry timeout) surfaces instead of silently parsing at a guessed precision.Docs
CLI send page (Token Amount Format), troubleshooting entry, SDK querying-data note, error-reference row. README and CHANGELOG updated.
Testing
New tests for
getTokenInfoand thegetTokenPoolConfigdecimals field (stubbedeth_call),parseTokenAmountsfallbacks including a registry-timeout regression, thegetDestTokenAmount/checkExecutepool and family-default fallbacks on both sides, a registered-pool-vs-stale-remote-pool regression, a direct-estimation regression asserting the converted balance override, and timeout propagation on every resolution path. 4017/4017 unit tests,npm run checkgreen. Also verified by hand against a live Sepolia token withoutdecimals(): two sends to Fuji with plaintoken=amount, both quoted the same fee and executed on destination.