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

Transactions with maxFeePerGas or gasPrice less than baseFeePerGas rejected only when gas is missing #24661

Closed
MicaiahReid opened this issue Apr 7, 2022 · 12 comments

Comments

@MicaiahReid
Copy link

Thanks for all the work you do! Hopefully this issue is clear.

System information

Geth version: 1.10.18-unstable, also tested with 1.10.15-stable
OS & Version: Linux Ubuntu 20.04
Commit hash : (if develop) 763b3f8d1f93b57795dd33b35e8b97ef9b9da99f

Expected behaviour

When sending a transaction with maxFeePerGas / gasPrice < baseFeePerGas and omitting gas, error in a way that points to the problem of the missing gas field.

Or don't require a gas field to be specified in cases where maxFeePerGas or gasPrice are lower than baseFeePerGas.

Actual behaviour

Sending a transaction with no maxFeePerGas / gasPrice and no gas field, the transaction is accepted.

Sending a transaction with maxFeePerGas / gasPrice < baseFeePerGas and no gas field, the transaction is rejected with an error suggesting that the effective gas price being lower than baseFeePerGas is the reason for the failure.

Sending a transaction with maxFeePerGas / gasPrice < baseFeePerGas and the gas field specified, the transaction is accepted.

Steps to reproduce the behaviour

const [from, to] = eth.accounts;

// fails with error below
eth.sendTransaction({from, to, gasPrice: "0x1"});
// Error: err: max fee per gas less than block base fee: 
// address 0x9ccab4d0Af2363835bEdc09b66Ac9130635e63f7, maxFeePerGas: 1 baseFee: 765625000 (supplied gas 6004776)

// tx successfully added to pending queue
eth.sendTransaction({from, to, gasPrice: "0x1", gas: 21000}); 

// fails with error below
eth.sendTransaction({from, to, maxFeePerGas: "0x1", maxPriorityFeePerGas: "0x0"});
//Error: err: max fee per gas less than block base fee: 
// address 0x9ccab4d0Af2363835bEdc09b66Ac9130635e63f7, maxFeePerGas: 1 baseFee: 765625000 (supplied gas 6004776)

// tx successfully added to pending queue
eth.sendTransaction({from, to, maxFeePerGas: "0x1", maxPriorityFeePerGas: "0x0", gas: 21000}); 

// tx successfully added to pending queue
eth.sendTransaction({from, to}); 
@karalabe
Copy link
Member

I kind of thins this works as expected, though maybe a bit unintuitive:

  • If you don't specify a gas limit for the transaction, it will be automatically estimated (since we can't just use an arbitrary amout; and erroring out would be bad UX). However, in order to estimate the gas, we need to actually execute the transaction, so the gas fields need to be valid in the context of the current block.
  • If you do specify a gas limit, then we can just blindly accept your transaction and store it locally.
  • If you don't supply eithee the gas price or gas limit, then first we pick a good gas price; and then having a good price we can correctly estimate. So that case will work.

So, all in all the question I think is whether we should reject a small gas price for all transactions or not. I think way back we did that, but then it may not allow you to deliberately send transactions with lower prices than current network climate. That would be bad too.

TL;DR I kind of think it's correct the way it is.

@MicaiahReid
Copy link
Author

@karalabe Thanks for the response.

However, in order to estimate the gas, we need to actually execute the transaction, so the gas fields need to be valid in the context of the current block.

If a user wants to estimate the gas usage of a transaction, why would the gas price need to be relevant? Wouldn't it be most intuitive to ignore gas prices for estimates of gas usage?

@rjl493456442
Copy link
Member

@MicaiahReid GasPrice can be used to check if the sender has enough funds to cover the cost.

@holiman
Copy link
Contributor

holiman commented Apr 28, 2022

why would the gas price need to be relevant? Wouldn't it be most intuitive to ignore gas prices for estimates of gas usage?

You're kind of right, intuitively. However, if we want to simulate a transaction in a block, the gasPrice is relevant, because it's "visible" on-chain, and it also affects the balance which is also visible on-chain, and also needs to be correct related to the basefee.

So it's not directly relevant, but it still needs to be set correctly even for estimation purposes.

@holiman
Copy link
Contributor

holiman commented Apr 28, 2022

