highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-26 22:08:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#22831: test: add addpeeraddress "tried", test addrman checks on restart with asmap

Browse Source 
cdaab90662 Add test for addrman consistency check on restart with asmap (Jon Atack)
869f136816 Add test for rpc addpeeraddress with "tried" argument (Jon Atack)
ef242f5213 Allow passing "tried" to rpc addpeeraddress to call CAddrMan::Good() (Jon Atack)

Pull request description:

  This pull adds a `tried` argument to RPC addpeeraddress and a regression test for the recent addrman/asmap changes and issue.

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

  The issue occurs upon 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`, build, and launch bitcoind with the `-asmap` and `-checkaddrman=1` configuration options enabled
  - restart bitcoind
  - bitcoind aborts on the second call to the addrman consistency checks in `CAddrMan::Check()`

  How to test this pull:

  - `git checkout 181a1207`, cherry pick the first commit of this branch, build, git checkout this branch, run `test/functional/rpc_net.py`, which should pass, and then run `test/functional/feature_asmap.py`, which should fail with the following output:

  ```
  AssertionError: Unexpected stderr bitcoind: ./addrman.h:739: void CAddrMan::Check() const: Assertion `false' failed.
  ```

ACKs for top commit:
  jnewbery:
    utACK cdaab90662
  mzumsande:
    re-ACK cdaab90662 (based on code review of diff to d586817)
  vasild:
    ACK cdaab90662

Tree-SHA512: 0251a18fea629b62486fc907d7ab0e96c6df6fadb9e4d62cff018bc681afb6ac31e0e7258809c0a88f91e4a36c4fb0b16ed294ce47ef30585217de89c3342399
This commit is contained in:
merge-script
2021-09-21 09:34:28 +02:00
parent 0c1a39390f cdaab90662
commit 223ad2fd0d
4 changed files with 64 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
test/functional/rpc_net.py
View File

@@ -239,7 +239,16 @@ class NetTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, "Network not recognized: Foo", self.nodes[0].getnodeaddresses, 1, "Foo")
def test_addpeeraddress(self):
"""RPC addpeeraddress sets the source address equal to the destination address.
If an address with the same /16 as an existing new entry is passed, it will be
placed in the same new bucket and have a 1/64 chance of the bucket positions
colliding (depending on the value of nKey in the addrman), in which case the
new address won't be added. The probability of collision can be reduced to
1/2^16 = 1/65536 by using an address from a different /16. We avoid this here
by first testing adding a tried table entry before testing adding a new table one.
"""
self.log.info("Test addpeeraddress")
self.restart_node(1, ["-checkaddrman=1"])
node = self.nodes[1]
self.log.debug("Test that addpeerinfo is a hidden RPC")
@@ -251,17 +260,25 @@ class NetTest(BitcoinTestFramework):
assert_equal(node.addpeeraddress(address="", port=8333), {"success": False})
assert_equal(node.getnodeaddresses(count=0), [])
self.log.debug("Test that adding a valid address succeeds")
assert_equal(node.addpeeraddress(address="1.2.3.4", port=8333), {"success": True})
addrs = node.getnodeaddresses(count=0)
assert_equal(len(addrs), 1)
assert_equal(addrs[0]["address"], "1.2.3.4")
assert_equal(addrs[0]["port"], 8333)
self.log.debug("Test that adding a valid address to the tried table succeeds")
assert_equal(node.addpeeraddress(address="1.2.3.4", tried=True, port=8333), {"success": True})
with node.assert_debug_log(expected_msgs=["Addrman checks started: new 0, tried 1, total 1"]):
addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks
assert_equal(len(addrs), 1)
assert_equal(addrs[0]["address"], "1.2.3.4")
assert_equal(addrs[0]["port"], 8333)
self.log.debug("Test that adding the same address again when already present fails")
assert_equal(node.addpeeraddress(address="1.2.3.4", port=8333), {"success": False})
self.log.debug("Test that adding an already-present tried address to the new and tried tables fails")
for value in [True, False]:
assert_equal(node.addpeeraddress(address="1.2.3.4", tried=value, port=8333), {"success": False})
assert_equal(len(node.getnodeaddresses(count=0)), 1)
self.log.debug("Test that adding a second address, this time to the new table, succeeds")
assert_equal(node.addpeeraddress(address="2.0.0.0", port=8333), {"success": True})
with node.assert_debug_log(expected_msgs=["Addrman checks started: new 1, tried 1, total 2"]):
addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks
assert_equal(len(addrs), 2)
if __name__ == '__main__':
NetTest().main()