Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

EIP-1559 Transaction Gas Fixes #1307

Merged
merged 38 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
1fbd277
add missed field to transaction test
MicaiahReid Oct 1, 2021
a2292fd
add effectiveGasPrice to transaction's "extra" data
MicaiahReid Oct 1, 2021
822e51f
fix miscalculation in gas price tests
MicaiahReid Oct 1, 2021
57d5134
fix bug with overwriting data on upgraded txs
MicaiahReid Oct 1, 2021
6b96fe6
add tests persisting accurate gasPrice for db eip-1559 txs
MicaiahReid Oct 1, 2021
cb341f0
fix bug with 0 baseFee being omitted from block
MicaiahReid Oct 1, 2021
d70eda4
actually fix bug with base fee that I previously made worser
MicaiahReid Oct 1, 2021
a456fc8
update eth_call to properly use eip-1559 gas tx data
MicaiahReid Oct 5, 2021
4ae2b34
add very basic tests for eth_call
MicaiahReid Oct 5, 2021
bed4ca3
remove unused import
MicaiahReid Oct 5, 2021
cd139e5
remove console.log
MicaiahReid Oct 5, 2021
7405b4e
change to falsey check
MicaiahReid Oct 6, 2021
742ab71
move initial updateEffectiveGasPrice call to api
MicaiahReid Oct 6, 2021
9d4311d
fix resultant test issues of moving updateEffectiveGasPrice
MicaiahReid Oct 6, 2021
8950354
add missed field to transaction test
MicaiahReid Oct 1, 2021
929f9c4
add effectiveGasPrice to transaction's "extra" data
MicaiahReid Oct 1, 2021
28cfd6e
fix miscalculation in gas price tests
MicaiahReid Oct 1, 2021
65ab405
fix bug with overwriting data on upgraded txs
MicaiahReid Oct 1, 2021
e03f7fd
add tests persisting accurate gasPrice for db eip-1559 txs
MicaiahReid Oct 1, 2021
5c9348e
fix bug with 0 baseFee being omitted from block
MicaiahReid Oct 1, 2021
232f818
actually fix bug with base fee that I previously made worser
MicaiahReid Oct 1, 2021
c82d960
update eth_call to properly use eip-1559 gas tx data
MicaiahReid Oct 5, 2021
63d9eb4
add very basic tests for eth_call
MicaiahReid Oct 5, 2021
9a0dbca
remove unused import
MicaiahReid Oct 5, 2021
23d7dd7
remove console.log
MicaiahReid Oct 5, 2021
db65f9f
change to falsey check
MicaiahReid Oct 6, 2021
e59a2ec
move initial updateEffectiveGasPrice call to api
MicaiahReid Oct 6, 2021
d6dd260
fix resultant test issues of moving updateEffectiveGasPrice
MicaiahReid Oct 6, 2021
54b9fb0
Merge branch 'fix/eip-1559-tx-gas' of github.com:trufflesuite/ganache…
MicaiahReid Oct 6, 2021
0ae7495
remove unused references
MicaiahReid Oct 6, 2021
5407218
move updateEffectiveGasPrice call to txpool
MicaiahReid Oct 7, 2021
56ae2d3
add common/blocks to our fake blockchain for tests. fix underpriced tx
MicaiahReid Oct 7, 2021
da0ced7
remove unnecessary function call
MicaiahReid Oct 7, 2021
6185f0d
move variable declaration context
MicaiahReid Oct 7, 2021
e8a7ca5
Apply suggestions from code review
MicaiahReid Oct 7, 2021
6c7c36d
remove unused contract functions
MicaiahReid Oct 7, 2021
af04b08
Merge branch 'fix/eip-1559-tx-gas' of github.com:trufflesuite/ganache…
MicaiahReid Oct 7, 2021
b9b185d
rename Example contract to EthCall
MicaiahReid Oct 7, 2021
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
10 changes: 8 additions & 2 deletions src/chains/ethereum/block/src/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ export class Block {
const hash = this.hash();
const txFn = this.getTxFn(includeFullTransactions);
const hashBuffer = hash.toBuffer();
const number = this.header.number.toBuffer();
const header = this.header;
const number = header.number.toBuffer();
const common = this._common;
const jsonTxs = this._rawTransactions.map((raw, index) => {
const [from, hash] = this._rawTransactionMetaData[index];
Expand All @@ -85,12 +86,17 @@ export class Block {
Quantity.from(index).toBuffer()
];
const tx = TransactionFactory.fromDatabaseTx(raw, common, extra);
// we could either parse the raw data to check if the tx is type 2,
// get the maxFeePerGas and maxPriorityFeePerGas, use those to calculate
// the effectiveGasPrice and add it to `extra` above, or we can just
// leave it out of extra and update the effectiveGasPrice after like this
tx.updateEffectiveGasPrice(header.baseFeePerGas);
return txFn(tx);
});

return {
hash,
...this.header,
...header,
size: Quantity.from(this._size),
transactions: jsonTxs,
uncles: [] as string[] // this.value.uncleHeaders.map(function(uncleHash) {return to.hex(uncleHash)})
Expand Down
2 changes: 1 addition & 1 deletion src/chains/ethereum/block/src/runtime-block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export class RuntimeBlock {
BUFFER_32_ZERO, // mixHash
BUFFER_8_ZERO // nonce
];
if (header.baseFeePerGas) {
if (header.baseFeePerGas !== undefined) {
rawHeader[15] = header.baseFeePerGas.buf;
}

Expand Down
45 changes: 41 additions & 4 deletions src/chains/ethereum/ethereum/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2496,6 +2496,7 @@ export default class EthereumApi implements Api {
@assertArgLength(1, 2)
async eth_call(transaction: any, blockNumber: QUANTITY | Tag = Tag.LATEST) {
const blockchain = this.#blockchain;
const common = this.#blockchain.common;
const blocks = blockchain.blocks;
const parentBlock = await blocks.get(blockNumber);
const parentHeader = parentBlock.header;
Expand All @@ -2522,6 +2523,44 @@ export default class EthereumApi implements Api {
data = Data.from(transaction.data);
}

// eth_call doesn't validate that the transaction has a sufficient
// "effectiveGasPrice". however, if `maxPriorityFeePerGas` or
// `maxFeePerGas` values are set, the baseFeePerGas is used to calculate
// the effectiveGasPrice, which is used to calculate tx costs/refunds.
const baseFeePerGasBigInt = Block.calcNextBaseFee(parentBlock);

let gasPrice: Quantity;
const hasGasPrice = typeof transaction.gasPrice !== "undefined";
if (!common.isActivatedEIP(1559)) {
gasPrice = Quantity.from(hasGasPrice ? 0 : transaction.gasPrice);
} else {
const hasMaxFeePerGas = typeof transaction.maxFeePerGas !== "undefined";
const hasMaxPriorityFeePerGas =
typeof transaction.maxPriorityFeePerGas !== "undefined";

if (hasGasPrice && (hasMaxFeePerGas || hasMaxPriorityFeePerGas)) {
throw new Error(
"both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified"
);
}
// User specified 1559 gas fields (or none), use those
let maxFeePerGas = 0n;
let maxPriorityFeePerGas = 0n;
if (hasMaxFeePerGas) {
maxFeePerGas = BigInt(transaction.maxFeePerGas);
}
if (hasMaxPriorityFeePerGas) {
maxPriorityFeePerGas = BigInt(transaction.maxPriorityFeePerGas);
}
if (maxPriorityFeePerGas > 0 || maxFeePerGas > 0) {
const a = maxFeePerGas - baseFeePerGasBigInt;
const tip = a < maxPriorityFeePerGas ? a : maxPriorityFeePerGas;
gasPrice = Quantity.from(baseFeePerGasBigInt + tip);
} else {
gasPrice = Quantity.from(0);
}
}

const block = new RuntimeBlock(
parentHeader.number,
parentHeader.parentHash,
Expand All @@ -2531,7 +2570,7 @@ export default class EthereumApi implements Api {
parentHeader.timestamp,
options.miner.difficulty,
parentHeader.totalDifficulty,
0n // no baseFeePerGas for eth_call
baseFeePerGasBigInt
);

const simulatedTransaction = {
Expand All @@ -2542,9 +2581,7 @@ export default class EthereumApi implements Api {
? blockchain.coinbase
: Address.from(transaction.from),
to: transaction.to == null ? null : Address.from(transaction.to),
gasPrice: Quantity.from(
transaction.gasPrice == null ? 0 : transaction.gasPrice
),
gasPrice,
value:
transaction.value == null ? null : Quantity.from(transaction.value),
data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import PromiseQueue from "@ganache/promise-queue";
import type Common from "@ethereumjs/common";
import { Data, Quantity } from "@ganache/utils";
import {
GanacheRawExtraTx,
TransactionFactory,
TypedRpcTransaction,
TypedTransaction
Expand Down Expand Up @@ -47,19 +48,20 @@ export default class TransactionManager extends Manager<NoOp> {
[Data.from(transactionHash).toString()]
);
if (tx == null) return null;
const extra = [
const blockHash = Data.from((tx as any).blockHash, 32);
const blockNumber = Quantity.from((tx as any).blockNumber);
const index = Quantity.from((tx as any).transactionIndex);

const extra: GanacheRawExtraTx = [
Data.from(tx.from, 20).toBuffer(),
Data.from((tx as any).hash, 32).toBuffer(),
Data.from((tx as any).blockHash, 32).toBuffer(),
Quantity.from((tx as any).blockNumber).toBuffer(),
Quantity.from((tx as any).transactionIndex).toBuffer()
] as any;
blockHash.toBuffer(),
blockNumber.toBuffer(),
index.toBuffer(),
Quantity.from(tx.gasPrice).toBuffer()
davidmurdoch marked this conversation as resolved.
Show resolved Hide resolved
];
const runTx = TransactionFactory.fromRpc(tx, fallback.common, extra);
return runTx.serializeForDb(
Data.from((tx as any).blockHash, 32),
Quantity.from((tx as any).blockNumber),
Quantity.from((tx as any).transactionIndex)
);
return runTx.serializeForDb(blockHash, blockNumber, index);
};

public async getRaw(transactionHash: Buffer): Promise<Buffer> {
Expand Down
16 changes: 14 additions & 2 deletions src/chains/ethereum/ethereum/src/transaction-pool.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import Emittery, { Typed } from "emittery";
import Emittery from "emittery";
import Blockchain from "./blockchain";
import { Heap } from "@ganache/utils";
import { Data, Quantity, JsonRpcErrorCode, ACCOUNT_ZERO } from "@ganache/utils";
import {
GAS_LIMIT,
INTRINSIC_GAS_TOO_LOW,
NONCE_TOO_LOW,
CodedError,
UNDERPRICED,
REPLACED
Expand Down Expand Up @@ -146,6 +145,19 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
if (transactorNoncePromise) {
await transactorNoncePromise;
}
// if the user called sendTransaction or sendRawTransaction, effectiveGasPrice
// hasn't been set yet on the tx. calculating the effectiveGasPrice requires
// the block context, so we need to set it outside of the transaction. this
// value is updated in the miner as we're more sure of what block the tx will
// actually go on, but we still need to set it here to check for valid
// transaction replacements of same origin/nonce transactions
if (
!transaction.effectiveGasPrice &&
this.#blockchain.common.isActivatedEIP(1559)
) {
const baseFeePerGas = this.#blockchain.blocks.latest.header.baseFeePerGas;
transaction.updateEffectiveGasPrice(baseFeePerGas);
}
MicaiahReid marked this conversation as resolved.
Show resolved Hide resolved

// we should _probably_ cache `highestNonce`, but it's actually a really hard thing to cache as the current highest
// nonce might be invalidated (like if the sender doesn't have enough funds), so we'd have to go back to the previous
Expand Down
163 changes: 163 additions & 0 deletions src/chains/ethereum/ethereum/tests/api/eth/call.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import assert from "assert";
import EthereumProvider from "../../../src/provider";
import getProvider from "../../helpers/getProvider";
import compile from "../../helpers/compile";
import { join } from "path";
import { BUFFER_EMPTY, Quantity, RPCQUANTITY_EMPTY } from "@ganache/utils";
import { RETURN_TYPES, RuntimeError } from "@ganache/ethereum-utils";

const contract = compile(join(__dirname, "./contracts/EthCall.sol"), {
contractName: "EthCall"
});

describe("api", () => {
describe("eth", () => {
describe("call", () => {
let provider: EthereumProvider;
let from, to: string;
let contractAddress: string;
let tx: object;

before(async () => {
provider = await getProvider({ wallet: { deterministic: true } });
[from, to] = await provider.send("eth_accounts");

await provider.send("eth_subscribe", ["newHeads"]);
const contractHash = await provider.send("eth_sendTransaction", [
{
from,
data: contract.code,
gasLimit: "0xfffff"
}
]);
await provider.once("message");
const receipt = await provider.send("eth_getTransactionReceipt", [
contractHash
]);
contractAddress = receipt.contractAddress;

tx = {
from,
to: contractAddress,
data: "0x3fa4f245" // code for the "value" of the contract
};
});

after(async () => {
provider && (await provider.disconnect());
});

it("executes a message call", async () => {
const result = await provider.send("eth_call", [tx, "latest"]);
// gets the contract's "value", which should be 5
assert.strictEqual(Quantity.from(result).toNumber(), 5);
});

it("does not create a transaction on the chain", async () => {
const beforeCall = await provider.send("eth_getBlockByNumber", [
"latest"
]);
await provider.send("eth_call", [tx, "latest"]);
const afterCall = await provider.send("eth_getBlockByNumber", [
"latest"
]);
assert.strictEqual(beforeCall.number, afterCall.number);
});

it("allows legacy 'gasPrice' based transactions", async () => {
const tx = {
from,
to: contractAddress,
data: "0x3fa4f245",
gasPrice: "0x1"
};
const result = await provider.send("eth_call", [tx, "latest"]);
// we can still get the result when the gasPrice is set
assert.strictEqual(Quantity.from(result).toNumber(), 5);
});

it("allows eip-1559 fee market transactions", async () => {
const tx = {
from,
to: contractAddress,
data: "0x3fa4f245",
maxFeePerGas: "0xff",
maxPriorityFeePerGas: "0xff"
};

const result = await provider.send("eth_call", [tx, "latest"]);
// we can still get the result when the maxFeePerGas/maxPriorityFeePerGas are set
assert.strictEqual(Quantity.from(result).toNumber(), 5);
});

it("allows gas price to be omitted", async () => {
const result = await provider.send("eth_call", [tx, "latest"]);
// we can get the value if no gas info is given at all
assert.strictEqual(Quantity.from(result).toNumber(), 5);
});

it("rejects transactions that specify legacy and eip-1559 transaction fields", async () => {
const tx = {
from,
to: contractAddress,
data: "0x3fa4f245",
maxFeePerGas: "0xff",
maxPriorityFeePerGas: "0xff",
gasPrice: "0x1"
};
const ethCallProm = provider.send("eth_call", [tx, "latest"]);
await assert.rejects(
ethCallProm,
new Error(
"both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified"
),
"didn't reject transaction with both legacy and eip-1559 gas fields"
);
});

it("returns empty result for transactions with insufficient gas when vmErrorsOnRpcResponse is disabled", async () => {
const tx = {
from,
to: contractAddress,
data: "0x3fa4f245",
gasLimit: "0xf"
};
const result = await provider.send("eth_call", [tx, "latest"]);
// the vm errored, but since vmErrorsOnRpcResponse is disabled, we just
// set the result to 0x
assert.strictEqual(result, "0x");
});

it("rejects transactions with insufficient gas when vmErrorsOnRpcResponse is enabled", async () => {
const vmErrorsProvider = await getProvider({
wallet: { deterministic: true },
chain: { vmErrorsOnRPCResponse: true }
});
const tx = {
from,
input: contract.code,
gas: "0xf"
};
const ethCallProm = vmErrorsProvider.send("eth_call", [tx, "latest"]);
const result = {
execResult: {
exceptionError: { error: "out of gas" },
returnValue: BUFFER_EMPTY,
runState: { programCounter: 0 }
}
} as any;
// since vmErrorsOnRpcResponse is enabled, the vm error should propogate
// through to here
await assert.rejects(
ethCallProm,
new RuntimeError(
RPCQUANTITY_EMPTY,
result,
RETURN_TYPES.RETURN_VALUE
),
"didn't reject transaction with insufficient gas"
);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
pragma solidity ^0.7.4;

contract EthCall {
uint public value;

constructor() public payable {
value = 5;
}

}
31 changes: 23 additions & 8 deletions src/chains/ethereum/ethereum/tests/api/eth/eth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,17 +491,32 @@ describe("api", () => {

it("eth_getTransactionByHash", async () => {
await provider.send("eth_subscribe", ["newHeads"]);
const hash = await provider.send("eth_sendTransaction", [
{
from: accounts[0],
to: accounts[1],
value: "0x1"
}
]);
const txJson = {
type: "0x2",
chainId: "0x539",
nonce: "0x0",
from: accounts[0],
to: accounts[1],
value: "0x1",
maxPriorityFeePerGas: "0xf",
maxFeePerGas: "0xfffffffff",
gas: "0x15f90",
input: "0x01",
accessList: []
} as any;
const hash = await provider.send("eth_sendTransaction", [txJson]);
const _message = await provider.once("message");
// we want these values set for when we check against the return data,
// but they shouldn't be used in eth_sendTransaction, so we'll set them now
txJson.transactionIndex = "0x0";
txJson.gasPrice = "0x342770cf";

const tx = await provider.send("eth_getTransactionByHash", [hash]);
assert(tx.transactionIndex, "0x0");

// loop over all of the data we set to verify it matches
for (const [key, value] of Object.entries(txJson)) {
assert.deepStrictEqual(value, tx[key]);
}
});
});
});
Loading