Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gas-fee): catch estimation errors #1121

Merged
merged 3 commits into from
Jun 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/ethereum-storage/src/gas-fee-definer.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import { BigNumber, providers, constants } from 'ethers';
import { GasDefinerProps } from './ethereum-storage-ethers';
import { estimateGasFees } from '@requestnetwork/utils';
import { LogTypes } from '@requestnetwork/types';

export class GasFeeDefiner {
private readonly logger: LogTypes.ILogger;
private readonly provider: providers.JsonRpcProvider;
private readonly gasPriceMin: BigNumber;

constructor({
logger,
provider,
gasPriceMin,
}: GasDefinerProps & { provider: providers.JsonRpcProvider }) {
}: GasDefinerProps & { logger?: LogTypes.ILogger; provider: providers.JsonRpcProvider }) {
this.logger = logger || console;
this.provider = provider;
this.gasPriceMin = gasPriceMin || constants.Zero;
}
Expand All @@ -18,6 +22,10 @@ export class GasFeeDefiner {
maxFeePerGas?: BigNumber;
maxPriorityFeePerGas?: BigNumber;
}> {
return estimateGasFees({ provider: this.provider, gasPriceMin: this.gasPriceMin });
return estimateGasFees({
logger: this.logger,
provider: this.provider,
gasPriceMin: this.gasPriceMin,
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ export const getSignerAndGasFees = async (
const signer = new hre.ethers.Wallet(hre.config.xdeploy.signer).connect(provider);

const txOverrides = (await isEip1559Supported(provider))
? await estimateGasFees({ provider })
? await estimateGasFees({ logger: console, provider })
: {};

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/smart-contracts/scripts-create2/xdeployer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const xdeploy = async (
let txOverrides: Overrides = {};

if (await isEip1559Supported(provider, console)) {
txOverrides = await estimateGasFees({ provider });
txOverrides = await estimateGasFees({ logger: console, provider });
}
txOverrides.gasLimit = hre.config.xdeploy.gasLimit;

Expand Down
38 changes: 24 additions & 14 deletions packages/utils/src/estimate-gas-fees.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BigNumber, constants, providers } from 'ethers';
import { suggestFees } from '@rainbow-me/fee-suggestions';
import { maxBigNumber } from './index';
import { LogTypes } from '@requestnetwork/types';

/**
* The function estimates gas fee with EIP-1559.
Expand All @@ -11,36 +12,45 @@ import { maxBigNumber } from './index';
* maxFeePerGas = baseFeePerGas + maxPriorityFeePerGas
* The baseFeePerGas depends on how full the previous blocks were.
* - maxPriorityFeePerGas: The maximum priority fee per unit of gas for this transaction.
* - gasPrice: Optional fallback: the gas price for this transaction.
*/
async function estimateGasFees({
logger,
provider,
gasPriceMin,
}: {
logger: LogTypes.ILogger;
provider: providers.Provider | providers.JsonRpcProvider;
gasPriceMin?: BigNumber;
}): Promise<{
maxFeePerGas?: BigNumber;
maxPriorityFeePerGas?: BigNumber;
}> {
const suggestedFee = await suggestFees(provider as providers.JsonRpcProvider);
try {
const suggestedFee = await suggestFees(provider as providers.JsonRpcProvider);

const baseFee = maxBigNumber(suggestedFee.baseFeeSuggestion, gasPriceMin || constants.Zero);
const baseFee = maxBigNumber(suggestedFee.baseFeeSuggestion, gasPriceMin || constants.Zero);

const maxPriorityFeePerGas = maxBigNumber(
suggestedFee.maxPriorityFeeSuggestions.urgent,
gasPriceMin || constants.Zero,
);
const maxFeePerGas = baseFee.add(maxPriorityFeePerGas);
const maxPriorityFeePerGas = maxBigNumber(
suggestedFee.maxPriorityFeeSuggestions.urgent,
gasPriceMin || constants.Zero,
);
const maxFeePerGas = baseFee.add(maxPriorityFeePerGas);

if (maxPriorityFeePerGas.eq(0) || maxFeePerGas.eq(0)) {
if (maxPriorityFeePerGas.eq(0) || maxFeePerGas.eq(0)) {
logger.warn(
`estimateGasFees: maxPriorityFeePerGas or maxFeePerGas too low (maxPriorityFeePerGas: ${maxPriorityFeePerGas.toString()} / maxFeePerGas: ${maxFeePerGas.toString()})`,
);
return {};
}

return {
maxPriorityFeePerGas,
maxFeePerGas,
};
} catch (e) {
logger.error(`estimateGasFees error: ${e}`);
alexandre-abrioux marked this conversation as resolved.
Show resolved Hide resolved
return {};
}

return {
maxPriorityFeePerGas,
maxFeePerGas,
};
}

export { estimateGasFees };
60 changes: 55 additions & 5 deletions packages/utils/test/estimate-gas-fee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@ const checkEstimation = (
expect(absRatio).toBeLessThan(ratioMax);
};

afterEach(() => {
jest.clearAllMocks();
});

describe('Gas fee estimation', () => {
it('Should not be undefined', async () => {
const estimation = await estimateGasFees({ provider });
const estimation = await estimateGasFees({ logger: console, provider });
expect(estimation.maxFeePerGas).toBeDefined();
expect(estimation.maxPriorityFeePerGas).toBeDefined();
});

it('Should return a lower estimation when the previous block is empty', async () => {
const firstEstimation = await estimateGasFees({ provider });
const firstEstimation = await estimateGasFees({ logger: console, provider });
await provider.send('evm_mine', []);
const secondEstimation = await estimateGasFees({ provider });
const secondEstimation = await estimateGasFees({ logger: console, provider });

expect(
firstEstimation.maxFeePerGas?.sub(secondEstimation.maxFeePerGas || 0).toNumber(),
Expand All @@ -45,12 +49,58 @@ describe('Gas fee estimation', () => {
});
}

const estimation = await estimateGasFees({ provider });
const estimation = await estimateGasFees({ logger: console, provider });
const tx = await wallet.sendTransaction({
to: dummyAddress,
value: BigNumber.from(1),
});
checkEstimation(estimation.maxFeePerGas as BigNumber, tx.maxFeePerGas as BigNumber, 0.1);
checkEstimation(estimation.maxFeePerGas as BigNumber, tx.maxFeePerGas as BigNumber, 0.1);
});

it('Should handle estimation errors properly', async () => {
jest.spyOn(provider, 'send').mockImplementation((command) => {
if (command !== 'eth_feeHistory') throw new Error('unhandled command');
return Promise.resolve({
// random data, found on https://docs.infura.io/networks/ethereum/json-rpc-methods/eth_feehistory#body
// not important in this test
baseFeePerGas: [
'0x3da8e7618',
'0x3e1ba3b1b',
'0x3dfd72b90',
'0x3d64eee76',
'0x3d4da2da0',
'0x3ccbcac6b',
],
gasUsedRatio: [
0.5290747666666666, 0.49240453333333334, 0.4615576, 0.49407083333333335, 0.4669053,
],
oldestBlock: '0xfab8ac',
// Here, return all rewards as > 5 Gwei.
// This is so that all blocks would be considered as outlier blocks in
// https://github.com/rainbow-me/fee-suggestions/blob/76b9fe14d3740c9df7cedf40b2f85cd8871ff9c2/src/utils.ts#L123C11-L123C22
// thus triggering an error.
reward: [
// 6000000000 wei
['x165A0BC00'],
// 7000000000 wei
['x1A13B8600'],
// 8000000000 wei
['x1DCD65000'],
],
});
});

const loggerMock = {
debug: jest.fn(),
error: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
};

const estimation = await estimateGasFees({ logger: loggerMock, provider });
expect(estimation).toStrictEqual({});
expect(loggerMock.error).toHaveBeenCalledWith(
'estimateGasFees error: Error: Error: ema was undefined',
);
});
});