It's interesting that we do allow a tx which can never ever be included in a block:

> eth.sendTransaction({from, to, maxFeePerGas: "0x1", maxPriorityFeePerGas: "0x0", gas: 21000});
INFO [04-28|10:56:18.604] Setting new local account                address=0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33
INFO [04-28|10:56:18.604] Submitted transaction                    hash=0x6ded9c3b9c4c32463f35304d0ee9a8fddc30f2c0859caff51dc14f4ce27a2efc from=0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33 nonce=0 recipient=0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33 value=0
"0x6ded9c3b9c4c32463f35304d0ee9a8fddc30f2c0859caff51dc14f4ce27a2efc"
> INFO [04-28|10:56:18.607] Commit new sealing work                  number=1 sealhash=3bcfb9..245d81 uncles=0 txs=0 gas=0 fees=0 elapsed="309.082µs"
WARN [04-28|10:56:18.607] Block sealing failed                     err="sealing paused while waiting for transactions"

> txpool
{
  content: {
    pending: {
      0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33: {
        0: {...}
      }
    },
    queued: {}
  },
  inspect: {
    pending: {
      0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33: {
        0: "0x1309c1ece82D0433EecB3a35DE2772aa0C11Ca33: 0 wei + 21000 gas × 1 wei"
      }
    },
    queued: {}
  },
  status: {
    pending: 1,
    queued: 0
  },
  contentFrom: function(),
  getContent: function(callback),
  getInspect: function(callback),
  getStatus: function(callback)

I guess we allow local transactions even when their gas price is below the theoretical lowest base price per gas (which is 7 iirc)

@fjl
Copy link
Contributor

fjl commented Apr 28, 2022

While we agree this issue is unfortunate, it's a consequence of how transactions are handled:

  • When gas is not specified, we need to estimate the gas by EXECUTING the tx. A transaction with maxFeePerGas < baseFeePerGas cannot be executed, and produces the error max fee per gas less than block base fee.
  • When gas is specified, we DO NOT EXECUTE the transaction, and thus the baseFee issue is not noticed. It could be argued that this is correct because the baseFee can still change and could work when the tx will be included.

@fjl fjl closed this as completed Apr 28, 2022
@MicaiahReid
Copy link
Author

@fjl Yes, I understand that when gas is not specified it needs to be estimated by executing the transaction. In Ganache we execute this transaction against a "fake" block with baseFeePerGas=0 so that the gas can be estimated regardless of what block it will be on. This allows us to behave consistently in what fields are required.

The block the transaction will eventually be included in will rarely be the same block that is used to estimate the transaction's gas, so why should that block be used to enforce a gas price?

When gas is specified, we DO NOT EXECUTE the transaction, and thus the baseFee issue is not noticed. It could be argued that this is correct because the baseFee can still change and could work when the tx will be included.

Even when gas isn't specified, the baseFee can change and could work when the transaction will be included. This is why I think it would make more sense to run the gas estimate against a block with the price disregarded.

@RippleLeaf
Copy link

@fjl Yes, I understand that when gas is not specified it needs to be estimated by executing the transaction. In Ganache we execute this transaction against a "fake" block with baseFeePerGas=0 so that the gas can be estimated regardless of what block it will be on. This allows us to behave consistently in what fields are required.

Hi MicaiahReid, this is kind off the topic but may I know how to set baseFeePerGas=0? I tried setting it in truffle-config.js but it does not work, and I couldn't find any online document about how to set it. Thank you very much!

@MicaiahReid
Copy link
Author

@RippleLeaf I think it may be more appropriate to open up a discussion question over at Truffle so I can help you with that 😄

@staminna
Copy link

staminna commented Sep 14, 2022

@RippleLeaf Did you manage to open a ticket over at Truffle as MicaiahRedi suggested?
Thanks I must followup aswell

@RippleLeaf
Copy link

@RippleLeaf Did you manage to open a ticket over at Truffle as MicaiahRedi suggested? Thanks I must followup aswell

Hi @staminna, yes I opened a discussion thread here. Hope this would be helpful!

@karalabe
Copy link
Member

karalabe commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants
@fjl @karalabe @holiman @staminna @rjl493456442 @MicaiahReid @RippleLeaf and others