Skip to content

Conversation

@theStack
Copy link
Contributor

The script message-capture-parser.py currently doesn't support parsing the BIP157 messages getcfilters, getcfheaders and getcfcheckpt, e.g.

$ ./contrib/message-capture/message-capture-parser.py msgs_recv.dat
...
    WARNING - Unrecognized message type b'getcfcheckpt' in /home/thestack/bitcoin/msgs_recv.dat
...

This PR fixes this by adding the missing message type mappings to the MESSAGEMAP in the test framework and add default-constructors for the corresponding msg_... classes.

Without the second commit, the following error message would occur:

  File "/home/thestack/bitcoin/./contrib/message-capture/message-capture-parser.py", line 141, in process_file
    msg = MESSAGEMAP[msgtype]()
TypeError: __init__() missing 2 required positional arguments: 'filter_type' and 'stop_hash'

theStack added 2 commits May 13, 2022 13:37
…ility)

In order to deserialize received or read messages via lookup in
MESSAGEMAP (e.g.: `t = MESSAGEMAP[msgtype]()`), the messages must have a
default constructor, i.e. there needs to be the possibility to
initialize them with zero arguments.
@theStack
Copy link
Contributor Author

Can be reproduced with the following input file:

$ xxd -p msgs_recv.dat
544fe2d1e1de050076657273696f6e00000000006f000000801101000900
000000000000812d7e620000000001000000000000000000000000000000
0000ffff7f000001301b010000000000000000000000000000000000ffff
000000000000e49e8d5430100b09192f707974686f6e2d7032702d746573
7465723a302e302e332fffffffff01d49ee2d1e1de050077747869647265
6c61790000000000001ca1e2d1e1de050076657261636b00000000000000
0000009fb7e2d1e1de050067657461646472000000000000000000c8bfe2
d1e1de0500706f6e670000000000000000080000006d37706c0c0fcf9a5a
11e3d1e1de050070696e6700000000000000000800000001000000000000
006f1ae4d1e1de05006765746366636865636b7074210000000006226e46
111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f1120
e4d1e1de050070696e670000000000000000080000000200000000000000

(Save the hex-lines into a file and convert it into the dump via $ xxd -p -r hexdump.txt > msgs_recv.dat).

@DrahtBot DrahtBot added the Tests label May 13, 2022
Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

tACK 5dc6d92

Reproduced error on master. Checked that adding default values to the constructor was required.

@maflcko maflcko merged commit e016c00 into bitcoin:master May 18, 2022
@theStack theStack deleted the 202205-test-support_BIP157_deser_via_messagemap branch May 18, 2022 17:49
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
…SSAGEMAP)

5dc6d92 test: make BIP157 messages default-constructible (MESSAGEMAP compatibility) (Sebastian Falbesoner)
71e4cfe test: p2p: add missing BIP157 message types to MESSAGEMAP (Sebastian Falbesoner)

Pull request description:

  The script [message-capture-parser.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/message-capture/message-capture-parser.py) currently doesn't support parsing the BIP157 messages `getcfilters`, `getcfheaders` and `getcfcheckpt`, e.g.
  ```
  $ ./contrib/message-capture/message-capture-parser.py msgs_recv.dat
  ...
      WARNING - Unrecognized message type b'getcfcheckpt' in /home/thestack/bitcoin/msgs_recv.dat
  ...
  ```

  This PR fixes this by adding the missing message type mappings to the [`MESSAGEMAP`](https://github.com/bitcoin/bitcoin/blob/225e5b57b2ee2bc1acd7f09c89ccccc15ef8c85f/test/functional/test_framework/p2p.py#L95-L127) in the test framework and add default-constructors for the corresponding `msg_`... classes.

  Without the second commit, the following error message would occur:
  ```
    File "/home/thestack/bitcoin/./contrib/message-capture/message-capture-parser.py", line 141, in process_file
      msg = MESSAGEMAP[msgtype]()
  TypeError: __init__() missing 2 required positional arguments: 'filter_type' and 'stop_hash'
  ```

ACKs for top commit:
  dunxen:
    tACK [5dc6d92](bitcoin@5dc6d92)

Tree-SHA512: d656c4d38a856373f01d7c293ae7d2b27378a9fc248048ebf2a64725ef8b498b3ddf4f420704abdb20d0c68ca548f1777602c5e73b66821a20c97ae618f1d63f
@bitcoin bitcoin locked and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants