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

Txpool: Enforce minimum gas fee on tx entry #803

Open
wants to merge 4 commits into
base: zkevm
Choose a base branch
from

Conversation

sieniven
Copy link

@sieniven sieniven commented Jul 17, 2024

Summary

Context

  • The issue with the current txpool with the zkevm node is that the pool is designed for post-london fork supporting EIP1559 transactions (baseFee subpool design)
  • However, zkevm node is currently set to pre-london fork, and all EIP1559 transactions are rejected form the mempool (
    isLondon := p.isLondon()
    if !isLondon && txn.Type == 0x2 {
    return UnsupportedTx
    }
    )
  • With the correct fixes that forkguards the base fee calculation for pre-london fork, we will need to enforce minimum fee tip on transactions entering the txpool

Copy link
Collaborator

@mandrigin mandrigin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick question -- will that produce an immediate error for user trying to use eth_sendRawTransaction?

Because the last thing we want is for a tx to be accepted and just disappear.

@sieniven
Copy link
Author

quick question -- will that produce an immediate error for user trying to use eth_sendRawTransaction?

Because the last thing we want is for a tx to be accepted and just disappear.

@mandrigin This PR enforces the correct behaviour which resolves this issue. More context: #774 (comment)

This PR rejects txs on the RPC end entirely on validateTx, and returns an txpool rejection error on the RPC.

// Drop non-local transactions under our own minimal accepted gas price or tip
if !isLocal && uint256.NewInt(p.cfg.MinFeeCap).Cmp(&txn.FeeCap) == 1 {
// Drop transactions under our minimal accepted gas price or tip
if uint256.NewInt(p.cfg.MinFeeCap).Cmp(&txn.FeeCap) == 1 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it break gasless (that come with 0 fee)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the spot. I think it might, let me resolve this.

Copy link
Collaborator

@mandrigin mandrigin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question about gasless -- it must keep working if allow-free-transactions is set to true

Copy link

sonarcloud bot commented Aug 21, 2024

Copy link
Author

@sieniven sieniven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mandrigin I am making some assumptions here, but it seems like the zkevm node on your side will only allow gasless txs if they were submitted locally. In this case this check is skipped entirely, since there isnt any other checks on gasless txs on the validate tx pipeline on tx entry into the pool. I have reverted the min gasprice check to only happen for non-local txs in commit - 3b33c6e

@V-Staykov V-Staykov linked an issue Sep 19, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Base fee calculation error on pre-london fork
2 participants