From 68ed895a015bcc73974f754d7894db8c00b3aea5 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Thu, 4 Apr 2024 10:55:14 +0200 Subject: [PATCH] fix(RREL-026): avoid using PromiseOrValue type in server (#143) * fix(RREL-026): avoid using PromiseOrValue type in server * refactor(HttpEnvelopingRequest): await properties * chore(RREL-026): apply PR suggestions --------- Co-authored-by: Francisco Tobar --- src/HttpServer.ts | 6 +-- src/RelayServer.ts | 52 +++++++++++------------- src/definitions/HttpEnvelopingRequest.ts | 38 +++++++++++++++++ src/relayServerUtils.ts | 47 +++++++++++---------- test/unit/RelayServer.test.ts | 18 ++++---- test/unit/relayServerUtils.test.ts | 18 ++++---- 6 files changed, 106 insertions(+), 73 deletions(-) create mode 100644 src/definitions/HttpEnvelopingRequest.ts diff --git a/src/HttpServer.ts b/src/HttpServer.ts index ee4a350..038b021 100644 --- a/src/HttpServer.ts +++ b/src/HttpServer.ts @@ -1,4 +1,3 @@ -import type { EnvelopingTxRequest } from '@rsksmart/rif-relay-client'; import bodyParser from 'body-parser'; import cors from 'cors'; import express, { Express, Request, Response } from 'express'; @@ -8,6 +7,7 @@ import jsonrpc, { Defined } from 'jsonrpc-lite'; import log from 'loglevel'; import configureDocumentation from './DocConfiguration'; import type { RelayServer } from './RelayServer'; +import type { HttpEnvelopingRequest } from './definitions/HttpEnvelopingRequest'; export type RootHandlerRequestBody = { id?: number; @@ -256,7 +256,7 @@ export class HttpServer { try { const { signedTx, txHash } = await this._relayServer.createRelayTransaction( - body as EnvelopingTxRequest + body as HttpEnvelopingRequest ); res.send({ signedTx, txHash }); } catch (e) { @@ -321,7 +321,7 @@ export class HttpServer { async estimateHandler(req: Request, res: Response): Promise { try { const estimation = await this._relayServer.estimateMaxPossibleGas( - req.body as EnvelopingTxRequest + req.body as HttpEnvelopingRequest ); res.send(estimation); } catch (e) { diff --git a/src/RelayServer.ts b/src/RelayServer.ts index 3d82899..004e937 100644 --- a/src/RelayServer.ts +++ b/src/RelayServer.ts @@ -30,7 +30,6 @@ import { PopulatedTransaction, BigNumber, providers, - BigNumberish, } from 'ethers'; import ow from 'ow'; import { @@ -45,7 +44,6 @@ import { } from './Utils'; import { AmountRequired } from './AmountRequired'; import { - EnvelopingTxRequest, RelayRequest, estimateRelayMaxPossibleGas, isDeployRequest, @@ -71,6 +69,7 @@ import { registerEventHandlers, } from './events'; import { SERVER_VERSION as version } from './version'; +import type { HttpEnvelopingRequest } from './definitions/HttpEnvelopingRequest'; type HubInfo = { relayWorkerAddress: string; @@ -245,7 +244,7 @@ export class RelayServer extends EventEmitter { }; } - validateInputTypes(envelopingTransaction: EnvelopingTxRequest): void { + validateInputTypes(envelopingTransaction: HttpEnvelopingRequest): void { if (isDeployTransaction(envelopingTransaction)) { ow( envelopingTransaction, @@ -259,30 +258,33 @@ export class RelayServer extends EventEmitter { } } - async validateInput(envelopingRequest: EnvelopingTxRequest): Promise { - const { metadata, relayRequest } = envelopingRequest; + async validateInput(envelopingRequest: HttpEnvelopingRequest): Promise { + const { + metadata: { relayHubAddress: relayHubAddressFromRequest }, + relayRequest, + } = envelopingRequest; const { app: { requestMinValidSeconds }, contracts: { relayHubAddress }, } = this.config; - const relayHubAddressValue = await metadata.relayHubAddress; - // Check that the relayHub is the correct one - if (relayHubAddressValue.toLowerCase() !== relayHubAddress.toLowerCase()) { + if ( + relayHubAddressFromRequest.toLowerCase() !== relayHubAddress.toLowerCase() + ) { throw new Error( - `Wrong hub address.\nRelay server's hub address: ${this.config.contracts.relayHubAddress}, request's hub address: ${relayHubAddressValue}\n` + `Wrong hub address.\nRelay server's hub address: ${this.config.contracts.relayHubAddress}, request's hub address: ${relayHubAddressFromRequest}\n` ); } - const feesReceiver = await relayRequest.relayData.feesReceiver; + const feesReceiver = relayRequest.relayData.feesReceiver; // Check the relayWorker (todo: once migrated to multiple relays, check if exists) if (feesReceiver.toLowerCase() !== this.feesReceiver.toLowerCase()) { throw new Error(`Wrong fees receiver address: ${feesReceiver}\n`); } - const gasPrice = await relayRequest.relayData.gasPrice; + const gasPrice = relayRequest.relayData.gasPrice; // Check that the gasPrice is initialized & acceptable if (this.gasPrice.gt(gasPrice)) { throw new Error( @@ -297,11 +299,8 @@ export class RelayServer extends EventEmitter { ); } - async validateVerifier( - envelopingRequest: EnvelopingTxRequest - ): Promise { - const callVerifier = await envelopingRequest.relayRequest.relayData - .callVerifier; + validateVerifier(envelopingRequest: HttpEnvelopingRequest): void { + const callVerifier = envelopingRequest.relayRequest.relayData.callVerifier; if (!this.isTrustedVerifier(callVerifier)) { throw new Error(`Invalid verifier: ${callVerifier}`); } @@ -318,10 +317,9 @@ export class RelayServer extends EventEmitter { } async validateRequestWithVerifier( - envelopingTransaction: EnvelopingTxRequest + envelopingTransaction: HttpEnvelopingRequest ): Promise { - const verifier = await envelopingTransaction.relayRequest.relayData - .callVerifier; + const verifier = envelopingTransaction.relayRequest.relayData.callVerifier; if (!this.isTrustedVerifier(verifier)) { throw new Error('Invalid verifier'); @@ -380,7 +378,7 @@ export class RelayServer extends EventEmitter { } async getMaxPossibleGas( - envelopingTransaction: EnvelopingTxRequest + envelopingTransaction: HttpEnvelopingRequest ): Promise { log.debug( `Enveloping transaction: ${JSON.stringify( @@ -418,7 +416,7 @@ export class RelayServer extends EventEmitter { async maxPossibleGasWithViewCall( transaction: PopulatedTransaction, - envelopingRequest: EnvelopingTxRequest, + envelopingRequest: HttpEnvelopingRequest, gasLimit: BigNumber ): Promise { log.debug('Relay Server - Request sent to the worker'); @@ -434,7 +432,7 @@ export class RelayServer extends EventEmitter { const { maxPossibleGas } = await maxPossibleGasVerification( transaction, - gasPrice as BigNumberish, + gasPrice, gasLimit, this.workerAddress ); @@ -443,7 +441,7 @@ export class RelayServer extends EventEmitter { } async estimateMaxPossibleGas( - envelopingRequest: EnvelopingTxRequest + envelopingRequest: HttpEnvelopingRequest ): Promise { log.debug( `EnvelopingRequest:${JSON.stringify(envelopingRequest, undefined, 4)}` @@ -492,7 +490,7 @@ export class RelayServer extends EventEmitter { } async createRelayTransaction( - envelopingTransaction: EnvelopingTxRequest + envelopingTransaction: HttpEnvelopingRequest ): Promise { log.debug(`dump request params: ${JSON.stringify(envelopingTransaction)}`); if (!this.isReady()) { @@ -557,10 +555,8 @@ export class RelayServer extends EventEmitter { const provider = getProvider(); - const relayHubAddress = await envelopingTransaction.metadata - .relayHubAddress; - const gasPrice = await envelopingTransaction.relayRequest.relayData - .gasPrice; + const relayHubAddress = envelopingTransaction.metadata.relayHubAddress; + const gasPrice = envelopingTransaction.relayRequest.relayData.gasPrice; const currentBlock = await provider.getBlockNumber(); const details: SendTransactionDetails = { diff --git a/src/definitions/HttpEnvelopingRequest.ts b/src/definitions/HttpEnvelopingRequest.ts new file mode 100644 index 0000000..9004ace --- /dev/null +++ b/src/definitions/HttpEnvelopingRequest.ts @@ -0,0 +1,38 @@ +import type { Either } from '@rsksmart/rif-relay-client/dist/common/utility.types'; +import type { BigNumberish } from 'ethers'; +import type { + RelayRequestBody as ClientRelayRequestBody, + DeployRequestBody as ClientDeployRequestBody, +} from '@rsksmart/rif-relay-client'; + +// IMPORTANT: The types defined here mirror the types defined in the client. +// see EnvelopingTxRequest in the rif-relay-client library + +type AwaitedWrapper = { [P in keyof T]: Awaited }; + +export declare type RelayRequestBody = AwaitedWrapper; + +export declare type DeployRequestBody = AwaitedWrapper; + +export declare type EnvelopingMetadata = { + relayHubAddress: RelayRequestBody['relayHub']; + relayMaxNonce: number; + signature: string; +}; + +export declare type EnvelopingRequestData = { + gasPrice: BigNumberish; + feesReceiver: string; + callForwarder: string; + callVerifier: string; +}; + +export declare type EnvelopingRequest = { + request: Either; + relayData: EnvelopingRequestData; +}; + +export declare type HttpEnvelopingRequest = { + relayRequest: EnvelopingRequest; + metadata: EnvelopingMetadata; +}; diff --git a/src/relayServerUtils.ts b/src/relayServerUtils.ts index 2f66cd4..932e59e 100644 --- a/src/relayServerUtils.ts +++ b/src/relayServerUtils.ts @@ -1,10 +1,7 @@ import { - EnvelopingRequest, estimateInternalCallGas, getExchangeRate, - EnvelopingTxRequest, isDeployRequest, - RelayRequestBody, } from '@rsksmart/rif-relay-client'; import { BigNumber as BigNumberJs } from 'bignumber.js'; import { constants, BigNumber, type BigNumberish } from 'ethers'; @@ -29,6 +26,11 @@ import { import log from 'loglevel'; import { INSUFFICIENT_TOKEN_AMOUNT } from './definitions/errorMessages.const'; import type { AppConfig } from './ServerConfigParams'; +import type { + EnvelopingRequest, + HttpEnvelopingRequest, + RelayRequestBody, +} from './definitions/HttpEnvelopingRequest'; const TRANSFER_HASH = 'a9059cbb'; const TRANSFER_FROM_HASH = '23b872dd'; @@ -48,7 +50,7 @@ async function calculateFee( maxPossibleGas: BigNumber, appConfig: AppConfig ): Promise { - if (await isSponsorshipAllowed(relayRequest, appConfig)) { + if (isSponsorshipAllowed(relayRequest, appConfig)) { return BigNumberJs(0); } @@ -64,7 +66,7 @@ async function calculateFee( } const { transferFeePercentage } = appConfig; - const data = await relayRequest.request.data; + const data = relayRequest.request.data; //Even if transferFeePercentage = 0, it has priority over gas fee if (transferFeePercentage >= 0 && isTransferOrTransferFrom(data.toString())) { const transferFee = await calculateFeeFromTransfer( @@ -101,8 +103,8 @@ async function calculateFixedUsdFee( envelopingRequest: EnvelopingRequest, fixedUsdFee: number ): Promise { - const tokenContract = await envelopingRequest.request.tokenContract; - const gasPrice = await envelopingRequest.relayData.gasPrice; + const tokenContract = envelopingRequest.request.tokenContract; + const gasPrice = envelopingRequest.relayData.gasPrice; const provider = getProvider(); @@ -148,8 +150,8 @@ async function calculateFeeFromTransfer( const valueInDecimal = BigNumberJs('0x' + valueHex); const feeInToken = valueInDecimal.multipliedBy(transferFeePercentage); - const tokenContract = await envelopingRequest.request.tokenContract; - const gasPrice = await envelopingRequest.relayData.gasPrice; + const tokenContract = envelopingRequest.request.tokenContract; + const gasPrice = envelopingRequest.relayData.gasPrice; return await convertTokenToGas(feeInToken, tokenContract, gasPrice); } @@ -166,15 +168,15 @@ function calculateFeeFromGas( ); } -async function isSponsorshipAllowed( +function isSponsorshipAllowed( envelopingRequest: EnvelopingRequest, config: AppConfig -): Promise { +): boolean { const { disableSponsoredTx, sponsoredDestinations } = config; return ( !disableSponsoredTx || - sponsoredDestinations.includes(await envelopingRequest.request.to) + sponsoredDestinations.includes(envelopingRequest.request.to) ); } @@ -184,8 +186,7 @@ function getMethodHashFromData(data: string) { async function validateIfGasAmountIsAcceptable({ relayRequest, -}: EnvelopingTxRequest) { - // TODO: For RIF Team +}: HttpEnvelopingRequest) { // The maxPossibleGas must be compared against the commitment signed with the user. // The relayServer must not allow a call that requires more gas than it was agreed with the user // For now, we can call estimateDestinationContractCallGas to get the "ACTUAL" gas required for the @@ -213,7 +214,7 @@ async function validateIfGasAmountIsAcceptable({ ); const { gas } = request as RelayRequestBody; - const gasValue = await gas; + const gasValue = gas; const bigGasFromRequestMaxAgreed = bigMaxEstimatedGasDeviation.multipliedBy( gasValue.toString() ); @@ -227,20 +228,18 @@ async function validateIfGasAmountIsAcceptable({ async function validateIfTokenAmountIsAcceptable( maxPossibleGas: BigNumber, - envelopingTransaction: EnvelopingTxRequest, + envelopingTransaction: HttpEnvelopingRequest, appConfig: AppConfig ) { - if ( - await isSponsorshipAllowed(envelopingTransaction.relayRequest, appConfig) - ) { + if (isSponsorshipAllowed(envelopingTransaction.relayRequest, appConfig)) { return; } const { request, relayData } = envelopingTransaction.relayRequest; - const tokenContract = await request.tokenContract; - const tokenAmount = await request.tokenAmount; - const gasPrice = await relayData.gasPrice; + const tokenContract = request.tokenContract; + const tokenAmount = request.tokenAmount; + const gasPrice = relayData.gasPrice; const tokenAmountInGas = await convertTokenToGas( tokenAmount, @@ -331,8 +330,8 @@ async function convertGasToTokenAndNative( relayRequest: EnvelopingRequest, initialEstimation: BigNumber ) { - const gasPrice = await relayRequest.relayData.gasPrice; - const tokenContractAddress = await relayRequest.request.tokenContract; + const gasPrice = relayRequest.relayData.gasPrice; + const tokenContractAddress = relayRequest.request.tokenContract; let xRate = '1'; let initialEstimationInNative: BigNumber = initialEstimation.mul(gasPrice); diff --git a/test/unit/RelayServer.test.ts b/test/unit/RelayServer.test.ts index 3ec9a8b..d43d650 100644 --- a/test/unit/RelayServer.test.ts +++ b/test/unit/RelayServer.test.ts @@ -5,7 +5,6 @@ import { TxStoreManager, } from '../../src'; import sinon, { SinonSpy, SinonStub, createStubInstance } from 'sinon'; -import type { EnvelopingTxRequest } from '@rsksmart/rif-relay-client'; import * as rifClient from '@rsksmart/rif-relay-client'; import { BigNumber, constants, providers, utils } from 'ethers'; import * as serverUtils from '../../src/Utils'; @@ -21,6 +20,7 @@ import { } from 'src/events/checkReplenish'; import * as replenish from 'src/ReplenishFunction'; import sinonChai from 'sinon-chai'; +import type { HttpEnvelopingRequest } from 'src/definitions/HttpEnvelopingRequest'; use(sinonChai); use(chaiAsPromised); @@ -155,7 +155,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as unknown as EnvelopingTxRequest); + } as unknown as HttpEnvelopingRequest); expect(onSpy).to.have.been.calledWith( EVENT_REPLENISH_CHECK_REQUIRED, relayServer, @@ -200,7 +200,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as unknown as EnvelopingTxRequest); + } as unknown as HttpEnvelopingRequest); expect(signedTx).to.be.eql(expectedTransactionDetails); expect(replenishStub).to.have.been.calledOnce; }); @@ -220,7 +220,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest + } as HttpEnvelopingRequest ); expect(maxPossibleGasEstimation.estimation).to.be.equal( @@ -243,7 +243,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest + } as HttpEnvelopingRequest ); expect(maxPossibleGasEstimation.estimation).to.eq( @@ -273,7 +273,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest); + } as HttpEnvelopingRequest); expect(maxPossibleGasWithFee.toString()).to.be.equal( FAKE_ESTIMATION_BEFORE_FEES.toString() @@ -294,7 +294,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest); + } as HttpEnvelopingRequest); expect(maxPossibleGasWithFee.toString()).to.eq( BigNumberJs(FAKE_ESTIMATION_BEFORE_FEES) @@ -320,7 +320,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest); + } as HttpEnvelopingRequest); const { maxPossibleGasWithFee } = await relayServer.getMaxPossibleGas({ relayRequest: { @@ -331,7 +331,7 @@ describe('RelayServer tests', function () { gasPrice: GAS_PRICE, }, }, - } as EnvelopingTxRequest); + } as HttpEnvelopingRequest); expect(estimatedGas.estimation.toString()).to.be.eq( maxPossibleGasWithFee.toString() diff --git a/test/unit/relayServerUtils.test.ts b/test/unit/relayServerUtils.test.ts index e919514..4718e20 100644 --- a/test/unit/relayServerUtils.test.ts +++ b/test/unit/relayServerUtils.test.ts @@ -1,9 +1,4 @@ import sinon, { SinonStub } from 'sinon'; -import type { - RelayRequestBody, - EnvelopingRequestData, - RelayRequest, -} from '@rsksmart/rif-relay-client'; import { BigNumber, constants } from 'ethers'; import { ERC20__factory, @@ -26,6 +21,11 @@ import chaiAsPromised from 'chai-as-promised'; import * as relayServerUtils from '../../src/relayServerUtils'; import * as relayClient from '@rsksmart/rif-relay-client'; import type { AppConfig } from 'src'; +import type { + EnvelopingRequest, + EnvelopingRequestData, + RelayRequestBody, +} from 'src/definitions/HttpEnvelopingRequest'; const ZERO_ADDRESS = constants.AddressZero; const FAKE_ESTIMATION_BEFORE_FEES = 100000; @@ -37,8 +37,8 @@ const FAKE_FIXED_USD_FEE = 2; const TOKEN_AMOUNT_TO_TRANSFER = '1000000000000000000'; //18 zeros const TOKEN_VALUE_IN_USD = 0.5; -function createRequest(request: Partial): RelayRequest { - const baseRequest: RelayRequest = { +function createRequest(request: Partial): EnvelopingRequest { + const baseRequest: EnvelopingRequest = { request: { relayHub: ZERO_ADDRESS, from: ZERO_ADDRESS, @@ -58,7 +58,7 @@ function createRequest(request: Partial): RelayRequest { callForwarder: ZERO_ADDRESS, callVerifier: ZERO_ADDRESS, } as EnvelopingRequestData, - } as RelayRequest; + } as EnvelopingRequest; return { request: { @@ -68,7 +68,7 @@ function createRequest(request: Partial): RelayRequest { relayData: { ...baseRequest.relayData, }, - }; + } as EnvelopingRequest; } use(chaiAsPromised);