Fix non-deterministic coverage of test DoS_mapOrphans #16878

davereikher · 2019-09-16T05:20:27Z

This pull request proposes a solution to make the test DoS_mapOrphans in denialofservice_tests.cpp have deterministic coverage.

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

In the former, if a random orphan was selected the index of which is bigger than the max. orphan index in mapOrphanTransactions, the last orphan was returned from RandomOrphan. If the random number generated was never large enough, this condition would not be fulfilled and the corresponding branch wouldn't run. The proposed solution is to force one of the 50 dependant orphans to depend on the last orphan in mapOrphanTransactions using the newly introduced function OrphanByIndex (and passing it a large uint256), forcing this branch to run at least once.

In the latter, if values for ECDSA R or S (or both) had no leading zeros, some code would not be executed. The solution was to find a constant signature that would be comprised of R and S values with leading zeros and calling CPubKey::Verify at the end of the test with this signature forcing this code to always run at least once at the end even if it hadn't throughout the test.

To test that the coverage is (at least highly likely) deterministic, I ran

contrib/devtools/test_deterministic_coverage.sh denialofservice_tests/DoS_mapOrphans 1000

and the result was deterministic coverage across 1000 runs.

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

DrahtBot · 2019-09-16T06:05:05Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

practicalswift · 2019-09-16T10:13:28Z

Concept ACK

Nice work! If you have the time: consider tackling the remaining test_deterministic_coverage.sh suppressions in follow-up PRs. Deterministic line coverage would be great to have! :)

practicalswift · 2019-09-16T10:15:45Z

src/test/denialofservice_tests.cpp

+        if (0 == i) {
+            // This block makes sure that the condition "if (it == mapOrphanTransactions.end())" in OrphanByIndex() gets called at least once.
+            // Otherwise test coverage is non-deterministic.
+            txPrev = OrphanByIndex(ArithToUint256(arith_uint256(LARGE_NUMBER)));


Could use std::numeric_limits<uint64_t>::max() instead of LARGE_NUMBER? :)

practicalswift · 2019-09-16T10:17:27Z

src/test/denialofservice_tests.cpp

@@ -391,7 +418,14 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
    // ... and 50 that depend on other orphans:
    for (int i = 0; i < 50; i++)
    {
-        CTransactionRef txPrev = RandomOrphan();
+        CTransactionRef txPrev;
+        if (0 == i) {


Nit: Could avoid non-repo-idiomatic yoda notation :)

davereikher · 2019-09-16T13:20:02Z

Concept ACK

Nice work! If you have the time: consider tackling the remaining test_deterministic_coverage.sh suppressions in follow-up PRs. Deterministic line coverage would be great to have! :)

Thanks for the review! I addressed your remarks in ffd3953. I indeed plan to tackle those in my free time.

8000

maflcko · 2019-09-16T13:24:56Z

src/test/denialofservice_tests.cpp

+    hash.SetHex(std::string("6af516422fef8a745aff6acdcc84076c77fb2ecd72bd5711df301230ac58fdd5"));
+
+    coverage_pubkey.Verify(hash, std::vector<unsigned char>(vch_sig_str.begin(), vch_sig_str.end()));
+}


Looks like the wrong location for this. I fail to understand what CPubKey has to do with the orphan map or even DoS?!

Thanks for the review! While trying to understand the non-determinism in the coverage, I used the script contrib/devtools/test_deterministic_coverage.sh, which uses gcovr to show the diff in which lines were and which ones were not executed between two runs with different coverage.

Over multiple runs of the script, the only two places where the coverage was non-deterministic were:

the function RandomOrphan in denialofservice_tests.cpp

lines 135-136, 147-148 in pubkey.cpp, in the function ecdsa_signature_parse_der_lax.

The latter function is called implicitly from the test DoS_mapOrphans via SignSignature (SignSignature => ProduceSignature => VerifyScript => EvalScript => CheckSig => VerifySignature => Verify => ecdsa_signature_parse_der_lax). The private key which generates the signatures in the test DoS_mapOrphans is randomly generated, so the R and S values of the ECDSA signatures for the inputs of the dependant orphans most of the time do not have leading zeros. The lines 135-136, 147-148 run only when there are leading zeroes in R, S, respectively. This results in those lines sometimes running and sometimes not.

