fix: remove unused dynamic fees support (#335)

As result of my effort to implement #334 directly into SDK, made some small changes that I would leave here: - util function that checks if the route exists from source chain router - removed the old dynamic fee code as it is not relevant anymore ## Related Issue Or Context    ## How Has This Been Tested? Testing details.    - Expanded unit tests ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation ## Checklist:   - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have ensured that all acceptance criteria (or expected behavior) from issue are met - [ ] I have updated the documentation locally and in chainbridge-docs. - [x] I have added tests to cover my changes. - [x] I have ensured that all the checks are passing and green, I've signed the CLA bot --------- Signed-off-by: Marin Petrunic <[email protected]> Co-authored-by: MakMuftic <[email protected]> Co-authored-by: Marin Petrunic <[email protected]>
sprintertech · Feb 13, 2024 · 63302fd · 63302fd
1 parent c97e684
commit 63302fd
Show file tree

Hide file tree

Showing 10 changed files with 146 additions and 140 deletions.
diff --git a/packages/sdk/src/chains/BaseAssetTransfer.ts b/packages/sdk/src/chains/BaseAssetTransfer.ts
@@ -4,6 +4,7 @@ import type {
   Environment,
   EvmResource,
   Fungible,
+  Resource,
   SubstrateResource,
   Transfer,
   TransferType,
@@ -72,6 +73,14 @@ export abstract class BaseAssetTransfer {
     return transfer;
   }
 
+  /**
+   * Returns if route is registered for resource between source domain of asset transfer and provided destination domain (on-chain check).
+   *
+   * @param {string} destinationDomainID - Destination domain ID
+   * @param resource - Resource for which we want to check if route is opened
+   */
+  abstract isRouteRegistered(destinationDomainID: string, resource: Resource): Promise<boolean>;
+
   /**
    * @param {Transfer} transfer Transfer to check
    * @param {string} destinationProviderUrl URL of the destination chain provider

diff --git a/packages/sdk/src/chains/EVM/assetTransfer.ts b/packages/sdk/src/chains/EVM/assetTransfer.ts
@@ -1,5 +1,5 @@
 import type { PopulatedTransaction, UnsignedTransaction, providers } from 'ethers';
-import { BigNumber } from 'ethers';
+import { BigNumber, utils, constants } from 'ethers';
 
 import type { ERC20, ERC721MinterBurnerPauser } from '@buildwithsygma/sygma-contracts';
 import {
@@ -13,18 +13,17 @@ import type {
   EvmResource,
   Fungible,
   NonFungible,
+  Resource,
   Transfer,
   TransferType,
 } from '../../types/index.js';
 import { Environment, FeeHandlerType, ResourceType } from '../../types/index.js';
-import { Config } from '../../index.js';
-import { getFeeOracleBaseURL } from '../../utils.js';
+import { Config, getFeeHandlerAddress } from '../../index.js';
 import { BaseAssetTransfer } from '../BaseAssetTransfer.js';
 import type { EvmFee } from './index.js';
 import {
   approve,
   calculateBasicfee,
-  calculateDynamicFee,
   createERCDepositData,
   erc20Transfer,
   erc721Transfer,
@@ -110,24 +109,6 @@ export class EVMAssetTransfer extends BaseAssetTransfer {
           sender: transfer.sender,
         });
       }
-      case FeeHandlerType.DYNAMIC: {
-        const fungibleTransfer = transfer as Transfer<Fungible>;
-        return await calculateDynamicFee({
-          provider: this.provider,
-          sender: transfer.sender,
-          fromDomainID: Number(transfer.from.id),
-          toDomainID: Number(transfer.to.id),
-          resourceID: transfer.resource.resourceId,
-          tokenAmount: fungibleTransfer.details.amount,
-          feeOracleBaseUrl: getFeeOracleBaseURL(this.environment),
-          feeHandlerAddress: feeHandlerAddress,
-          depositData: createERCDepositData(
-            fungibleTransfer.details.amount,
-            fungibleTransfer.details.recipient,
-            fungibleTransfer.details.parachainId,
-          ),
-        });
-      }
       case FeeHandlerType.PERCENTAGE: {
         const fungibleTransfer = transfer as Transfer<Fungible>;
         return await getPercentageFee({
@@ -253,6 +234,20 @@ export class EVMAssetTransfer extends BaseAssetTransfer {
     }
   }
 
+  override async isRouteRegistered(
+    destinationDomainID: string,
+    resource: Resource,
+  ): Promise<boolean> {
+    const config = this.config.getSourceDomainConfig() as EthereumConfig;
+    const registeredHandler = await getFeeHandlerAddress(
+      this.provider,
+      config.feeRouter,
+      destinationDomainID,
+      resource.resourceId,
+    );
+    return utils.isAddress(registeredHandler) && registeredHandler != constants.AddressZero;
+  }
+
   private async getERC20Approvals(
     erc20: ERC20,
     fee: EvmFee,

diff --git a/packages/sdk/src/chains/EVM/fee/feeHandler.ts b/packages/sdk/src/chains/EVM/fee/feeHandler.ts
@@ -26,7 +26,7 @@ import type { ethers } from 'ethers';
  * @returns {Promise<string>} A promise that resolves to the fee handler address.
  */
 export const getFeeHandlerAddress = async (
-  signerOrProvider: ethers.providers.JsonRpcProvider | ethers.Signer,
+  signerOrProvider: ethers.providers.BaseProvider,
   feeRouterAddress: string,
   domainId: string,
   resourceId: string,

diff --git a/packages/sdk/src/chains/EVM/genericMessage.ts b/packages/sdk/src/chains/EVM/genericMessage.ts
@@ -2,15 +2,13 @@ import type { PopulatedTransaction, providers } from 'ethers';
 
 import { Bridge__factory, FeeHandlerRouter__factory } from '@buildwithsygma/sygma-contracts';
 
-import { getFeeOracleBaseURL } from '../../utils.js';
 import type { EthereumConfig, GenericMessage, Transfer, TransferType } from '../../types/index.js';
 import { Environment, FeeHandlerType, ResourceType } from '../../types/index.js';
 
 import { Config } from '../../config.js';
 
 import type { EvmFee } from './types/index.js';
-import { calculateBasicfee, calculateDynamicFee } from './fee/index.js';
-import { createPermissionlessGenericDepositData } from './helpers.js';
+import { calculateBasicfee } from './fee/index.js';
 import { genericMessageTransfer } from './utils/index.js';
 
 export class EVMGenericMessageTransfer {
@@ -59,27 +57,6 @@ export class EVMGenericMessageTransfer {
           sender: transfer.sender,
         });
       }
-      case FeeHandlerType.DYNAMIC: {
-        const genericTransfer = transfer as Transfer<GenericMessage>;
-        return await calculateDynamicFee({
-          provider: this.provider,
-          sender: transfer.sender,
-          fromDomainID: Number(transfer.from.id),
-          toDomainID: Number(transfer.to.id),
-          resourceID: transfer.resource.resourceId,
-          feeOracleBaseUrl: getFeeOracleBaseURL(this.environment),
-          feeHandlerAddress: feeHandlerAddress,
-          depositData: createPermissionlessGenericDepositData(
-            genericTransfer.details.destinationFunctionSignature,
-            genericTransfer.details.destinationContractAddress,
-            genericTransfer.details.maxFee,
-            transfer.sender,
-            genericTransfer.details.executionData,
-          ),
-          tokenAmount: genericTransfer.details.tokenAmount,
-          maxFee: genericTransfer.details.maxFee,
-        });
-      }
       default:
         throw new Error(`Unsupported fee handler type`);
     }

diff --git a/packages/sdk/src/chains/Substrate/assetTransfer.ts b/packages/sdk/src/chains/Substrate/assetTransfer.ts
@@ -2,7 +2,13 @@ import type { ApiPromise, SubmittableResult } from '@polkadot/api';
 import type { SubmittableExtrinsic } from '@polkadot/api/types';
 import type { U256 } from '@polkadot/types';
 import { BN } from '@polkadot/util';
-import type { Fungible, SubstrateResource, Transfer, TransferType } from '../../types/index.js';
+import type {
+  Fungible,
+  SubstrateResource,
+  Transfer,
+  TransferType,
+  Resource,
+} from '../../types/index.js';
 import { Environment, FeeHandlerType, ResourceType } from '../../types/index.js';
 import { Config } from '../../index.js';
 import { BaseAssetTransfer } from '../BaseAssetTransfer.js';
@@ -123,4 +129,16 @@ export class SubstrateAssetTransfer extends BaseAssetTransfer {
         );
     }
   }
+
+  override async isRouteRegistered(
+    destinationDomainID: string,
+    resource: Resource,
+  ): Promise<boolean> {
+    const feeHandler = await getFeeHandler(
+      this.apiPromise,
+      Number(destinationDomainID),
+      (resource as SubstrateResource).xcmMultiAssetId,
+    );
+    return feeHandler != FeeHandlerType.UNDEFINED;
+  }
 }
diff --git a/packages/sdk/src/chains/Substrate/utils/getFeeHandler.ts b/packages/sdk/src/chains/Substrate/utils/getFeeHandler.ts
@@ -2,12 +2,13 @@ import type { ApiPromise } from '@polkadot/api';
 import type { Enum, Option } from '@polkadot/types';
 import { FeeHandlerType } from '../../../types/index.js';
 import type { XcmMultiAssetIdType } from '../types/index.js';
+
 /**
  * Retrieves the fee handler for a given domainId and asset.
  *
  * @category Fee
  * @param {ApiPromise} api - The Substrate API instance.
- * @param {number} domainId - The ID of the domain.
+ * @param {number} destinationDomainId - The ID of the domain.
  * @param {XcmMultiAssetIdType} xcmMultiAssetId - The XCM MultiAsset ID of the asset. {@link https://github.com/sygmaprotocol/sygma-substrate-pallets#multiasset | More details}
  * @returns {Promise} A promise that resolves to a FeeHandlerType object.
  * @throws {Error} Unable to retrieve fee from pallet
@@ -23,7 +24,7 @@ export const getFeeHandler = async (
   ]);
 
   if (feeHandler.isNone) {
-    throw new Error('No Fee Handler configured');
+    return FeeHandlerType.UNDEFINED;
   }
 
   const feeHandlerName = feeHandler.unwrap().toString();

diff --git a/packages/sdk/src/types/types.ts b/packages/sdk/src/types/types.ts
@@ -50,6 +50,7 @@ export enum FeeHandlerType {
   DYNAMIC = 'oracle',
   BASIC = 'basic',
   PERCENTAGE = 'percentage',
+  UNDEFINED = 'undefined',
 }
 
 type AssetTransfer = {

diff --git a/packages/sdk/src/utils.ts b/packages/sdk/src/utils.ts
@@ -2,22 +2,6 @@ import { IndexerUrl, ExplorerUrl } from './constants.js';
 import type { TransferStatus, TransferStatusResponse } from './types/index.js';
 import { Environment } from './types/index.js';
 
-export const DEVNET_FEE_ORACLE_BASE_URL: string = 'https://fee-oracle.develop.buildwithsygma.com/';
-export const TESTNET_FEE_ORACLE_BASE_URL: string = 'https://fee-oracle.test.buildwithsygma.com/';
-export const MAINNET_FEE_ORACLE_BASE_URL: string = '';
-
-export function getFeeOracleBaseURL(environment?: Environment): string {
-  switch (environment) {
-    case Environment.DEVNET:
-      return DEVNET_FEE_ORACLE_BASE_URL;
-    case Environment.TESTNET:
-      return TESTNET_FEE_ORACLE_BASE_URL;
-    case Environment.MAINNET:
-    default:
-      return MAINNET_FEE_ORACLE_BASE_URL;
-  }
-}
-
 /**
  * @@description Get the status of a transfer using transaction hash and optionally domain id
  */

