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

Add EIP-1559 compatibility validation for transaction creation #1693

Merged
merged 6 commits into from
Sep 27, 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
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
Loading