Skip to content

Commit

Permalink
fix: gas limit estimation (#4897)
Browse files Browse the repository at this point in the history 
Remove the calculated fallback `gas` property from the `eth_estimateGas` request.
Remove all gas fee properties from the `eth_estimateGas` request.
Calculate fallback gas, max gas limit, and buffer using fractions rather than decimals.
  • Loading branch information
@matthewwalsh0
matthewwalsh0 authored Nov 7, 2024
1 parent 99b320b commit d116136
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 59 deletions.
182 changes: 136 additions & 46 deletions packages/transaction-controller/src/utils/gas.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* eslint-disable jsdoc/require-jsdoc */

import { query } from '@metamask/controller-utils';
import type EthQuery from '@metamask/eth-query';

Expand All @@ -12,7 +10,8 @@ import {
updateGas,
FIXED_GAS,
DEFAULT_GAS_MULTIPLIER,
GAS_ESTIMATE_FALLBACK_MULTIPLIER,
GAS_ESTIMATE_FALLBACK_BLOCK_PERCENT,
MAX_GAS_BLOCK_PERCENT,
} from './gas';

jest.mock('@metamask/controller-utils', () => ({
Expand All @@ -21,20 +20,18 @@ jest.mock('@metamask/controller-utils', () => ({
}));

const GAS_MOCK = 100;
const BLOCK_GAS_LIMIT_MOCK = 1234567;
const BLOCK_GAS_LIMIT_MOCK = 123456789;
const BLOCK_NUMBER_MOCK = '0x5678';
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const ETH_QUERY_MOCK = {} as any as EthQuery;
const ETH_QUERY_MOCK = {} as unknown as EthQuery;
const FALLBACK_MULTIPLIER = GAS_ESTIMATE_FALLBACK_BLOCK_PERCENT / 100;
const MAX_GAS_MULTIPLIER = MAX_GAS_BLOCK_PERCENT / 100;

const TRANSACTION_META_MOCK = {
txParams: {
data: '0x1',
to: '0x2',
},
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any as TransactionMeta;
} as unknown as TransactionMeta;

const UPDATE_GAS_REQUEST_MOCK = {
txMeta: TRANSACTION_META_MOCK,
Expand All @@ -43,6 +40,11 @@ const UPDATE_GAS_REQUEST_MOCK = {
ethQuery: ETH_QUERY_MOCK,
} as UpdateGasRequest;

/**
* Converts number to hex string.
* @param value - The number to convert.
* @returns The hex string.
*/
function toHex(value: number) {
return `0x${value.toString(16)}`;
}
Expand All @@ -51,22 +53,26 @@ describe('gas', () => {
const queryMock = jest.mocked(query);
let updateGasRequest: UpdateGasRequest;

/**
* Mocks query responses.
* @param options - The options.
* @param options.getCodeResponse - The response for getCode.
* @param options.getBlockByNumberResponse - The response for getBlockByNumber.
* @param options.estimateGasResponse - The response for estimateGas.
* @param options.estimateGasError - The error for estimateGas.
*/
function mockQuery({
getCodeResponse,
getBlockByNumberResponse,
estimateGasResponse,
estimateGasError,
}: {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getCodeResponse?: any;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getBlockByNumberResponse?: any;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
estimateGasResponse?: any;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
estimateGasError?: any;
}) {
Expand All @@ -85,6 +91,9 @@ describe('gas', () => {
}
}

/**
* Assert that estimateGas was not called.
*/
function expectEstimateGasNotCalled() {
expect(queryMock).not.toHaveBeenCalledWith(
expect.anything(),
Expand All @@ -95,6 +104,7 @@ describe('gas', () => {

beforeEach(() => {
updateGasRequest = JSON.parse(JSON.stringify(UPDATE_GAS_REQUEST_MOCK));
jest.resetAllMocks();
});

describe('updateGas', () => {
Expand Down Expand Up @@ -149,8 +159,10 @@ describe('gas', () => {
);
});

it('to estimate if estimate greater than 90% of block gas limit', async () => {
const estimatedGas = Math.ceil(BLOCK_GAS_LIMIT_MOCK * 0.9 + 10);
it('to estimate if estimate greater than percentage of block gas limit', async () => {
const estimatedGas = Math.ceil(
BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER + 10,
);

mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
Expand All @@ -165,10 +177,12 @@ describe('gas', () => {
);
});

it('to padded estimate if padded estimate less than 90% of block gas limit', async () => {
const blockGasLimit90Percent = BLOCK_GAS_LIMIT_MOCK * 0.9;
const estimatedGasPadded = Math.ceil(blockGasLimit90Percent - 10);
const estimatedGas = Math.round(estimatedGasPadded / 1.5);
it('to padded estimate if padded estimate less than percentage of block gas limit', async () => {
const maxGasLimit = BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER;
const estimatedGasPadded = Math.floor(maxGasLimit) - 10;
const estimatedGas = Math.ceil(
estimatedGasPadded / DEFAULT_GAS_MULTIPLIER,
);

mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
Expand All @@ -185,9 +199,9 @@ describe('gas', () => {
);
});

it('to padded estimate using chain multiplier if padded estimate less than 90% of block gas limit', async () => {
const blockGasLimit90Percent = BLOCK_GAS_LIMIT_MOCK * 0.9;
const estimatedGasPadded = Math.ceil(blockGasLimit90Percent - 10);
it('to padded estimate using chain multiplier if padded estimate less than percentage of block gas limit', async () => {
const maxGasLimit = BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER;
const estimatedGasPadded = Math.ceil(maxGasLimit - 10);
const estimatedGas = estimatedGasPadded; // Optimism multiplier is 1

updateGasRequest.chainId = CHAIN_IDS.OPTIMISM;
Expand All @@ -207,10 +221,14 @@ describe('gas', () => {
);
});

it('to 90% of block gas limit if padded estimate only is greater than 90% of block gas limit', async () => {
const blockGasLimit90Percent = Math.round(BLOCK_GAS_LIMIT_MOCK * 0.9);
const estimatedGasPadded = blockGasLimit90Percent + 10;
const estimatedGas = Math.ceil(estimatedGasPadded / 1.5);
it('to percentage of block gas limit if padded estimate only is greater than percentage of block gas limit', async () => {
const maxGasLimit = Math.round(
BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER,
);
const estimatedGasPadded = maxGasLimit + 10;
const estimatedGas = Math.ceil(
estimatedGasPadded / DEFAULT_GAS_MULTIPLIER,
);

mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
Expand All @@ -219,9 +237,7 @@ describe('gas', () => {

await updateGas(updateGasRequest);

expect(updateGasRequest.txMeta.txParams.gas).toBe(
toHex(blockGasLimit90Percent),
);
expect(updateGasRequest.txMeta.txParams.gas).toBe(toHex(maxGasLimit));
expect(updateGasRequest.txMeta.originalGasEstimate).toBe(
updateGasRequest.txMeta.txParams.gas,
);
Expand Down Expand Up @@ -267,7 +283,7 @@ describe('gas', () => {
describe('on estimate query error', () => {
it('sets gas to 35% of block gas limit', async () => {
const fallbackGas = Math.floor(
BLOCK_GAS_LIMIT_MOCK * GAS_ESTIMATE_FALLBACK_MULTIPLIER,
BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER,
);

mockQuery({
Expand Down Expand Up @@ -357,7 +373,7 @@ describe('gas', () => {

it('returns estimated gas as 35% of block gas limit on error', async () => {
const fallbackGas = Math.floor(
BLOCK_GAS_LIMIT_MOCK * GAS_ESTIMATE_FALLBACK_MULTIPLIER,
BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER,
);

mockQuery({
Expand All @@ -378,47 +394,121 @@ describe('gas', () => {
simulationFails: expect.any(Object),
});
});

it('removes gas fee properties from estimate request', async () => {
mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
estimateGasResponse: toHex(GAS_MOCK),
});

await estimateGas(
{
...TRANSACTION_META_MOCK.txParams,
gasPrice: '0x1',
maxFeePerGas: '0x2',
maxPriorityFeePerGas: '0x3',
},
ETH_QUERY_MOCK,
);

expect(queryMock).toHaveBeenCalledWith(ETH_QUERY_MOCK, 'estimateGas', [
{
...TRANSACTION_META_MOCK.txParams,
value: expect.anything(),
},
]);
});

it('normalizes data in estimate request', async () => {
mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
estimateGasResponse: toHex(GAS_MOCK),
});

await estimateGas(
{
...TRANSACTION_META_MOCK.txParams,
data: '123',
},
ETH_QUERY_MOCK,
);

expect(queryMock).toHaveBeenCalledWith(ETH_QUERY_MOCK, 'estimateGas', [
expect.objectContaining({
...TRANSACTION_META_MOCK.txParams,
data: '0x123',
}),
]);
});

it('normalizes value in estimate request', async () => {
mockQuery({
getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
estimateGasResponse: toHex(GAS_MOCK),
});

await estimateGas(
{
...TRANSACTION_META_MOCK.txParams,
value: undefined,
},
ETH_QUERY_MOCK,
);

expect(queryMock).toHaveBeenCalledWith(ETH_QUERY_MOCK, 'estimateGas', [
{
...TRANSACTION_META_MOCK.txParams,
value: '0x0',
},
]);
});
});

describe('addGasBuffer', () => {
it('returns estimated gas if greater than 90% of block gas limit', () => {
const estimatedGas = Math.ceil(BLOCK_GAS_LIMIT_MOCK * 0.9 + 10);
it('returns estimated gas if greater than percentage of block gas limit', () => {
const estimatedGas = Math.ceil(
BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER + 10,
);

const result = addGasBuffer(
toHex(estimatedGas),
toHex(BLOCK_GAS_LIMIT_MOCK),
1.5,
DEFAULT_GAS_MULTIPLIER,
);

expect(result).toBe(toHex(estimatedGas));
});

it('returns padded estimate if less than 90% of block gas limit', () => {
const blockGasLimit90Percent = BLOCK_GAS_LIMIT_MOCK * 0.9;
const estimatedGasPadded = Math.ceil(blockGasLimit90Percent - 10);
const estimatedGas = Math.round(estimatedGasPadded / 1.5);
it('returns padded estimate if less than percentage of block gas limit', () => {
const maxGasLimit = BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER;
const estimatedGasPadded = Math.floor(maxGasLimit - 10);
const estimatedGas = Math.ceil(
estimatedGasPadded / DEFAULT_GAS_MULTIPLIER,
);

const result = addGasBuffer(
toHex(estimatedGas),
toHex(BLOCK_GAS_LIMIT_MOCK),
1.5,
DEFAULT_GAS_MULTIPLIER,
);

expect(result).toBe(toHex(estimatedGasPadded));
});

it('returns 90% of block gas limit if padded estimate only is greater than 90% of block gas limit', () => {
const blockGasLimit90Percent = Math.round(BLOCK_GAS_LIMIT_MOCK * 0.9);
const estimatedGasPadded = blockGasLimit90Percent + 10;
const estimatedGas = Math.ceil(estimatedGasPadded / 1.5);
it('returns percentage of block gas limit if padded estimate only is greater than percentage of block gas limit', () => {
const maxGasLimit = Math.round(BLOCK_GAS_LIMIT_MOCK * MAX_GAS_MULTIPLIER);
const estimatedGasPadded = maxGasLimit + 10;
const estimatedGas = Math.ceil(
estimatedGasPadded / DEFAULT_GAS_MULTIPLIER,
);

const result = addGasBuffer(
toHex(estimatedGas),
toHex(BLOCK_GAS_LIMIT_MOCK),
1.5,
DEFAULT_GAS_MULTIPLIER,
);

expect(result).toBe(toHex(blockGasLimit90Percent));
expect(result).toBe(toHex(maxGasLimit));
});
});
});
Loading

0 comments on commit d116136

Please sign in to comment.