My proposed solution is to create a constant signature with leading zeroes in both R and S and force the function ecdsa_signature_parse_der_lax to run on that signature by calling CPubkey::Verify on that signature and a corresponding pubkey and hash. This forces the lines 135-136, 147-148 in pubkey.cpp to always run at least once, resulting in deterministic test coverage.

I see. A smaller alternative would be to set the key to a constant?

That could be nice actually if it's acceptable. I didn't want to do it initially because I didn't want to modify the test too much and restricted myself to 'parasitic' changes.
On the other hand - if I'm not mistaken, this will not work directly, since, if we have transaction B which is using as an input an output from transaction A, part of the data used to calculate the hash of that input for B (this hash is then signed to produce R and S) is the hash of the transaction A, which contains a random element (the initial non-dependant 50 orphans are created with tx.vin[0].prevout.hash = InsecureRand256()), so R and S would again be random.

I guess I could use an engineered, non-random value for tx.vin[0].prevout.hash for one of the orphans along with an engineered fixed key. But then again, this test would loose the fuzzing feature of making a different key every run.

What do you think?

InsecureRand256 is an alias for g_insecure_rand_ctx.rand256(), so it can be made deterministic with a call to SeedInsecureRand(/* deterministic */ true).

this test would loose the fuzzing feature of making a different key every run.

This "fuzzing feature" is useless, as a failure could not be reproduced (when it occurs) and thus not debugged, nor fixed.

When fuzzing is desired, the seed would have to be written to a file or stdout.

I agree that fuzz tests should be separate and I wasn't familiar with src/test/fuzz. Thanks for pointing me there. However, reading through #16320, there are some comments about the positive value of non-determinism in tests (I was trying to understand if the direction is to completely eliminate non-determinism). I think that an effort should be made when writing new tests towards determinism and separating the fuzzy parts into the fuzz framework, but since this test already has non-determinism, I think I will stick with writing the seed to stdout. Separating the fuzzy from the non-fuzzy parts should perhaps be a part of another pull request.
That brings the question - there are a lot of tests that use random values. I completely agree that randomness in tests is practically useless unless the seed is known. What do you think about setting and writing the seed to stdout as part of the BasicTestingSetup fixture so that seeds for all tests are known? This would also prevent code duplication when multiple tests require this.

We already use BOOST_TEST_RANDOM https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/rt_param_reference/random.html, which is logged. So might as well use that as the seed for our test rng?

Sounds good. Do you suggest seeding the rng with BOOST_TEST_RANDOM in BasicTestingSetup or only in this test for now? I'm thinking the latter, since otherwise it changes the behaviour of all tests that use random numbers when running them locally and it would require adjusting the test execution scripts, as now the seed must be provided externally. For example, test_deterministic_coverage.sh will always show deterministic coverage when run locally unless we provide a different value of BOOST_TEST_RANDOM every time it's run. I feel handling those issues would be out of the scope of this pull request and may be done separately?

test_deterministic_coverage.sh will always show deterministic coverage when run locally unless we provide a different value of BOOST_TEST_RANDOM every time it's run. I feel handling those issues would be out of the scope of this pull request and may be done separately?

I think this is the way the script should work (report non-determinism even when the rng seed is pinned to a constant). As suggested by you this should be done in a separate pull request. Are you interested in working on this as well? Otherwise I will create an issue for others to pick up.

Yes, that sounds interesting, since IMO recording the seed for any test that uses random values is important. I'll check whether pinning the seed still results in non determinism between runs and based on that I'll see what can be done about it and about setting the seed to BOOST_TEST_RANDOM in BasicTestingSetup.
For now, I guess setting the seed to BOOST_TEST_RANDOM just inside this test would be enough?

davereikher · 2019-09-20T11:48:50Z

Could somebody explain why it's not allowed to include boost/test/unit_test_parameters.hpp?

A new Boost dependency in the form of "boost/test/unit_test_parameters.hpp" appears to have been introduced:

src/test/denialofservice_tests.cpp:#include <boost/test/unit_test_parameters.hpp>

^---- failure generated from test/lint/lint-includes.sh

I need it to access the BOOST_TEST_RANDOM parameter from within a test. Is there a better way to do it?

maflcko · 2019-09-20T12:05:19Z

I guess, you'd have to add it like this:

diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh
index d27e45a23f..c9e1d57ee1 100755
--- a/test/lint/lint-includes.sh
+++ b/test/lint/lint-includes.sh
@@ -68,6 +68,7 @@ EXPECTED_BOOST_INCLUDES=(
     boost/signals2/last_value.hpp
     boost/signals2/signal.hpp
     boost/test/unit_test.hpp
+    boost/test/unit_test_parameters.hpp
     boost/thread.hpp
     boost/thread/condition_variable.hpp
     boost/thread/mutex.hpp

src/test/denialofservice_tests.cpp

src/test/setup_common.h

davereikher · 2019-09-22T07:17:44Z

@MarcoFalke I see that the changes I made regarding setting the seed to BOOST_TEST_RANDOM are causing some trouble - some Travis builds fail while succeeding for me locally so I'm not sure how to debug that as well as the fact that the boost interface might change and I also feel that this is going a bit out of scope for this pull request which aims to make DoS_mapOrphans have deterministic coverage.
Would it be ok if I leave the test with a random, unknown seed for now (that was the case before anyway) and just leave the changes that make the coverage deterministic for this test? I will then attempt to tackle the problem of setting the seed to BOOST_TEST_RANDOM in BasicTestingSetup in a separate pull request?
Overall, I think this would still add positive value, as this test would have deterministic coverage, as opposed to before, with all else being equal.

practicalswift · 2019-09-22T19:24:09Z

ACK ffd3953 -- diff looks correct

davereikher · 2019-09-24T12:00:59Z

How can I remove the "Waiting for author" label?

maflcko · 2019-10-03T19:45:04Z

Have you seen #16978, which should fix the reproducibility issue

maflcko · 2019-10-03T19:46:58Z

src/test/denialofservice_tests.cpp

+    hash.SetHex(std::string("6af516422fef8a745aff6acdcc84076c77fb2ecd72bd5711df301230ac58fdd5"));
+
+    coverage_pubkey.Verify(hash, std::vector<unsigned char>(vch_sig_str.begin(), vch_sig_str.end()));
+}


Still don't like this, maybe it can be removed after #16878 (comment) ?

Actually, I ran the DoS_mapOrphans test to test the initial version of #16978 and even though the seed was the same between different runs, I still got non-deterministic behaviour for this test. The non-determinism originated from the size of the mapOrphanTransactions map. It was different for each run. I didn't dive into the source code to understand why exactly that is but I suspect that it's due to concurrency issues, since the seed was fixed between runs (I printed some uint256's generated by InsecureRand256() to make sure and they were the same between runs), unless there is another, differently seeded RNG used in the process of populating mapOrphanTransactions. In any case, if I understand correctly, #16978 is a great solution for setting the seed externally and for logging it, but it won't remove the non-determinism between test runs of this particular test, since I suspect that the non-determinism in this test arises due to concurrency and not due to seeding of an RNG. Though I agree that the fix is ugly, this is unfortunately the only way I can see of fixing the non-deterministic coverage issue for this particular test.

Hmm, ok. Will see if other have an opinion on this.

davereikher · 2019-10-24T14:13:40Z

@laanwj This PR is waiting for additional reviews, so I don't think this should be labeled as 'Waiting for author'?

jonatack

ACK ffd3953 tested rebased on current master

Feel free to ignore the nits below.

jonatack · 2020-06-11T17:22:38Z

src/test/denialofservice_tests.cpp

@@ -16,6 +16,8 @@
 #include <util/system.h>
 #include <util/time.h>
 #include <validation.h>
+#include <arith_uint256.h>
+#include <pubkey.h>


jonatack · 2020-06-11T17:38:42Z

src/test/denialofservice_tests.cpp

+/** This function runs CPubkey::Verify with parameters that result in all branches of the function ecdsa_signature_parse_der_lax to run. Namely, the signature literal
+ * in vch_sig_str in the following function is comprised of R and S values which both contain leading zeroes, forcing some branches of said function to run which
+ * would otherwise not. This function is called at the end of the DoS_mapOrphans test to force deterministic coverage.
+ */


suggestion

-/** This function runs CPubkey::Verify with parameters that result in all branches of the function ecdsa_signature_parse_der_lax to run. Namely, the signature literal - * in vch_sig_str in the following function is comprised of R and S values which both contain leading zeroes, forcing some branches of said function to run which - * would otherwise not. This function is called at the end of the DoS_mapOrphans test to force deterministic coverage. +/** This function runs CPubkey::Verify with parameters that result in all + * branches of the function ecdsa_signature_parse_der_lax to run. Namely, the + * signature literal in vch_sig_str in the following function is comprised of R + * and S values which both contain leading zeroes, forcing some branches of said + * function to run which would otherwise not. This function is called at the end + * of the DoS_mapOrphans test to force deterministic coverage. */

