Page MenuHomePhabricator

Make test DoS_mapOrphans deterministic
Changes PlannedPublic

Authored by PiRK on Sep 2 2021, 08:54.


Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task

The RandomOrphan function and the function ecdsa_signature_parse_der_lax
in pubkey.cpp were causing non-deterministic test coverage.

Force seed in the beginning of the test to make it deterministic.
The seed is selected carefully so that all branches of the function
ecdsa_signature_parse_der_lax are executed. Prior to this fix, the test
was exhibiting non-deterministic coverage since none of the ECDSA
signatures that were generated during the test had leading zeroes in
either R, S, or both, resulting in some branches of said function not
being executed. The seed ensures that both conditions are hit.

Removed denialofservice_tests test entry from the list of non-deterministic
tests in the coverage script.

This is a backport of core#16878

Depends on D10030

Test Plan

5 runs of coverage testing to detect non-determinism:

ninja test_bitcoin
../contrib/devtools/ 5

Event Timeline

PiRK requested review of this revision.Sep 2 2021, 08:54

Failed tests logs:

====== Bitcoin ABC functional tests: ======

------- Stdout: -------
2021-09-02T08:56:15.483000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210902_085613/abc_p2p_proof_inventory_1
2021-09-02T08:56:17.134000Z TestFramework (INFO): Test sending a proof to our peers
2021-09-02T08:56:25.265000Z TestFramework (INFO): Test that we don't send the same inv several times
2021-09-02T08:56:27.348000Z TestFramework (INFO): Test a peer is created on proof reception
2021-09-02T08:56:27.571000Z TestFramework (INFO): Test receiving a proof with missing utxo is orphaned
2021-09-02T08:56:43.467000Z TestFramework (INFO): Nodes should eventually get the proof from their peer
2021-09-02T08:57:44.454000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/", line 127, in main
  File "/work/test/functional/", line 310, in run_test
  File "/work/test/functional/", line 195, in test_proof_relay
  File "/work/test/functional/test_framework/", line 598, in sync_proofs
    "".join("\n  {!r}".format(m) for m in nodes_proofs),
AssertionError: Proofs sync timed out after 60s:
  {86673552667144960821180270182930488354574360048056424164474128394676370479795, 1071494625972861108302275591791667205878014819203852579343650234449198321030, 70539085136772750248149448726828305746036568039574916191913862117229161555044}
  {86673552667144960821180270182930488354574360048056424164474128394676370479795, 1071494625972861108302275591791667205878014819203852579343650234449198321030, 70539085136772750248149448726828305746036568039574916191913862117229161555044}
  {86673552667144960821180270182930488354574360048056424164474128394676370479795, 1071494625972861108302275591791667205878014819203852579343650234449198321030, 70539085136772750248149448726828305746036568039574916191913862117229161555044}
  {49357428675336966640306988750810744970724397195565995089372818783743066328008, 86673552667144960821180270182930488354574360048056424164474128394676370479795, 70539085136772750248149448726828305746036568039574916191913862117229161555044, 67905616073175015349207207432815567458172380676709443880243830079445802282458}
  {49357428675336966640306988750810744970724397195565995089372818783743066328008, 86673552667144960821180270182930488354574360048056424164474128394676370479795, 67905616073175015349207207432815567458172380676709443880243830079445802282458, 70539085136772750248149448726828305746036568039574916191913862117229161555044}
2021-09-02T08:57:44.505000Z TestFramework (INFO): Stopping nodes
2021-09-02T08:57:44.812000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210902_085613/abc_p2p_proof_inventory_1
2021-09-02T08:57:44.812000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210902_085613/abc_p2p_proof_inventory_1/test_framework.log
2021-09-02T08:57:44.812000Z TestFramework (ERROR): 
2021-09-02T08:57:44.812000Z TestFramework (ERROR): Hint: Call /work/test/functional/ '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210902_085613/abc_p2p_proof_inventory_1' to consolidate all logs
2021-09-02T08:57:44.812000Z TestFramework (ERROR): 
2021-09-02T08:57:44.812000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-09-02T08:57:44.812000Z TestFramework (ERROR):
2021-09-02T08:57:44.813000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests:

deadalnix requested changes to this revision.Sep 2 2021, 12:09
deadalnix added a subscriber: deadalnix.

Test suite is broken.

This revision now requires changes to proceed.Sep 2 2021, 12:09

I'll keep investigating the CI test failure, but it is clearly unrelated to this diff.

deadalnix requested changes to this revision.Sep 2 2021, 14:45

It seems that this comes after core added some tooling related to determinism in tests. Why is that out of order? Is there a plan to get that as well?

This revision now requires changes to proceed.Sep 2 2021, 14:45
PiRK planned changes to this revision.Sep 2 2021, 16:11

I'm having a look at how to backport . It will take me some time, I'm running into difficulties while running src/test/test_bitcoin through that script.

PiRK edited the summary of this revision. (Show Details)
PiRK edited the test plan for this revision. (Show Details)

rebase on D10030 and run the deterministic coverage script as a test plan

PiRK planned changes to this revision.Sep 3 2021, 09:19

More work to be done on D10030

PiRK added a task: Restricted Maniphest Task.Sep 3 2021, 14:25