Swaps: logic, data flow, and performance fixes #5787
Conversation
christianbaroni
requested review from
brunobar79,
jinchung,
greg-schrammel,
walmat and
benisgold
Need to replace with proper currency formatting
Used for a migration but it doesn't look like we're actually using the userAssetsStore favorites anywhere — may want to instead remove the migration
Shields it from WalletScreen re-renders
| sliderXPosition: SLIDER_WIDTH / 2, | ||
| }) | ||
| ); | ||
| const initialInputNativeValue = addCommasToNumber((initialInputAmount * (initialSelectedInputAsset?.price?.value ?? 0)).toFixed(2)); |
There was a problem hiding this comment.
we should have native formatter functions with the bigint work Bruno is doing
| const inputValues = useSharedValue<{ [key in inputKeys]: number | string }>({ | ||
| inputAmount: 0, | ||
| inputNativeValue: 0, | ||
| inputAmount: initialInputAmount, |
There was a problem hiding this comment.
open question / note to self: what's happening for the scenario for internal deeplinks if we want to "BUY" a specific token
There was a problem hiding this comment.
Follow-up question before I can answer that one, but if we're prefilling an asset to buy, are we prefilling the asset to sell as the largest asset on the same network?
In not: We can do this no problem and just leave the output values as 0
If so: We'll have to prefetch a quote to show the initial output values based on that quote.
| @@ -87,20 +129,20 @@ export function useSwapInputsController({ | |||
| if (inputMethod.value === 'outputAmount') { | |||
| return valueBasedDecimalFormatter({ | |||
| amount: inputValues.value.inputAmount, | |||
| usdTokenPrice: internalSelectedInputAsset.value.nativePrice, | |||
| usdTokenPrice: inputNativePrice.value, | |||
| roundingMode: 'up', | |||
| precisionAdjustment: -1, | |||
| isStablecoin: internalSelectedInputAsset.value?.type === 'stablecoin' ?? false, | |||
There was a problem hiding this comment.
note to check: are we sure this type for stablecoin is actually set? (wasn't seeing it locally a few commits ago)
|
|
||
| return niceIncrementFormatter({ | ||
| incrementDecimalPlaces: incrementDecimalPlaces.value, | ||
| inputAssetBalance: balance, | ||
| inputAssetUsdPrice: internalSelectedInputAsset.value.nativePrice, | ||
| inputAssetUsdPrice: inputNativePrice.value, | ||
| niceIncrement: niceIncrement.value, | ||
| percentageToSwap: percentageToSwap.value, | ||
| sliderXPosition: sliderXPosition.value, |
There was a problem hiding this comment.
github won't let me add a comment on unchanged code for line 154: in formattedInputNativeValue and same for formattedOutputNativeValue: we need to remove USD specific hardcoded fallback values ($0.00)
| }: { | ||
| data: Quote | CrosschainQuote | QuoteError | null; | ||
| outputAmount?: number; | ||
| inputAmount?: number; | ||
| inputPrice?: number | null; | ||
| outputPrice?: number | null; | ||
| }) => { | ||
| 'worklet'; | ||
|
|
There was a problem hiding this comment.
for lines 234 - 245: for discussion: we're doing this slider update in multiple places - what if we turned the slider into an animated reaction and react when: the input amount changes and the input method is not the slider
isLoading from useSwapEstimatedGasFee changes erratically, so this approximates the extra gas fee fetch time by using a delayed version of isFetching
[ERROR] Error: userAssetsQueryFunction: {
"message": "must at least pass 1 valid address: '[]'"
}
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
What changed (plus any additional context for devs)
Screen recordings / screenshots
What to test
Fixes APP-1463
Fixes APP-1523
Fixes APP-1513
Fixes APP-1502
Fixes APP-1492
Fixes APP-1505
Fixes APP-1536
Fixes APP-1535
Fixes APP-1352
Fixes APP-1488
Fixes APP-1330