Page MenuHomePhabricator

[avalanche] Use the ProofComparator for sets and maps
ClosedPublic

Authored by Fabien on May 13 2022, 23:27.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC6f21aa633d70: [avalanche] Use the ProofComparator for sets and maps
Summary

There are places where the comparator is not important as long as the ordering is consistent, and the default comparator is used. This assumes that the key actually has a default comparator. This diff adds a simple comparator and makes explicit use of it where possible so updating the key type will not be an issue as long as it is managed by the comparator. There is no change in behavior.

Test Plan
ninja check-avalanche

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_explicit_comparator
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19029
Build 37814: Build Difflint-circular-dependencies · build-without-wallet · build-diff · build-clang-tidy · build-clang · build-debug
Build 37813: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.May 13 2022, 23:27
Fabien updated this revision to Diff 33512.

Add missing include

ProofComparator does complex computation, when it was before just comparing pointers. This solves a real problem, but the solution is not good.

deadalnix requested changes to this revision.May 13 2022, 23:34
This revision now requires changes to proceed.May 13 2022, 23:34

Failed tests logs:

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

------- Stdout: -------
2022-05-13T23:33:00.604000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220513_233221/abc_p2p_getavaaddr_16
2022-05-13T23:33:02.889000Z TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:21024 due to [Errno 104] Connection reset by peer
2022-05-13T23:33:09.343000Z TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:21024 due to [Errno 104] Connection reset by peer
2022-05-13T23:33:12.631000Z TestFramework (INFO): Check we send a getavaaddr message to our avalanche outbound peers
2022-05-13T23:33:15.088000Z TestFramework (INFO): Check we send a getavaaddr message to our manually connected peers that support avalanche
2022-05-13T23:33:20.621000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
            lambda: p.message_count.get(
                "getavaaddr", 0) > 1, timeout=5)
'''
2022-05-13T23:33:20.621000Z 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/abc_p2p_getavaaddr.py", line 386, in run_test
    self.getavaaddr_manual_test()
  File "/work/test/functional/abc_p2p_getavaaddr.py", line 297, in getavaaddr_manual_test
    "getavaaddr", 0) > 1, timeout=5)
  File "/work/test/functional/test_framework/test_framework.py", line 677, in wait_until
    timeout_factor=self.options.timeout_factor)
  File "/work/test/functional/test_framework/util.py", line 284, in wait_until_helper
    "Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
            lambda: p.message_count.get(
                "getavaaddr", 0) > 1, timeout=5)
''' not true after 5.0 seconds
2022-05-13T23:33:20.672000Z TestFramework (INFO): Stopping nodes
2022-05-13T23:33:20.925000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220513_233221/abc_p2p_getavaaddr_16
2022-05-13T23:33:20.925000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220513_233221/abc_p2p_getavaaddr_16/test_framework.log
2022-05-13T23:33:20.925000Z TestFramework (ERROR): 
2022-05-13T23:33:20.926000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20220513_233221/abc_p2p_getavaaddr_16' to consolidate all logs
2022-05-13T23:33:20.926000Z TestFramework (ERROR): 
2022-05-13T23:33:20.926000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2022-05-13T23:33:20.926000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2022-05-13T23:33:20.926000Z TestFramework (ERROR):

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

Fabien edited the summary of this revision. (Show Details)

Use a simple comparator, by id

deadalnix requested changes to this revision.May 14 2022, 01:32

No, compare the address.

This revision now requires changes to proceed.May 14 2022, 01:32
This revision is now accepted and ready to land.May 19 2022, 17:22