improve: Use base fee multiplier option in SDK to resolve gas fees #1969
Conversation
Builds on across-protocol/sdk#801 and applies relayer's base fee multiplier to new SDK parameter to scale the base fee
…ansaction` method
| maxFeePerGas: feeData.maxFeePerGas, | ||
| maxPriorityFeePerGas: scaleByNumber(feeData.maxPriorityFeePerGas, priorityScaler), |
There was a problem hiding this comment.
If we are retaining the ability to scale the priority fee (which we probably need to), then we'd still need to adjust the base fee here. Otherwise we'd be eating into our base fee multiplier by increasing the priority fee.
There was a problem hiding this comment.
ok so do you suggest keeping the existing code here?
nicholaspai
changed the title
src/clients/ProfitClient.ts
Outdated
| @@ -205,7 +205,9 @@ export class ProfitClient { | |||
|
|
|||
| private async _getTotalGasCost(deposit: Deposit, relayer: string): Promise<TransactionCostEstimate> { | |||
| try { | |||
| return await this.relayerFeeQueries[deposit.destinationChainId].getGasCosts(deposit, relayer); | |||
| return await this.relayerFeeQueries[deposit.destinationChainId].getGasCosts(deposit, relayer, { | |||
| baseFeeMultiplier: this.gasPadding, | |||
There was a problem hiding this comment.
Using gasPadding here should align the relayer with the API
There was a problem hiding this comment.
gasPadding has the valid range [0, 2] scaled to 18 decimals, so we need clamp this to a minimum of 1e18.
There was a problem hiding this comment.
I'm not 100% sure we want to do this though. The intention of the base fee multiplier is to account for time-based uncertainty around when the fill will be made. That's a problem in the API, but it's not a problem for the relayer because it's doing this calculation JIT, so it has a lot more certainty about when the fill would be submitted.
Additionally, the intention of gasPadding is to add some padding for the fill-time gas consumption (i.e. how many units of gas are consumed). This change would instead pad the base fee, which may then be discounted by the gasMultiplier.
tldr I think the relayer should probably retain its existing behaviour here, such that:
- gasPadding is applied to the estimated gas consumption.
- baseFeeMultiplier is 1x because the relayer does not really need to consider uncertainty about the base fee when the transaction lands (the base fee is already returned from the pending block).
| provider, | ||
| priorityFeeScaler, | ||
| maxFeePerGasScaler, | ||
| await contract.populateTransaction[method](...(args as Array<unknown>), { value }) |
There was a problem hiding this comment.
getting gas price here should be same as the gas price used in estimating the fill cost in the ProfitClient
Not true anymore, I agree with Paul's comment: https://github.com/across-protocol/relayer/pull/1969/files#r1901762081
| // Pass in unsignedTx here for better Linea gas price estimations via the Linea Viem provider. | ||
| const feeData = await gasPriceOracle.getGasPriceEstimate(provider, { | ||
| chainId, | ||
| baseFeeMultiplier: toBNWei(maxFeePerGasScaler), |
There was a problem hiding this comment.
We might want to clamp this to 1e18 as well.
nicholaspai
requested review from
mrice32,
james-a-morris and
dohaki
as code owners
Builds on across-protocol/sdk#801 and applies relayer's base fee multiplier to new SDK parameter to scale the base fee in one use case:
Estimating fill costs via the ProfitClientWe can test this by querying
/api/gas-pricesagainst the prod API deployment. The gas prices should be mostly in line otherwise the relayer will see too many deposits as unprofitable potentially