backport: Merge bitcoin#21029, 21060, 19509, 21107, 25117 #5766
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_25
branch
2 times, most recently
from
58ee969
to
f7af474
Compare
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
changed the title
|
This pull request has conflicts, please rebase. |
|
This pull request has conflicts, please rebase. |
vijaydasmp
force-pushed
the
bp22_25
branch
3 times, most recently
from
0c134ec
to
cebe110
Compare
|
This pull request has conflicts, please rebase. |
|
This pull request has conflicts, please rebase. |
vijaydasmp
changed the title
vijaydasmp
changed the title
UdjinM6
requested changes
Feb 6, 2024
vijaydasmp
changed the title
vijaydasmp
changed the title
UdjinM6
reviewed
Feb 7, 2024
|
Hello @PastaPastaPasta , requesting review |
|
Hello @PastaPastaPasta , Please review |
PastaPastaPasta
approved these changes
Feb 14, 2024
…s" exists) 71430ae bitcoin-cli: Correct docs (no "generatenewaddress" exists) (Luke Dashjr) Pull request description: ACKs for top commit: jonatack: ACK 71430ae Tree-SHA512: 45ee7a51080598141fac4446563bdf99a59bfaa0ee00d9e092511087a968e8535d9208683ed589be1c975531be6d0319d33720f1f0dc1c3635c7d5ea6d726a41
572fd0f doc: More precise -debug and -debugexclude doc (wodry) Pull request description: I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category. This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent. ACKs for top commit: laanwj: ACK 572fd0f MarcoFalke: ACK 572fd0f theStack: ACK 572fd0f Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
bff7c66 Add documentation to contrib folder (Troy Giorshev) 381f77b Add Message Capture Test (Troy Giorshev) e4f378a Add capture parser (Troy Giorshev) 4d1a582 Call CaptureMessage at appropriate locations (Troy Giorshev) f2a77ff Add CaptureMessage (Troy Giorshev) dbf779d Clean PushMessage and ProcessMessages (Troy Giorshev) Pull request description: This PR introduces per-peer message capture into Bitcoin Core. 📓 ## Purpose The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?". ## Functionality When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue. The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so: ``` message_capture/203.0.113.7:56072/msgs_recv.dat message_capture/203.0.113.7:56072/msgs_sent.dat ``` Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option. ## Future Maintenance I sympathize greatly with anyone who says "the best code is no code". The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool. Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small. ## FAQ "Why not just use Wireshark" Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark. Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done. Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable. ACKs for top commit: MarcoFalke: re-ACK bff7c66 only some minor changes: 👚 jnewbery: utACK bff7c66 theStack: re-ACK bff7c66 Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
…annotations 9913419 test: remove type: comments in favour of actual annotations (fanquake) Pull request description: Now that we require Python 3.6+, we should be using variable type annotations directly rather than `# type:` comments. Also takes care of the discarded value issue in p2p_message_capture.py. See: https://github.com/bitcoin/bitcoin/pull/19509/files#r571674446. ACKs for top commit: MarcoFalke: review ACK 9913419 jnewbery: Code review ACK 9913419 Tree-SHA512: 63aba5eef6c1320578f66cf8a6d85ac9dbab9d30b0d21e6e966be8216e63606de12321320c2958c67933bf68d10f2e76e9c43928e5989614cea34dde4187aad8
PastaPastaPasta
force-pushed
the
bp22_25
branch
from
71bab44
to
639705e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.