Repository navigation
fix(uniswap-v3): stop aToken sells failing on the pool's input check - #1543
Merged
Merged
Conversation
|
Crate versions that have been updated:
Runtime version has been increased. |
- sell gas counts 5 ERC20_VIEW reads, 4 aToken + 1 allowance from #1545 - bumped runtime to 456, dispatcher to 1.9.0, integration tests to 1.127.0
enthusiastmartin
approved these changes
Oct 28, 2026
|
Quick benchmark at commit 442efd2 has been executed successfully. |
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.
Description
A router sell whose UniswapV3 hop takes an Aave aToken as input (e.g. aDOT → HOLLAR)
failed for some amounts with a bare
DispatchError::Other, while amounts 1 wei higheror lower went through.
An aToken stores scaled balances:
balanceOfisscaled.rayMul(index)and a transfermoves
amount.rayDiv(index), both rounding half up. The two roundings can leave thepool's
balanceOf1 wei short ofamount_in, andUniswapV3Pool.swapreverts on itsinput check (
IIA). The executor then replaced the revert with"uniswapv3: swap failed",a
DispatchError::Otherwhose text is dropped when the error is encoded, so on chainonly
Otherwas visible.execute_sellnow reads the input token's aToken state before swapping —UNDERLYING_ASSET_ADDRESS(),POOL(),getReserveNormalizedIncome(underlying)andscaledBalanceOf(pool)— and predicts the pool's exactbalanceOfincrease with Aave'srounding. While that increase would fall short of the amount, the amount is stepped
down 1 wei at a time, up to 1,000 steps. The amount actually sold is what gets reported
to the oracle and in the trade event.
A failed swap now returns
pallet_dispatcher::Error::UniswapV3SwapFailedinstead ofOther, on both the sell and the buy path.Note for reviewers
Only amounts that would revert are changed. Anything that goes through today is sold
unchanged. This matters for ICE: the UniswapV3 simulator quotes the exact pool output at
its sample points, so shrinking a trade that would have succeeded could break
conservation for a whole batch. A simpler rule that only looks at the transfer's own
rounding (no pool read) changes about half of all amounts instead of only the failing
ones.
Everything falls back to today's behaviour: a non-aToken input, any failed read, or no
passing amount within the cap sells the full amount unchanged. The cap only matters for
an index very close to 1.0, i.e. a freshly listed reserve.
The sell path makes four extra view calls, so
worst_case_gasnow returns the larger ofthe buy and sell paths instead of assuming the buy path bounds both. Every UniswapV3
trade reserves ~300k more gas worth of weight.
Two things left out on purpose:
~17 wei of aDOT at an index ≥ 1.03). The next trade through the router sells it.
Returning it to the seller would mean extra EVM reads and transfers in the router on
every trade, and the router's weights are benchmarked.
IIA, because inexactOutputthe pooldecides the input amount. They now fail with
UniswapV3SwapFailedrather thanOther.UniswapV3SwapFailedis appended at the end of the enum, so no existing variant indexshifts.
Related Issue
Fixes #1538
Motivation and Context
The outcome depended only on the last wei of the amount and on the current Aave index,
so neither a quote nor a dry run at the best block could predict it. Users paid the fee
and saw nothing but
Other.How Has This Been Tested?
cargo test -p hydradx-runtime --lib uniswap_v3_trade_executor::tests— 19 passed, 8 newcargo test -p pallet-dispatcher— 14 passedcargo test -p runtime-integration-tests uniswap_v3_router— 33 passed, 1 ignoredcargo clippy --all-targets -- -D warningson the three changed crates andcargo fmt --checkcleanNew test
router_sell_should_succeed_at_every_amount_when_the_input_is_an_atokensellsaDOT → HOLLAR through the router at 32 consecutive amounts on the mainnet snapshot.
Without the fix it fails at
1_000_000_007withOther("uniswapv3: swap failed"); withit all 32 go through.
execute_sell_should_fail_with_a_named_error_when_the_swap_revertsandtrade_weight_should_cover_the_atoken_sell_pathcover the new error and the weight.The unit tests pin the rounding prediction and the step-down: a 1 wei shortfall stepped
down, an amount kept when it already passes, index 1.0, zero amount, zero index, the cap,
a realistic index of 1.07, and overflow.
Versions:
hydradx-runtime448.0.0 /spec_version448,pallet-dispatcher1.8.0,runtime-integration-tests1.119.0.Checklist: