55556a64a8 test: Remove struct import from messages.py (MarcoFalke)
fa3fa86dda scripted-diff: test: Use int from_bytes and to_bytes over struct packing (MarcoFalke)
fafc0d68ee test: Use int from_bytes and to_bytes over struct packing (MarcoFalke)
fa3886b7c6 test: Treat msg_version.relay as unsigned (MarcoFalke)
Pull request description:
`struct` has many issues in messages.py:
* For unpacking, it requires to specify the length a second time, even when it is already clear from the `f.read(num_bytes)` context.
* For unpacking, it is designed to support a long format string and returning a tuple of many values. However, except for 3 instances in `messages.py`, usually only a single value is unpacked and all those cases require an `[0]` access.
* For packing and unpacking of a single value, the format string consists of characters that may be confusing and may need to be looked up in the documentation, as opposed to using easy to understand self-documenting code.
I presume the above issues lead to accidentally treat `msg_version.relay` as a "signed bool", which is fine, but confusing.
Fix all issues by using the built-in `int` helpers `to_bytes` and `from_bytes` via a scripted diff.
Review notes:
* `struct.unpack` throws an error if the number of bytes passed is incorrect. `int.from_bytes` doesn't know about "missing" bytes and treats an empty byte array as `int(0)`. "Extraneous" bytes should never happen, because all `read` calls are limited in this file. If it is important to keep this error behavior, a helper `int_from_stream(stream, num_bytes, bytes, byteorder, *, **kwargs)` can be added, which checks the number of bytes read from the stream.
* For `struct.pack` and `int.to_bytes` the error behavior is the same, although the error messages are not identical.
ACKs for top commit:
stickies-v:
ACK 55556a64a8
theStack:
re-ACK 55556a64a8
Tree-SHA512: 1cef8cdfd763fb424ed4b8be850a834b83fd0ef47fbea626a29784eb4f4832d44e42c4fe05b298b6070a933ef278b0222289a9955a97c86707e091e20bbb247a
78c06a38c4