diff --git a/packages/sdk/test/chains/EVM/assetTransfer.test.ts b/packages/sdk/test/chains/EVM/assetTransfer.test.ts
@@ -1,54 +1,47 @@
 import axios from 'axios';
-import { BigNumber, providers } from 'ethers';
+import type { providers } from 'ethers';
+import { BigNumber } from 'ethers';
 import MockAdapter from 'axios-mock-adapter';
 
-import {
-  Transfer,
-  ResourceType,
-  FeeHandlerType,
-  Environment,
-  NonFungible,
-  Fungible,
-  Network,
-} from '../../../src/types';
-import { testingConfigData } from '../../constants';
-import { ConfigUrl } from '../../../src/constants';
-import { EVMAssetTransfer } from '../../../src/chains/EVM';
-import * as EVM from '../../../src/chains/EVM';
-import { DEVNET_FEE_ORACLE_BASE_URL } from '../../../src/utils';
+import type { Transfer, NonFungible, Fungible } from '../../../src/types/index.js';
+import { ResourceType, FeeHandlerType, Environment, Network } from '../../../src/types/index.js';
+import { testingConfigData } from '../../constants.js';
+import { ConfigUrl } from '../../../src/constants.js';
+import { EVMAssetTransfer } from '../../../src/chains/EVM/index.js';
+import * as EVM from '../../../src/chains/EVM/index.js';
 
 const feeHandlerAddressFunction = jest.fn();
 const resourceHandlerFunction = jest.fn();
 jest.mock(
   '@buildwithsygma/sygma-contracts',
   () =>
-  ({
-    ...jest.requireActual('@buildwithsygma/sygma-contracts'),
-    FeeHandlerRouter__factory: {
-      connect: () => {
-        return {
-          _domainResourceIDToFeeHandlerAddress: feeHandlerAddressFunction,
-        };
+    ({
+      ...jest.requireActual('@buildwithsygma/sygma-contracts'),
+      FeeHandlerRouter__factory: {
+        connect: () => {
+          return {
+            _domainResourceIDToFeeHandlerAddress: feeHandlerAddressFunction,
+          };
+        },
       },
-    },
-    ERC20__factory: {
-      connect: () => {
-        return {};
+      ERC20__factory: {
+        connect: () => {
+          return {};
+        },
       },
-    },
-    ERC721MinterBurnerPauser__factory: {
-      connect: () => {
-        return {};
+      ERC721MinterBurnerPauser__factory: {
+        connect: () => {
+          return {};
+        },
       },
-    },
-    Bridge__factory: {
-      connect: () => {
-        return {
-          _resourceIDToHandlerAddress: resourceHandlerFunction,
-        };
+      Bridge__factory: {
+        connect: () => {
+          return {
+            _resourceIDToHandlerAddress: resourceHandlerFunction,
+          };
+        },
       },
-    },
-  } as unknown),
+    }) as unknown,
 );
 const axiosMock = new MockAdapter(axios);
 const mockProvider: Partial<providers.Provider> = {
@@ -62,11 +55,6 @@ const calculateBasicFeeMock = jest.spyOn(EVM, 'calculateBasicfee').mockResolvedV
   type: FeeHandlerType.BASIC,
   handlerAddress: '0xC2a1E379E2d255F42f3F8cA7Be32E3C3E1767622',
 });
-const calculateDynamicFeeMock = jest.spyOn(EVM, 'calculateDynamicFee').mockResolvedValue({
-  fee: BigNumber.from('100'),
-  type: FeeHandlerType.DYNAMIC,
-  handlerAddress: '0xe495c86962DcA7208ECcF2020A273395AcE8da3e',
-});
 const isApprovedMock = jest.spyOn(EVM, 'isApproved');
 const getAllowanceMock = jest.spyOn(EVM, 'getERC20Allowance');
 const approveMock = jest.spyOn(EVM, 'approve');
@@ -160,31 +148,6 @@ describe('EVM asset transfer', () => {
       });
     });
 
-    it('Should successfully get dynamic fee', async function () {
-      feeHandlerAddressFunction.mockResolvedValue('0xe495c86962DcA7208ECcF2020A273395AcE8da3e');
-
-      const fee = await assetTransfer.getFee(transfer);
-
-      expect(fee).toEqual({
-        fee: BigNumber.from('100'),
-        type: FeeHandlerType.DYNAMIC,
-        handlerAddress: '0xe495c86962DcA7208ECcF2020A273395AcE8da3e',
-      });
-      expect(calculateDynamicFeeMock).toBeCalled();
-      expect(calculateDynamicFeeMock).toBeCalledWith({
-        feeHandlerAddress: '0xe495c86962DcA7208ECcF2020A273395AcE8da3e',
-        feeOracleBaseUrl: DEVNET_FEE_ORACLE_BASE_URL,
-        toDomainID: transfer.to.id,
-        fromDomainID: transfer.from.id,
-        provider: mockProvider,
-        resourceID: transfer.resource.resourceId,
-        sender: transfer.sender,
-        tokenAmount: '200',
-        depositData:
-          '0x00000000000000000000000000000000000000000000000000000000000000c80000000000000000000000000000000000000000000000000000000000000014557abec0cb31aa925577441d54c090987c2ed818',
-      });
-    });
-
     it('Should throw error for unsupported fee handler', async function () {
       feeHandlerAddressFunction.mockResolvedValue('0xunsupported');
 
@@ -300,4 +263,25 @@ describe('EVM asset transfer', () => {
       expect(erc721TransferMock).toBeCalled();
     });
   });
+
+  describe('isRouteRegistered', () => {
+    it('should return true if fee handler address is registered', async () => {
+      feeHandlerAddressFunction.mockResolvedValue('0xC2a1E379E2d255F42f3F8cA7Be32E3C3E1767622');
+
+      const at = await assetTransfer.isRouteRegistered('1', transfer.resource);
+      expect(at).toBe(true);
+    });
+    it('should return false if fee handler address is zero', async () => {
+      feeHandlerAddressFunction.mockResolvedValue('0x0000000000000000000000000000000000000000');
+
+      const at = await assetTransfer.isRouteRegistered('1', transfer.resource);
+      expect(at).toBe(false);
+    });
+    it('should return false if fee handler address is zero', async () => {
+      feeHandlerAddressFunction.mockResolvedValue('0xunsupported');
+
+      const at = await assetTransfer.isRouteRegistered('1', transfer.resource);
+      expect(at).toBe(false);
+    });
+  });
 });