Fix: [TP-1670] ERC20 approvals are not additive (#1079)

Co-authored-by: Pano Skylakis <[email protected]>

packages/internal/dex/sdk/src/errors/exchangeError.ts

@@ -21,8 +21,8 @@ export enum ExchangeErrorCode {
  * Contains a type that corresponds to a {@link ExchangeError} and an optional message string.
  */
 export type ExchangeErrorType = {
-  type: ExchangeError,
-  message: string,
+  type: ExchangeError;
+  message: string;
 };
 
 /**
@@ -97,15 +97,3 @@ export class ApproveError extends ExchangeError {
     super(message, ExchangeErrorCode.APPROVE_ERROR);
   }
 }
-
-export class AlreadyApprovedError extends ExchangeError {
-  public tokenAddress: string;
-
-  public spenderAddress: string;
-
-  constructor(amountApproved: string, tokenAddress: string, spenderAddress: string) {
-    super(`already approved ${amountApproved} tokens`, ExchangeErrorCode.ALREADY_APPROVED_ERROR);
-    this.tokenAddress = tokenAddress;
-    this.spenderAddress = spenderAddress;
-  }
-}

packages/internal/dex/sdk/src/exchange.getUnsignedSwapTxFromAmountIn.test.ts

@@ -363,8 +363,8 @@ describe('getUnsignedSwapTxFromAmountIn', () => {
 
         const decodedResults = erc20ContractInterface.decodeFunctionData('approve', tx.approval.transaction.data);
         expect(decodedResults[0]).toEqual(TEST_ROUTER_ADDRESS);
-        // we have already approved 1000000000000000000, so we expect to approve 1000000000000000000 more
-        expect(decodedResults[1].toString()).toEqual(APPROVED_AMOUNT.value.toString());
+        // we have already approved 1000000000000000000 but this is not enough, so we expect to approve the full amount
+        expect(decodedResults[1].toString()).toEqual(amountIn.value.toString());
         expect(tx.approval.transaction.to).toEqual(params.inputToken);
         expect(tx.approval.transaction.from).toEqual(params.fromAddress);
         expect(tx.approval.transaction.value).toEqual(0); // we do not want to send any ETH

packages/internal/dex/sdk/src/lib/transactionUtils/approval.test.ts

@@ -63,7 +63,7 @@ describe('getApprovalTransaction', () => {
   });
 
   describe('when the allowance is less than the given amount', () => {
-    it('should create an unsigned approve transaction with the difference as the amount to approve', async () => {
+    it('should create an unsigned approve transaction with the full amount of the input token', async () => {
       const erc20Contract = (Contract as unknown as jest.Mock).mockImplementation(() => ({
         allowance: jest.fn().mockResolvedValue(BigNumber.from('1000000000000000000')),
       }));
@@ -85,8 +85,6 @@ describe('getApprovalTransaction', () => {
       // Mock the contract instance creation
       erc20ContractFactory.connect.mockReturnValue(erc20Contract);
 
-      const expectedAmountToApprove = tokenInAmount.value.sub(existingAllowance);
-
       const result = await getApproveTransaction(provider, TEST_FROM_ADDRESS, tokenInAmount, spenderAddress);
 
       expectToBeDefined(result?.data);
@@ -97,7 +95,7 @@ describe('getApprovalTransaction', () => {
       const erc20ContractInterface = ERC20__factory.createInterface();
       const decodedResults = erc20ContractInterface.decodeFunctionData('approve', result.data);
       expect(decodedResults[0]).toEqual(TEST_ROUTER_ADDRESS);
-      expect(decodedResults[1].toString()).toEqual(expectedAmountToApprove.toString());
+      expect(decodedResults[1].toString()).toEqual(tokenInAmount.value.toString());
       expect(erc20Contract.mock.calls.length).toEqual(1);
     });
   });

packages/internal/dex/sdk/src/lib/transactionUtils/approval.ts

@@ -1,10 +1,10 @@
 import { JsonRpcProvider, TransactionRequest } from '@ethersproject/providers';
 import { BigNumber } from '@ethersproject/bignumber';
 import { ERC20__factory } from 'contracts/types/factories/ERC20__factory';
-import { ApproveError, AlreadyApprovedError } from 'errors';
+import { ApproveError } from 'errors';
 import { ethers } from 'ethers';
 import { TradeType } from '@uniswap/sdk-core';
-import { isERC20Amount, newAmount, toPublicAmount } from 'lib/utils';
+import { isERC20Amount, toPublicAmount } from 'lib/utils';
 import { CoinAmount, Coin, ERC20 } from 'types';
 import { SecondaryFee, TransactionDetails } from '../../types';
 import { calculateGasFee } from './gas';
@@ -15,26 +15,25 @@ type PreparedApproval = {
 };
 
 /**
- * Get the amount of an ERC20 token that needs to be approved by
- * checking the existing allowance for the spender
+ * Check if the spender needs approval for the token
  *
  * @param provider - The provider to use for the call
  * @param ownerAddress - The address of the owner of the token
  * @param tokenAmount  - The amount of the token to approve
  * @param spenderAddress - The address of the spender
  * @returns - The amount of the token that needs to be approved
  */
-const getERC20AmountToApprove = async (
+const doesSpenderNeedApproval = async (
   provider: JsonRpcProvider,
   ownerAddress: string,
   tokenAmount: CoinAmount<ERC20>,
   spenderAddress: string,
-): Promise<CoinAmount<ERC20>> => {
+): Promise<boolean> => {
   // create an instance of the ERC20 token contract
   const erc20Contract = ERC20__factory.connect(tokenAmount.token.address, provider);
 
   // get the allowance for the token spender
-  // minimum is 0 - no allowance
+  // the minimum allowance is 0 - no allowance
   let allowance: BigNumber;
   try {
     allowance = await erc20Contract.allowance(ownerAddress, spenderAddress);
@@ -43,13 +42,13 @@ const getERC20AmountToApprove = async (
     throw new ApproveError(`failed to get allowance: ${message}`);
   }
 
-  // get the amount that needs to be approved
+  // check if approval is needed
   const requiredAmount = tokenAmount.value.sub(allowance);
   if (requiredAmount.isNegative() || requiredAmount.isZero()) {
-    throw new AlreadyApprovedError(tokenAmount.toString(), tokenAmount.token.address, spenderAddress);
+    return false;
   }
 
-  return newAmount(requiredAmount, tokenAmount.token);
+  return true;
 };
 
 /**
@@ -116,19 +115,10 @@ export const getApproveTransaction = async (
   tokenAmount: CoinAmount<ERC20>,
   spenderAddress: string,
 ): Promise<TransactionRequest | null> => {
-  let amountToApprove: CoinAmount<ERC20>;
-  try {
-    amountToApprove = await getERC20AmountToApprove(provider, ownerAddress, tokenAmount, spenderAddress);
-  } catch (e) {
-    if (e instanceof AlreadyApprovedError) {
-      // already approved for the required amount, nothing to do
-      return null;
-    }
-
-    throw e;
-  }
+  const needsApproval = await doesSpenderNeedApproval(provider, ownerAddress, tokenAmount, spenderAddress);
 
-  return getUnsignedERC20ApproveTransaction(ownerAddress, amountToApprove, spenderAddress);
+  // @dev approvals are not additive, so we need to approve the full amount
+  return needsApproval ? getUnsignedERC20ApproveTransaction(ownerAddress, tokenAmount, spenderAddress) : null;
 };
 
 export async function getApproveGasEstimate(