Skip to content

Commit

Permalink
fix: convert active rate to fraction to calculate opposite amount (#5322
Browse files Browse the repository at this point in the history
)

* fix: convert active rate to fraction to calculate opposite amount

* test: explain problem with multiplying price
  • Loading branch information
shoom3301 authored Jan 23, 2025
1 parent a30d5bd commit 8afae8b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export function useUpdateCurrencyAmount() {
outputCurrency,
})

const newInputAmount = (field as Field) === Field.INPUT ? amount : calculatedAmount
const newOutputAmount = (field as Field) === Field.OUTPUT ? amount : calculatedAmount
const newInputAmount = field === Field.INPUT ? amount : calculatedAmount
const newOutputAmount = field === Field.OUTPUT ? amount : calculatedAmount

const update: Partial<Writeable<LimitOrdersRawState>> = {
orderKind,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { USDC_MAINNET, WETH_MAINNET } from '@cowprotocol/common-const'
import { CurrencyAmount, Fraction, Price } from '@uniswap/sdk-core'

import JSBI from 'jsbi'

describe('calculateAmountForRate', () => {
it('When multiply an amount to a price instead of fraction, then the result will be zero', () => {
const priceAsFraction = new Fraction('320778000000000000000000000000000000', '99995600000000000000000000000000') // 3207.921148
const price = new Price({
baseAmount: CurrencyAmount.fromRawAmount(WETH_MAINNET, '1000000000000000000'),
quoteAmount: CurrencyAmount.fromRawAmount(USDC_MAINNET, '3208940687'),
})
const value = CurrencyAmount.fromRawAmount(WETH_MAINNET, '1000000')

const resultAfterFraction = value.multiply(priceAsFraction).quotient.toString()
const resultAfterPrice = value.multiply(price).quotient.toString()

expect(resultAfterFraction).toBe('3207921148') // 3207.921148
expect(resultAfterPrice).toBe('0')
})

it('When multiply an amount to a price numerator and divide to denominator, then the result will be zero', () => {
const price = new Price({
baseAmount: CurrencyAmount.fromRawAmount(WETH_MAINNET, '1000000000000000000'),
quoteAmount: CurrencyAmount.fromRawAmount(USDC_MAINNET, '3208940687'),
})
const value = CurrencyAmount.fromRawAmount(WETH_MAINNET, '1000000')

expect(value.numerator.toString()).toBe('1000000')
expect(value.denominator.toString()).toBe('1')

expect(price.numerator.toString()).toBe('3208940687')
expect(price.denominator.toString()).toBe('1000000000000000000')

/**
* The code bellow is the same if we multiply CurrencyAmount to Price
* I just revealed it to demonstrate a problem
* When we do this multiplication, we get a new Fraction where numerator and denominator have different length
*/
expect(JSBI.multiply(value.numerator, price.numerator).toString()).toBe('3208940687000000')
expect(JSBI.multiply(value.denominator, price.denominator).toString()).toBe('1000000000000000000')

const multiplied = new Fraction(
JSBI.multiply(value.numerator, price.numerator),
JSBI.multiply(value.denominator, price.denominator),
)

/**
* This is still valid fraction, and it does make sense, but let's see what we get bellow
*/
expect(multiplied.toFixed(10)).toBe('0.0032089407')

expect(multiplied.numerator.toString()).toBe('3208940687000000') // length 16
expect(multiplied.denominator.toString()).toBe('1000000000000000000') // length 19

/**
* Inside of JSBI.divide() it compares number lengths using __absoluteCompare() and if they are different it just returns zero
* *** It happens, because the Price was constructed from CurrencyAmounts with different decimals
* *** To avoid that FractionUtils.fromPrice() in order to normalize the Fraction
*/
expect((JSBI as any).__absoluteCompare(multiplied.numerator, multiplied.denominator)).toBe(-1)
expect(JSBI.divide(multiplied.numerator, multiplied.denominator).toString()).toBe('0')
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FractionUtils } from '@cowprotocol/common-utils'
import { Currency, CurrencyAmount, Fraction } from '@uniswap/sdk-core'
import { Currency, CurrencyAmount, Fraction, Price } from '@uniswap/sdk-core'

import { Field } from 'legacy/state/types'

Expand Down Expand Up @@ -28,15 +28,16 @@ export function calculateAmountForRate({
const parsedValue = FractionUtils.adjustDecimalsAtoms(
amount,
field === Field.INPUT ? inputDecimals : outputDecimals,
field === Field.INPUT ? outputDecimals : inputDecimals
field === Field.INPUT ? outputDecimals : inputDecimals,
)
const activeRateAsFraction = activeRate instanceof Price ? FractionUtils.fromPrice(activeRate) : activeRate

if (field === Field.INPUT) {
return CurrencyAmount.fromRawAmount(outputCurrency, parsedValue.multiply(activeRate).quotient)
return CurrencyAmount.fromRawAmount(outputCurrency, parsedValue.multiply(activeRateAsFraction).quotient)
}

if (field === Field.OUTPUT) {
return CurrencyAmount.fromRawAmount(inputCurrency, parsedValue.divide(activeRate).quotient)
return CurrencyAmount.fromRawAmount(inputCurrency, parsedValue.divide(activeRateAsFraction).quotient)
}

return null
Expand Down

0 comments on commit 8afae8b

Please sign in to comment.