Skip to content

Commit

Permalink
OG-1039: Fix "max priority fee per" exceeding "max fee per gas (#968)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
forshtat authored Apr 11, 2023
1 parent b9abb2f commit a435a57
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
15 changes: 13 additions & 2 deletions packages/common/src/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions packages/dev/test/provider/RelayClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion packages/provider/src/RelaySelectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a435a57

Please sign in to comment.