davereikher · 2020-06-14T11:39:10Z

Thanks very much for the review @jonatack ! I rebased on the latest master and addressed the nits in d90308b.
Could somebody please re-run the travis job? Looks like it timed out on ARM.

maflcko

the changes are incomplete because they don't print the seed, so the "fuzzing feature" doesn't add any value because if a test failed, it would be impossible to reproduce without the seed.

I think that fuzzing should be done in the fuzz tests and not unit tests and my suggestion would be to make the test deterministic here.

maflcko · 2020-06-14T12:28:45Z

src/test/denialofservice_tests.cpp

+        if (i == 0) {
+            // This block makes sure that the condition "if (it == mapOrphanTransactions.end())" in OrphanByIndex() gets called at least once.
+            // Otherwise test coverage is non-deterministic.
+            txPrev = OrphanByIndex(ArithToUint256(arith_uint256(std::numeric_limits<uint64_t>::max())));


instead of hardcoding special cases, I'd prefer if the txids were just derived from a deterministic fast random context. If you want, you can explicitly seed it so that the condition is hit at least once.

Ok, I can do that. The RNG can indeed be seeded so that the condition is hit at least once. However, the ForceCoverageInPubKeyVerify function must remain, since the non-determinism is not caused by different seeds, but is there due to concurrency issues (please refer to my comment here for some details). I therefore don't see another way to remove the non-determinism other than this. Is that ok?

Generally the tests run in a single thread, so there shouldn't be any concurrency issues. Are you sure the non-determinism is not caused by the random private key, which can also be picked deterministically?

Sorry for the long delay, you are right, the additional non-determinism was caused by the fact that the private key is generated by another RNG and not by concurrency issues. I removed the function I added to force coverage and forced the key to be generated by the same RNG for this test. I seeded it so that all branches of the function ecdsa_signature_parse_der_lax run. However, after a few dozen times of running this test with the script contrib/devtools/test_deterministic_coverage.sh, there was another place where the coverage was not deterministic, in the function Loop() in checkqueue.h. I'm looking into it.

davereikher · 2020-07-16T06:46:55Z

I'm running this test multiple times with the contrib/devtools/test_deterministic_coverage.sh script and out of 590 runs it detected non-deterministic coverage twice, in a seemingly unrelated location. This line

bitcoin/src/checkqueue.h

Line 99 in 9a714c5

nIdle--;

very rarely doesn't run. I'm thinking that maybe some tests which are not in the list of non-deterministic tests in this script:

bitcoin/contrib/devtools/test_deterministic_coverage.sh

Line 16 in 9a714c5

NON_DETERMINISTIC_TESTS=(

might also have non-deterministic behavior if run enough times with the script.
@practicalswift Can you say how many times the script ran for each test before it was considered to be deterministic? I guess there is some threshold number of runs such that if non-deterministic behavior doesn't appear during those runs then this test is considered to be deterministic 'enough'. Is 2 out of 590 runs above or below that threshold?

maflcko · 2020-07-16T07:49:21Z

You don't need to solve all issues. As long as something is an improvement, it should be good to go

practicalswift · 2020-07-16T08:22:11Z

@davereikher

@practicalswift Can you say how many times the script ran for each test before it was considered to be deterministic? 3D11 I guess there is some threshold number of runs such that if non-deterministic behavior doesn't appear during those runs then this test is considered to be deterministic 'enough'. Is 2 out of 590 runs above or below that threshold?

Personally I think 2 out of 590 runs is a great improvement compared to the current state of things: great job! :)

Thanks for tackling testing non-determinism!

davereikher · 2020-07-16T09:00:13Z

Personally I think 2 out of 590 runs is a great improvement compared to the current state of things: great job! :)

Thanks for tackling testing non-determinism!

Thanks @practicalswift , so do you think this test should be removed from the list of non-deterministic tests then?

davereikher · 2020-07-19T14:09:32Z

Ok, I think it's done. As stated, there is still some non-determinism, but it occurs once every several hundred rounds in an unrelated location. I'm assuming it's below the threshold of non-determinism, so I removed this test from the list of non-deterministic tests in contrib/devtools/test_deterministic_coverage.sh.

