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

## [Unreleased]

### Added

- Calculate operator fee for OP stack networks and include it in `layer1GasFee` ([#6979](https://github.com/MetaMask/core/pull/6979))

## [61.1.0]

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@ import type { TypedTransaction } from '@ethereumjs/tx';
import { TransactionFactory } from '@ethereumjs/tx';
import { Contract } from '@ethersproject/contracts';
import type { Provider } from '@metamask/network-controller';
import type { Hex } from '@metamask/utils';
import { add0x, type Hex } from '@metamask/utils';
import BN from 'bn.js';

import { OracleLayer1GasFeeFlow } from './OracleLayer1GasFeeFlow';
import { CHAIN_IDS } from '../constants';
import type { TransactionControllerMessenger } from '../TransactionController';
import type { Layer1GasFeeFlowRequest, TransactionMeta } from '../types';
import { TransactionStatus } from '../types';
import {
type Layer1GasFeeFlowRequest,
type TransactionMeta,
TransactionStatus,
} from '../types';
import { bnFromHex, padHexToEvenLength } from '../utils/utils';

jest.mock('@ethersproject/contracts', () => ({
Contract: jest.fn(),
Expand Down Expand Up @@ -36,7 +41,8 @@ const TRANSACTION_META_MOCK: TransactionMeta = {

const SERIALIZED_TRANSACTION_MOCK = '0x1234';
const ORACLE_ADDRESS_MOCK = '0x5678' as Hex;
const LAYER_1_FEE_MOCK = '0x9ABCD';
const LAYER_1_FEE_MOCK = '0x09abcd';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Hexadecimal Leading Zero Mismatch

The test constant LAYER_1_FEE_MOCK was changed from '0x9ABCD' to '0x09abcd' (adding a leading zero), but the implementation calls toHexString() on BigNumber values which strips leading zeros. This causes a mismatch between the expected value ('0x09abcd') and the actual value ('0x9abcd') returned by the code, making tests fail. The constant should remain '0x9ABCD' (or the implementation should preserve leading zeros differently).

Fix in Cursor Fix in Web

const OPERATOR_FEE_MOCK = '0x5';
const DEFAULT_GAS_PRICE_ORACLE_ADDRESS =
'0x420000000000000000000000000000000000000F';

Expand Down Expand Up @@ -98,9 +104,9 @@ class DefaultOracleLayer1GasFeeFlow extends OracleLayer1GasFeeFlow {

describe('OracleLayer1GasFeeFlow', () => {
const contractMock = jest.mocked(Contract);
const contractGetL1FeeMock: jest.MockedFn<
() => Promise<{ toHexString: () => string }>
> = jest.fn();
const contractGetL1FeeMock: jest.MockedFn<() => Promise<BN>> = jest.fn();
const contractGetOperatorFeeMock: jest.MockedFn<() => Promise<BN>> =
jest.fn();

let request: Layer1GasFeeFlowRequest;

Expand All @@ -112,13 +118,14 @@ describe('OracleLayer1GasFeeFlow', () => {

contractMock.mockClear();
contractGetL1FeeMock.mockClear();
contractGetOperatorFeeMock.mockClear();

contractGetL1FeeMock.mockResolvedValue({
toHexString: () => LAYER_1_FEE_MOCK,
});
contractGetL1FeeMock.mockResolvedValue(bnFromHex(LAYER_1_FEE_MOCK));
contractGetOperatorFeeMock.mockResolvedValue(new BN(0));

contractMock.mockReturnValue({
getL1Fee: contractGetL1FeeMock,
getOperatorFee: contractGetOperatorFeeMock,
} as unknown as Contract);
});

Expand Down Expand Up @@ -156,6 +163,7 @@ describe('OracleLayer1GasFeeFlow', () => {
expect(contractGetL1FeeMock).toHaveBeenCalledWith(
serializedTransactionMock,
);
expect(contractGetOperatorFeeMock).not.toHaveBeenCalled();
});

it('signs transaction with dummy key if supported by flow', async () => {
Expand All @@ -180,6 +188,7 @@ describe('OracleLayer1GasFeeFlow', () => {
});

expect(typedTransactionMock.sign).toHaveBeenCalledTimes(1);
expect(contractGetOperatorFeeMock).not.toHaveBeenCalled();
});

describe('throws', () => {
Expand Down Expand Up @@ -228,5 +237,79 @@ describe('OracleLayer1GasFeeFlow', () => {
expect(oracleAddress).toBe(DEFAULT_GAS_PRICE_ORACLE_ADDRESS);
expect(typedTransactionMock.sign).not.toHaveBeenCalled();
});

it('adds operator fee when gas used is available', async () => {
const gasUsed = '0x5208';
request = {
...request,
transactionMeta: {
...request.transactionMeta,
gasUsed,
},
};

contractGetOperatorFeeMock.mockResolvedValueOnce(
bnFromHex(OPERATOR_FEE_MOCK),
);

const flow = new MockOracleLayer1GasFeeFlow(false);
const response = await flow.getLayer1Fee(request);

expect(contractGetOperatorFeeMock).toHaveBeenCalledTimes(1);
expect(contractGetOperatorFeeMock).toHaveBeenCalledWith(gasUsed);
expect(response).toStrictEqual({
layer1Fee: add0x(
padHexToEvenLength(
bnFromHex(LAYER_1_FEE_MOCK)
.add(bnFromHex(OPERATOR_FEE_MOCK))
.toString(16),
),
) as Hex,
});
});

it('defaults operator fee to zero when call fails', async () => {
const gasUsed = '0x1';
request = {
...request,
transactionMeta: {
...request.transactionMeta,
gasUsed,
},
};

contractGetOperatorFeeMock.mockRejectedValueOnce(new Error('revert'));

const flow = new MockOracleLayer1GasFeeFlow(false);
const response = await flow.getLayer1Fee(request);

expect(contractGetOperatorFeeMock).toHaveBeenCalledTimes(1);
expect(response).toStrictEqual({
layer1Fee: LAYER_1_FEE_MOCK,
});
});

it('defaults operator fee to zero when call returns undefined', async () => {
const gasUsed = '0x2';
request = {
...request,
transactionMeta: {
...request.transactionMeta,
gasUsed,
},
};

contractGetOperatorFeeMock.mockResolvedValueOnce(
undefined as unknown as BN,
);

const flow = new MockOracleLayer1GasFeeFlow(false);
const response = await flow.getLayer1Fee(request);

expect(contractGetOperatorFeeMock).toHaveBeenCalledTimes(1);
expect(response).toStrictEqual({
layer1Fee: LAYER_1_FEE_MOCK,
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Contract } from '@ethersproject/contracts';
import { Web3Provider, type ExternalProvider } from '@ethersproject/providers';
import type { Hex } from '@metamask/utils';
import { createModuleLogger } from '@metamask/utils';
import { add0x, createModuleLogger } from '@metamask/utils';
import BN from 'bn.js';

import { projectLogger } from '../logger';
import type { TransactionControllerMessenger } from '../TransactionController';
Expand All @@ -12,17 +13,52 @@ import type {
TransactionMeta,
} from '../types';
import { prepareTransaction } from '../utils/prepare';
import { padHexToEvenLength, toBN } from '../utils/utils';

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

const ZERO = new BN(0);

const DUMMY_KEY =
'abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789';

const GAS_PRICE_ORACLE_ABI = [
{
inputs: [{ internalType: 'bytes', name: '_data', type: 'bytes' }],
inputs: [
{
internalType: 'bytes',
name: '_data',
type: 'bytes',
},
],
name: 'getL1Fee',
outputs: [{ internalType: 'uint256', name: '', type: 'uint256' }],
outputs: [
{
internalType: 'uint256',
name: '',
type: 'uint256',
},
],
stateMutability: 'view',
type: 'function',
},
// only available post Isthmus
{
inputs: [
{
internalType: 'uint256',
name: '_gasUsed',
type: 'uint256',
},
],
name: 'getOperatorFee',
outputs: [
{
internalType: 'uint256',
name: '',
type: 'uint256',
},
],
stateMutability: 'view',
type: 'function',
},
Expand Down Expand Up @@ -73,25 +109,37 @@ export abstract class OracleLayer1GasFeeFlow implements Layer1GasFeeFlow {
request: Layer1GasFeeFlowRequest,
): Promise<Layer1GasFeeFlowResponse> {
try {
return await this.#getOracleLayer1GasFee(request);
const { provider, transactionMeta } = request;

const contract = this.#getGasPriceOracleContract(
provider,
transactionMeta.chainId,
);

const oracleFee = await this.#getOracleLayer1GasFee(
contract,
transactionMeta,
);
const operatorFee = await this.#getOperatorLayer1GasFee(
contract,
transactionMeta,
);

return {
layer1Fee: add0x(
padHexToEvenLength(oracleFee.add(operatorFee).toString(16)),
) as Hex,
};
} catch (error) {
log('Failed to get oracle layer 1 gas fee', error);
throw new Error(`Failed to get oracle layer 1 gas fee`);
}
}

async #getOracleLayer1GasFee(
request: Layer1GasFeeFlowRequest,
): Promise<Layer1GasFeeFlowResponse> {
const { provider, transactionMeta } = request;

const contract = new Contract(
this.getOracleAddressForChain(transactionMeta.chainId),
GAS_PRICE_ORACLE_ABI,
// Network controller provider type is incompatible with ethers provider
new Web3Provider(provider as unknown as ExternalProvider),
);

contract: Contract,
transactionMeta: TransactionMeta,
): Promise<BN> {
const serializedTransaction = this.#buildUnserializedTransaction(
transactionMeta,
this.shouldSignTransaction(),
Expand All @@ -103,9 +151,31 @@ export abstract class OracleLayer1GasFeeFlow implements Layer1GasFeeFlow {
throw new Error('No value returned from oracle contract');
}

return {
layer1Fee: result.toHexString(),
};
return toBN(result);
}

async #getOperatorLayer1GasFee(
contract: Contract,
transactionMeta: TransactionMeta,
): Promise<BN> {
const { gasUsed } = transactionMeta;

if (!gasUsed) {
return ZERO;
}

try {
const result = await contract.getOperatorFee(gasUsed);

if (result === undefined) {
return ZERO;
}

return toBN(result);
} catch (error) {
log('Failed to get operator layer 1 gas fee', error);
return ZERO;
}
}

#buildUnserializedTransaction(
Expand Down Expand Up @@ -134,4 +204,16 @@ export abstract class OracleLayer1GasFeeFlow implements Layer1GasFeeFlow {
gasLimit: transactionMeta.txParams.gas,
};
}

#getGasPriceOracleContract(
provider: Layer1GasFeeFlowRequest['provider'],
chainId: Hex,
) {
return new Contract(
this.getOracleAddressForChain(chainId),
GAS_PRICE_ORACLE_ABI,
// Network controller provider type is incompatible with ethers provider
new Web3Provider(provider as unknown as ExternalProvider),
);
}
}
Loading
Loading