From c5bca7767e3f3d43e3d0bd3c9e9420321ee9907a Mon Sep 17 00:00:00 2001 From: Richard Moore Date: Mon, 14 Jun 2021 22:24:14 -0400 Subject: [PATCH] Refactored eip-1559 logic (#1610). --- packages/abstract-provider/src.ts/index.ts | 26 ++++++- packages/abstract-signer/src.ts/index.ts | 91 ++++++++-------------- packages/providers/src.ts/index.ts | 2 + packages/transactions/src.ts/index.ts | 12 +-- 4 files changed, 65 insertions(+), 66 deletions(-) diff --git a/packages/abstract-provider/src.ts/index.ts b/packages/abstract-provider/src.ts/index.ts index c8d479f406..419b0c86c2 100644 --- a/packages/abstract-provider/src.ts/index.ts +++ b/packages/abstract-provider/src.ts/index.ts @@ -3,7 +3,7 @@ import { BigNumber, BigNumberish } from "@ethersproject/bignumber"; import { BytesLike, isHexString } from "@ethersproject/bytes"; import { Network } from "@ethersproject/networks"; -import { Deferrable, Description, defineReadOnly } from "@ethersproject/properties"; +import { Deferrable, Description, defineReadOnly, resolveProperties } from "@ethersproject/properties"; import { AccessListish, Transaction } from "@ethersproject/transactions"; import { OnceBlockable } from "@ethersproject/web"; @@ -117,6 +117,12 @@ export interface TransactionReceipt { status?: number }; +export interface FeeData { + maxFeePerGas: null | BigNumber; + maxPriorityFeePerGas: null | BigNumber; + gasPrice: null | BigNumber; +} + export interface EventFilter { address?: string; topics?: Array | null>; @@ -211,7 +217,6 @@ export type Listener = (...args: Array) => void; /////////////////////////////// // Exported Abstracts - export abstract class Provider implements OnceBlockable { // Network @@ -220,6 +225,23 @@ export abstract class Provider implements OnceBlockable { // Latest State abstract getBlockNumber(): Promise; abstract getGasPrice(): Promise; + async getFeeData(): Promise { + const { block, gasPrice } = await resolveProperties({ + block: this.getBlock(-1), + gasPrice: this.getGasPrice() + }); + + let maxFeePerGas = null, maxPriorityFeePerGas = null; + + if (block && block.baseFee) { + maxFeePerGas = block.baseFee.mul(2); + //maxPriorityFeePerGas = BigNumber.from("1000000000"); + // @TODO: This needs to come from somewhere. + maxPriorityFeePerGas = BigNumber.from("1"); + } + + return { maxFeePerGas, maxPriorityFeePerGas, gasPrice }; + } // Account abstract getBalance(addressOrName: string | Promise, blockTag?: BlockTag | Promise): Promise; diff --git a/packages/abstract-signer/src.ts/index.ts b/packages/abstract-signer/src.ts/index.ts index 659c5757bb..e13eb2ad60 100644 --- a/packages/abstract-signer/src.ts/index.ts +++ b/packages/abstract-signer/src.ts/index.ts @@ -1,6 +1,6 @@ "use strict"; -import { BlockTag, Provider, TransactionRequest, TransactionResponse } from "@ethersproject/abstract-provider"; +import { BlockTag, FeeData, Provider, TransactionRequest, TransactionResponse } from "@ethersproject/abstract-provider"; import { BigNumber, BigNumberish } from "@ethersproject/bignumber"; import { Bytes, BytesLike } from "@ethersproject/bytes"; import { Deferrable, defineReadOnly, resolveProperties, shallowCopy } from "@ethersproject/properties"; @@ -19,12 +19,6 @@ const forwardErrors = [ Logger.errors.REPLACEMENT_UNDERPRICED, ]; -export interface FeeData { - maxFeePerGas: null | BigNumber; - maxPriorityFeePerGas: null | BigNumber; - gasPrice: null | BigNumber; -} - // EIP-712 Typed Data // See: https://eips.ethereum.org/EIPS/eip-712 @@ -146,21 +140,8 @@ export abstract class Signer { } async getFeeData(): Promise { - this._checkProvider("getFeeStats"); - - const { block, gasPrice } = await resolveProperties({ - block: this.provider.getBlock(-1), - gasPrice: this.provider.getGasPrice() - }); - - let maxFeePerGas = null, maxPriorityFeePerGas = null; - - if (block && block.baseFee) { - maxFeePerGas = block.baseFee.mul(2); - maxPriorityFeePerGas = BigNumber.from("1000000000"); - } - - return { maxFeePerGas, maxPriorityFeePerGas, gasPrice }; + this._checkProvider("getFeeData"); + return await this.provider.getFeeData(); } @@ -230,26 +211,23 @@ export abstract class Signer { }); } - if ((tx.type === 2 || tx.type == null) && (tx.maxFeePerGas != null && tx.maxPriorityFeePerGas != null)) { - // Fully-formed EIP-1559 transaction - - // Check the gasPrice == maxFeePerGas - if (tx.gasPrice != null && !BigNumber.from(tx.gasPrice).eq((tx.maxFeePerGas))) { - logger.throwArgumentError("gasPrice/maxFeePerGas mismatch", "transaction", transaction); - } + // Do not allow mixing pre-eip-1559 and eip-1559 proerties + const hasEip1559 = (tx.maxFeePerGas != null || tx.maxPriorityFeePerGas != null); + if (tx.gasPrice != null && (tx.type === 2 || hasEip1559)) { + logger.throwArgumentError("eip-1559 transaction do not support gasPrice", "transaction", transaction); + } else if ((tx.type === -1 || tx.type === 1) && hasEip1559) { + logger.throwArgumentError("pre-eip-1559 transaction do not support maxFeePerGas/maxPriorityFeePerGas", "transaction", transaction); + } + if ((tx.type === 2 || tx.type == null) && (tx.maxFeePerGas != null && tx.maxPriorityFeePerGas != null)) { + // Fully-formed EIP-1559 transaction (skip getFeeData) tx.type = 2; } else if (tx.type === -1 || tx.type === 1) { - // Explicit EIP-2930 or Legacy transaction - - // Do not allow EIP-1559 properties - if (tx.maxFeePerGas != null || tx.maxPriorityFeePerGas != null) { - logger.throwArgumentError(`transaction type ${ tx.type } does not support eip-1559 keys`, "transaction", transaction); - } + // Explicit Legacy or EIP-2930 transaction + // Populate missing gasPrice if (tx.gasPrice == null) { tx.gasPrice = this.getGasPrice(); } - tx.type = (tx.accessList ? 1: -1); } else { @@ -262,23 +240,19 @@ export abstract class Signer { if (feeData.maxFeePerGas != null && feeData.maxPriorityFeePerGas != null) { // The network supports EIP-1559! - if (tx.gasPrice != null && tx.maxFeePerGas == null && tx.maxPriorityFeePerGas == null) { - // Legacy or EIP-2930 transaction, but without its type set - tx.type = (tx.accessList ? 1: -1); + // Upgrade transaction from null to eip-1559 + tx.type = 2; + + if (tx.gasPrice != null) { + // Using legacy gasPrice property on an eip-1559 network, + // so use gasPrice as both fee properties + const gasPrice = tx.gasPrice; + delete tx.gasPrice; + tx.maxFeePerGas = gasPrice; + tx.maxPriorityFeePerGas = gasPrice; } else { - // Use EIP-1559; no gas price or one EIP-1559 property was specified - - // Check that gasPrice == maxFeePerGas - if (tx.gasPrice != null) { - // The first condition fails only if gasPrice and maxPriorityFeePerGas - // were specified, which is a weird thing to do - if (tx.maxFeePerGas == null || !BigNumber.from(tx.gasPrice).eq((tx.maxFeePerGas))) { - logger.throwArgumentError("gasPrice/maxFeePerGas mismatch", "transaction", transaction); - } - } - - tx.type = 2; + // Populate missing fee data if (tx.maxFeePerGas == null) { tx.maxFeePerGas = feeData.maxFeePerGas; } if (tx.maxPriorityFeePerGas == null) { tx.maxPriorityFeePerGas = feeData.maxPriorityFeePerGas; } } @@ -287,14 +261,17 @@ export abstract class Signer { // Network doesn't support EIP-1559... // ...but they are trying to use EIP-1559 properties - if (tx.maxFeePerGas != null || tx.maxPriorityFeePerGas != null) { + if (hasEip1559) { logger.throwError("network does not support EIP-1559", Logger.errors.UNSUPPORTED_OPERATION, { operation: "populateTransaction" }); } - tx.gasPrice = feeData.gasPrice; - tx.type = (tx.accessList ? 1: -1); + // Populate missing fee data + if (tx.gasPrice == null) { tx.gasPrice = feeData.gasPrice; } + + // Explicitly set untyped transaction to legacy + tx.type = -1; } else { // getFeeData has failed us. @@ -306,11 +283,7 @@ export abstract class Signer { } else if (tx.type === 2) { // Explicitly using EIP-1559 - // Check gasPrice == maxFeePerGas - if (tx.gasPrice != null && !BigNumber.from(tx.gasPrice).eq((tx.maxFeePerGas))) { - logger.throwArgumentError("gasPrice/maxFeePerGas mismatch", "transaction", transaction); - } - + // Populate missing fee data if (tx.maxFeePerGas == null) { tx.maxFeePerGas = feeData.maxFeePerGas; } if (tx.maxPriorityFeePerGas == null) { tx.maxPriorityFeePerGas = feeData.maxPriorityFeePerGas; } } diff --git a/packages/providers/src.ts/index.ts b/packages/providers/src.ts/index.ts index 5a355f5490..cc31f8acad 100644 --- a/packages/providers/src.ts/index.ts +++ b/packages/providers/src.ts/index.ts @@ -4,6 +4,7 @@ import { Block, BlockTag, EventType, + FeeData, Filter, Log, Listener, @@ -150,6 +151,7 @@ export { Block, BlockTag, EventType, + FeeData, Filter, Log, Listener, diff --git a/packages/transactions/src.ts/index.ts b/packages/transactions/src.ts/index.ts index 4e8cc68bca..4045942bbf 100644 --- a/packages/transactions/src.ts/index.ts +++ b/packages/transactions/src.ts/index.ts @@ -103,7 +103,7 @@ const transactionFields = [ ]; const allowedTransactionKeys: { [ key: string ]: boolean } = { - chainId: true, data: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true + chainId: true, data: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true, type: true } export function computeAddress(key: BytesLike | string): string { @@ -368,15 +368,16 @@ function _parseEip1559(payload: Uint8Array): Transaction { value: handleNumber(transaction[6]), data: transaction[7], accessList: accessListify(transaction[8]), - hash: keccak256(payload) }; // Unsigned EIP-1559 Transaction if (transaction.length === 9) { return tx; } + tx.hash = keccak256(payload); + _parseEipSignature(tx, transaction.slice(9), _serializeEip2930); - return null; + return tx; } function _parseEip2930(payload: Uint8Array): Transaction { @@ -395,13 +396,14 @@ function _parseEip2930(payload: Uint8Array): Transaction { to: handleAddress(transaction[4]), value: handleNumber(transaction[5]), data: transaction[6], - accessList: accessListify(transaction[7]), - hash: keccak256(payload) + accessList: accessListify(transaction[7]) }; // Unsigned EIP-2930 Transaction if (transaction.length === 8) { return tx; } + tx.hash = keccak256(payload); + _parseEipSignature(tx, transaction.slice(8), _serializeEip2930); return tx;