Make test DoS_mapOrphans deterministic
AbandonedPublic
Actions

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

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Summary

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:

cmake -GNinja .. -DENABLE_COVERAGE=ON
ninja test_bitcoin
../contrib/devtools/test_deterministic_coverage.sh 5

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr16878

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16533
Build 32928: Build Diff	lint-circular-dependencies · build-clang · build-diff · build-without-wallet · build-debug · build-clang-tidy
Build 32927: arc lint + arc unit

Event Timeline

PiRK created this revision.Sep 2 2021, 08:54

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 2 2021, 08:54

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

teamcity edited the summary of this revision. (Show Details)Sep 2 2021, 08:54

Harbormaster completed remote builds in B16516: Diff 29735.Sep 2 2021, 09:02

Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

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

------- 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/test_framework.py", line 127, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 310, in run_test
    self.test_proof_relay()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 195, in test_proof_relay
    self.sync_proofs()
  File "/work/test/functional/test_framework/test_framework.py", 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/combine_logs.py '/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): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-09-02T08:57:44.813000Z TestFramework (ERROR):

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

Test suite is broken.

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

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

PiRK edited the summary of this revision. (Show Details)Sep 2 2021, 14:36

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

Harbormaster completed remote builds in B16529: Diff 29761.Sep 2 2021, 15:07

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