diff options
| author | MarcoFalke <falke.marco@gmail.com> | 2022-03-08 16:48:18 +0100 |
|---|---|---|
| committer | MarcoFalke <falke.marco@gmail.com> | 2022-03-08 16:48:22 +0100 |
| commit | b07fdd7f9eef5f1498af056f3ef11e44795578c0 (patch) | |
| tree | ecee71c792ca2353f9b4e4ae9ed1c485b89fd58e /test | |
| parent | 64a4483dc6798a9a7d8327d320a17b3c4d7d4ee0 (diff) | |
| parent | fa097d074bc1afcc2a52976796bb618f7c6a68b3 (diff) | |
| download | bitcoin-b07fdd7f9eef5f1498af056f3ef11e44795578c0.tar.xz | |
Merge bitcoin/bitcoin#24312: addrman: Log too low compat value
fa097d074bc1afcc2a52976796bb618f7c6a68b3 addrman: Log too low compat value (MarcoFalke)
Pull request description:
Before this patch, when writing a negative `lowest_compatible` value, it would be read as a positive value. For example `-32` will be read as `224`. There is generally nothing wrong with that. Though, similarly there shouldn't be anything wrong with refusing to read a negative value. I find the code after this patch more logical than before. Also, this allows dropping a file-wide sanitizer suppression.
In practice none of this should ever happen. Bitcoin Core would never write a negative `lowest_compatible` in normal operation, unless the file storage is later corrupted by external influence.
ACKs for top commit:
mzumsande:
re-ACK fa097d074bc1afcc2a52976796bb618f7c6a68b3
Tree-SHA512: 9aae7b8fe666f52f667f149667025e0160cef1a793cc4d392e36608f65c2bee8096da429235118f40a3368f327aabe30f3732ae78c5874648ea6f423f2687b65
Diffstat (limited to 'test')
| -rwxr-xr-x | test/functional/feature_addrman.py | 11 | ||||
| -rw-r--r-- | test/sanitizer_suppressions/ubsan | 1 |
2 files changed, 11 insertions, 1 deletions
diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 0fdefaa9c3..5e49d0214a 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -68,6 +68,17 @@ class AddrmanTest(BitcoinTestFramework): self.start_node(0, extra_args=["-checkaddrman=1"]) assert_equal(self.nodes[0].getnodeaddresses(), []) + self.log.info("Check that addrman with negative lowest_compatible cannot be read") + self.stop_node(0) + write_addrman(peers_dat, lowest_compatible=-32) + self.nodes[0].assert_start_raises_init_error( + expected_msg=init_error( + "Corrupted addrman database: The compat value \\(0\\) is lower " + "than the expected minimum value 32.: (.+)" + ), + match=ErrorMatch.FULL_REGEX, + ) + self.log.info("Check that addrman from future is overwritten with new addrman") self.stop_node(0) write_addrman(peers_dat, lowest_compatible=111) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index b06dd253be..e6cfe5f81a 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -62,7 +62,6 @@ implicit-integer-sign-change:script/bitcoinconsensus.cpp implicit-integer-sign-change:script/interpreter.cpp implicit-integer-sign-change:serialize.h implicit-integer-sign-change:txmempool.cpp -implicit-signed-integer-truncation:addrman.cpp implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/ shift-base:arith_uint256.cpp |