4d2fa97031a6f31da984d4c2c105447ed692c6ed [addrman] Clean up ctor (John Newbery)
7e6e65918f75211b517fc887f5d90df8edd52ced [addrman] inline Clear() into CAddrMan ctor (John Newbery)
406be5ff9699874dc1d38d11f036e33cbdb820c9 [addrman] Remove all public uses of CAddrMan.Clear() from the tests (John Newbery)
ed9ba8af08f857bda3ce2f77413317374c22d7b4 [tests] Remove CAddrMan.Clear() call from CAddrDB::Read() (John Newbery)
e8e7392311edf44278d76743bebe902d4ac94662 [addrman] Don't call Clear() if parsing peers.dat fails (John Newbery)
181a1207ba6bd179d181f3e2534ef8676565ce72 [addrman] Move peers.dat parsing to init.cpp (John Newbery)
Pull request description:
`CAddrMan::Clear()` exists to reset the internal state of `CAddrMan`. It's currently used in two places:
- on startup, if deserializing peers.dat fails, `Clear()` is called to reset to an empty addrman
- in tests, `Clear()` is called to reset the addrman for more tests
In both cases, we can simply destruct the `CAddrMan` and construct a new, empty addrman. That approach is safer - it's possible that `Clear()` could 'reset' the addrman to a state that's not equivalent to a freshly constructed addrman (one actual example of this is that `Clear()` does not clear the `m_tried_collisions` set). On the other hand, if we destruct and then construct a fresh addrman, we're guaranteed that the new object is empty.
This wasn't possible when addrman was initially implemented, since it was a global, and so it would only be destructed on shutdown. However, addrman is now owned by `node.context`, so we have control over its destruction/construction.
ACKs for top commit:
laanwj:
Code review ACK 4d2fa97031a6f31da984d4c2c105447ed692c6ed
vasild:
ACK 4d2fa97031a6f31da984d4c2c105447ed692c6ed
Zero-1729:
crACK 4d2fa97031a6f31da984d4c2c105447ed692c6ed
Tree-SHA512: f715bf2cbff4f8c3a9dbc613f8c7f11846b065d6807faf3c7d346a0b0b29cbe7ce1dc0509465c2c9b88a8ad55299c9182ea53f5f743e47502a69a0f375e09408
602c8eb8f0