Skip to content

Commit

Permalink
Merge bitcoin#28176: tests: add coverage to feature_addrman.py
Browse files Browse the repository at this point in the history
380130d test: add coverage to feature_addrman.py (kevkevin)

Pull request description:

  I added two new tests that will cover the nNew and nTried tests which add coverage to the if block by checking values larger than our range since we only check for negative values now

  adding coverage to these lines
  https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L273
  https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L280

  our test seem to only cover the `nTried < 0` and `nNew < 0` scenarios

ACKs for top commit:
  ismaelsadeeq:
    ACK 380130d, code looks good to me 🍃 .
  0xB10C:
    Re-ACK 380130d

Tree-SHA512: a063bd9ca4d2d536a27c8c22a28fb13759a96f19cd8ba6cb8879cf7f65046d4ff6e8f70df17feaffd0d0d08ef914cb18a11258d313a4841c811a7e7ae4df6d5b
  • Loading branch information
fanquake committed Oct 2, 2023
2 parents 50f250a + 380130d commit 8909667
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
23 changes: 20 additions & 3 deletions test/functional/feature_addrman.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
import struct

from test_framework.messages import ser_uint256, hash256
from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE
from test_framework.p2p import MAGIC_BYTES
from test_framework.test_framework import BitcoinTestFramework
from test_framework.test_node import ErrorMatch
from test_framework.util import assert_equal


def serialize_addrman(
*,
format=1,
Expand Down Expand Up @@ -117,17 +117,34 @@ def run_test(self):

self.log.info("Check that corrupt addrman cannot be read (len_tried)")
self.stop_node(0)
max_len_tried = ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE
write_addrman(peers_dat, len_tried=-1)
self.nodes[0].assert_start_raises_init_error(
expected_msg=init_error("Corrupt AddrMan serialization: nTried=-1, should be in \\[0, 16384\\]:.*"),
expected_msg=init_error(f"Corrupt AddrMan serialization: nTried=-1, should be in \\[0, {max_len_tried}\\]:.*"),
match=ErrorMatch.FULL_REGEX,
)

self.log.info("Check that corrupt addrman cannot be read (large len_tried)")
write_addrman(peers_dat, len_tried=max_len_tried + 1)
self.nodes[0].assert_start_raises_init_error(
expected_msg=init_error(f"Corrupt AddrMan serialization: nTried={max_len_tried + 1}, should be in \\[0, {max_len_tried}\\]:.*"),
match=ErrorMatch.FULL_REGEX,
)

self.log.info("Check that corrupt addrman cannot be read (len_new)")
self.stop_node(0)
max_len_new = ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE
write_addrman(peers_dat, len_new=-1)
self.nodes[0].assert_start_raises_init_error(
expected_msg=init_error("Corrupt AddrMan serialization: nNew=-1, should be in \\[0, 65536\\]:.*"),
expected_msg=init_error(f"Corrupt AddrMan serialization: nNew=-1, should be in \\[0, {max_len_new}\\]:.*"),
match=ErrorMatch.FULL_REGEX,
)

self.log.info("Check that corrupt addrman cannot be read (large len_new)")
self.stop_node(0)
write_addrman(peers_dat, len_new=max_len_new + 1)
self.nodes[0].assert_start_raises_init_error(
expected_msg=init_error(f"Corrupt AddrMan serialization: nNew={max_len_new + 1}, should be in \\[0, {max_len_new}\\]:.*"),
match=ErrorMatch.FULL_REGEX,
)

Expand Down
5 changes: 5 additions & 0 deletions test/functional/test_framework/netutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
STATE_LISTEN = '0A'
# STATE_CLOSING = '0B'

# Address manager size constants as defined in addrman_impl.h
ADDRMAN_NEW_BUCKET_COUNT = 1 << 10
ADDRMAN_TRIED_BUCKET_COUNT = 1 << 8
ADDRMAN_BUCKET_SIZE = 1 << 6

def get_socket_inodes(pid):
'''
Get list of socket inodes for process pid.
Expand Down

0 comments on commit 8909667

Please sign in to comment.