Improve swap fee estimation (#4168)

* Improve swap fee estimation * Fix tests * Update copy * Fix color * Fix legacy gas price calculation * Add test
MyCryptoHQ · Oct 19, 2021 · 731c90c · 731c90c
1 parent 18c742e
commit 731c90c
Show file tree

Hide file tree

Showing 21 changed files with 226 additions and 64 deletions.
diff --git a/src/components/GasSelector.tsx b/src/components/GasSelector.tsx
@@ -106,10 +106,11 @@ export default function GasSelector({
 
     try {
       const { network } = account;
-      const [gas, fetchedNonce] = await Promise.all([
+      const [gasEstimate, fetchedNonce] = await Promise.all([
         fetchUniversalGasPriceEstimate(network, account),
         getNonce(network, account.address)
       ]);
+      const { estimate: gas } = gasEstimate;
       setGasPrice({
         gasPrice: gas.gasPrice ?? '',
         maxFeePerGas: gas.maxFeePerGas ?? '',

diff --git a/src/components/TokenMigration/components/TokenMigrationForm.tsx b/src/components/TokenMigration/components/TokenMigrationForm.tsx
@@ -182,7 +182,7 @@ export const TokenMigrationFormUI = ({
   const handleSubmit = () => {
     if (isFormValid) {
       fetchUniversalGasPriceEstimate(values.network, values.account)
-        .then((gas) => {
+        .then(({ estimate: gas }) => {
           onComplete({ ...values, ...gas });
         })
         .catch(console.error);

diff --git a/src/components/TransactionFeeEIP1559.tsx b/src/components/TransactionFeeEIP1559.tsx
@@ -7,7 +7,7 @@ import { GasLimitField, GasPriceField } from '@features/SendAssets/components';
 import { COLORS } from '@theme';
 import { translateRaw } from '@translations';
 import { Asset, Fiat } from '@types';
-import { bigify, bigNumGasPriceToViewableGwei, gasStringsToMaxGasNumber } from '@utils';
+import { calculateMinMaxFee } from '@utils';
 
 import Box from './Box';
 import { default as Currency } from './Currency';
@@ -63,26 +63,16 @@ export const TransactionFeeEIP1559 = ({
   const [editMode, setEditMode] = useState(false);
   const handleToggleEditMode = () => setEditMode(!editMode);
 
-  const viewableBaseFee = baseFee && bigify(bigNumGasPriceToViewableGwei(baseFee));
-
-  const maxFee = gasStringsToMaxGasNumber(maxFeePerGas, gasLimit);
-  const maxFeeFiat = maxFee.multipliedBy(baseAssetRate);
-  const hasFiatValue = maxFeeFiat.gt(0);
-
-  const minMaxFee =
-    viewableBaseFee &&
-    BigNumber.min(
-      bigify(maxPriorityFeePerGas).gt(viewableBaseFee)
-        ? bigify(maxPriorityFeePerGas).plus(viewableBaseFee)
-        : viewableBaseFee,
-      maxFeePerGas
-    );
-
-  const minFee = minMaxFee ? gasStringsToMaxGasNumber(minMaxFee.toString(), gasLimit) : maxFee;
-  const minFeeFiat = minFee.multipliedBy(baseAssetRate);
-
-  const avgFee = minFee.plus(maxFee).dividedBy(2);
-  const avgFeeFiat = avgFee.multipliedBy(baseAssetRate);
+  const {
+    viewableBaseFee,
+    minFee,
+    minFeeFiat,
+    avgFee,
+    avgFeeFiat,
+    maxFee,
+    maxFeeFiat,
+    hasFiatValue
+  } = calculateMinMaxFee({ baseFee, baseAssetRate, maxFeePerGas, maxPriorityFeePerGas, gasLimit });
 
   return (
     <Box opacity={disabled ? '0.5' : undefined}>

diff --git a/src/components/TransactionFlow/helpers.spec.ts b/src/components/TransactionFlow/helpers.spec.ts
@@ -14,7 +14,7 @@ import {
 import { fetchUniversalGasPriceEstimate } from '@services/ApiService/Gas';
 import { translateRaw } from '@translations';
 import { ITxGasLimit, ITxNonce, ITxObject, ITxStatus, ITxType } from '@types';
-import { generateUUID, noOp } from '@utils';
+import { bigify, generateUUID, noOp } from '@utils';
 
 import {
   calculateReplacementGasPrice,
@@ -24,7 +24,7 @@ import {
 
 jest.mock('@services/ApiService/Gas', () => ({
   ...jest.requireActual('@services/ApiService/Gas'),
-  fetchUniversalGasPriceEstimate: jest.fn().mockResolvedValueOnce({ gasPrice: '500' })
+  fetchUniversalGasPriceEstimate: jest.fn().mockResolvedValueOnce({ estimate: { gasPrice: '500' } })
 }));
 
 describe('calculateReplacementGasPrice', () => {
@@ -39,7 +39,7 @@ describe('calculateReplacementGasPrice', () => {
   it('correctly determines tx gas price with too low fast gas price', () => {
     (fetchUniversalGasPriceEstimate as jest.MockedFunction<
       typeof fetchUniversalGasPriceEstimate
-    >).mockResolvedValueOnce({ gasPrice: '1' });
+    >).mockResolvedValueOnce({ estimate: { gasPrice: '1' } });
     return expect(
       calculateReplacementGasPrice(fTxConfig, { ...fNetworks[0], supportsEIP1559: false })
     ).resolves.toStrictEqual({
@@ -50,7 +50,10 @@ describe('calculateReplacementGasPrice', () => {
   it('correctly determines tx gas price for eip 1559', () => {
     (fetchUniversalGasPriceEstimate as jest.MockedFunction<
       typeof fetchUniversalGasPriceEstimate
-    >).mockResolvedValueOnce({ maxFeePerGas: '1', maxPriorityFeePerGas: '1' });
+    >).mockResolvedValueOnce({
+      baseFee: bigify(1000000000),
+      estimate: { maxFeePerGas: '1', maxPriorityFeePerGas: '1' }
+    });
     return expect(
       calculateReplacementGasPrice(fTxConfigEIP1559, { ...fNetworks[0], supportsEIP1559: true })
     ).resolves.toStrictEqual({
@@ -62,7 +65,10 @@ describe('calculateReplacementGasPrice', () => {
   it('correctly determines tx gas price for eip 1559 when new price too high', () => {
     (fetchUniversalGasPriceEstimate as jest.MockedFunction<
       typeof fetchUniversalGasPriceEstimate
-    >).mockResolvedValueOnce({ maxFeePerGas: '100', maxPriorityFeePerGas: '10' });
+    >).mockResolvedValueOnce({
+      baseFee: bigify(100000000000),
+      estimate: { maxFeePerGas: '100', maxPriorityFeePerGas: '10' }
+    });
     return expect(
       calculateReplacementGasPrice(fTxConfigEIP1559, { ...fNetworks[0], supportsEIP1559: true })
     ).resolves.toStrictEqual({

diff --git a/src/components/TransactionFlow/helpers.ts b/src/components/TransactionFlow/helpers.ts
@@ -143,7 +143,7 @@ export const constructSenderFromTxConfig = (
 
 // replacement gas price must be at least 10% higher than the replaced tx's gas price
 export const calculateReplacementGasPrice = async (txConfig: ITxConfig, network: Network) => {
-  const gas = await fetchUniversalGasPriceEstimate(network, txConfig.senderAccount);
+  const { estimate: gas } = await fetchUniversalGasPriceEstimate(network, txConfig.senderAccount);
 
   return isType2Tx(txConfig.rawTransaction)
     ? {

diff --git a/src/features/PurchaseMembership/components/MembershipPurchaseForm.tsx b/src/features/PurchaseMembership/components/MembershipPurchaseForm.tsx
@@ -263,9 +263,11 @@ export const MembershipFormUI = ({
                 loading={isSubmitting}
                 onClick={() => {
                   if (isValid) {
-                    fetchUniversalGasPriceEstimate(values.network, values.account).then((gas) => {
-                      onComplete({ ...values, ...gas });
-                    });
+                    fetchUniversalGasPriceEstimate(values.network, values.account).then(
+                      ({ estimate: gas }) => {
+                        onComplete({ ...values, ...gas });
+                      }
+                    );
                   }
                 }}
               >

diff --git a/src/features/SwapAssets/SwapAssetsFlow.test.tsx b/src/features/SwapAssets/SwapAssetsFlow.test.tsx
@@ -11,7 +11,7 @@ import SwapAssetsFlow from './SwapAssetsFlow';
 
 jest.mock('@services/ApiService/Gas', () => ({
   ...jest.requireActual('@services/ApiService/Gas'),
-  fetchUniversalGasPriceEstimate: () => Promise.resolve({ gasPrice: '20' }),
+  fetchUniversalGasPriceEstimate: () => Promise.resolve({ estimate: { gasPrice: '20' } }),
   getGasEstimate: () => Promise.resolve(21000)
 }));
 

diff --git a/src/features/SwapAssets/SwapAssetsFlow.tsx b/src/features/SwapAssets/SwapAssetsFlow.tsx
@@ -61,7 +61,8 @@ const SwapAssetsFlow = (props: RouteComponentProps) => {
     approvalTx,
     isEstimatingGas,
     tradeTx,
-    selectedNetwork
+    selectedNetwork,
+    gas
   }: SwapFormState = formState;
 
   const [assetPair, setAssetPair] = useState({});
@@ -110,7 +111,8 @@ const SwapAssetsFlow = (props: RouteComponentProps) => {
         approvalTx,
         isEstimatingGas,
         isSubmitting,
-        selectedNetwork
+        selectedNetwork,
+        gas
       },
       actions: {
         handleFromAssetSelected,

diff --git a/src/features/SwapAssets/components/SwapAssets.tsx b/src/features/SwapAssets/components/SwapAssets.tsx
@@ -32,9 +32,9 @@ import {
 import { SPACING } from '@theme';
 import translate, { translateRaw } from '@translations';
 import { Asset, ISwapAsset, Network, NetworkId, StoreAccount } from '@types';
-import { bigify, getTimeDifference, sortByLabel, totalTxFeeToString, useInterval } from '@utils';
+import { getTimeDifference, sortByLabel, useInterval } from '@utils';
 
-import { getAccountsWithAssetBalance, getUnselectedAssets } from '../helpers';
+import { getAccountsWithAssetBalance, getEstimatedGasFee, getUnselectedAssets } from '../helpers';
 import { SwapFormState } from '../types';
 import { SwapQuote } from './SwapQuote';
 
@@ -96,7 +96,8 @@ const SwapAssets = (props: Props) => {
     gasPrice,
     isEstimatingGas,
     expiration,
-    setNetwork
+    setNetwork,
+    gas
   } = props;
 
   const settings = useSelector(getSettings);
@@ -147,13 +148,12 @@ const SwapAssets = (props: Props) => {
     calculateNewFromAmountDebounced(value);
   };
 
-  const estimatedGasFee =
-    gasPrice &&
-    tradeGasLimit &&
-    totalTxFeeToString(
-      gasPrice,
-      bigify(tradeGasLimit).plus(approvalGasLimit ? approvalGasLimit : 0)
-    );
+  const estimatedGasFee = getEstimatedGasFee({
+    tradeGasLimit,
+    approvalGasLimit,
+    baseAssetRate,
+    gas
+  });
 
   // Accounts with a balance of the chosen asset and base asset
   const filteredAccounts = fromAsset

diff --git a/src/features/SwapAssets/components/SwapQuote.tsx b/src/features/SwapAssets/components/SwapQuote.tsx
@@ -53,7 +53,7 @@ export const SwapQuote = ({
       </Heading>
       <LinkApp href="#" variant="opacityLink">
         <Box variant="rowAlign" onClick={() => handleRefreshQuote()}>
-          <Icon type="refresh" width="16px" />
+          <Icon type="refresh" width="16px" color="BLUE_BRIGHT" />
           <Text ml={SPACING.XS} mb={0}>
             {translateRaw('GET_NEW_QUOTE')}
           </Text>

diff --git a/src/features/SwapAssets/helpers.test.ts b/src/features/SwapAssets/helpers.test.ts
@@ -0,0 +1,33 @@
+import { ITxGasLimit } from '@types';
+import { bigify } from '@utils';
+
+import { getEstimatedGasFee } from './helpers';
+
+describe('getEstimatedGasFee', () => {
+  it('supports legacy gas', () => {
+    expect(
+      getEstimatedGasFee({
+        tradeGasLimit: '0x3f7b1' as ITxGasLimit,
+        approvalGasLimit: '0x3f7b1' as ITxGasLimit,
+        baseAssetRate: 0,
+        gas: {
+          estimate: { gasPrice: '90', maxFeePerGas: undefined, maxPriorityFeePerGas: undefined }
+        }
+      })
+    ).toStrictEqual('0.046803');
+  });
+
+  it('supports eip 1559 gas', () => {
+    expect(
+      getEstimatedGasFee({
+        tradeGasLimit: '0x3f7b1' as ITxGasLimit,
+        approvalGasLimit: '0x3f7b1' as ITxGasLimit,
+        baseAssetRate: 0,
+        gas: {
+          estimate: { maxFeePerGas: '200', maxPriorityFeePerGas: '5', gasPrice: undefined },
+          baseFee: bigify('200000000000')
+        }
+      })
+    ).toStrictEqual('0.104007');
+  });
+});
diff --git a/src/features/SwapAssets/helpers.ts b/src/features/SwapAssets/helpers.ts
@@ -1,8 +1,25 @@
+import BigNumber from 'bignumber.js';
+
 import { WALLET_STEPS } from '@components';
 import { DEFAULT_ASSET_DECIMAL } from '@config';
-import { getAssetByTicker, getAssetByUUID } from '@services';
-import { ISwapAsset, ITxConfig, ITxObject, StoreAccount, StoreAsset, TUuid } from '@types';
-import { toTokenBase, weiToFloat } from '@utils';
+import { getAssetByTicker, getAssetByUUID, UniversalGasEstimationResult } from '@services';
+import {
+  ISwapAsset,
+  ITxConfig,
+  ITxGasLimit,
+  ITxObject,
+  StoreAccount,
+  StoreAsset,
+  TUuid
+} from '@types';
+import {
+  bigify,
+  calculateMinMaxFee,
+  inputGasPriceToHex,
+  totalTxFeeToString,
+  toTokenBase,
+  weiToFloat
+} from '@utils';
 
 export const makeSwapTxConfig = (assets: StoreAsset[]) => (
   transaction: ITxObject,
@@ -28,6 +45,42 @@ export const makeSwapTxConfig = (assets: StoreAsset[]) => (
   return txConfig;
 };
 
+export const getEstimatedGasFee = ({
+  tradeGasLimit,
+  approvalGasLimit,
+  baseAssetRate,
+  gas
+}: {
+  tradeGasLimit?: ITxGasLimit;
+  approvalGasLimit?: ITxGasLimit;
+  baseAssetRate?: number;
+  gas?: { estimate: UniversalGasEstimationResult; baseFee?: BigNumber };
+}) => {
+  if (tradeGasLimit && gas?.estimate.maxFeePerGas) {
+    const { avgFee } = calculateMinMaxFee({
+      baseFee: gas.baseFee,
+      ...gas.estimate,
+      gasLimit:
+        tradeGasLimit &&
+        bigify(tradeGasLimit)
+          .plus(approvalGasLimit ? approvalGasLimit : 0)
+          .toString(),
+      baseAssetRate
+    });
+
+    return avgFee.toFixed(6);
+  }
+
+  return (
+    gas?.estimate.gasPrice &&
+    tradeGasLimit &&
+    totalTxFeeToString(
+      inputGasPriceToHex(gas.estimate.gasPrice),
+      bigify(tradeGasLimit).plus(approvalGasLimit ? approvalGasLimit : 0)
+    )
+  );
+};
+
 // filter accounts based on wallet type and sufficient balance
 export const getAccountsWithAssetBalance = (
   accounts: StoreAccount[],

diff --git a/src/features/SwapAssets/types.ts b/src/features/SwapAssets/types.ts
@@ -1,6 +1,7 @@
 import { BigNumber } from 'bignumber.js';
 import { DistributiveOmit } from 'react-redux';
 
+import { UniversalGasEstimationResult } from '@services';
 import {
   ISwapAsset,
   ITxGasLimit,
@@ -62,6 +63,7 @@ export interface SwapFormState {
     txType: ITxType;
     metadata: ITxMetadata;
   };
+  gas?: { estimate: UniversalGasEstimationResult; baseFee?: BigNumber };
 }
 
 export interface IAssetPair {

diff --git a/src/helpers/transaction.ts b/src/helpers/transaction.ts
@@ -443,7 +443,7 @@ export const appendGasPrice = (network: Network, account: StoreAccount) => async
     return tx as TxBeforeGasLimit;
   }
   const gas = await fetchUniversalGasPriceEstimate(network, account)
-    .then((r) => mapObjIndexed((v) => v && inputGasPriceToHex(v), r))
+    .then(({ estimate: r }) => mapObjIndexed((v) => v && inputGasPriceToHex(v), r))
     .catch((err) => {
       throw new Error(`getGasPriceEstimate: ${err}`);
     });

diff --git a/src/services/ApiService/Dex/Dex.spec.ts b/src/services/ApiService/Dex/Dex.spec.ts
@@ -13,13 +13,14 @@ import {
   TUuid,
   WalletId
 } from '@types';
+import { bigify } from '@utils';
 
 import { DexService } from '.';
 import { formatTradeTx } from './Dex';
 
 jest.mock('@services/ApiService/Gas', () => ({
   ...jest.requireActual('@services/ApiService/Gas'),
-  fetchUniversalGasPriceEstimate: jest.fn().mockResolvedValue({ gasPrice: '154' })
+  fetchUniversalGasPriceEstimate: jest.fn().mockResolvedValue({ estimate: { gasPrice: '154' } })
 }));
 
 describe('SwapFlow', () => {
@@ -64,7 +65,10 @@ describe('SwapFlow', () => {
     it('returns the expected two transactions for a multi tx swap using eip1559', async () => {
       (fetchUniversalGasPriceEstimate as jest.MockedFunction<
         typeof fetchUniversalGasPriceEstimate
-      >).mockResolvedValueOnce({ maxFeePerGas: '100', maxPriorityFeePerGas: '10' });
+      >).mockResolvedValueOnce({
+        baseFee: bigify(100000000000),
+        estimate: { maxFeePerGas: '100', maxPriorityFeePerGas: '10' }
+      });
       const promise = DexService.instance.getOrderDetailsFrom(
         { ...fNetwork, supportsEIP1559: true },
         { ...fAccount, wallet: WalletId.LEDGER_NANO_S_NEW },

diff --git a/src/services/ApiService/Dex/Dex.ts b/src/services/ApiService/Dex/Dex.ts
@@ -120,7 +120,8 @@ export default class DexService {
       })
     });
 
-    const gas = await fetchUniversalGasPriceEstimate(network, account);
+    const gasResult = await fetchUniversalGasPriceEstimate(network, account);
+    const { estimate: gas } = gasResult;
 
     const gasPrice = gas.gasPrice ?? gas.maxFeePerGas;
 
@@ -153,6 +154,7 @@ export default class DexService {
       expiration: Date.now() / 1000 + DEX_TRADE_EXPIRATION,
       approvalTx,
       tradeGasLimit,
+      gas: gasResult,
       tradeTx: formatTradeTx({
         account: account!,
         to: data.to,