Skip to content

Commit

Permalink
Make test DoS_mapOrphans deterministic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davereikher committed Jul 21, 2020
1 parent 090d877 commit 4455949
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
1 change: 0 additions & 1 deletion contrib/devtools/test_deterministic_coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ GCOV_EXECUTABLE="gcov"
NON_DETERMINISTIC_TESTS=(
"blockfilter_index_tests/blockfilter_index_initial_sync" # src/checkqueue.h: In CCheckQueue::Loop(): while (queue.empty()) { ... }
"coinselector_tests/knapsack_solver_test" # coinselector_tests.cpp: if (equal_sets(setCoinsRet, setCoinsRet2))
"denialofservice_tests/DoS_mapOrphans" # denialofservice_tests.cpp: it = mapOrphanTransactions.lower_bound(InsecureRand256());
"fs_tests/fsbridge_fstream" # deterministic test failure?
"miner_tests/CreateNewBlock_validity" # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
"scheduler_tests/manythreads" # scheduler.cpp: CScheduler::serviceQueue()
Expand Down
20 changes: 19 additions & 1 deletion src/test/denialofservice_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

// Unit tests for denial-of-service detection/prevention code

#include <arith_uint256.h>
#include <banman.h>
#include <chainparams.h>
#include <net.h>
#include <net_processing.h>
#include <pubkey.h>
#include <script/sign.h>
#include <script/signingprovider.h>
#include <script/standard.h>
Expand Down Expand Up @@ -314,10 +316,26 @@ static CTransactionRef RandomOrphan()
return it->second.tx;
}

static void MakeNewKeyWithFastRandomContext(CKey& key)
{
std::vector<unsigned char> keydata;
keydata = g_insecure_rand_ctx.randbytes(32);
key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true);
assert(key.IsValid());
}

BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
{
// This test had non-deterministic coverage due to
// randomly selected seeds.
// This seed is chosen so that all branches of the function
// ecdsa_signature_parse_der_lax are executed during this test.
// Specifically branches that run only when an ECDSA
// signature's R and S values have leading zeros.
g_insecure_rand_ctx = FastRandomContext(ArithToUint256(arith_uint256(33)));

CKey key;
key.MakeNewKey(true);
MakeNewKeyWithFastRandomContext(key);
FillableSigningProvider keystore;
BOOST_CHECK(keystore.AddKey(key));

Expand Down

0 comments on commit 4455949

Please sign in to comment.