fix(pegged-swap): correct PeggedPrice lt-quote conversions for mixed-decimal pairs - #83
Merged
Merged
Conversation
…decimal pairs PeggedPrice mixed two definitions of its marginal rate: the canonical raw gt-per-lt e18 (used by fromReserves, toGtPerLtE18 and the gt-quote branches) and a human-unit rate (used by the lt-quote branches of toHuman and fromHuman). The definitions only coincide when both tokens have equal decimals, so mixed-decimal pairs (e.g. 6/18) quoted in the lower-address token were off by 10^|dGt-dLt|. Fix the two lt-quote branches to invert the raw rate with the proper raw->human decimals adjustment: scaled/marginal = 10^(18 + gtDecimals) divided by the counterpart, instead of 10^(gtDecimals + 18 + ltDecimals) over (counterpart * 10^gtDecimals). Equal-decimals pairs are unaffected.
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.
Change Summary
What does this PR change?
Fixes
PeggedPrice(swap-vm SDK) producing wrong values when a mixed-decimals pegged pair is quoted in the lower-address (lt) token.The class mixed two definitions of its internal marginal rate: the canonical raw gt-per-lt rate in 1e18 fixed-point (used by
fromReserves,toGtPerLtE18(),PeggedSwapCalculator.computeFixedAllocation, and the gt-quote branches oftoHuman/fromHuman) and a human-unit rate (used only by the lt-quote branches oftoHumanandfromHuman). The two definitions coincide only when both tokens have equal decimals; for mixed-decimals pairs (e.g. a 6/18 USDC/DAI-style pool) lt-quoted prices were off by10^|gtDecimals - ltDecimals|. Example: a 6/18 pair with equal-value reserves returnedtoHuman(gt) === '1'(correct) buttoHuman(lt) === '0'instead of'1'.This PR makes raw gt-per-lt e18 the single canonical meaning and fixes the two lt-quote branches to invert it with the proper raw→human decimals adjustment:
toHuman, lt-quote branch: scaled value is now10^(18 + gtDecimals) / marginalE18(was10^(gtDecimals + 18 + ltDecimals) / (marginalE18 * 10^gtDecimals), which dropped the10^(gtDecimals - ltDecimals)adjustment).fromHuman, lt-quote branch:marginalE18is now10^(18 + gtDecimals) / parsed(was the matching buggy inverse, which made lt-quote round-trips self-consistently mask the bug).fromReserves,fromGtPerLtE18/toGtPerLtE18(the internalgtPerLtRawstorage round-trips consistently), the gt-quote branches, and all equal-decimals behavior are unchanged.PeggedSwapCalculatorand the e2e specs consumetoGtPerLtE18()/gt-quotefromHumanonly, so they are unaffected (verified by running their tests).Related Issue/Ticket:
No tracker ticket. Found while implementing opening-price support in 1inch/aqua-api PR #149, which currently works around this bug by converting
toGtPerLtE18()to a sqrt price and rendering throughinstructions.concentrate.Price; that workaround can be simplified once this fix ships.Testing & Verification
How was this tested?
Added regression tests in
pegged-price.test.ts:fromReserves(...).toHuman()returns the true price in both quote directions (equal-value reserves, price exactly'1'both ways; previously the lt quote returned'0').fromHumanwith lt quote read back with gt quote gives the exact inverse ('2000'↔'0.0005'), and vice versa (previously off by 1e12).Existing lt-quote round-trip tests were kept as-is — they passed before only because the two buggy branches were self-consistent inverses; they still pass now that both branches are correct.
Verification commands (all green):
pnpm --filter @1inch/swap-vm-sdk test(44 files, 600 tests),pnpm test,pnpm lint,pnpm lint:types(afterpnpm build:contracts+pnpm build). Also manually reproduced the numeric repro from the report against the fixed build.Risk Assessment
Risk Level:
Risks & Impact
This is a behavioral change for any consumer that quotes a mixed-decimals pegged price in the lower-address token via
toHuman/fromHuman: those calls previously returned values off by10^|gtDecimals - ltDecimals|and now return correct ones. Consumers that compensated for the bug downstream (like the aqua-api workaround above) should drop their compensation when upgrading. Equal-decimals pairs,fromReserves,toGtPerLtE18(), gt-quote paths, andPeggedSwapCalculatorare unaffected, and JSON round-trips are unchanged. Rollback is a standard revert / version pin.