harden: say a swap's output token is unknown rather than calling it ETH (closes #353) #356
Reference in New Issue
Block a user
Delete Branch "harden/353-v4-undetermined-token-out"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #353.
The determination: null means undetermined, not native ETH
The investigation the issue asked for, from the encoding rather than from this
repo's code.
Currencyis an address, and native ETH isaddress(0).Uniswap/v4-core,src/types/Currency.sol:type Currency is address;, withCurrency public constant ADDRESS_ZERO = Currency.wrap(address(0));andisAddressZero()comparing against it. So native ETH has a value in theencoding; it is not an absence.
Currencyis ABI-encoded as a plain address word. A user-defined valuetype over
addresshas theaddressABI encoding — one 32-byte word. EveryV4 field this decoder reads is such a word:
SETTLE (Currency, uint256, bool),TAKE (Currency, address, uint256),ExactInputParams.currencyIn,PathKey.intermediateCurrency,PoolKey.currency0/currency1.Uniswap/v4-periphery,src/libraries/Actions.solconfirms the action IDsthis file decodes:
SWAP_EXACT_IN_SINGLE = 0x06,SWAP_EXACT_IN = 0x07,SETTLE = 0x0b,TAKE = 0x0e.42-character hex string for an
addressword; foraddress(0)it returns thetruthy
"0x0000000000000000000000000000000000000000", whichtokenInfo()already maps to ETH. A test asserts exactly this decode, so the claim is
checked rather than argued.
Empirical corroboration already in the suite:
tests/uniswap.test.jsdecodes areal mainnet USDT->ETH V4 swap (the repo's first-ever swap) and gets
Swap USDT -> ETH. Its native-ETH output side reaches the decoder as the zeroaddress, not as null.
Conclusion: undetermined.
decodeV4Swap()returns null fortokenOutonlywhen no sub-action yielded a currency at all — a
paramsblob that did notdecode, no
TAKE, or anExactInputParamswith an emptypath. The two ways aswap genuinely outputs ETH are both already handled without null: the zero
address (named ETH by
tokenInfo()), andUNWRAP_WETH, caught byhasUnwrapWethbeforetokenInfo()is consulted. So the conservative branchand the correct branch coincide here.
The change
src/shared/uniswap.jsonly:outputTokenis no longer passed totokenInfo(). It resolves to{symbol: null, decimals: null, address: null}, soMin. receivedtakes thebase-unit refusal from
#340 rather than being scaled at 18,
and the swap title falls back to
Uniswap Swaprather than naming ETH.Token Outgains a third branch readingUnknown (not named in the calldata). Without it the line would go missingentirely for this population, which is the failure
#346 fixed — a
Min. receivedfigure with nothing saying what is being received. It carries no
addressandno
isToken: a refusal is not a token.if (v4.tokenOut)gate now also clears a stale token. A V4 stepthat supersedes an earlier step's
Min. receivedwhile naming no currencypreviously left the earlier hop's token on screen, so the new figure was read
against the wrong token. Same two lines, same defect; disclosed here because
it is one case wider than the issue's literal text.
resolveTokenDecimals(), the display helpers, the bundled token list and thedecode path itself are untouched.
Fail-first evidence
tests/uniswapUndeterminedTokenOut.test.jsadded, thensrc/shared/uniswap.jsstashed back to head (bd0a626) andmake testrun.Five of the eight tests failed:
Test Suites: 1 failed, 51 passed/Tests: 5 failed, 933 passed.The other three tests pass on both sides by design: they pin the native-ETH
interpretation (zero address stays
ETHat 18 decimals, and the rawcoder.decodeof a zero-addressCurrencyis asserted truthy) so it cannotsilently change.
Verification
make checkgreen on this branch, rebased ontonextatbd0a626:Test Suites: 52 passed, 52 total,Tests: 938 passed, 938 total,check-censored: 176 tracked file(s) inspected. Lint ran in the pinnedcontainer and executed rather than replaying cache —
#11 [lint 1/1] RUN make lint/$ eslint . && prettier --check ./#11 DONE 5.7son the run following the source edit. No containers or imagesleft behind.
Not established
Nothing about this determination. One adjacent observation, deliberately not
acted on: the same null-means-ETH collapse exists on the input side —
tokenInfo(null)for a nullinputTokenyieldsToken In: ETH (native), andWRAP_ETHalready sets the explicit zero address, so null there is likewiseundetermined. Out of scope here; reported separately rather than widened into
this PR.
`tokenInfo(null)` answers `{symbol: "ETH", decimals: 18}`, and the V4 arm of `decode()` could take a step's `amountOutMin` while leaving `outputToken` null, because the token assignment was gated on `if (v4.tokenOut)`. The approval screen then named ETH as the output and formatted `Min. received` at 18 decimals for a swap whose output currency the calldata never stated. Null is not how V4 spells native ETH. v4-core declares `type Currency is address` and wraps `address(0)` for native ETH, and a `Currency` is ABI-encoded as a plain address word, so every decode site here gets back the truthy string `0x0000000000000000000000000000000000000000` for it — which `tokenInfo()` already names ETH — and an UNWRAP_WETH output is caught separately. Nothing that genuinely outputs ETH arrives as null; only a step whose output currency did not decode does. So a null output token is now treated as undetermined: `Token Out` reads `Unknown (not named in the calldata)` and `Min. received` falls to the base-unit refusal from #340 instead of being scaled at 18. A V4 step that supersedes an earlier step's `Min. received` without naming a currency also clears the earlier step's token, so that figure is never read against the token a previous hop named.PASS — #353 satisfied; determination independently confirmed, fail-first reproduced verbatim (5 failed / 938 passed), real mainnet USDT->ETH V4 fixture still renders
Token Out: ETH/Min. received: 0.0002 ETH/Swap USDT → ETHby execution,make checkgreen in an independent clone with the lint stage executing (#11 [lint 1/1] RUN make lint...DONE 5.3s, notCACHED), one commit, basenext, no Claude/Anthropic references.Anomalies (not defects, disclosed rather than omitted):
execute()(e.g.PERMIT2_PERMITalone) previously renderedToken Out: ETHwith titleSwap USDT → ETH; it now rendersUnknown (not named in the calldata)with titleUniswap Swap. Verified by execution on both sides. Strictly more truthful, so accepted, but it is a behaviour change on transactions that have no output token at all and is untested.address/isToken. None are counted as fail-first coverage, so the claim is not inflated — the label is just imprecise.