Skip to content

Commit

Permalink
Added EIP-2930 support (#1364).
Browse files Browse the repository at this point in the history
  • Loading branch information
@ricmoo
ricmoo committed Mar 26, 2021
1 parent 1db4ce1 commit c47d2eb
Show file tree
Hide file tree
Showing 13 changed files with 487 additions and 40 deletions.
5 changes: 4 additions & 1 deletion packages/abstract-provider/src.ts/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,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 { Transaction } from "@ethersproject/transactions";
import { AccessListish, Transaction } from "@ethersproject/transactions";
import { OnceBlockable } from "@ethersproject/web";

import { Logger } from "@ethersproject/logger";
Expand All @@ -26,6 +26,9 @@ export type TransactionRequest = {
data?: BytesLike,
value?: BigNumberish,
chainId?: number

type?: number;
accessList?: AccessListish;
}

export interface TransactionResponse extends Transaction {
Expand Down
2 changes: 1 addition & 1 deletion packages/abstract-signer/src.ts/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { version } from "./_version";
const logger = new Logger(version);

const allowedTransactionKeys: Array<string> = [
"chainId", "data", "from", "gasLimit", "gasPrice", "nonce", "to", "value"
"accessList", "chainId", "data", "from", "gasLimit", "gasPrice", "nonce", "to", "type", "value"
];

const forwardErrors = [
Expand Down
2 changes: 2 additions & 0 deletions packages/bytes/src.ts/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ export function splitSignature(signature: SignatureLike): Signature {
if (result.recoveryParam == null) {
if (result.v == null) {
logger.throwArgumentError("signature missing v and recoveryParam", "signature", signature);
} else if (result.v === 0 || result.v === 1) {
result.recoveryParam = result.v;

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 26, 2021 •
edited
Loading

How exactly does this work? This implies that { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } are correct, but the check below throws an argument error if they were both specified as such?

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 26, 2021 •
edited
Loading

Also conflicts with the isBytesLike(signature) path above?

This comment has been minimized.

Sign in to view

Copy link
@feri42

feri42 Aug 27, 2021 •
edited
Loading

but the check below throws an argument error if they were both specified as such

Looks like that, but the test below and the assignment above are mutually exclusive. If I had to guess, this tells me that if v is 0 or 1 then recoveryParam must be the same, but if v is greater than 1 then the formula 1 - (result.v % 2) applies.

This comment has been minimized.

Sign in to view

Copy link
@feri42

feri42 Aug 27, 2021

Also conflicts with the isBytesLike(signature) path above?

How? in that path recoveryParam is set to something >= 27, so we never get to this part.

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 27, 2021 •
edited
Loading

c47d2eb#diff-047e7ebfdbd5c41e762cda03593bd15ed8b3121dece67262729dfcbde7040818R331-R352
Has:

if (isBytesLike(signature)) {
…
  // Compute recoveryParam from v
  result.recoveryParam = 1 - (result.v % 2);

So, if signature was passed as a bytes-like instead of an object, this logic does not apply, and { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } are unreachable.

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 27, 2021

Same below, if signature was passed as an object already containing a non-nullish recoveryParam, { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } will throw.

In short:

  • If signature is a bytes-like, { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } are unreachable.
  • If signature is an object containing v but not recoveryParam, then { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } could be created by this new codepath.
  • If signature is an object containing pre-defined recoveryParam together with v, { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } will throw.

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 30, 2021

сс @ricmoo perhaps?

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Aug 30, 2021

ethers.js/packages/bytes/src.ts/index.ts

Lines 344 to 348 in c47d2eb

if (result.v === 0 || result.v === 1) {
result.v += 27;
} else {
logger.throwArgumentError("signature invalid v byte", "signature", signature);
}

Ah. I see that the binary form input can't generate a v of 0 of 1.

Still looks inconsistent with non-binary { v: 0, recoveryParam: 0 } and { v: 1, recoveryParam: 1 } codepaths though?

This comment has been minimized.

Sign in to view

Copy link
@ricmoo

ricmoo Aug 31, 2021

Author Member

Heya! Just saw this thread. The double-checking of the parameters agreeing has a lot of nuance, but I think it works. Can you give an example where it does not? Is it a problem where a result cannot then be re-serialized?

This comment has been minimized.

Sign in to view

Copy link
@ChALkeR

ChALkeR Sep 24, 2021

@ricmoo I filed #2084 with more info and a testcase.
} else {
result.recoveryParam = 1 - (result.v % 2);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/contracts/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"@ethersproject/bytes": "^5.0.9",
"@ethersproject/constants": "^5.0.8",
"@ethersproject/logger": "^5.0.8",
"@ethersproject/properties": "^5.0.7"
"@ethersproject/properties": "^5.0.7",
"@ethersproject/transactions": "^5.0.11"
},
"description": "Contract abstraction meta-class for ethers.",
"ethereum": "donations.ethers.eth",
Expand Down
16 changes: 14 additions & 2 deletions packages/contracts/src.ts/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getAddress, getContractAddress } from "@ethersproject/address";
import { BigNumber, BigNumberish } from "@ethersproject/bignumber";
import { arrayify, BytesLike, concat, hexlify, isBytes, isHexString } from "@ethersproject/bytes";
import { Deferrable, defineReadOnly, deepCopy, getStatic, resolveProperties, shallowCopy } from "@ethersproject/properties";
// @TOOD remove dependences transactions
import { AccessList, accessListify, AccessListish } from "@ethersproject/transactions";

import { Logger } from "@ethersproject/logger";
import { version } from "./_version";
Expand All @@ -18,6 +18,8 @@ export interface Overrides {
gasLimit?: BigNumberish | Promise<BigNumberish>;
gasPrice?: BigNumberish | Promise<BigNumberish>;
nonce?: BigNumberish | Promise<BigNumberish>;
type?: number;
accessList?: AccessListish;
};

export interface PayableOverrides extends Overrides {
Expand Down Expand Up @@ -45,6 +47,9 @@ export interface PopulatedTransaction {
data?: string;
value?: BigNumber;
chainId?: number;

type?: number;
accessList?: AccessList;
};

export type EventFilter = {
Expand Down Expand Up @@ -94,7 +99,8 @@ export interface ContractTransaction extends TransactionResponse {
///////////////////////////////

const allowedTransactionKeys: { [ key: string ]: boolean } = {
chainId: true, data: true, from: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true
chainId: true, data: true, from: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true,
type: true, accessList: true,
}

async function resolveName(resolver: Signer | Provider, nameOrPromise: string | Promise<string>): Promise<string> {
Expand Down Expand Up @@ -212,6 +218,9 @@ async function populateTransaction(contract: Contract, fragment: FunctionFragmen
if (ro.gasPrice != null) { tx.gasPrice = BigNumber.from(ro.gasPrice); }
if (ro.from != null) { tx.from = ro.from; }

if (ro.type != null) { tx.type = ro.type; }
if (ro.accessList != null) { tx.accessList = accessListify(ro.accessList); }

// If there was no "gasLimit" override, but the ABI specifies a default, use it
if (tx.gasLimit == null && fragment.gas != null) {
// Conmpute the intrinisic gas cost for this transaction
Expand Down Expand Up @@ -247,6 +256,9 @@ async function populateTransaction(contract: Contract, fragment: FunctionFragmen
delete overrides.from;
delete overrides.value;

delete overrides.type;
delete overrides.accessList;

// Make sure there are no stray overrides, which may indicate a
// typo or using an unsupported key.
const leftovers = Object.keys(overrides).filter((key) => ((<any>overrides)[key] != null));
Expand Down
20 changes: 18 additions & 2 deletions packages/networks/src.ts/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,30 @@ function ethDefaultProvider(network: string | Network): Renetworkable {
}

if (providers.AlchemyProvider) {
// These networks are currently faulty on Alchemy as their
// network does not handle the Berlin hardfork, which is
// live on these ones.
// @TODO: This goes away once AlchemyAPI has upgraded their nodes
const skip = [ "goerli", "ropsten", "rinkeby" ];
try {
providerList.push(new providers.AlchemyProvider(network, options.alchemy));
const provider = new providers.AlchemyProvider(network, options.alchemy);
if (provider.network && skip.indexOf(provider.network.name) === -1) {
providerList.push(provider);
}
} catch(error) { }
}

if (providers.PocketProvider) {
// These networks are currently faulty on Alchemy as their
// network does not handle the Berlin hardfork, which is
// live on these ones.
// @TODO: This goes away once Pocket has upgraded their nodes
const skip = [ "goerli", "ropsten", "rinkeby" ];
try {
providerList.push(new providers.PocketProvider(network));
const provider = new providers.PocketProvider(network);
if (provider.network && skip.indexOf(provider.network.name) === -1) {
providerList.push(provider);
}
} catch(error) { }
}

Expand Down
10 changes: 10 additions & 0 deletions packages/providers/src.ts/base-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,15 @@ export class BaseProvider extends Provider implements EnsProvider {
tx[key] = Promise.resolve(values[key]).then((v) => (v ? BigNumber.from(v): null));
});

["type"].forEach((key) => {
if (values[key] == null) { return; }
tx[key] = Promise.resolve(values[key]).then((v) => ((v != null) ? v: null));
});

if (values.accessList) {
tx.accessList = this.formatter.accessList(values.accessList);
}

["data"].forEach((key) => {
if (values[key] == null) { return; }
tx[key] = Promise.resolve(values[key]).then((v) => (v ? hexlify(v): null));
Expand Down Expand Up @@ -1175,6 +1184,7 @@ export class BaseProvider extends Provider implements EnsProvider {
const params = await resolveProperties({
transaction: this._getTransactionRequest(transaction)
});

const result = await this.perform("estimateGas", params);
try {
return BigNumber.from(result);
Expand Down
24 changes: 21 additions & 3 deletions packages/providers/src.ts/etherscan-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ function getTransactionPostData(transaction: TransactionRequest): Record<string,
const result: Record<string, string> = { };
for (let key in transaction) {
if ((<any>transaction)[key] == null) { continue; }
let value = hexlify((<any>transaction)[key]);
let value = (<any>transaction)[key];
// Quantity-types require no leading zero, unless 0
if ((<any>{ gasLimit: true, gasPrice: true, nonce: true, value: true })[key]) {
value = hexValue(value);
if ((<any>{ type: true, gasLimit: true, gasPrice: true, nonce: true, value: true })[key]) {
value = hexValue(hexlify(value));
} else if (key === "accessList") {
value = value;
} else {
value = hexlify(value);
}
result[key] = value;
}
Expand Down Expand Up @@ -293,6 +297,13 @@ export class EtherscanProvider extends BaseProvider{


case "call": {
if (params.transaction.type != null) {
logger.throwError("Etherscan does not currently support Berlin", Logger.errors.UNSUPPORTED_OPERATION, {
operation: "call",
transaction: params.transaction
});
}

if (params.blockTag !== "latest") {
throw new Error("EtherscanProvider does not support blockTag for call");
}
Expand All @@ -310,6 +321,13 @@ export class EtherscanProvider extends BaseProvider{
}

case "estimateGas": {
if (params.transaction.type != null) {
logger.throwError("Etherscan does not currently support Berlin", Logger.errors.UNSUPPORTED_OPERATION, {
operation: "estimateGas",
transaction: params.transaction
});
}

const postData = getTransactionPostData(params.transaction);
postData.module = "proxy";
postData.action = "eth_estimateGas";
Expand Down
34 changes: 12 additions & 22 deletions packages/providers/src.ts/formatter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { BigNumber } from "@ethersproject/bignumber";
import { hexDataLength, hexDataSlice, hexValue, hexZeroPad, isHexString } from "@ethersproject/bytes";
import { AddressZero } from "@ethersproject/constants";
import { shallowCopy } from "@ethersproject/properties";
import { parse as parseTransaction } from "@ethersproject/transactions";
import { AccessList, accessListify, parse as parseTransaction } from "@ethersproject/transactions";

import { Logger } from "@ethersproject/logger";
import { version } from "./_version";
Expand Down Expand Up @@ -51,6 +51,9 @@ export class Formatter {
formats.transaction = {
hash: hash,

type: Formatter.allowNull(number, null),
accessList: Formatter.allowNull(this.accessList.bind(this), null),

blockHash: Formatter.allowNull(hash, null),
blockNumber: Formatter.allowNull(number, null),
transactionIndex: Formatter.allowNull(number, null),
Expand Down Expand Up @@ -83,6 +86,8 @@ export class Formatter {
to: Formatter.allowNull(address),
value: Formatter.allowNull(bigNumber),
data: Formatter.allowNull(strictData),
type: Formatter.allowNull(number),
accessList: Formatter.allowNull(this.accessList.bind(this), null),
};

formats.receiptLog = {
Expand Down Expand Up @@ -162,6 +167,10 @@ export class Formatter {
return formats;
}

accessList(accessList: Array<any>): AccessList {
return accessListify(accessList || []);
}

// Requires a BigNumberish that is within the IEEE754 safe integer range; returns a number
// Strict! Used on input.
number(number: any): number {
Expand Down Expand Up @@ -308,28 +317,9 @@ export class Formatter {
transaction.creates = this.contractAddress(transaction);
}

// @TODO: use transaction.serialize? Have to add support for including v, r, and s...
/*
if (!transaction.raw) {
// Very loose providers (e.g. TestRPC) do not provide a signature or raw
if (transaction.v && transaction.r && transaction.s) {
let raw = [
stripZeros(hexlify(transaction.nonce)),
stripZeros(hexlify(transaction.gasPrice)),
stripZeros(hexlify(transaction.gasLimit)),
(transaction.to || "0x"),
stripZeros(hexlify(transaction.value || "0x")),
hexlify(transaction.data || "0x"),
stripZeros(hexlify(transaction.v || "0x")),
stripZeros(hexlify(transaction.r)),
stripZeros(hexlify(transaction.s)),
];
transaction.raw = rlpEncode(raw);
}
if (transaction.type === 1 && transaction.accessList == null) {
transaction.accessList = [ ];
}
*/

const result: TransactionResponse = Formatter.check(this.formats.transaction, transaction);

Expand Down
15 changes: 11 additions & 4 deletions packages/providers/src.ts/json-rpc-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { _TypedDataEncoder } from "@ethersproject/hash";
import { Network, Networkish } from "@ethersproject/networks";
import { checkProperties, deepCopy, Deferrable, defineReadOnly, getStatic, resolveProperties, shallowCopy } from "@ethersproject/properties";
import { toUtf8Bytes } from "@ethersproject/strings";
import { AccessList, accessListify } from "@ethersproject/transactions";
import { ConnectionInfo, fetchJson, poll } from "@ethersproject/web";

import { Logger } from "@ethersproject/logger";
Expand Down Expand Up @@ -264,7 +265,8 @@ class UncheckedJsonRpcSigner extends JsonRpcSigner {
}

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, accessList: true
}

export class JsonRpcProvider extends BaseProvider {
Expand Down Expand Up @@ -529,20 +531,21 @@ export class JsonRpcProvider extends BaseProvider {
// before this is called
// @TODO: This will likely be removed in future versions and prepareRequest
// will be the preferred method for this.
static hexlifyTransaction(transaction: TransactionRequest, allowExtra?: { [key: string]: boolean }): { [key: string]: string } {
static hexlifyTransaction(transaction: TransactionRequest, allowExtra?: { [key: string]: boolean }): { [key: string]: string | AccessList } {
// Check only allowed properties are given
const allowed = shallowCopy(allowedTransactionKeys);
if (allowExtra) {
for (const key in allowExtra) {
if (allowExtra[key]) { allowed[key] = true; }
}
}

checkProperties(transaction, allowed);

const result: { [key: string]: string } = {};
const result: { [key: string]: string | AccessList } = {};

// Some nodes (INFURA ropsten; INFURA mainnet is fine) do not like leading zeros.
["gasLimit", "gasPrice", "nonce", "value"].forEach(function(key) {
["gasLimit", "gasPrice", "type", "nonce", "value"].forEach(function(key) {
if ((<any>transaction)[key] == null) { return; }
const value = hexValue((<any>transaction)[key]);
if (key === "gasLimit") { key = "gas"; }
Expand All @@ -554,6 +557,10 @@ export class JsonRpcProvider extends BaseProvider {
result[key] = hexlify((<any>transaction)[key]);
});

if ((<any>transaction).accessList) {
result["accessList"] = accessListify((<any>transaction).accessList);
}

return result;
}
}
Loading

0 comments on commit c47d2eb

Please sign in to comment.