Add test for addrman consistency check on restart with asmap

PR #22697 introduced a reproducible issue in commit 181a1207 that causes the
addrman tried table to fail consistency checks and significantly lose peer
entries when the `-asmap` configuration option is used.

The issue occurs on bitcoind restart due to an initialization order change
in `src/init.cpp` in that commit whereby CAddrman asmap is set after
deserializing `peers.dat`, rather than before.

Issue reported on the `#bitcoin-core-dev` IRC channel starting at
https://www.erisian.com.au/bitcoin-core-dev/log-2021-08-23.html#l-263.

```
addrman lost 22813 new and 2 tried addresses due to collisions or invalid addresses
ADDRMAN CONSISTENCY CHECK FAILED!!! err=-17
bitcoind: ./addrman.h:707: void CAddrMan::Check() const: Assertion `false' failed. Aborted
```

How to reproduce:
- `git checkout 181a1207` and recompile
- launch bitcoind with `-asmap` and `-checkaddrman=1` config options
- restart bitcoind
- bitcoind aborts on second call to `CAddrMan::Check()`

This commit adds a regression test to reproduce the case; it passes or fails
with the same error.

Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
This commit is contained in:
Jon Atack 2021-08-30 12:11:47 +02:00
parent 869f136816
commit cdaab90662
No known key found for this signature in database
GPG Key ID: 4F5721B3D0E3921D

View File

@ -14,9 +14,11 @@ Verify node behaviour and debug log when launching bitcoind in these cases:
4. `bitcoind -asmap/-asmap=` with no file specified, using the default asmap 4. `bitcoind -asmap/-asmap=` with no file specified, using the default asmap
5. `bitcoind -asmap` with no file specified and a missing default asmap file 5. `bitcoind -asmap` restart with an addrman containing new and tried entries
6. `bitcoind -asmap` with an empty (unparsable) default asmap file 6. `bitcoind -asmap` with no file specified and a missing default asmap file
7. `bitcoind -asmap` with an empty (unparsable) default asmap file
The tests are order-independent. The tests are order-independent.
@ -37,6 +39,12 @@ def expected_messages(filename):
class AsmapTest(BitcoinTestFramework): class AsmapTest(BitcoinTestFramework):
def set_test_params(self): def set_test_params(self):
self.num_nodes = 1 self.num_nodes = 1
self.extra_args = [["-checkaddrman=1"]] # Do addrman checks on all operations.
def fill_addrman(self, node_id):
"""Add 2 tried addresses to the addrman, followed by 2 new addresses."""
for addr, tried in [[0, True], [1, True], [2, False], [3, False]]:
self.nodes[node_id].addpeeraddress(address=f"101.{addr}.0.0", tried=tried, port=8333)
def test_without_asmap_arg(self): def test_without_asmap_arg(self):
self.log.info('Test bitcoind with no -asmap arg passed') self.log.info('Test bitcoind with no -asmap arg passed')
@ -72,6 +80,22 @@ class AsmapTest(BitcoinTestFramework):
self.start_node(0, [arg]) self.start_node(0, [arg])
os.remove(self.default_asmap) os.remove(self.default_asmap)
def test_asmap_interaction_with_addrman_containing_entries(self):
self.log.info("Test bitcoind -asmap restart with addrman containing new and tried entries")
self.stop_node(0)
shutil.copyfile(self.asmap_raw, self.default_asmap)
self.start_node(0, ["-asmap", "-checkaddrman=1"])
self.fill_addrman(node_id=0)
self.restart_node(0, ["-asmap", "-checkaddrman=1"])
with self.node.assert_debug_log(
expected_msgs=[
"Addrman checks started: new 2, tried 2, total 4",
"Addrman checks completed successfully",
]
):
self.node.getnodeaddresses() # getnodeaddresses re-runs the addrman checks
os.remove(self.default_asmap)
def test_default_asmap_with_missing_file(self): def test_default_asmap_with_missing_file(self):
self.log.info('Test bitcoind -asmap with missing default map file') self.log.info('Test bitcoind -asmap with missing default map file')
self.stop_node(0) self.stop_node(0)
@ -97,6 +121,7 @@ class AsmapTest(BitcoinTestFramework):
self.test_asmap_with_absolute_path() self.test_asmap_with_absolute_path()
self.test_asmap_with_relative_path() self.test_asmap_with_relative_path()
self.test_default_asmap() self.test_default_asmap()
self.test_asmap_interaction_with_addrman_containing_entries()
self.test_default_asmap_with_missing_file() self.test_default_asmap_with_missing_file()
self.test_empty_asmap() self.test_empty_asmap()