addrman: Fix format string in deserialize error #22879
Merged
Conversation
This file contains hidden or 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
2222d91 to
fa17c5f
Compare
|
Context for other reviewers: c42f/tinyformat#82 |
Also add a regression test.
fa17c5f to
fab0b55
Compare
I think the changes here make sense even without the issue (and whether it is a bug or not). |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. |
jonatack
reviewed
Sep 6, 2021
There was a problem hiding this comment.
ACK fab0b55 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected
2021-09-06T13:36:53.543000Z TestFramework (INFO): Check that addrman from future cannot be read
2021-09-06T13:36:55.715000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/jon/projects/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/home/jon/projects/bitcoin/bitcoin/test/functional/feature_addrman.py", line 61, in run_test
self.start_node(0)
File "/usr/lib/python3.9/contextlib.py", line 126, in __exit__
next(self.gen)
File "/home/jon/projects/bitcoin/bitcoin/test/functional/test_framework/test_node.py", line 400, in assert_debug_log
self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
File "/home/jon/projects/bitcoin/bitcoin/test/functional/test_framework/test_node.py", line 166, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: 1. It is compatible with formats >=111, but the maximum supported by this version of Bitcoin Core is 3.', 'Recreating peers.dat']" does not partially match log:
...
- 2021-09-06T13:36:53.840640Z [init] [util/system.h:51] [error] ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: \x01. It is compatible with formats >=111, but the maximum supported by this version of Bitcoin Core is 3.: iostream error
| self.stop_node(0) | ||
| write_addrman(peers_dat, lowest_compatible=111) | ||
| with self.nodes[0].assert_debug_log([ | ||
| f'ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: 1. It is compatible with formats >=111, but the maximum supported by this version of {self.config["environment"]["PACKAGE_NAME"]} is 3.', |
There was a problem hiding this comment.
nit, line length
- f'ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: 1. It is compatible with formats >=111, but the maximum supported by this version of {self.config["environment"]["PACKAGE_NAME"]} is 3.',
+ "ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of "
+ "addrman database: 1. It is compatible with formats >=111, but the maximum "
+ f"supported by this version of {self.config['environment']['PACKAGE_NAME']} is 3.",There was a problem hiding this comment.
Thanks, will fix on next force push, if there is one
Right. Didn't seem worth it to open a separate pull for this "trivial style" fix. I found it while working on this pull, so at least it is loosely related 😅 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 11, 2021
fab0b55 addrman: Fix format string in deserialize error (MarcoFalke) facce4c test: Remove useless overwrite (MarcoFalke) Pull request description: The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later). Work around the behaviour change in compilers by pinning the underlying type of the format arguments. Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later). ACKs for top commit: jonatack: ACK fab0b55 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected mzumsande: ACK fab0b55 Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Oct 10, 2021
Also add a regression test. Github-Pull: bitcoin#22879 Rebased-From: fab0b55
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Dec 14, 2021
Also add a regression test. Github-Pull: bitcoin#22879 Rebased-From: fab0b55
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later).
Work around the behaviour change in compilers by pinning the underlying type of the format arguments.
Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later).