fix: reject transaction if maxFeePerGas is less than blockBaseFeePerGas of the pending block

[jeffsmale90]

Fixes #2176

[davidmurdoch]

Can you rebase this on current develop?

[MicaiahReid]

So with further testing I discovered that transactions with too small gas price were accepted if the gas limit is specified. I opened an issue with Geth, and they pretty much said it's expected behavior. Here is a summary of how they handle these cases and their reasoning:

Gas Limit	Gas Price	Result	Notes
Unspecified	Unspecified	No error	Geth chooses a good gas price, then uses that price to estimate gas limit.
Specified	< baseFeePerGas	No error	Geth blindly accepts the gas price because the transaction has a gas limit.
Unspecified	< baseFeePerGas	Error: maxFeePerGas is less than block base fee.	Because no gas limit was specified, Geth needs to estimate it. To estimate it, it runs the transaction against the next block. Because the gas price is too low, it's rejected.

I think the key difference between us and Geth in this case is that our eth_estimateGas function does not enforce a baseFeePerGas. This can be seen when we make the RuntimeBlock in eth_estimateGas:

const block = new RuntimeBlock(
  Quantity.from((parentHeader.number.toBigInt() || 0n) + 1n),
  parentHeader.parentHash,
  parentHeader.miner,
  tx.gas.toBuffer(),
  parentHeader.gasUsed.toBuffer(),
  parentHeader.timestamp,
  options.miner.difficulty,
  parentHeader.totalDifficulty,
  0n // no baseFeePerGas for estimates
);

So I think rather than this current change, we may just want to pass in the next block's baseFeePerGas rather than 0n. Thoughts? @davidmurdoch @jeffsmale90

[MicaiahReid]

Upon further inspection, it doesn't look like we call eth_estimateGas as liberally as Geth does. We only estimate gas for transactions that omit gasLimit if their --miner.defaultTransactionGasLimit="estimate", so my suggested fix wouldn't work in all cases.

Regardless, the current implementation now rejects the transaction in more cases than Geth. In my eyes, we could:

1. Match Geth exactly by setting the baseFeePerGas in the eth_estimateGas function as mentioned above, and checking that the gas price is greater than the baseFeePerGas in the autofillDefaultTransactionValues branch that uses the default gas limit.
2. Add a --chain.mode="geth" option that allows a user to use ganache in a more strict mode (following geth's rules) and a more lenient mode (we don't ever error for gas being too low).
3. Leave ganache as is to be more lenient and, in our eyes, intuitive.

[jeffsmale90]

@MicaiahReid @davidmurdoch I think the best way forward is to discard this change, and consider adding a strict "Geth" mode as @MicaiahReid recommended where we pass the actual next block baseFeePerGas into vm.runTx (instead of 0).

What do you guys think?

[MicaiahReid]

@jeffsmale90 I'm on board for that!
I think in all the actions would be:

• make an issue for a general Geth compatibility mode feature
• update In Geth chain-mode, logic to accept/reject transactions based on gas price/limit should match Geth #2176 with details on exactly how Geth does it
• mark In Geth chain-mode, logic to accept/reject transactions based on gas price/limit should match Geth #2176 as blocked by the issue you make for the Geth compatibility mode feature
• (optionally) make a label that we can apply to those two issues and any future geth compatibility mode features

@davidmurdoch thoughts?

[davidmurdoch]

@MicaiahReid That sounds like a great plan to me!

[jeffsmale90]

Closing this PR, after following the actions @MicaiahReid listed above.