maflcko

ACK

maflcko · 2020-07-20T06:29:09Z

src/test/denialofservice_tests.cpp

+    do {
+        keydata = g_insecure_rand_ctx.randbytes(32);
+        key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true);
+    } while (!key.IsValid());


nit: This can simply say

Suggested change

} while (!key.IsValid());

assert(key.IsValid());

Wouldn't it fail the test if the generated key is invalid instead of retrying?

Yes, but that should never happen, because the test is deterministic

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.

maflcko · 2020-07-21T07:05:50Z

ACK 4455949

jonatack

ACK 4455949

Oops, too late for the merge.

cecbf6c Use secure.h header for secure allocators (Fuzzbawls) d9f67da net: add ifaddrs.h include (fanquake) e906436 build: check if -lsocket is required with *ifaddrs (fanquake) 414f405 rand: only try and use freeifaddrs if available (fanquake) 3a039d6 build: avoid getifaddrs when unavailable (Cory Fields) 77bddd7 Use GetStrongRandBytes in gmp bignum initialization (Fuzzbawls) b70b26f Fix typo in comment in randomenv.cpp (Fuzzbawls) fec460c Put bounds on the number of CPUID leaves explored (Pieter Wuille) 41ab1ff Fix CPUID subleaf iteration (Pieter Wuille) 8a9bbb1 Move events_hasher into RNGState() (Pieter Wuille) 88c2ae5 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 81d382f doc: correct random.h docs after bitcoin#17270 (fanquake) f363ea9 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo) 7d6ddcb Run background seeding periodically instead of unpredictably (Pieter Wuille) 4679181 Add information gathered through getauxval() (Pieter Wuille) 88d97d0 Feed CPUID data into RNG (Pieter Wuille) 8f5b9c9 Use sysctl for seeding on MacOS/BSD (Pieter Wuille) 67de246 Gather additional entropy from the environment (Pieter Wuille) 6142e1f Seed randomness with process id / thread id / various clocks (Pieter Wuille) 7bde8b7 [MOVEONLY] Move cpuid code from random to compat/cpuid (Fuzzbawls) 52b5336 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille) 27cf995 doc: minor corrections in random.cpp (fanquake) fccd2b8 doc: correct function name in ReportHardwareRand() (fanquake) 909473e Fix FreeBSD build by including utilstrencodings.h (Fuzzbawls) 630931f break circular dependency: random/sync -> util -> random/sync (Fuzzbawls) 5eed08c random: remove call to RAND_screen() (Windows only) (fanquake) ada9868 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake) 22a7121 Fix non-deterministic coverage of test DoS_mapOrphans (Fuzzbawls) 79e7fd3 Add ChaCha20 bench (Jonas Schnelli) 6966aa9 Add ChaCha20 encryption option (XOR) (Jonas Schnelli) 28c9cdb tests: Add script checking for deterministic line coverage (practicalswift) c82e359 test: Make bloom tests deterministic (MarcoFalke) 7b33223 Document strenghtening (Pieter Wuille) 0190dec Add hash strengthening to the RNG (Pieter Wuille) 67e336d Use RdSeed when available, and reduce RdRand load (Pieter Wuille) 4ffda1f Document RNG design in random.h (Pieter Wuille) 2b6381e Use secure allocator for RNG state (Pieter Wuille) 080deb3 Encapsulate RNGState better (Pieter Wuille) 787d72f DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille) 5bc2583 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille) 774899f Remove hwrand_initialized. (Pieter Wuille) 698d133 Switch all RNG code to the built-in PRNG. (Pieter Wuille) 038a45a Integrate util/system's CInit into RNGState (Fuzzbawls) 5f20e62 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille) 298f97c Add thread safety annotations to RNG state (Pieter Wuille) 2326535 Rename some hardware RNG related functions (Pieter Wuille) d76ee83 Automatically initialize RNG on first use. (Pieter Wuille) 1a5dbc5 Don't log RandAddSeedPerfmon details (Pieter Wuille) 32e6c42 Simplify testing RNG code (Fuzzbawls) 972effa Make unit tests use the insecure_rand_ctx exclusively (Fuzzbawls) af52bf5 Use a FastRandomContext in LimitOrphanTxSize (Fuzzbawls) 746d466 Introduce a Shuffle for FastRandomContext and use it in wallet (Fuzzbawls) 1cdf124 Use a local FastRandomContext in a few more places in net (Fuzzbawls) e862564 Make addrman use its local RNG exclusively (Fuzzbawls) 94b2ead Make FastRandomContext support standard C++11 RNG interface (Pieter Wuille) Pull request description: This is a collection of upstream PRs that have been backported to bring our RNG (`src/random`) code more up-to-date. The following upstream PRs have been included here: - bitcoin#12742 - bitcoin#14624 - some of this had already been merged previously - bitcoin#14955 - bitcoin#15250 - bitcoin#15224 - bitcoin#15324 - bitcoin#15296 - bitcoin#15512 - bitcoin#16878 - bitcoin#17151 - bitcoin#17191 - bitcoin#13236 - bitcoin#13314 - bitcoin#17169 - bitcoin#17270 - omitted last commit as our testing framework doesn't support it currently - omitted bitcoin@64e1e02, to be pulled in after our time utility is updated in a separate PR - bitcoin#17573 - bitcoin#17507 - bitcoin#17670 - bitcoin#17527 - bitcoin#14127 - bitcoin#21486 ACKs for top commit: furszy: ACK cecbf6c with a minor nit that can be easily tackled later. random-zebra: rebase utACK cecbf6c and merging... Tree-SHA512: 3463b693cc9bddc1ec15228d264a794f5c2f159073fafa2ccf6e2563abfeb4369e49505f97ca84f2478ca792bd07b66d2cd83c58044d6a0cae6af42d22f5784b

