Drop only invalid entries when reading banlist.json#22362
Drop only invalid entries when reading banlist.json#22362laanwj merged 1 commit intobitcoin:masterfrom
Conversation
|
Concept ACK |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
0075737 to
77335ff
Compare
|
Concept ACK. Still, a mistake in one entry of user input should not prevent the whole ban thing from working. |
8023243 to
fa7f08d
Compare
src/test/banman_tests.cpp
Outdated
There was a problem hiding this comment.
Dropping this seems to break the test, but I don't understand why. Perhaps a comment?
There was a problem hiding this comment.
Ban entries from the past (banned_until=778) will be dropped, unless the time is less than that.
|
ACK fa7f08ddad1c7741ead1589730b56262ee9aeb9c |
|
Rebased to fix silent and explicit merge conflict. Should be trivial to re-ACK |
|
ACK eeee738d1c4b584968b816309dff78c8011d34bf |
src/net_types.cpp
Outdated
There was a problem hiding this comment.
I think this error message needs to include the context (maybe "ban list entry" instead of "entry").
|
Code review ACK eeee738d1c4b584968b816309dff78c8011d34bf |
Currently all entries in the file are dropped. Fix that by only dropping the invalid ones
|
Thanks, should be trivial to re-review with: git range-diff bitcoin-core/master eeee738d1c faa6c3d44c --word-diff-regex=. |
|
Re-ACK faa6c3d |
faa6c3d net: Drop only invalid entries when reading banlist.json (MarcoFalke) Pull request description: All entries will be dropped when there is at least one invalid one in `banlist.json`. Fix this by only dropping invalid ones. Also suggested in bitcoin#20966 (comment) ACKs for top commit: laanwj: Re-ACK faa6c3d Tree-SHA512: 5a58e7f1dcabf78d0c65d8c6d5d997063af1efeaa50ca7730fc00056fda7e0061b6f7a38907ea045fe667c9f61d392e01e556b425a95e6b126e3c41cd33deb83
All entries will be dropped when there is at least one invalid one in
banlist.json. Fix this by only dropping invalid ones.Also suggested in #20966 (comment)