Skip to content
Open
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
5 changes: 5 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Fix `LineaGasFeeFlow` silently under-computing fee estimates for the `medium` and `high` levels ([#10045](https://github.com/MetaMask/core/pull/10045))
- `BN.muln()` was called with the fractional multipliers (`1.35`, `1.7`, `1.05`, `1.1`) used to derive Linea gas fee levels; `muln` truncates non-integer input per 26-bit word instead of throwing, so the previous implementation silently returned an under-computed value (e.g. `1 gwei * 1.05` returned `1003023795` instead of `1050000000`).

### Changed

- Bump `@metamask/remote-feature-flag-controller` from `^6.0.0` to `^6.1.0` ([#9980](https://github.com/MetaMask/core/pull/9980))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ describe('LineaGasFeeFlow', () => {

expect(priorityFees).toStrictEqual([
LINEA_RESPONSE_MOCK.priorityFeePerGas,
'0x23a3d70a3',
'0x25658bf25',
'0x23d70a3d6',
'0x258bf258b',
]);

expect(rpcRequestMock).toHaveBeenCalledTimes(1);
Expand All @@ -138,11 +138,41 @@ describe('LineaGasFeeFlow', () => {

expect(maxFees).toStrictEqual([
'0x333333333',
'0x3a7ae1479',
'0x42428f5c1',
'0x3ae147ae0',
'0x428f5c28e',
]);
});

it('applies fractional multipliers without integer truncation', async () => {
// Values chosen so a naive `BN.muln(1.35)` truncates to the wrong
// integer per 26-bit word instead of throwing - proving the
// implementation multiplies via an arbitrary-precision path.
const basePerGas = 30_000_000_000n; // 30 gwei
const priorityPerGas = 1_000_000_000n; // 1 gwei

rpcRequestMock.mockResolvedValue({
baseFeePerGas: `0x${basePerGas.toString(16)}`,
priorityFeePerGas: `0x${priorityPerGas.toString(16)}`,
});

const flow = new LineaGasFeeFlow();
const response = await flow.getGasFees(request);
const estimates = response.estimates as FeeMarketGasFeeEstimates;

expect(BigInt(estimates.medium.maxPriorityFeePerGas)).toBe(
(priorityPerGas * 105n) / 100n,
);
expect(BigInt(estimates.high.maxPriorityFeePerGas)).toBe(
(priorityPerGas * 110n) / 100n,
);
expect(BigInt(estimates.medium.maxFeePerGas)).toBe(
(basePerGas * 135n) / 100n + (priorityPerGas * 105n) / 100n,
);
expect(BigInt(estimates.high.maxFeePerGas)).toBe(
(basePerGas * 170n) / 100n + (priorityPerGas * 110n) / 100n,
);
});

it('uses default flow if error', async () => {
jest
.spyOn(DefaultGasFeeFlow.prototype, 'getGasFees')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { ChainId, hexToBN, toHex } from '@metamask/controller-utils';
import {
ChainId,
fractionBN,
hexToBN,
toHex,
} from '@metamask/controller-utils';
import type { NetworkClientId } from '@metamask/network-controller';
import { createModuleLogger } from '@metamask/utils';
import type { Hex } from '@metamask/utils';
Expand Down Expand Up @@ -28,6 +33,16 @@ type FeesByLevel = {

const log = createModuleLogger(projectLogger, 'linea-gas-fee-flow');

/**
* Precision used to convert a decimal multiplier (e.g. 1.35) into an integer
* numerator/denominator pair for `fractionBN`, which does the multiplication
* as an arbitrary-precision BN operation. `BN.muln` cannot be used directly
* with a fractional multiplier: it silently truncates to the integer part on
* a per-26-bit-word basis instead of throwing, so `base.muln(1.35)` returns a
* wrong, under-computed result rather than failing loudly.
*/
const MULTIPLIER_PRECISION = 100;

const LINEA_CHAIN_IDS: Hex[] = [
ChainId['linea-mainnet'],
ChainId['linea-goerli'],
Expand Down Expand Up @@ -150,9 +165,9 @@ export class LineaGasFeeFlow implements GasFeeFlow {
multipliers: { low: number; medium: number; high: number },
): FeesByLevel {
const base = hexToBN(value);
const low = base.muln(multipliers.low);
const medium = base.muln(multipliers.medium);
const high = base.muln(multipliers.high);
const low = this.#applyMultiplier(base, multipliers.low);
const medium = this.#applyMultiplier(base, multipliers.medium);
const high = this.#applyMultiplier(base, multipliers.high);

return {
low,
Expand All @@ -161,6 +176,25 @@ export class LineaGasFeeFlow implements GasFeeFlow {
};
}

/**
* Multiplies a BN value by a decimal multiplier, quantized to
* `MULTIPLIER_PRECISION` decimal places, without `BN.muln`'s word-boundary
* truncation. All multipliers this flow uses have at most two decimal
* places, so this is exact for them; it is not a general-purpose
* arbitrary-precision decimal multiply.
*
* @param value - The value to multiply.
* @param multiplier - The decimal multiplier (e.g. 1.35).
* @returns The multiplied value.
*/
#applyMultiplier(value: BN, multiplier: number): BN {
return fractionBN(
value,
Math.round(multiplier * MULTIPLIER_PRECISION),
MULTIPLIER_PRECISION,
);
}

#getMaxFees(
baseFees: Record<GasFeeEstimateLevel, BN>,
priorityFees: Record<GasFeeEstimateLevel, BN>,
Expand Down
5 changes: 5 additions & 0 deletions packages/user-operation-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Fix `normalizeGasEstimate` silently under-computing the gas buffer for estimates that need more than one 26-bit `BN` word (values at or above ~67.1M) ([#10045](https://github.com/MetaMask/core/pull/10045))
- Same root cause as the `LineaGasFeeFlow` fix in `@metamask/transaction-controller`: `BN.muln(1.5)` truncates instead of throwing once the value crosses the 26-bit-word boundary (~67.1M), e.g. `100000000 * 1.5` previously returned `116445568` instead of `150000000`.

## [41.2.9]

### Changed
Expand Down
20 changes: 20 additions & 0 deletions packages/user-operation-controller/src/utils/gas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,26 @@ describe('gas', () => {
);
});

it('applies the gas buffer multiplier without integer truncation for large estimates', async () => {
// 100_000_000 is above the 2^26 threshold at which `BN.muln` with a
// fractional multiplier silently truncates instead of throwing -
// `muln(1.5)` on this value previously returned 116445568 instead of
// the correct 150000000.
bundlerMock.estimateUserOperationGas.mockResolvedValue({
callGasLimit: 100_000_000,
preVerificationGas: 456,
verificationGasLimit: 789,
});

await callUpdateGas();

expect(metadata.userOperation).toStrictEqual(
expect.objectContaining({
callGasLimit: '0x8f0d180',
}),
);
});

it('if estimates are hexadecimal strings', async () => {
bundlerMock.estimateUserOperationGas.mockResolvedValue(
ESTIMATE_RESPONSE_HEX_MOCK,
Expand Down
20 changes: 18 additions & 2 deletions packages/user-operation-controller/src/utils/gas.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { hexToBN } from '@metamask/controller-utils';
import { fractionBN, hexToBN } from '@metamask/controller-utils';
import { add0x } from '@metamask/utils';
import BN from 'bn.js';

Expand All @@ -17,6 +17,18 @@ const log = createModuleLogger(projectLogger, 'gas');
*/
const GAS_ESTIMATE_MULTIPLIER = 1.5;

/**
* Precision used to convert `GAS_ESTIMATE_MULTIPLIER` into an integer
* numerator/denominator pair for `fractionBN`. `BN.muln` cannot be used
* directly with a fractional multiplier: it silently truncates to the
* integer part on a per-26-bit-word basis instead of throwing, so
* `value.muln(1.5)` can return a wrong, under-computed result once the gas
* estimate needs more than one 26-bit word (values at or above ~67.1M, i.e.
* 2^26, are susceptible, though not every value above that boundary is
* actually affected).
*/
const GAS_ESTIMATE_MULTIPLIER_PRECISION = 100;

/**
* Populates the gas properties for a user operation.
*
Expand Down Expand Up @@ -87,7 +99,11 @@ function normalizeGasEstimate(rawValue: string | number): string {
const value =
typeof rawValue === 'string' ? hexToBN(rawValue) : new BN(rawValue);

const bufferedValue = value.muln(GAS_ESTIMATE_MULTIPLIER);
const bufferedValue = fractionBN(
value,
Math.round(GAS_ESTIMATE_MULTIPLIER * GAS_ESTIMATE_MULTIPLIER_PRECISION),
GAS_ESTIMATE_MULTIPLIER_PRECISION,
);

return add0x(bufferedValue.toString(16));
}