Skip to content

Commit

Permalink
Add EIP-1559 compatibility validation for transaction creation (#1693)
Browse files Browse the repository at this point in the history
## Explanation

This PR aims to include a validation existent in the Extension in the `TransactionController` core. The validation gets the current network and current account to check if they support EIP-1559 transactions. If the transaction is an EIP-1559 transaction, meaning it contains `maxFeePerGas` and `maxPriorityFeePerGas` values, but the network or account does not support EIP-1559, then an error is thrown.

In order to support it two callbacks are passed to the controller constructor `getCurrentAccountEIP1559Compatibility` and
`getCurrentNetworkEIP1559Compatibility`.

## Changelog

### `@metamask/transaction-controller`

- **BREAKING**: Add required `getCurrentAccountEIP1559Compatibility` and `getCurrentNetworkEIP1559Compatibility` callback arguments to constructor.
  • Loading branch information
vinistevam authored Sep 27, 2023
1 parent 762882d commit db2aef2
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,8 @@ describe('TransactionController', () => {
{
blockTracker: finalNetwork.blockTracker,
getNetworkState: () => finalNetwork.state,
getCurrentAccountEIP1559Compatibility: () => true,
getCurrentNetworkEIP1559Compatibility: () => true,
messenger,
onNetworkStateChange: finalNetwork.subscribe,
provider: finalNetwork.provider,
Expand Down
28 changes: 27 additions & 1 deletion packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ export class TransactionController extends BaseController<

private readonly getNetworkState: () => NetworkState;

private readonly getCurrentAccountEIP1559Compatibility: () => Promise<boolean>;

private readonly getCurrentNetworkEIP1559Compatibility: () => Promise<boolean>;

private readonly messagingSystem: TransactionControllerMessenger;

private readonly incomingTransactionHelper: IncomingTransactionHelper;
Expand Down Expand Up @@ -232,6 +236,8 @@ export class TransactionController extends BaseController<
* @param options.blockTracker - The block tracker used to poll for new blocks data.
* @param options.disableHistory - Whether to disable storing history in transaction metadata.
* @param options.disableSendFlowHistory - Explicitly disable transaction metadata history.
* @param options.getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559.
* @param options.getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559.
* @param options.getNetworkState - Gets the state of the network controller.
* @param options.getSelectedAddress - Gets the address of the currently selected account.
* @param options.incomingTransactions - Configuration options for incoming transaction support.
Expand All @@ -250,6 +256,8 @@ export class TransactionController extends BaseController<
blockTracker,
disableHistory,
disableSendFlowHistory,
getCurrentAccountEIP1559Compatibility,
getCurrentNetworkEIP1559Compatibility,
getNetworkState,
getSelectedAddress,
incomingTransactions = {},
Expand All @@ -260,6 +268,8 @@ export class TransactionController extends BaseController<
blockTracker: BlockTracker;
disableHistory: boolean;
disableSendFlowHistory: boolean;
getCurrentAccountEIP1559Compatibility: () => Promise<boolean>;
getCurrentNetworkEIP1559Compatibility: () => Promise<boolean>;
getNetworkState: () => NetworkState;
getSelectedAddress: () => string;
incomingTransactions: {
Expand Down Expand Up @@ -297,6 +307,10 @@ export class TransactionController extends BaseController<
this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false;
this.isHistoryDisabled = disableHistory ?? false;
this.registry = new MethodRegistry({ provider });
this.getCurrentAccountEIP1559Compatibility =
getCurrentAccountEIP1559Compatibility;
this.getCurrentNetworkEIP1559Compatibility =
getCurrentNetworkEIP1559Compatibility;

this.nonceTracker = new NonceTracker({
provider,
Expand Down Expand Up @@ -426,7 +440,8 @@ export class TransactionController extends BaseController<
const chainId = this.getChainId();
const { transactions } = this.state;
txParams = normalizeTxParams(txParams);
validateTxParams(txParams);
const isEIP1559Compatible = await this.getEIP1559Compatibility();
validateTxParams(txParams, isEIP1559Compatible);

const dappSuggestedGasFees = this.generateDappSuggestedGasFees(
txParams,
Expand Down Expand Up @@ -1800,6 +1815,17 @@ export class TransactionController extends BaseController<
transactionMeta.v = addHexPrefix(signedTx.v.toString(16));
}
}

private async getEIP1559Compatibility() {
const currentNetworkIsEIP1559Compatible =
await this.getCurrentNetworkEIP1559Compatibility();
const currentAccountIsEIP1559Compatible =
this.getCurrentAccountEIP1559Compatibility?.() ?? true;

return (
currentNetworkIsEIP1559Compatible && currentAccountIsEIP1559Compatible
);
}
}

export default TransactionController;
17 changes: 17 additions & 0 deletions packages/transaction-controller/src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,23 @@ describe('utils', () => {
} as any),
).not.toThrow();
});

it('throws if params specifies an EIP-1559 transaction but the current network does not support EIP-1559', () => {
expect(() =>
util.validateTxParams(
{
from: '0x3244e191f1b4903970224322180f1fbbc415696b',
maxFeePerGas: '2',
maxPriorityFeePerGas: '3',
} as any,
false,
),
).toThrow(
rpcErrors.invalidParams(
'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559',
),
);
});
});

describe('isEIP1559Transaction', () => {
Expand Down
16 changes: 13 additions & 3 deletions packages/transaction-controller/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,12 @@ export function normalizeTxParams(txParams: TransactionParams) {
* the event of any validation error.
*
* @param txParams - Transaction params object to validate.
* @param isEIP1559Compatible - whether or not the current network supports EIP-1559 transactions.
*/
export function validateTxParams(txParams: TransactionParams) {
export function validateTxParams(
txParams: TransactionParams,
isEIP1559Compatible = true,
) {
if (
!txParams.from ||
typeof txParams.from !== 'string' ||
Expand All @@ -66,6 +70,12 @@ export function validateTxParams(txParams: TransactionParams) {
);
}

if (isEIP1559Transaction(txParams) && !isEIP1559Compatible) {
throw rpcErrors.invalidParams(
'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559',
);
}

if (txParams.to === '0x' || txParams.to === undefined) {
if (txParams.data) {
delete txParams.to;
Expand Down Expand Up @@ -114,14 +124,14 @@ export function validateTxParams(txParams: TransactionParams) {
* @param txParams - Transaction params object to add.
* @returns Boolean that is true if the transaction is EIP-1559 (has maxFeePerGas and maxPriorityFeePerGas), otherwise returns false.
*/
export const isEIP1559Transaction = (txParams: TransactionParams): boolean => {
export function isEIP1559Transaction(txParams: TransactionParams): boolean {
const hasOwnProp = (obj: TransactionParams, key: string) =>
Object.prototype.hasOwnProperty.call(obj, key);
return (
hasOwnProp(txParams, 'maxFeePerGas') &&
hasOwnProp(txParams, 'maxPriorityFeePerGas')
);
};
}

export const validateGasValues = (
gasValues: GasPriceValue | FeeMarketEIP1559Values,
Expand Down

0 comments on commit db2aef2

Please sign in to comment.