Page MenuHomePhabricator

[avalanche] Make sure the finalization status is maintained when a proof is promoted from dangling
ClosedPublic

Authored by Fabien on Oct 30 2023, 17:15.

Details

Summary

Dangling proofs can be promoted to the peer set if our peers have it as per the remote proofs algorithm. But if the proof was recently finalized, the finalization status would be erroneously lost. This diff fixes this bug and demonstrates it with a test (the last assert would fail on the finalized_proof_count without the fix).

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Oct 30 2023, 17:15
bytesofman added inline comments.
test/functional/abc_p2p_avalanche_remote_proofs.py
329 ↗(On Diff #42857)

(the last assert would fail on the finalized_proof_count without the fix).

This one?

365 ↗(On Diff #42857)

What does wait_until do -- if the condition here never occurs, does the test fail? Is there a timeout?

PiRK added inline comments.
src/avalanche/processor.cpp
167 ↗(On Diff #42857)

shouln't this log message be in the below if scope?

test/functional/abc_p2p_avalanche_remote_proofs.py
333 ↗(On Diff #42857)

assert bool_var (no need for is True)

src/avalanche/processor.cpp
167 ↗(On Diff #42857)

nevermind, i'm probably just confused (promoting != finalizing)

src/avalanche/processor.cpp
167 ↗(On Diff #42857)

Yes all the proofs in this loop are the ones promoted from the dangling pool to the valid pool, the message apply to any below case

test/functional/abc_p2p_avalanche_remote_proofs.py
365 ↗(On Diff #42857)

It times out, default is 60s but there are rules to adapt automatically (e.g. it's extended when there are sanitizers running as they slow down the node)

367 ↗(On Diff #42857)

@bytesofman This one

Simplify the assert is True

This revision is now accepted and ready to land.Oct 30 2023, 18:56

Failed tests logs:

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

------- Stdout: -------
2023-10-30T18:58:23.507000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20231030_185739/abc_p2p_compactproofs_11
2023-10-30T18:58:23.945000Z TestFramework (INFO): Check we send a getavaproofs message to our avalanche outbound peers
2023-10-30T18:58:25.837000Z TestFramework (INFO): Check we send periodic getavaproofs message to some of our peers
2023-10-30T18:58:25.888000Z TestFramework (INFO): After the first avaproofs has been received, all the peers are requested periodically
2023-10-30T18:58:26.153000Z TestFramework (INFO): Empty avaproofs will not trigger any request
2023-10-30T18:58:26.155000Z TestFramework (INFO): Check we send a getavaproofs message to our manually connected peers that support avalanche
2023-10-30T18:58:27.735000Z TestFramework (INFO): Check the node responds to getavaproofs messages
2023-10-30T18:58:41.967000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
    def avapeer_connected():
        node_list = []
        try:
            node_list = node.getavalanchepeerinfo(proofid_hex)[0]["node_list"]
        except BaseException:
            pass

        return n.nodeid in node_list
'''
2023-10-30T18:58:42.694000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 142, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_compactproofs.py", line 736, in run_test
    self.test_respond_getavaproofs()
  File "/work/test/functional/abc_p2p_compactproofs.py", line 288, in test_respond_getavaproofs
    p = get_ava_p2p_interface(self, node)
  File "/work/test/functional/test_framework/avatools.py", line 411, in get_ava_p2p_interface
    wait_until_helper(avapeer_connected, timeout=5)
  File "/work/test/functional/test_framework/util.py", line 298, in wait_until_helper
    raise AssertionError(
AssertionError: Predicate ''''
    def avapeer_connected():
        node_list = []
        try:
            node_list = node.getavalanchepeerinfo(proofid_hex)[0]["node_list"]
        except BaseException:
            pass

        return n.nodeid in node_list
''' not true after 5.0 seconds
2023-10-30T18:58:42.750000Z TestFramework (INFO): Stopping nodes
2023-10-30T18:58:43.002000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20231030_185739/abc_p2p_compactproofs_11
2023-10-30T18:58:43.002000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20231030_185739/abc_p2p_compactproofs_11/test_framework.log
2023-10-30T18:58:43.003000Z TestFramework (ERROR): 
2023-10-30T18:58:43.003000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20231030_185739/abc_p2p_compactproofs_11' to consolidate all logs
2023-10-30T18:58:43.004000Z TestFramework (ERROR): 
2023-10-30T18:58:43.004000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-10-30T18:58:43.004000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-10-30T18:58:43.005000Z TestFramework (ERROR):

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

This revision was landed with ongoing or failed builds.Oct 30 2023, 19:23
This revision was automatically updated to reflect the committed changes.