refactor: pr comments

src/RelayClient.ts

@@ -61,6 +61,7 @@ import {
   estimateInternalCallGas,
   estimatePaymentGas,
   getSmartWalletAddress,
+  isCustomSmartWalletDeployment,
   isDataEmpty,
   maxPossibleGasVerification,
   validateRelayResponse,
@@ -93,8 +94,7 @@ class RelayClient extends EnvelopingEventEmitter {
   }
 
   private _getEnvelopingRequestDetails = async (
-    envelopingRequest: UserDefinedEnvelopingRequest,
-    isCustom = false
+    envelopingRequest: UserDefinedEnvelopingRequest
   ): Promise<EnvelopingRequest> => {
     const isDeployment: boolean = isDeployRequest(
       envelopingRequest as EnvelopingRequest
@@ -134,10 +134,6 @@ 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)) {
@@ -302,10 +298,23 @@ 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,
-      options?.isCustom
+      isCustom
     );
 
     const updatedRelayRequest: EnvelopingRequest = {
@@ -326,7 +335,6 @@ class RelayClient extends EnvelopingEventEmitter {
       relayHubAddress: await relayHub,
       signature: await accountManager.sign(updatedRelayRequest, signerWallet),
       relayMaxNonce,
-      isCustom: options?.isCustom,
     };
     const httpRequest: EnvelopingTxRequest = {
       relayRequest: updatedRelayRequest,
@@ -346,11 +354,10 @@ 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 },
@@ -485,8 +492,7 @@ class RelayClient extends EnvelopingEventEmitter {
     options?: RelayTxOptions
   ): Promise<HubEnvelopingTx> {
     const envelopingRequestDetails = await this._getEnvelopingRequestDetails(
-      envelopingRequest,
-      options?.isCustom
+      envelopingRequest
     );
 
     //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 method was replaced by {@link PaymentGasEstimationParams}
+ * @deprecated The type was replaced by {@link PaymentGasEstimationParams}
  */
 type TokenGasEstimationParams = Pick<EnvelopingTxRequest, 'relayRequest'> &
   Pick<
@@ -65,7 +65,6 @@ type IgnoreVerifications = 'relayHub' | 'workerBalance' | 'verifiers';
 type RelayTxOptions = {
   signerWallet?: Wallet;
   ignoreVerifications?: Array<IgnoreVerifications>;
-  isCustom?: boolean;
 };
 
 type SmartWalletAddressTxOptions = {

src/common/relayHub.types.ts

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

src/gasEstimator/gasEstimator.ts

@@ -1,17 +1,19 @@
 import { BigNumber, utils } from 'ethers';
-import { getSmartWalletAddress, estimatePaymentGas } from '../utils';
+import {
+  getSmartWalletAddress,
+  estimatePaymentGas,
+  isCustomSmartWalletDeployment,
+} 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,
-  options?: RelayTxOptions
+  relayWorkerAddress: string
 ): Promise<BigNumber> => {
   const {
     relayRequest,
@@ -24,9 +26,10 @@ const estimateRelayMaxPossibleGas = async (
 
   const smartWalletIndex = await index;
 
-  const callForwarder = relayRequest.relayData.callForwarder.toString();
+  const callForwarder = await relayRequest.relayData.callForwarder;
+
+  const isCustom = await isCustomSmartWalletDeployment(relayRequest);
 
-  const isCustom = options?.isCustom;
   const preDeploySWAddress = isSmartWalletDeploy
     ? await getSmartWalletAddress({
         owner: await from,

src/utils.ts

@@ -8,6 +8,7 @@ import {
 } from 'ethers';
 import { BigNumber as BigNumberJs } from 'bignumber.js';
 import {
+  DestinationContractHandler__factory,
   ICustomSmartWalletFactory__factory,
   IERC20__factory,
   ISmartWalletFactory__factory,
@@ -30,6 +31,7 @@ import type {
   DeployRequest,
   RelayRequest,
   PaymentGasEstimationParams,
+  EnvelopingRequest,
 } from './common';
 import {
   MISSING_SMART_WALLET_ADDRESS,
@@ -39,7 +41,7 @@ import {
 import RelayClient from './RelayClient';
 import type { HttpClient } from './api/common';
 
-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_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_NATIVE_ESTIMATED_CORRECTION = 10500;
 const ESTIMATED_GAS_CORRECTION_FACTOR = 1;
 const SHA3_NULL_S = utils.keccak256('0x00');
@@ -288,7 +290,7 @@ const applyInternalEstimationCorrection = (
  * requested transaction and validate its signature.
  */
 const validateRelayResponse = (
-  request: EnvelopingTxRequest,
+  envelopingRequest: EnvelopingTxRequest,
   transaction: Transaction,
   relayWorkerAddress: string
 ): void => {
@@ -301,7 +303,7 @@ const validateRelayResponse = (
   const {
     metadata: { signature, relayMaxNonce },
     relayRequest,
-  } = request;
+  } = envelopingRequest;
   const requestMaxNonce = BigNumber.from(relayMaxNonce).toNumber();
   log.debug('validateRelayResponse - Transaction is', transaction);
 
@@ -313,7 +315,7 @@ const validateRelayResponse = (
     throw Error('Transaction has no signer');
   }
 
-  const isDeploy = isDeployTransaction(request);
+  const isDeploy = isDeployTransaction(envelopingRequest);
 
   const provider = getProvider();
   const envelopingConfig = getEnvelopingConfig();
@@ -442,6 +444,37 @@ 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,
@@ -458,4 +491,5 @@ 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,6 +424,8 @@ 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,
@@ -433,7 +435,8 @@ describe('RelayClient', function () {
               ...FAKE_RELAY_REQUEST.request,
               tokenAmount: constants.Zero,
             },
-          }
+          },
+          isCustom
         );
 
         expect(tokenGas).to.be.eql(constants.Zero);
@@ -443,7 +446,8 @@ describe('RelayClient', function () {
         const request = { ...FAKE_RELAY_REQUEST };
         const tokenGas = await relayClient._prepareTokenGas(
           FAKE_HUB_INFO.feesReceiver,
-          request
+          request,
+          isCustom
         );
 
         const {
@@ -457,13 +461,17 @@ 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;
@@ -482,7 +490,8 @@ describe('RelayClient', function () {
               ...FAKE_DEPLOY_REQUEST.request,
               tokenGas: constants.Zero,
             },
-          }
+          },
+          isCustom
         );
 
         expect(tokenGas).to.be.equal(expectedValue);