-
Notifications
You must be signed in to change notification settings - Fork 108
chore(l1): move WARN
logs to DEBUG
when a tx is not able to be added to the mempool
#4841
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
base: main
Are you sure you want to change the base?
Conversation
WARN
logs to DEBUG
when a tx is not able to be added to the mempool
match e { | ||
// Some of the more common mempool errors are logged at debug level, we might want to move all of them to debug in the future | ||
ethrex_blockchain::error::MempoolError::NonceTooLow | ||
| ethrex_blockchain::error::MempoolError::NotEnoughBalance | ||
| ethrex_blockchain::error::MempoolError::UnderpricedReplacement => { | ||
log_peer_debug( | ||
&state.node, | ||
&format!("Error adding transaction common: {e}"), | ||
); | ||
continue; | ||
} | ||
_ => { | ||
log_peer_warn( | ||
&state.node, | ||
&format!("Error adding transaction: {e}"), | ||
); | ||
continue; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we agree we can move them all to debug, otherwise this tackles 99% of all logs that are seen in the current sepolia and mainnet executions
PS: the errors aren't matched rn, Im looking into it
Lines of code reportTotal lines added: Detailed view
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses log spam by downgrading common mempool-related transaction errors from WARN to DEBUG level. The changes reduce noise in production logs when transactions fail to be added to the mempool for common, expected reasons.
- Move three specific mempool errors (
NonceTooLow
,NotEnoughBalance
,UnderpricedReplacement
) from WARN to DEBUG logging - Add client name information to peer logging functions for better debugging context
- Implement
client_name()
method to extract client information from node version strings
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
crates/networking/p2p/types.rs | Adds client_name() method to extract client name from node version |
crates/networking/p2p/rlpx/utils.rs | Updates logging functions to include client name in log format |
crates/networking/p2p/rlpx/connection/server.rs | Changes specific mempool errors from WARN to DEBUG level logging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Motivation
We have some
WARN
logs spamming the node while on Sepolia or Mainnet, we want to remove them given taht they don't require attention.Description
This PR move the most common of these errors (the one that are spammed during most of mainnet executions) to debug:
Attempted to replace a pooled transaction with an underpriced transaction
Nonce for account too low
Account does not have enough balance to cover the tx cost
We could move them all to debug instead but wanted some feedback first. It also adds information to the peer logs related to the client. We checked that these logs doesn't come from the same source of clients, here is a summary of a node in mainnet running for a couple of hours
This is how a typical mainnet log looked like before:
We may want to further investigate if they are a constant influx of new transactions that we need to discard or if we are recieving the same transactions multiple times, for that I created #4842