Skip to content

Refactor: Use single lookup for transaction validation #17712

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Refactor: Use single lookup for transaction validation #17712

wants to merge 2 commits into from
+3 −2

Conversation

malik672
Copy link
Contributor

@malik672 malik672 commented Aug 2, 2025

would call it optimize but doubt this check is hardly ever hit tbh

@malik672 malik672 requested a review from mattsse as a code owner August 2, 2025 13:13
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Aug 2, 2025
mattsse
mattsse requested changes Aug 2, 2025
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

actually, let's just convert this to a debug_assert instead

we did the same for the pending pool, and these checks are mostly safeguards but since this is in the hotpath we should just make this a debug_assert

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Aug 2, 2025
@malik672
Copy link
Contributor Author

malik672 commented Aug 2, 2025

okay, what's the command for the linter ?

@malik672
Copy link
Contributor Author

malik672 commented Aug 2, 2025

actually, let's just convert this to a debug_assert instead

we did the same for the pending pool, and these checks are mostly safeguards but since this is in the hotpath we should just make this a debug_assert

can the contains() function be removed

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Aug 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
S-stale This issue/PR is stale and will close with no further activity
Projects
Reth Tracker
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@malik672 @mattsse