-
Notifications
You must be signed in to change notification settings - Fork 38.7k
log: log wtxids when possible, add TXPACKAGES category #28364
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
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
instagibbs
left a comment
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.
ACK 17e327d95ae9e55353f94acee126b227cf906c3f
modulo that one missing log, for completeness
17e327d to
168db3b
Compare
|
reACK 168db3b |
- Whenever a tx is erased. Allows somebody to see which transactions have been erased due to expiry/overflow, not just how many. - Whenever a tx is added to a peer's workset. - AcceptToMemoryPool when a tx is accepted, mirroring the one logged for a tx received from a peer. This allows someone to see all of the transactions that are accepted to mempool just by looking for ATMP logs. - MEMPOOLREJ when a tx is rejected, mirroring the one logged for a tx received from a peer. This allows someone to see all of the transaction rejections by looking at MEMPOOLREJ logs.
This comment isn't in the right place, as detection of a tx in recent_rejects would cause the function to exit much earlier. Move the comment to the right place and tweak the first sentence for accuracy.
168db3b to
a3b55c9
Compare
|
reACK a3b55c9 |
brunoerg
left a comment
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.
crACK a3b55c9
Code looks great and m_recent_rejects check is done into AlreadyHaveTx so the local change makes sense for me. I just was in doubt if this comment could be in AlreadyHaveTx but seems ok to leave it in net_processing side.
mzumsande
left a comment
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.
Code review ACK a3b55c9
jonatack
left a comment
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.
Concept ACK, happy to see improved or more useful logging.
|
ACK a3b55c9 |
Summary: ``` - Additional orphan-related logging to make testing and debugging easier. - Add TXPACKAGES category for logging. - Move a nearby comment block that was in the wrong place. ``` Backport of [[bitcoin/bitcoin#28364 | core#28364]]. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D16318
This was taken from #28031 (see #27463 for project tracking).
TXPACKAGEScategory for logging.