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

fix(node): harden transaction inclusion logic to account for fee market #1239

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

raulk
Copy link
Contributor

@raulk raulk commented Dec 19, 2024

This PR improves correctness of tx inclusion logic on two fronts.

  1. Begins rejecting messages with invalid limits at CheckTx time:
    • Gas fee cap below minimal base fee.
    • Gas limit above maximum block gas limit.

Some type shenanigans via downcasts were necessary to deal with #1241. Tested by extending a materializer test.

  1. Exclude non-includable messages at proposal time.

    • I removed the MessagesSelector trait because it was unusable. The vector holding selectors cannot used boxed objects because the trait is not object-safe as it has methods with generic parameters. Also, there's no point over-engineering this prematurely.
    • Also removed the decreasing sorting by gas limit; we prefer to randomize for security (otherwise an attacker could drip brutally overestimated transactions just below the block gas limit to DoS the chain).
  2. Randomize message selection to prevent attackers from predicting or controlling message inclusion.


This change is Reviewable

@raulk raulk requested a review from a team as a code owner December 19, 2024 15:11
@raulk raulk marked this pull request as draft December 19, 2024 15:11
@raulk raulk marked this pull request as ready for review January 2, 2025 16:00
- gas fee cap below minimal base fee.
- gas limit above maximum block gas limit.
@raulk raulk force-pushed the raulk/fix/reject-below-base-fee branch from 05c6790 to cd07cec Compare January 2, 2025 16:35
@raulk raulk changed the title fix: reject block if it includes messages below the base fee. fix(node): reject block if it includes messages below the base fee. Jan 2, 2025
@raulk raulk requested a review from cryptoAtwill January 3, 2025 15:14
@raulk raulk changed the title fix(node): reject block if it includes messages below the base fee. fix(node): harden transaction inclusion logic to account for fee market Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

3 participants