Conversation
…mals The instruction scaled the Chainlink answer to 1e18 and compared it against a swap price computed from raw token amounts. Those match only when both tokens have 18 decimals. On an 18/6 pair the swap price is 1e12 smaller, so the oracle is found better on every fill and min(priceRatio, 2e18 - maxPriceDecay) returns the cap. Takes tokenInDecimals and tokenOutDecimals in the encoding and scales to 10 ** (18 + tokenOutDecimals - tokenInDecimals). On an 18/18 pair that exponent is 18, so behaviour there is unchanged. Refs 1inch#31
SteMak
left a comment
SteMak
left a comment
There was a problem hiding this comment.
The PR proposes to modify OraclePriceAdjuster opcode implementing support for non-18 decimals tokens while keeping the opcode itself non-functional.
The OraclePriceAdjuster is not a production-ready opcode and is not part of deployed code. The code requires deep refactoring due to precision issues, inability to proceed reversed feeds (using USD/ETH feed for both ETH -> USD and USD -> ETH swaps), price-based instead of amount-based math.
Though, it's hard to validate the solution itself does not have precision issues.
I'd reject this PR as out-of-focus. There is no need to fix minors in demo code that requires complete rewrite.
| /// @dev Encoding: [uint64 maxPriceDecay, uint16 maxStaleness, uint8 oracleDecimals, uint8 tokenInDecimals, uint8 tokenOutDecimals, address oracleAddress] | ||
| /// maxStaleness = 0 skips the staleness check, oracleDecimals = 0 fetches decimals from the oracle | ||
| /// @dev Supports only single direction swaps, adjustment is applied only if favorable for the taker | ||
| /// @dev tokenInDecimals and tokenOutDecimals describe the swap direction the instruction runs on. |
There was a problem hiding this comment.
Decimals are just decimals, they do not describe the swap direction - technically invalid claim
|
|
||
| /// @notice Scales an oracle answer to the units the swap price is computed in | ||
| /// @dev A single net exponent, so the answer's low digits survive a scale-down | ||
| function scaleAnswer( |
There was a problem hiding this comment.
Helper functions should be placed below exec
There was a problem hiding this comment.
Function per 2-3 LoC is a poor style
| } else if (oracleDecimals > DECIMALS) { | ||
| oraclePrice = oraclePrice / 10 ** (oracleDecimals - DECIMALS); | ||
| } | ||
| // Convert oracle price to the scale currentPrice below is computed in, which is 1e18 only |
There was a problem hiding this comment.
currentPrice is not a literal
|
Out of development focus. The opcode is demo and requires complete rewrite or needs to be removed. |
2 participants
Follow-up to #31, which reported the scale mismatch and was closed as out of focus. What that report
did not say is that it pays out rather than just misbehaving, so here is the fix in case it is worth
having.
oraclePriceis scaled to 1e18,currentPriceisamountOut * 1e18 / amountInin raw tokenamounts. Those agree only on an 18/18 pair. On 18/6 the swap price is 1e12 smaller, so
oraclePrice > currentPriceis true on every fill andmin(priceRatio, 2e18 - maxPriceDecay)returns the cap. With
maxPriceDecay = 0that is 2x. There is no safe setting, sincemaxPriceDecay < ONEis required at build.The added test shows it. Reverting just the scaling line and rerunning gives:
The fix scales to
10 ** (18 + tokenOutDecimals - tokenInDecimals)instead. On an 18/18 pair thatexponent is 18, so nothing changes there. Decimals are declared in the program rather than read from
the tokens to keep two external calls off the hot path.
This changes the encoding, so it is breaking for anyone building the instruction today. Happy to do
it another way if you would rather not.
803 passing locally, up from 797.
.gas-snapshotnot regenerated; per AGENTS.md new functions areinformational there.
Note
High Risk
Changes swap payout logic tied to oracle comparison and uses a breaking instruction encoding; incorrect decimals would mis-price adjustments, though the fix addresses a known overpayment on common decimal pairs.
Overview
Fixes OraclePriceAdjuster comparing Chainlink answers at a fixed 1e18 scale while currentPrice uses raw amountOut / amountIn, which diverges on mixed-decimal pairs (e.g. 18/6) and could treat every fill as oracle-favorable—capping at up to 2× output when maxPriceDecay is minimal.
The instruction encoding now includes
tokenInDecimalsandtokenOutDecimals, andscaleAnswerrescales the feed to10 ** (18 + tokenOutDecimals - tokenInDecimals)so it matches the swap price units (unchanged for 18/18 pairs).build/parse/sizeOfare updated accordingly—breaking for existing bytecode builders.Adds
PriceOracleMock, integration tests on a WETH/USDC-style pair (match feed, better/worse feed, cap), and unit tests forscaleAnswer.Reviewed by Cursor Bugbot for commit 490941b. Bugbot is set up for automated code reviews on this repo. Configure here.