From bf5764c68c482fb5aa9485cb62809d5551030ba0 Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Wed, 28 Aug 2024 15:18:00 -0400 Subject: [PATCH 1/7] who knows --- src/handlers/web3.ts | 4 ++++ src/raps/actions/crosschainSwap.ts | 6 +++++- src/raps/actions/swap.ts | 9 ++++++--- src/raps/actions/unlock.ts | 7 ++++++- src/raps/unlockAndCrosschainSwap.ts | 12 +++++++++--- src/raps/unlockAndSwap.ts | 11 ++++++++--- 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/handlers/web3.ts b/src/handlers/web3.ts index 23996995cf2..2d793470994 100644 --- a/src/handlers/web3.ts +++ b/src/handlers/web3.ts @@ -433,6 +433,10 @@ export async function estimateGasWithPadding( ? contractCallEstimateGas(...(callArguments ?? []), txPayloadToEstimate) : p.estimateGas(cleanTxPayload)); + if (!BigNumber.isBigNumber(estimatedGas)) { + throw new Error('Invalid gas limit type'); + } + const lastBlockGasLimit = addBuffer(gasLimit.toString(), 0.9); const paddedGas = addBuffer(estimatedGas.toString(), paddingFactor.toString()); logger.debug('[web3]: ⛽ GAS CALCULATIONS!', { diff --git a/src/raps/actions/crosschainSwap.ts b/src/raps/actions/crosschainSwap.ts index 92aea428ffc..23e020e180b 100644 --- a/src/raps/actions/crosschainSwap.ts +++ b/src/raps/actions/crosschainSwap.ts @@ -71,7 +71,11 @@ export const estimateCrosschainSwapGasLimit = async ({ SWAP_GAS_PADDING ); - return gasLimit || getCrosschainSwapDefaultGasLimit(quote); + if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit))) { + return getCrosschainSwapDefaultGasLimit(quote) || getDefaultGasLimitForTrade(quote, chainId); + } + + return gasLimit; } catch (error) { return getCrosschainSwapDefaultGasLimit(quote); } diff --git a/src/raps/actions/swap.ts b/src/raps/actions/swap.ts index 7a060b2e173..97558363f54 100644 --- a/src/raps/actions/swap.ts +++ b/src/raps/actions/swap.ts @@ -93,9 +93,9 @@ export const estimateSwapGasLimit = async ({ WRAP_GAS_PADDING ); - return gasLimit || String(quote?.defaultGasLimit) || String(default_estimate); + return gasLimit || quote?.defaultGasLimit || default_estimate; } catch (e) { - return String(quote?.defaultGasLimit) || String(default_estimate); + return quote?.defaultGasLimit || default_estimate; } // Swap } else { @@ -116,8 +116,11 @@ export const estimateSwapGasLimit = async ({ } const gasLimit = await estimateGasWithPadding(params, method, methodArgs, provider, SWAP_GAS_PADDING); + if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit))) { + return getDefaultGasLimitForTrade(quote, chainId); + } - return gasLimit || getDefaultGasLimitForTrade(quote, chainId); + return gasLimit; } catch (error) { return getDefaultGasLimitForTrade(quote, chainId); } diff --git a/src/raps/actions/unlock.ts b/src/raps/actions/unlock.ts index 08592cc3597..5fe0898d9d6 100644 --- a/src/raps/actions/unlock.ts +++ b/src/raps/actions/unlock.ts @@ -93,7 +93,12 @@ export const estimateApprove = async ({ const gasLimit = await tokenContract.estimateGas.approve(spender, MaxUint256, { from: owner, }); - return gasLimit ? gasLimit.toString() : `${gasUnits.basic_approval}`; + + if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit))) { + return `${gasUnits.basic_approval}`; + } + + return gasLimit.toString(); } catch (error) { logger.error(new RainbowError('[raps/unlock]: error estimateApprove'), { message: (error as Error)?.message, diff --git a/src/raps/unlockAndCrosschainSwap.ts b/src/raps/unlockAndCrosschainSwap.ts index d1ff8fa64f3..d081b00aa8f 100644 --- a/src/raps/unlockAndCrosschainSwap.ts +++ b/src/raps/unlockAndCrosschainSwap.ts @@ -53,10 +53,8 @@ export const estimateUnlockAndCrosschainSwap = async ({ }); } - let unlockGasLimit; - if (swapAssetNeedsUnlocking) { - unlockGasLimit = await estimateApprove({ + const unlockGasLimit = await estimateApprove({ owner: accountAddress, tokenAddress: sellTokenAddress, spender: allowanceTarget, @@ -71,8 +69,16 @@ export const estimateUnlockAndCrosschainSwap = async ({ quote, }); + if (swapGasLimit === null || swapGasLimit === undefined || isNaN(Number(swapGasLimit))) { + return null; + } + const gasLimit = gasLimits.concat(swapGasLimit).reduce((acc, limit) => add(acc, limit), '0'); + if (isNaN(Number(gasLimit))) { + return null; + } + return gasLimit.toString(); }; diff --git a/src/raps/unlockAndSwap.ts b/src/raps/unlockAndSwap.ts index 614e66b992c..012971ac4ab 100644 --- a/src/raps/unlockAndSwap.ts +++ b/src/raps/unlockAndSwap.ts @@ -67,10 +67,8 @@ export const estimateUnlockAndSwap = async ({ } } - let unlockGasLimit; - if (swapAssetNeedsUnlocking) { - unlockGasLimit = await estimateApprove({ + const unlockGasLimit = await estimateApprove({ owner: accountAddress, tokenAddress: sellTokenAddress, spender: getRainbowRouterContractAddress(chainId), @@ -85,7 +83,14 @@ export const estimateUnlockAndSwap = async ({ quote, }); + if (swapGasLimit === null || swapGasLimit === undefined || isNaN(Number(swapGasLimit))) { + return null; + } + const gasLimit = gasLimits.concat(swapGasLimit).reduce((acc, limit) => add(acc, limit), '0'); + if (isNaN(Number(gasLimit))) { + return null; + } return gasLimit.toString(); }; From 8bcb787694080dfa8b8099675ba549f606333027 Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Fri, 30 Aug 2024 12:06:11 -0400 Subject: [PATCH 2/7] more error checking for if NaN numbers exist --- src/raps/actions/crosschainSwap.ts | 2 +- src/raps/actions/swap.ts | 6 +++++- src/raps/unlockAndCrosschainSwap.ts | 1 - src/raps/unlockAndSwap.ts | 3 --- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/raps/actions/crosschainSwap.ts b/src/raps/actions/crosschainSwap.ts index 23e020e180b..ac474447bc8 100644 --- a/src/raps/actions/crosschainSwap.ts +++ b/src/raps/actions/crosschainSwap.ts @@ -77,7 +77,7 @@ export const estimateCrosschainSwapGasLimit = async ({ return gasLimit; } catch (error) { - return getCrosschainSwapDefaultGasLimit(quote); + return getCrosschainSwapDefaultGasLimit(quote) || getDefaultGasLimitForTrade(quote, chainId); } }; diff --git a/src/raps/actions/swap.ts b/src/raps/actions/swap.ts index 97558363f54..b099c686916 100644 --- a/src/raps/actions/swap.ts +++ b/src/raps/actions/swap.ts @@ -93,7 +93,11 @@ export const estimateSwapGasLimit = async ({ WRAP_GAS_PADDING ); - return gasLimit || quote?.defaultGasLimit || default_estimate; + if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit))) { + return quote?.defaultGasLimit || default_estimate; + } + + return gasLimit; } catch (e) { return quote?.defaultGasLimit || default_estimate; } diff --git a/src/raps/unlockAndCrosschainSwap.ts b/src/raps/unlockAndCrosschainSwap.ts index d081b00aa8f..a5d3b20ed21 100644 --- a/src/raps/unlockAndCrosschainSwap.ts +++ b/src/raps/unlockAndCrosschainSwap.ts @@ -74,7 +74,6 @@ export const estimateUnlockAndCrosschainSwap = async ({ } const gasLimit = gasLimits.concat(swapGasLimit).reduce((acc, limit) => add(acc, limit), '0'); - if (isNaN(Number(gasLimit))) { return null; } diff --git a/src/raps/unlockAndSwap.ts b/src/raps/unlockAndSwap.ts index 012971ac4ab..249b8b86b27 100644 --- a/src/raps/unlockAndSwap.ts +++ b/src/raps/unlockAndSwap.ts @@ -43,7 +43,6 @@ export const estimateUnlockAndSwap = async ({ let swapAssetNeedsUnlocking = false; const nativeAsset = isLowerCaseMatch(ETH_ADDRESS_AGGREGATOR, sellTokenAddress) || isNativeAsset(sellTokenAddress, chainId); - if (!isNativeAssetUnwrapping && !nativeAsset) { swapAssetNeedsUnlocking = await assetNeedsUnlocking({ owner: accountAddress, @@ -65,9 +64,7 @@ export const estimateUnlockAndSwap = async ({ if (gasLimitFromMetadata) { return gasLimitFromMetadata; } - } - if (swapAssetNeedsUnlocking) { const unlockGasLimit = await estimateApprove({ owner: accountAddress, tokenAddress: sellTokenAddress, From 67c559ddb0d61243cc85cc1fdfe9791f9ab74567 Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Fri, 30 Aug 2024 12:27:26 -0400 Subject: [PATCH 3/7] more boundary checks --- .../screens/Swap/hooks/useEstimatedGasFee.ts | 17 +++++++++++++++-- .../providers/SyncSwapStateAndSharedValues.tsx | 11 +++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts index ab3729c448c..f42f158d5a4 100644 --- a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts +++ b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts @@ -19,9 +19,22 @@ function safeBigInt(value: string) { } } +const isFeeNaN = (value: string | undefined) => isNaN(Number(value)) || !value; + export function calculateGasFee(gasSettings: GasSettings, gasLimit: string) { - const amount = gasSettings.isEIP1559 ? add(gasSettings.maxBaseFee, gasSettings.maxPriorityFee || '0') : gasSettings.gasPrice; - return multiply(gasLimit, amount); + if (gasSettings.isEIP1559) { + if (isFeeNaN(gasSettings.maxBaseFee) || isFeeNaN(gasSettings.maxPriorityFee)) { + return null; + } + + return add(gasSettings.maxBaseFee, gasSettings.maxPriorityFee); + } + + if (isFeeNaN(gasSettings.gasPrice)) { + return null; + } + + return multiply(gasLimit, gasSettings.gasPrice); } export function useEstimatedGasFee({ diff --git a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx index 474f44943a7..15bc2b54132 100644 --- a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx +++ b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx @@ -129,12 +129,23 @@ export function SyncGasStateToSharedValues() { hasEnoughFundsForGas.value = undefined; if (!gasSettings || !estimatedGasLimit || !quote || 'error' in quote || isLoadingNativeNetworkAsset) return; + // NOTE: if we don't have a gas price or max base fee or max priority fee, we can't calculate the gas fee + if ( + (gasSettings.isEIP1559 && !(gasSettings.maxBaseFee || gasSettings.maxPriorityFee)) || + (!gasSettings.isEIP1559 && !gasSettings.gasPrice) + ) { + return; + } + if (!userNativeNetworkAsset) { hasEnoughFundsForGas.value = false; return; } const gasFee = calculateGasFee(gasSettings, estimatedGasLimit); + if (!gasFee || isNaN(Number(gasFee))) { + return; + } const nativeGasFee = divWorklet(gasFee, powWorklet(10, userNativeNetworkAsset.decimals)); From 6b278ef0fb9e39657d9308dacc922773ebc4767c Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Fri, 30 Aug 2024 13:14:32 -0400 Subject: [PATCH 4/7] fix lint --- src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts index f42f158d5a4..0e30fdd02a9 100644 --- a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts +++ b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts @@ -53,6 +53,7 @@ export function useEstimatedGasFee({ if (!gasLimit || !gasSettings || !nativeNetworkAsset?.price) return; const fee = calculateGasFee(gasSettings, gasLimit); + if (!fee) return; const networkAssetPrice = nativeNetworkAsset.price.value?.toString(); if (!networkAssetPrice) return `${formatNumber(weiToGwei(fee))} Gwei`; From debf8b6feb9d77ec1766709af619eeff0dff33a1 Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Wed, 4 Sep 2024 11:31:00 -0400 Subject: [PATCH 5/7] code review changes --- src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts | 2 +- .../screens/Swap/providers/SyncSwapStateAndSharedValues.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts index 0e30fdd02a9..88a5b717573 100644 --- a/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts +++ b/src/__swaps__/screens/Swap/hooks/useEstimatedGasFee.ts @@ -19,7 +19,7 @@ function safeBigInt(value: string) { } } -const isFeeNaN = (value: string | undefined) => isNaN(Number(value)) || !value; +const isFeeNaN = (value: string | undefined) => isNaN(Number(value)) || typeof value === 'undefined'; export function calculateGasFee(gasSettings: GasSettings, gasLimit: string) { if (gasSettings.isEIP1559) { diff --git a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx index 15bc2b54132..3cf024f750c 100644 --- a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx +++ b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx @@ -143,7 +143,7 @@ export function SyncGasStateToSharedValues() { } const gasFee = calculateGasFee(gasSettings, estimatedGasLimit); - if (!gasFee || isNaN(Number(gasFee))) { + if (gasFee === null || isNaN(Number(gasFee))) { return; } From b6c45384846c37b0479697ef2169a0d2c9a1db3f Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Wed, 4 Sep 2024 11:34:35 -0400 Subject: [PATCH 6/7] change calculate gas fee worklet with new changes --- .../SyncSwapStateAndSharedValues.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx index afaff055cd8..fa047483948 100644 --- a/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx +++ b/src/__swaps__/screens/Swap/providers/SyncSwapStateAndSharedValues.tsx @@ -79,10 +79,28 @@ export const SyncQuoteSharedValuesToState = () => { return null; }; +const isFeeNaNWorklet = (value: string | undefined) => { + 'worklet'; + + return isNaN(Number(value)) || typeof value === 'undefined'; +}; + export function calculateGasFeeWorklet(gasSettings: GasSettings, gasLimit: string) { 'worklet'; - const amount = gasSettings.isEIP1559 ? sumWorklet(gasSettings.maxBaseFee, gasSettings.maxPriorityFee || '0') : gasSettings.gasPrice; - return mulWorklet(gasLimit, amount); + + if (gasSettings.isEIP1559) { + if (isFeeNaNWorklet(gasSettings.maxBaseFee) || isFeeNaNWorklet(gasSettings.maxPriorityFee)) { + return null; + } + + return sumWorklet(gasSettings.maxBaseFee || '0', gasSettings.maxPriorityFee || '0'); + } + + if (isFeeNaNWorklet(gasSettings.gasPrice)) { + return null; + } + + return mulWorklet(gasLimit, gasSettings.gasPrice); } export function formatUnitsWorklet(value: string, decimals: number) { From 54214348d4c2076e927671fb92686c169b9a6902 Mon Sep 17 00:00:00 2001 From: Matthew Wall Date: Tue, 10 Sep 2024 14:12:06 -0400 Subject: [PATCH 7/7] Update src/raps/actions/unlock.ts --- src/raps/actions/unlock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/raps/actions/unlock.ts b/src/raps/actions/unlock.ts index fb31e766888..b2de3ff610c 100644 --- a/src/raps/actions/unlock.ts +++ b/src/raps/actions/unlock.ts @@ -92,7 +92,7 @@ export const estimateApprove = async ({ from: owner, }); - if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit))) { + if (gasLimit === null || gasLimit === undefined || isNaN(Number(gasLimit.toString()))) { return `${gasUnits.basic_approval}`; }