Revert "refactor: pr comments"

This reverts commit 865d18d.

src/RelayClient.ts

@@ -61,7 +61,6 @@ import {
   estimateInternalCallGas,
   estimatePaymentGas,
   getSmartWalletAddress,
-  isCustomSmartWalletDeployment,
   isDataEmpty,
   maxPossibleGasVerification,
   validateRelayResponse,
@@ -94,7 +93,8 @@ class RelayClient extends EnvelopingEventEmitter {
   }
 
   private _getEnvelopingRequestDetails = async (
-    envelopingRequest: UserDefinedEnvelopingRequest
+    envelopingRequest: UserDefinedEnvelopingRequest,
+    isCustom = false
   ): Promise<EnvelopingRequest> => {
     const isDeployment: boolean = isDeployRequest(
       envelopingRequest as EnvelopingRequest
@@ -134,6 +134,10 @@ class RelayClient extends EnvelopingEventEmitter {
     const tokenAmount =
       (await envelopingRequest.request.tokenAmount) ?? constants.Zero;
 
+    if (!isCustom && this._isContractCallInvalid(to, data, value)) {
+      throw new Error('Contract execution needs data or value to be sent.');
+    }
+
     const { index } = request as UserDefinedDeployRequestBody;
 
     if (isDeployment && isNullOrUndefined(await index)) {
@@ -298,23 +302,10 @@ class RelayClient extends EnvelopingEventEmitter {
       throw new Error('FeesReceiver has to be a valid non-zero address');
     }
 
-    // At this point all the properties from the envelopingRequest were validated and awaited.
-    const isCustom = await isCustomSmartWalletDeployment(envelopingRequest);
-
-    const { to, data, value } = envelopingRequest.request as {
-      to: string;
-      data: string;
-      value: BigNumberish;
-    };
-
-    if (!isCustom && this._isContractCallInvalid(to, data, value)) {
-      throw new Error('Contract execution needs data or value to be sent.');
-    }
-
     const tokenGas = await this._prepareTokenGas(
       feesReceiver,
       envelopingRequest,
-      isCustom
+      options?.isCustom
     );
 
     const updatedRelayRequest: EnvelopingRequest = {
@@ -335,6 +326,7 @@ class RelayClient extends EnvelopingEventEmitter {
       relayHubAddress: await relayHub,
       signature: await accountManager.sign(updatedRelayRequest, signerWallet),
       relayMaxNonce,
+      isCustom: options?.isCustom,
     };
     const httpRequest: EnvelopingTxRequest = {
       relayRequest: updatedRelayRequest,
@@ -354,10 +346,11 @@ class RelayClient extends EnvelopingEventEmitter {
     return httpRequest;
   }
 
+  // At this point all the properties from the envelopingRequest were validated and awaited.
   private async _prepareTokenGas(
     feesReceiver: string,
     envelopingRequest: EnvelopingRequest,
-    isCustom: boolean
+    isCustom?: boolean
   ): Promise<BigNumber> {
     const {
       request: { tokenGas, tokenAmount },
@@ -492,7 +485,8 @@ class RelayClient extends EnvelopingEventEmitter {
     options?: RelayTxOptions
   ): Promise<HubEnvelopingTx> {
     const envelopingRequestDetails = await this._getEnvelopingRequestDetails(
-      envelopingRequest
+      envelopingRequest,
+      options?.isCustom
     );
 
     //FIXME we should implement the relay selection strategy

src/common/relayClient.types.ts

@@ -32,7 +32,7 @@ type PaymentGasEstimationParams = Pick<EnvelopingTxRequest, 'relayRequest'> &
 
 //FIXME name standardization
 /**
- * @deprecated The type was replaced by {@link PaymentGasEstimationParams}
+ * @deprecated The method was replaced by {@link PaymentGasEstimationParams}
  */
 type TokenGasEstimationParams = Pick<EnvelopingTxRequest, 'relayRequest'> &
   Pick<
@@ -65,6 +65,7 @@ type IgnoreVerifications = 'relayHub' | 'workerBalance' | 'verifiers';
 type RelayTxOptions = {
   signerWallet?: Wallet;
   ignoreVerifications?: Array<IgnoreVerifications>;
+  isCustom?: boolean;
 };
 
 type SmartWalletAddressTxOptions = {

src/common/relayHub.types.ts

@@ -30,6 +30,7 @@ type EnvelopingMetadata = {
   relayHubAddress: RelayRequestBody['relayHub'];
   relayMaxNonce: number;
   signature: string;
+  isCustom?: boolean;
 };
 
 export type { HubInfo, EnvelopingMetadata, RelayInfo, RelayManagerData };

src/gasEstimator/gasEstimator.ts

@@ -1,19 +1,17 @@
 import { BigNumber, utils } from 'ethers';
-import {
-  getSmartWalletAddress,
-  estimatePaymentGas,
-  isCustomSmartWalletDeployment,
-} from '../utils';
+import { getSmartWalletAddress, estimatePaymentGas } from '../utils';
 import { isDeployRequest } from '../common/relayRequest.utils';
 import type { EnvelopingTxRequest } from '../common/relayTransaction.types';
 import {
   standardMaxPossibleGasEstimation,
   linearFitMaxPossibleGasEstimation,
 } from './utils';
+import type { RelayTxOptions } from 'src/common';
 
 const estimateRelayMaxPossibleGas = async (
   envelopingRequest: EnvelopingTxRequest,
-  relayWorkerAddress: string
+  relayWorkerAddress: string,
+  options?: RelayTxOptions
 ): Promise<BigNumber> => {
   const {
     relayRequest,
@@ -26,10 +24,9 @@ const estimateRelayMaxPossibleGas = async (
 
   const smartWalletIndex = await index;
 
-  const callForwarder = await relayRequest.relayData.callForwarder;
-
-  const isCustom = await isCustomSmartWalletDeployment(relayRequest);
+  const callForwarder = relayRequest.relayData.callForwarder.toString();
 
+  const isCustom = options?.isCustom;
   const preDeploySWAddress = isSmartWalletDeploy
     ? await getSmartWalletAddress({
         owner: await from,

src/utils.ts

@@ -8,7 +8,6 @@ import {
 } from 'ethers';
 import { BigNumber as BigNumberJs } from 'bignumber.js';
 import {
-  DestinationContractHandler__factory,
   ICustomSmartWalletFactory__factory,
   IERC20__factory,
   ISmartWalletFactory__factory,
@@ -31,7 +30,6 @@ import type {
   DeployRequest,
   RelayRequest,
   PaymentGasEstimationParams,
-  EnvelopingRequest,
 } from './common';
 import {
   MISSING_SMART_WALLET_ADDRESS,
@@ -41,7 +39,7 @@ import {
 import RelayClient from './RelayClient';
 import type { HttpClient } from './api/common';
 
-const INTERNAL_TRANSACTION_ESTIMATED_CORRECTION = 19500; // When estimating the gas an internal call is going to spend, we need to substract some gas inherent to send the parameters to the blockchain
+const INTERNAL_TRANSACTION_ESTIMATED_CORRECTION = 18500; // When estimating the gas an internal call is going to spend, we need to substract some gas inherent to send the parameters to the blockchain
 const INTERNAL_TRANSACTION_NATIVE_ESTIMATED_CORRECTION = 10500;
 const ESTIMATED_GAS_CORRECTION_FACTOR = 1;
 const SHA3_NULL_S = utils.keccak256('0x00');
@@ -290,7 +288,7 @@ const applyInternalEstimationCorrection = (
  * requested transaction and validate its signature.
  */
 const validateRelayResponse = (
-  envelopingRequest: EnvelopingTxRequest,
+  request: EnvelopingTxRequest,
   transaction: Transaction,
   relayWorkerAddress: string
 ): void => {
@@ -303,7 +301,7 @@ const validateRelayResponse = (
   const {
     metadata: { signature, relayMaxNonce },
     relayRequest,
-  } = envelopingRequest;
+  } = request;
   const requestMaxNonce = BigNumber.from(relayMaxNonce).toNumber();
   log.debug('validateRelayResponse - Transaction is', transaction);
 
@@ -315,7 +313,7 @@ const validateRelayResponse = (
     throw Error('Transaction has no signer');
   }
 
-  const isDeploy = isDeployTransaction(envelopingRequest);
+  const isDeploy = isDeployTransaction(request);
 
   const provider = getProvider();
   const envelopingConfig = getEnvelopingConfig();
@@ -444,37 +442,6 @@ const isDataEmpty = (data: string) => {
   return ['', '0x', '0x00'].includes(data);
 };
 
-const isCustomSmartWalletDeployment = async ({
-  request,
-  relayData,
-}: EnvelopingRequest): Promise<boolean> => {
-  const index = await request.index;
-  if (!index) {
-    return false;
-  }
-
-  const to = await request.to;
-  if (to == constants.AddressZero) {
-    return false;
-  }
-
-  try {
-    const callVerifier = await relayData.callVerifier;
-    const provider = getProvider();
-    const verifier = DestinationContractHandler__factory.connect(
-      callVerifier,
-      provider
-    );
-    await verifier.acceptsContract(to);
-
-    return false;
-  } catch (error) {
-    log.warn(error);
-  }
-
-  return true;
-};
-
 export {
   estimateInternalCallGas,
   estimatePaymentGas,
@@ -491,5 +458,4 @@ export {
   getRelayClientGenerator,
   maxPossibleGasVerification,
   isDataEmpty,
-  isCustomSmartWalletDeployment,
 };

test/RelayClient.test.ts

@@ -152,7 +152,7 @@ describe('RelayClient', function () {
       _prepareTokenGas: (
         feesReceiver: string,
         envelopingRequest: EnvelopingRequest,
-        isCustom: boolean
+        isCustom?: boolean
       ) => Promise<BigNumber>;
       _calculateGasPrice(): Promise<BigNumber>;
       _getEnvelopingRequestDetails: (
@@ -424,8 +424,6 @@ describe('RelayClient', function () {
     });
 
     describe('_prepareTokenGas', function () {
-      const isCustom = false;
-
       it('should return zero if token amount is zero', async function () {
         const tokenGas = await relayClient._prepareTokenGas(
           FAKE_HUB_INFO.feesReceiver,
@@ -435,8 +433,7 @@ describe('RelayClient', function () {
               ...FAKE_RELAY_REQUEST.request,
               tokenAmount: constants.Zero,
             },
-          },
-          isCustom
+          }
         );
 
         expect(tokenGas).to.be.eql(constants.Zero);
@@ -446,8 +443,7 @@ describe('RelayClient', function () {
         const request = { ...FAKE_RELAY_REQUEST };
         const tokenGas = await relayClient._prepareTokenGas(
           FAKE_HUB_INFO.feesReceiver,
-          request,
-          isCustom
+          request
         );
 
         const {
@@ -461,17 +457,13 @@ describe('RelayClient', function () {
         const addressStub = sandbox.stub(relayUtils, 'getSmartWalletAddress');
         const estimationStub = sandbox.stub(relayUtils, 'estimatePaymentGas');
 
-        await relayClient._prepareTokenGas(
-          FAKE_HUB_INFO.feesReceiver,
-          {
-            ...FAKE_DEPLOY_REQUEST,
-            request: {
-              ...FAKE_DEPLOY_REQUEST.request,
-              tokenGas: constants.Zero,
-            },
+        await relayClient._prepareTokenGas(FAKE_HUB_INFO.feesReceiver, {
+          ...FAKE_DEPLOY_REQUEST,
+          request: {
+            ...FAKE_DEPLOY_REQUEST.request,
+            tokenGas: constants.Zero,
           },
-          isCustom
-        );
+        });
 
         expect(addressStub).to.be.calledOnce;
         expect(estimationStub).to.be.calledOnce;
@@ -490,8 +482,7 @@ describe('RelayClient', function () {
               ...FAKE_DEPLOY_REQUEST.request,
               tokenGas: constants.Zero,
             },
-          },
-          isCustom
+          }
         );
 
         expect(tokenGas).to.be.equal(expectedValue);