fanquake added the Tests label Sep 16, 2019

practicalswift reviewed Sep 16, 2019

View reviewed changes

davereikher force-pushed the make_denialofservice_tests_deterministic branch from bf2637c to ffd3953 Compare September 16, 2019 13:13

maflcko reviewed Sep 16, 2019

View reviewed changes

maflcko added the Waiting for author label Sep 17, 2019

maflcko reviewed Sep 20, 2019

View reviewed changes

src/test/denialofservice_tests.cpp Outdated Show resolved Hide resolved

davereikher force-pushed the make_denialofservice_tests_deterministic branch from bae45e8 to f2e7f8b Compare September 20, 2019 13:35

maflcko reviewed Sep 20, 2019

View reviewed changes

src/test/setup_common.h Outdated Show resolved Hide resolved

davereikher force-pushed the make_denialofservice_tests_deterministic branch from f2e7f8b to ffd3953 Compare September 22, 2019 16:38

fanquake removed the Waiting for author label Sep 24, 2019

maflcko reviewed Oct 3, 2019

View reviewed changes

laanwj added the Waiting for author label Oct 15, 2019

fanquake removed the Waiting for author label Oct 24, 2019

davereikher mentioned this pull request Dec 12, 2019

Suggestion on how to mitigate non-determinism in tests #17731

Closed

DrahtBot mentioned this pull request Jun 11, 2020

banman: Limit resources consumed by misbehaving node deprioitisation #19243

Closed

jonatack reviewed Jun 11, 2020

View reviewed changes

davereikher force-pushed the make_denialofservice_tests_deterministic branch 3 times, most recently from 92d76de to d90308b Compare June 14, 2020 10:30

maflcko suggested changes Jun 14, 2020

View reviewed changes

DrahtBot mentioned this pull request Jun 25, 2020

refactor: Drop g_orphan_list global #19374

Closed

davereikher force-pushed the make_denialofservice_tests_deterministic branch 2 times, most recently from c158708 to 5059382 Compare July 19, 2020 12:43

maflcko approved these changes Jul 20, 2020

View reviewed changes

davereikher force-pushed the make_denialofservice_tests_deterministic branch from 5059382 to 4455949 Compare July 21, 2020 06:19

maflcko merged commit b763ae0 into bitcoin:master Jul 21, 2020

jonatack reviewed Jul 21, 2020

View reviewed changes

Fuzzbawls mentioned this pull request Mar 30, 2021

[Refactor] Update RNG code from upstream PIVX-Project/PIVX#2278

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Fix non-deterministic coverage of test DoS_mapOrphans #16878

Fix non-deterministic coverage of test DoS_mapOrphans #16878

Uh oh!

Conversation

Uh oh!

Uh oh!

Conflicts

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!