Skip to content

Commit

Permalink
Fix weird issues with rounding (#5822)
Browse files Browse the repository at this point in the history
* addresses most of the issues with stablecoins and beyond 2nd order magnitude rounding

* add test suite

* rm console

* fix precision rounding function

* fix some safe math rounding on max amount

* rm old function

* fix lint

* clean up numberFormatter

* revert change to safemath

* fix some flakiness

* remove logs app wide

* add more tests

* Apply suggestions from code review

* add constants and adjust fallbacks
  • Loading branch information
walmat authored Jun 11, 2024
1 parent 7cdcc7a commit 2654e86
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 36 deletions.
1 change: 0 additions & 1 deletion src/__swaps__/safe-math/SafeMath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ export function floorWorklet(num: string | number): string {
export function roundWorklet(num: string | number): string {
'worklet';
const numStr = toStringWorklet(num);

if (!isNumberStringWorklet(numStr)) {
throw new Error('Argument must be a numeric string or number');
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { niceIncrementFormatter } from '@/__swaps__/utils/swaps';
import { SLIDER_WIDTH } from '../constants';

type TestCase = {
incrementDecimalPlaces: number;
inputAssetBalance: number | string;
assetBalanceDisplay: string;
inputAssetUsdPrice: number;
niceIncrement: number | string;
percentageToSwap: number;
sliderXPosition: number;
stripSeparators?: boolean;
isStablecoin?: boolean;
} & {
testName: string;
expectedResult: string;
};

const TEST_CASES: TestCase[] = [
{
incrementDecimalPlaces: 0,
inputAssetBalance: 45.47364224817269,
assetBalanceDisplay: '45.47364225',
inputAssetUsdPrice: 0.9995363790000001,
niceIncrement: '1',
percentageToSwap: 0.5,
sliderXPosition: SLIDER_WIDTH / 2,
stripSeparators: true,
isStablecoin: true,
testName: 'DAI Stablecoin',
expectedResult: '22.74',
},
{
incrementDecimalPlaces: 2,
inputAssetBalance: 100,
assetBalanceDisplay: '100.00',
inputAssetUsdPrice: 10,
niceIncrement: '0.1',
percentageToSwap: 0,
sliderXPosition: 0,
stripSeparators: false,
isStablecoin: false,
testName: 'Zero percent swap',
expectedResult: '0.00',
},
{
incrementDecimalPlaces: 2,
inputAssetBalance: 100,
assetBalanceDisplay: '100.00',
inputAssetUsdPrice: 10,
niceIncrement: '0.1',
percentageToSwap: 1,
sliderXPosition: SLIDER_WIDTH,
stripSeparators: false,
isStablecoin: false,
testName: 'Full swap',
expectedResult: '100.00',
},
{
incrementDecimalPlaces: 2,
inputAssetBalance: 123.456,
assetBalanceDisplay: '123.46',
inputAssetUsdPrice: 1,
niceIncrement: '0.05',
percentageToSwap: 0.25,
sliderXPosition: SLIDER_WIDTH / 4,
stripSeparators: true,
isStablecoin: false,
testName: 'Quarter swap with fractional increment',
expectedResult: '30.86',
},
{
incrementDecimalPlaces: 0,
inputAssetBalance: '1000',
assetBalanceDisplay: '1,000',
inputAssetUsdPrice: 0.5,
niceIncrement: '100',
percentageToSwap: 0.75,
sliderXPosition: (3 * SLIDER_WIDTH) / 4,
stripSeparators: false,
isStablecoin: false,
testName: 'Large increment test',
expectedResult: '750',
},
];

describe('NiceIncrementFormatter', () => {
beforeAll(() => {
jest.mock('react-native-reanimated', () => require('react-native-reanimated/mock'));
});

TEST_CASES.forEach(({ testName, expectedResult, ...params }, index) => {
// eslint-disable-next-line jest/valid-title
test(testName || `test-${index}`, () => {
expect(niceIncrementFormatter({ ...params })).toBe(expectedResult);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const TokenToSellListComponent = () => {
ListEmptyComponent={<ListEmpty />}
keyExtractor={uniqueId => uniqueId}
renderItem={({ item: uniqueId }) => {
return <CoinRow onPress={(asset: ParsedSearchAsset | null) => handleSelectToken(asset)} output={false} uniqueId={uniqueId} />;
return <CoinRow onPress={asset => handleSelectToken(asset)} output={false} uniqueId={uniqueId} />;
}}
/>
</Stack>
Expand Down
3 changes: 3 additions & 0 deletions src/__swaps__/screens/Swap/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ export const ETH_COLOR_DARK_ACCENT = '#9CA4AD';

export const LONG_PRESS_DELAY_DURATION = 200;
export const LONG_PRESS_REPEAT_DURATION = 69;

export const STABLECOIN_MINIMUM_SIGNIFICANT_DECIMALS = 2;
export const MAXIMUM_SIGNIFICANT_DECIMALS = 6;
//
// /---- END constants ----/ //

Expand Down
26 changes: 26 additions & 0 deletions src/__swaps__/screens/Swap/hooks/useSwapInputsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,21 @@ import { divWorklet, equalWorklet, greaterThanWorklet, mulWorklet, toFixedWorkle

function getInitialInputValues(initialSelectedInputAsset: ExtendedAnimatedAssetWithColors | null) {
const initialBalance = Number(initialSelectedInputAsset?.balance.amount) || 0;
const assetBalanceDisplay = initialSelectedInputAsset?.balance.display ?? '';
const initialNiceIncrement = findNiceIncrement(initialBalance);
const initialDecimalPlaces = countDecimalPlaces(initialNiceIncrement);
const isStablecoin = initialSelectedInputAsset?.type === 'stablecoin';

const initialInputAmount = niceIncrementFormatter({
incrementDecimalPlaces: initialDecimalPlaces,
inputAssetBalance: initialBalance,
assetBalanceDisplay,
inputAssetUsdPrice: initialSelectedInputAsset?.price?.value ?? 0,
niceIncrement: initialNiceIncrement,
percentageToSwap: 0.5,
sliderXPosition: SLIDER_WIDTH / 2,
stripSeparators: true,
isStablecoin,
});

const initialInputNativeValue = addCommasToNumber(
Expand Down Expand Up @@ -119,10 +123,13 @@ export function useSwapInputsController({
return addCommasToNumber(inputValues.value.inputAmount, '0');
}

const assetBalanceDisplay = internalSelectedInputAsset.value.balance.display ?? '';

if (inputMethod.value === 'outputAmount') {
return valueBasedDecimalFormatter({
amount: inputValues.value.inputAmount,
usdTokenPrice: inputNativePrice.value,
assetBalanceDisplay,
roundingMode: 'up',
precisionAdjustment: -1,
isStablecoin: internalSelectedInputAsset.value?.type === 'stablecoin' ?? false,
Expand All @@ -131,14 +138,17 @@ export function useSwapInputsController({
}

const balance = internalSelectedInputAsset.value?.balance.amount || 0;
const isStablecoin = internalSelectedInputAsset.value?.type === 'stablecoin' ?? false;

return niceIncrementFormatter({
incrementDecimalPlaces: incrementDecimalPlaces.value,
inputAssetBalance: balance,
assetBalanceDisplay,
inputAssetUsdPrice: inputNativePrice.value,
niceIncrement: niceIncrement.value,
percentageToSwap: percentageToSwap.value,
sliderXPosition: sliderXPosition.value,
isStablecoin,
});
});

Expand Down Expand Up @@ -166,9 +176,13 @@ export function useSwapInputsController({
if (inputMethod.value === 'outputAmount' || typeof inputValues.value.outputAmount === 'string') {
return addCommasToNumber(inputValues.value.outputAmount, '0');
}

const assetBalanceDisplay = internalSelectedOutputAsset.value.balance.display ?? '';

return valueBasedDecimalFormatter({
amount: inputValues.value.outputAmount,
usdTokenPrice: outputNativePrice.value,
assetBalanceDisplay,
roundingMode: 'down',
precisionAdjustment: -1,
isStablecoin: internalSelectedOutputAsset.value?.type === 'stablecoin' ?? false,
Expand Down Expand Up @@ -674,6 +688,7 @@ export function useSwapInputsController({
if (!internalSelectedInputAsset.value) return;

const balance = Number(internalSelectedInputAsset.value.balance.amount);

if (!balance) {
inputValues.modify(values => {
return {
Expand All @@ -687,14 +702,18 @@ export function useSwapInputsController({
return;
}

const assetBalanceDisplay = internalSelectedInputAsset.value.balance.display;

const inputAmount = niceIncrementFormatter({
incrementDecimalPlaces: incrementDecimalPlaces.value,
inputAssetBalance: balance,
assetBalanceDisplay,
inputAssetUsdPrice: inputNativePrice.value,
niceIncrement: niceIncrement.value,
percentageToSwap: percentageToSwap.value,
sliderXPosition: sliderXPosition.value,
stripSeparators: true,
isStablecoin: internalSelectedInputAsset.value?.type === 'stablecoin' ?? false,
});
const inputNativeValue = mulWorklet(inputAmount, inputNativePrice.value);
inputValues.modify(values => {
Expand Down Expand Up @@ -844,14 +863,18 @@ export function useSwapInputsController({
return;
}

const assetBalanceDisplay = internalSelectedInputAsset.value?.balance.display ?? '';

const inputAmount = niceIncrementFormatter({
incrementDecimalPlaces: incrementDecimalPlaces.value,
inputAssetBalance: balance,
assetBalanceDisplay,
inputAssetUsdPrice: inputNativePrice.value,
niceIncrement: niceIncrement.value,
percentageToSwap: percentageToSwap.value,
sliderXPosition: sliderXPosition.value,
stripSeparators: true,
isStablecoin: internalSelectedInputAsset.value?.type === 'stablecoin' ?? false,
});

const inputNativeValue = mulWorklet(inputAmount, inputNativePrice.value);
Expand All @@ -869,11 +892,14 @@ export function useSwapInputsController({
const inputNativePrice = internalSelectedInputAsset.value?.nativePrice || internalSelectedInputAsset.value?.price?.value || 0;
const outputNativePrice = internalSelectedOutputAsset.value?.nativePrice || internalSelectedOutputAsset.value?.price?.value || 0;

const assetBalanceDisplay = current.assetToSell?.balance.display ?? '';

const inputAmount = Number(
valueBasedDecimalFormatter({
amount:
inputNativePrice > 0 ? divWorklet(inputValues.value.inputNativeValue, inputNativePrice) : inputValues.value.outputAmount,
usdTokenPrice: inputNativePrice,
assetBalanceDisplay,
roundingMode: 'up',
precisionAdjustment: -1,
isStablecoin: current.assetToSell?.type === 'stablecoin' ?? false,
Expand Down
Loading

0 comments on commit 2654e86

Please sign in to comment.