Page MenuHomePhabricator

[avalanche] Properly remove pending node when possible
ClosedPublic

Authored by Fabien on Apr 4 2022, 16:05.

Details

Summary

Calling removeNode attempt to remove from the pending node set only after it checked the node is in the regulat set, which is obviously an impossible case.

This diff restores the expected behavior and remove the node appropriately. It closes the gap in the unit tests that failed to catch this issue.

Test Plan
ninja check-avalanche

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_fix_pendingnode_removal
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18723
Build 37237: Build Diffbuild-diff · build-without-wallet · lint-circular-dependencies · build-debug · build-clang-tidy · build-clang
Build 37236: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Apr 4 2022, 16:05

Failed tests logs:

====== Bitcoin ABC functional tests: feature_config_args.py ======

------- Stdout: -------
2022-04-04T16:09:21.459000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220404_160804/feature_config_args_32
2022-04-04T16:09:22.920000Z TestFramework (INFO): Test config args logging
2022-04-04T16:09:23.441000Z TestFramework (INFO): Test seed peers
2022-04-04T16:09:30.042000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 136, in main
    self.run_test()
  File "/work/test/functional/feature_config_args.py", line 247, in run_test
    self.test_seed_peers()
  File "/work/test/functional/feature_config_args.py", line 242, in test_seed_peers
    self.nodes[0].setmocktime(start + 65)
  File "/usr/lib/python3.7/contextlib.py", line 119, in __exit__
    next(self.gen)
  File "/work/test/functional/test_framework/test_node.py", line 535, in assert_debug_log
    str(expected_msgs), print_log))
  File "/work/test/functional/test_framework/test_node.py", line 213, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['Adding fixed seeds as 60 seconds have passed and addrman is empty']" does not partially match log:

 - 2022-04-04T16:09:28.034307Z (mocktime: 2022-04-04T16:09:27Z) [../../src/httpserver.cpp:252] [http_request_cb] Received a POST request for / from 127.0.0.1:41696
 - 2022-04-04T16:09:28.034402Z (mocktime: 2022-04-04T16:09:27Z) [../../src/rpc/request.cpp:183] [parse] ThreadRPCServer method=setmocktime user=__cookie__
 - 2022-04-04T16:09:28.034524Z (mocktime: 2022-04-04T16:10:32Z) [../../src/./util/system.h:468] [TraceThread] opencon thread start


2022-04-04T16:09:30.093000Z TestFramework (INFO): Stopping nodes
2022-04-04T16:09:30.345000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220404_160804/feature_config_args_32
2022-04-04T16:09:30.345000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220404_160804/feature_config_args_32/test_framework.log
2022-04-04T16:09:30.345000Z TestFramework (ERROR): 
2022-04-04T16:09:30.345000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220404_160804/feature_config_args_32' to consolidate all logs
2022-04-04T16:09:30.345000Z TestFramework (ERROR): 
2022-04-04T16:09:30.345000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2022-04-04T16:09:30.345000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2022-04-04T16:09:30.345000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: feature_config_args.py

tyler-smith added a subscriber: tyler-smith.

The code makes sense and seems to be working. I can't tell why the build is failing it it appears to be unrelated to this change.

This revision is now accepted and ready to land.Apr 6 2022, 06:29
This revision was landed with ongoing or failed builds.Apr 6 2022, 12:19
This revision was automatically updated to reflect the committed changes.