fix/payment-gas-estimation (#91)

* feat(utils): estimatePaymentGas

* refactor(utils): estimation values

* refactor(RelayClient): isCustom handler

* refactor(rRlayClient): method name and test

Author: franciscotobar

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@rsksmart/rif-relay-client",
-  "version": "2.2.0",
+  "version": "2.2.1",
   "private": false,
   "description": "This project contains all the client code for the rif relay system.",
   "license": "MIT",

src/RelayClient.ts

@@ -59,8 +59,9 @@ import EnvelopingEventEmitter, {
 import { standardMaxPossibleGasEstimation } from './gasEstimator';
 import {
   estimateInternalCallGas,
-  estimateTokenTransferGas,
+  estimatePaymentGas,
   getSmartWalletAddress,
+  isDataEmpty,
   maxPossibleGasVerification,
   validateRelayResponse,
 } from './utils';
@@ -92,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
@@ -132,7 +134,7 @@ class RelayClient extends EnvelopingEventEmitter {
     const tokenAmount =
       (await envelopingRequest.request.tokenAmount) ?? constants.Zero;
 
-    if (this._isContractCallInvalid(to, data, value)) {
+    if (!isCustom && this._isCallInvalid(to, data, value)) {
       throw new Error('Contract execution needs data or value to be sent.');
     }
 
@@ -264,14 +266,14 @@ class RelayClient extends EnvelopingEventEmitter {
     return completeRequest;
   };
 
-  private _isContractCallInvalid(
+  private _isCallInvalid(
     to: string,
     data: BytesLike,
     value: BigNumberish
   ): boolean {
     return (
       to != constants.AddressZero &&
-      data === '0x00' &&
+      isDataEmpty(data.toString()) &&
       BigNumber.from(value).isZero()
     );
   }
@@ -324,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,
@@ -350,8 +353,8 @@ class RelayClient extends EnvelopingEventEmitter {
     isCustom?: boolean
   ): Promise<BigNumber> {
     const {
-      request: { tokenGas, tokenAmount, tokenContract },
-      relayData: { gasPrice, callForwarder },
+      request: { tokenGas, tokenAmount },
+      relayData: { callForwarder },
     } = envelopingRequest;
 
     const currentTokenAmount = BigNumber.from(tokenAmount);
@@ -387,25 +390,16 @@ class RelayClient extends EnvelopingEventEmitter {
         })
       : await callForwarder;
 
-    const isNativePayment = (await tokenContract) === constants.AddressZero;
-
-    return isNativePayment
-      ? await estimateInternalCallGas({
-          from: origin,
-          to,
-          gasPrice,
-          data,
-        })
-      : await estimateTokenTransferGas({
-          relayRequest: {
-            ...envelopingRequest,
-            relayData: {
-              ...envelopingRequest.relayData,
-              feesReceiver,
-            },
-          },
-          preDeploySWAddress: origin,
-        });
+    return await estimatePaymentGas({
+      relayRequest: {
+        ...envelopingRequest,
+        relayData: {
+          ...envelopingRequest.relayData,
+          feesReceiver,
+        },
+      },
+      preDeploySWAddress: origin,
+    });
   }
 
   private async _calculateGasPrice(): Promise<BigNumber> {
@@ -491,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

@@ -21,7 +21,19 @@ type RequestConfig = {
   estimatedGasCorrectionFactor?: BigNumberish;
 };
 
+type PaymentGasEstimationParams = Pick<EnvelopingTxRequest, 'relayRequest'> &
+  Pick<
+    RequestConfig,
+    | 'isSmartWalletDeploy'
+    | 'preDeploySWAddress'
+    | 'internalEstimationCorrection'
+    | 'estimatedGasCorrectionFactor'
+  >;
+
 //FIXME name standardization
+/**
+ * @deprecated The type was replaced by {@link PaymentGasEstimationParams}
+ */
 type TokenGasEstimationParams = Pick<EnvelopingTxRequest, 'relayRequest'> &
   Pick<
     RequestConfig,
@@ -36,6 +48,7 @@ type EstimateInternalGasParams = Pick<
   RelayRequestBody,
   'data' | 'to' | 'from'
 > &
+  Partial<Pick<RelayRequestBody, 'value'>> &
   Pick<EnvelopingRequestData, 'gasPrice'> &
   Pick<
     RequestConfig,
@@ -67,6 +80,7 @@ type SmartWalletAddressTxOptions = {
 
 export type {
   RequestConfig,
+  PaymentGasEstimationParams,
   TokenGasEstimationParams,
   EstimateInternalGasParams,
   HubEnvelopingTx,

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,5 +1,5 @@
 import { BigNumber, utils } from 'ethers';
-import { getSmartWalletAddress, estimateTokenTransferGas } from '../utils';
+import { getSmartWalletAddress, estimatePaymentGas } from '../utils';
 import { isDeployRequest } from '../common/relayRequest.utils';
 import type { EnvelopingTxRequest } from '../common/relayTransaction.types';
 import {
@@ -39,7 +39,7 @@ const estimateRelayMaxPossibleGas = async (
       })
     : undefined;
 
-  const tokenEstimation = await estimateTokenTransferGas({
+  const tokenEstimation = await estimatePaymentGas({
     relayRequest: {
       ...relayRequest,
       request: {

src/utils.ts

@@ -29,6 +29,7 @@ import type {
   EnvelopingTxRequest,
   DeployRequest,
   RelayRequest,
+  PaymentGasEstimationParams,
 } from './common';
 import {
   MISSING_SMART_WALLET_ADDRESS,
@@ -38,9 +39,10 @@ import {
 import RelayClient from './RelayClient';
 import type { HttpClient } from './api/common';
 
-const INTERNAL_TRANSACTION_ESTIMATED_CORRECTION = 20000; // 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('0x');
+const SHA3_NULL_S = utils.keccak256('0x00');
 const FACTOR = 0.25;
 const GAS_VERIFICATION_ATTEMPTS = 4;
 
@@ -61,6 +63,13 @@ const estimateInternalCallGas = async ({
 
   let estimation: BigNumber = await provider.estimateGas(estimateGasParams);
 
+  const data = await estimateGasParams.data;
+
+  if (isDataEmpty(data.toString())) {
+    internalEstimationCorrection =
+      INTERNAL_TRANSACTION_NATIVE_ESTIMATED_CORRECTION;
+  }
+
   estimation = applyInternalEstimationCorrection(
     estimation,
     internalEstimationCorrection
@@ -69,6 +78,73 @@ const estimateInternalCallGas = async ({
   return applyGasCorrectionFactor(estimation, estimatedGasCorrectionFactor);
 };
 
+const estimatePaymentGas = async ({
+  internalEstimationCorrection,
+  estimatedGasCorrectionFactor,
+  preDeploySWAddress,
+  relayRequest,
+}: PaymentGasEstimationParams): Promise<BigNumber> => {
+  const {
+    request: { tokenContract, tokenAmount, to },
+    relayData: { callForwarder, gasPrice, feesReceiver },
+  } = relayRequest;
+
+  if (tokenAmount.toString() === '0') {
+    return constants.Zero;
+  }
+
+  let tokenOrigin: PromiseOrValue<string> | undefined;
+
+  if (isDeployRequest(relayRequest)) {
+    tokenOrigin = preDeploySWAddress;
+
+    // If it is a deploy and tokenGas was not defined, then the smartwallet address
+    // is required to estimate the token gas. This value should be calculated prior to
+    // the call to this function
+    if (!tokenOrigin || tokenOrigin === constants.AddressZero) {
+      throw Error(MISSING_SMART_WALLET_ADDRESS);
+    }
+  } else {
+    tokenOrigin = callForwarder;
+
+    if (tokenOrigin === constants.AddressZero) {
+      throw Error(MISSING_CALL_FORWARDER);
+    }
+  }
+
+  const isNativePayment = (await tokenContract) === constants.AddressZero;
+
+  if (isNativePayment) {
+    return await estimateInternalCallGas({
+      from: tokenOrigin,
+      to,
+      gasPrice,
+      value: tokenAmount,
+      data: '0x',
+    });
+  }
+
+  const provider = getProvider();
+  const erc20 = IERC20__factory.connect(await tokenContract, provider);
+  const gasCost = await erc20.estimateGas.transfer(feesReceiver, tokenAmount, {
+    from: tokenOrigin,
+    gasPrice,
+  });
+
+  const internalCallCost = applyInternalEstimationCorrection(
+    gasCost,
+    internalEstimationCorrection
+  );
+
+  return applyGasCorrectionFactor(
+    internalCallCost,
+    estimatedGasCorrectionFactor
+  );
+};
+
+/**
+ * @deprecated The method was replaced by {@link estimatePaymentGas}
+ */
 const estimateTokenTransferGas = async ({
   internalEstimationCorrection,
   estimatedGasCorrectionFactor,
@@ -142,7 +218,7 @@ const getSmartWalletAddress = async ({
 
   const recovererAddress = recoverer ?? constants.AddressZero;
 
-  const logicParamsHash = data ?? SHA3_NULL_S;
+  const logicParamsHash = data ? utils.keccak256(data) : SHA3_NULL_S;
 
   const logic = to ?? constants.AddressZero;
 
@@ -362,17 +438,24 @@ const applyFactor = (value: BigNumberish, factor: number) => {
   );
 };
 
+const isDataEmpty = (data: string) => {
+  return ['', '0x', '0x00'].includes(data);
+};
+
 export {
   estimateInternalCallGas,
+  estimatePaymentGas,
   estimateTokenTransferGas,
   getSmartWalletAddress,
   applyGasCorrectionFactor,
   applyInternalEstimationCorrection,
   INTERNAL_TRANSACTION_ESTIMATED_CORRECTION,
+  INTERNAL_TRANSACTION_NATIVE_ESTIMATED_CORRECTION,
   ESTIMATED_GAS_CORRECTION_FACTOR,
   SHA3_NULL_S,
   validateRelayResponse,
   useEnveloping,
   getRelayClientGenerator,
   maxPossibleGasVerification,
+  isDataEmpty,
 };