From a435a57c75bb54c7c364496652670601a9035bc1 Mon Sep 17 00:00:00 2001
From: Alex Forshtat <forshtat1@gmail.com>
Date: Tue, 11 Apr 2023 15:24:36 +0400
Subject: [PATCH] OG-1039: Fix "max priority fee per" exceeding "max fee per
 gas (#968)

The "max priority fee per" gas after adjustment for a "Relay Ping
Response" may exceed "max fee per gas" leading to a broken GSN client
state.
---
 packages/common/src/Utils.ts                  | 15 ++++++++++--
 .../dev/test/provider/RelayClient.test.ts     | 23 +++++++++++++++++++
 .../provider/src/RelaySelectionManager.ts     |  2 +-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/packages/common/src/Utils.ts b/packages/common/src/Utils.ts
index 9f8027d78..106abffe5 100644
--- a/packages/common/src/Utils.ts
+++ b/packages/common/src/Utils.ts
@@ -34,6 +34,7 @@ import { ethers } from 'ethers'
 import { keccak256 } from 'ethers/lib/utils'
 import { RelayRequest } from './EIP712/RelayRequest'
 import { PartialRelayInfo } from './types/RelayInfo'
+import { LoggerInterface } from './LoggerInterface'
 
 export function removeHexPrefix (hex: string): string {
   if (hex == null || typeof hex.replace !== 'function') {
@@ -494,18 +495,28 @@ export function adjustGasCostParameterUp (
  */
 export function adjustRelayRequestForPingResponse (
   feesIn: EIP1559Fees,
-  relayInfo: PartialRelayInfo
+  relayInfo: PartialRelayInfo,
+  logger: LoggerInterface
 ): RelaySelectionResult {
   const maxPriorityFeePerGas = adjustGasCostParameterUp(
     parseInt(feesIn.maxPriorityFeePerGas),
     parseInt(relayInfo.pingResponse.minMaxPriorityFeePerGas)
   )
 
-  const maxFeePerGas = adjustGasCostParameterUp(
+  let maxFeePerGas = adjustGasCostParameterUp(
     parseInt(feesIn.maxFeePerGas),
     parseInt(relayInfo.pingResponse.minMaxFeePerGas)
   )
 
+  if (maxPriorityFeePerGas.newValue > maxFeePerGas.newValue) {
+    logger.warn(`Attention: for relay ${relayInfo.relayInfo.relayUrl} had to adjust 'maxFeePerGas' to be equal 'maxPriorityFeePerGas' (from ${maxFeePerGas.newValue} to ${maxPriorityFeePerGas.newValue}) to avoid RPC error.`)
+    // reusing 'adjustGasCostParameterUp' but with new priority fee as minimum to get correct 'deltaPercent' calculation for the sorting
+    maxFeePerGas = adjustGasCostParameterUp(
+      parseInt(feesIn.maxFeePerGas),
+      maxPriorityFeePerGas.newValue
+    )
+  }
+
   const updatedGasFees: EIP1559Fees = {
     maxPriorityFeePerGas: toHex(maxPriorityFeePerGas.newValue),
     maxFeePerGas: toHex(maxFeePerGas.newValue)
diff --git a/packages/dev/test/provider/RelayClient.test.ts b/packages/dev/test/provider/RelayClient.test.ts
index f1240a70c..81fbc697e 100644
--- a/packages/dev/test/provider/RelayClient.test.ts
+++ b/packages/dev/test/provider/RelayClient.test.ts
@@ -28,18 +28,21 @@ import {
   Address,
   ConfigResponse,
   ContractInteractor,
+  EIP1559Fees,
   GsnTransactionDetails,
   HttpClient,
   HttpWrapper,
   LoggerConfiguration,
   LoggerInterface,
   ObjectMap,
+  PartialRelayInfo,
   PaymasterType,
   PingResponse,
   RegistrarRelayInfo,
   RelayInfo,
   RelayRequest,
   RelayTransactionRequest,
+  adjustRelayRequestForPingResponse,
   constants,
   defaultEnvironment,
   getRawTxOptions,
@@ -639,6 +642,26 @@ contract('RelayClient', function (accounts) {
       const calculatedGasPrice = await relayClient.calculateGasFees()
       assert.equal(calculatedGasPrice.maxPriorityFeePerGas, `0x${minMaxPriorityFeePerGas.toString(16)}`)
     })
+
+    it('should avoid setting adjusted max priority fee above max fee', async function () {
+      const feesIn: EIP1559Fees = {
+        maxFeePerGas: '40',
+        maxPriorityFeePerGas: '80'
+      }
+      const relayInfo: PartialRelayInfo = {
+        // @ts-ignore
+        relayInfo: {},
+        // @ts-ignore
+        pingResponse: {
+          minMaxFeePerGas: '300',
+          minMaxPriorityFeePerGas: '440'
+        }
+      }
+      const result = adjustRelayRequestForPingResponse(feesIn, relayInfo, console)
+      assert.equal(parseInt(result.updatedGasFees.maxFeePerGas), 440)
+      assert.equal(parseInt(result.updatedGasFees.maxPriorityFeePerGas), 440)
+      assert.equal(result.maxDeltaPercent, 1000)
+    })
   })
 
   describe('#_attemptRelay()', function () {
diff --git a/packages/provider/src/RelaySelectionManager.ts b/packages/provider/src/RelaySelectionManager.ts
index cb755378a..b41c58f97 100644
--- a/packages/provider/src/RelaySelectionManager.ts
+++ b/packages/provider/src/RelaySelectionManager.ts
@@ -261,7 +261,7 @@ export class RelaySelectionManager {
     const skippedRelays: string[] = []
     const adjustedArray = allPingResults.results
       .map(it => {
-        return adjustRelayRequestForPingResponse(this.gsnTransactionDetails, it)
+        return adjustRelayRequestForPingResponse(this.gsnTransactionDetails, it, this.logger)
       })
       .filter(it => {
         const isGasPriceWithinSlack = it.maxDeltaPercent <= this.config.gasPriceSlackPercent