Fix non-deterministic coverage of test DoS

davereikher commented at 5:20 am on September 16, 2019: contributor

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.

fanquake added the label Tests on Sep 16, 2019

DrahtBot commented at 6:05 am on September 16, 2019: member

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

Conflicts

No conflicts as of last run.

practicalswift commented at 10:13 am on September 16, 2019: contributor

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! :)

in src/test/denialofservice_tests.cpp:425 in bf2637cc32 outdated

417@@ -391,7 +418,14 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
418     // ... and 50 that depend on other orphans:
419     for (int i = 0; i < 50; i++)
420     {
421-        CTransactionRef txPrev = RandomOrphan();
422+        CTransactionRef txPrev;
423+        if (0 == i) {
424+            // This block makes sure that the condition "if (it == mapOrphanTransactions.end())" in OrphanByIndex() gets called at least once.
425+            // Otherwise test coverage is non-deterministic.
426+            txPrev = OrphanByIndex(ArithToUint256(arith_uint256(LARGE_NUMBER)));

practicalswift commented at 10:15 am on September 16, 2019:

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

in src/test/denialofservice_tests.cpp:422 in bf2637cc32 outdated

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

practicalswift commented at 10:17 am on September 16, 2019:

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

davereikher force-pushed on Sep 16, 2019

davereikher commented at 1:20 pm on September 16, 2019: contributor

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.

in src/test/denialofservice_tests.cpp:325 in ffd3953757 outdated

389+        "\xad\xf6\x6a\x9c\x2f\xd9\xd4\xf3\xcc\xf4\xc8\x4c\x38\xbb\xd0\xac\xde\xa7\x3d\x66\x28\xa0\xe0", 70);
390+    uint256 hash;
391+    hash.SetHex(std::string("6af516422fef8a745aff6acdcc84076c77fb2ecd72bd5711df301230ac58fdd5"));
392+
393+    coverage_pubkey.Verify(hash, std::vector<unsigned char>(vch_sig_str.begin(), vch_sig_str.end()));
394+}

MarcoFalke commented at 1:24 pm on September 16, 2019:

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

davereikher commented at 1:52 pm on September 16, 2019:

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.

MarcoFalke commented at 2:03 pm on September 16, 2019:

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

davereikher commented at 2:41 pm on September 16, 2019:

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?

MarcoFalke commented at 2:50 pm on September 16, 2019:

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.

davereikher commented at 10:20 am on September 17, 2019:

I see. Then I can just set a seed which guarantees the widest coverage in the beginning of the test and that should take care of the non-determinism in both RandomOrphan and ecdsa_signature_parse_der_lax, making the code much simpler.

Still, I would argue that it’s best to leave the fuzzing and write the seed to a file. That would result in an a better coverage over a long period of time. There might be some branches that run very rarely and setting the seed to a constant would prevent them from ever running in the future in the context of this test, reducing it’s long-term coverage. As an example the conditions on lines 151, 157, 160 in pubkey.cpp, in ecdsa_signature_parse_der_lax may evaluate to false or true. Although their evaluation was deterministic throughout the 1000+ runs I tried and I haven’t gone in-depth to try and understand whether they can at all run under this test’s conditions or whether they run extremely rarely, but the latter option can be true for other branches I haven’t checked. Unless handling artifacts created by tests involves a lot of overhead, the only disadvantage I see to leaving the fuzzing is more lines of code in the test, some of it being irrelevant to the functionality of the test itself, but I think that is taken care of by a detailed comment explaining the logic for the extra code.

MarcoFalke commented at 10:56 am on September 17, 2019:

Yeah, up to you if you feel strongly. My opinion is that we shouldn’t mix unit tests with fuzz tests, which are designed and compiled separately, see https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md

davereikher commented at 5:54 am on September 18, 2019:

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.

MarcoFalke commented at 10:29 am on September 18, 2019:

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?

davereikher commented at 2:08 pm on September 18, 2019:

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?

MarcoFalke commented at 2:16 pm on September 18, 2019:

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.

davereikher commented at 2:55 pm on September 18, 2019:

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?

MarcoFalke commented at 7:46 pm on October 3, 2019:

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

davereikher commented at 9:50 am on October 5, 2019:

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.

MarcoFalke commented at 5:41 pm on October 7, 2019:

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

MarcoFalke added the label Waiting for author on Sep 17, 2019

davereikher commented at 11:48 am on September 20, 2019: contributor

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

0A new Boost dependency in the form of "boost/test/unit_test_parameters.hpp" appears to have been introduced:
1
2src/test/denialofservice_tests.cpp:#include <boost/test/unit_test_parameters.hpp>
3
4^---- 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?

MarcoFalke commented at 12:05 pm on September 20, 2019: member

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

 0diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh
 1index d27e45a23f..c9e1d57ee1 100755
 2--- a/test/lint/lint-includes.sh
 3+++ b/test/lint/lint-includes.sh
 4@@ -68,6 +68,7 @@ EXPECTED_BOOST_INCLUDES=(
 5     boost/signals2/last_value.hpp
 6     boost/signals2/signal.hpp
 7     boost/test/unit_test.hpp
 8+    boost/test/unit_test_parameters.hpp
 9     boost/thread.hpp
10     boost/thread/condition_variable.hpp
11     boost/thread/mutex.hpp

in src/test/denialofservice_tests.cpp:397 in bae45e8ab4 outdated

393@@ -393,6 +394,8 @@ static void ForceCoverageInPubKeyVerify()
394 
395 BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
396 {
397+    SeedInsecureRand(ArithToUint256(arith_uint256(boost::unit_test::runtime_config::get<unsigned>( boost::unit_test::runtime_config::btrt_random_seed ))));

MarcoFalke commented at 12:12 pm on September 20, 2019:

I guess you could move this logic into a helper SeedInsecureRandWithBoostRandomSeed?

davereikher force-pushed on Sep 20, 2019

in src/test/setup_common.h:55 in f2e7f8b02b outdated

50+    g_insecure_rand_ctx = FastRandomContext(seed);
51+}
52+
53+static inline void SeedInsecureRandWithBoostRandomSeed()
54+{
55+    SeedInsecureRand(ArithToUint256(arith_uint256(boost::unit_test::runtime_config::get<unsigned>( boost::unit_test::runtime_config::btrt_random_seed ))));

MarcoFalke commented at 3:11 pm on September 20, 2019:

not sure how stable that boost interface is. They seem to change it every couple of versions

davereikher commented at 12:07 pm on September 21, 2019:

Is there a better way to access this value? Maybe directly access the environment variable using std::getenv?

davereikher commented at 7:17 am on September 22, 2019: contributor

@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.

davereikher force-pushed on Sep 22, 2019

practicalswift commented at 7:24 pm on September 22, 2019: contributor

ACK ffd3953757051ac50cea4129edbd9e9d5be39d0f – diff looks correct

davereikher commented at 12:00 pm on September 24, 2019: contributor

How can I remove the “Waiting for author” label?

fanquake removed the label Waiting for author on Sep 24, 2019

MarcoFalke commented at 7:45 pm on October 3, 2019: member

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

laanwj added the label Waiting for author on Oct 15, 2019

davereikher commented at 2:13 pm on October 24, 2019: contributor

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

fanquake removed the label Waiting for author on Oct 24, 2019

in src/test/denialofservice_tests.cpp:20 in ffd3953757 outdated

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

jonatack commented at 5:22 pm on June 11, 2020:

nit: sort

in src/test/denialofservice_tests.cpp:382 in ffd3953757 outdated

376+}
377+
378+/** 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
379+ * 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
380+ * would otherwise not. This function is called at the end of the DoS_mapOrphans test to force deterministic coverage.
381+ */

jonatack commented at 5:38 pm on June 11, 2020:

suggestion

0-/** 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
1- * 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
2- * would otherwise not. This function is called at the end of the DoS_mapOrphans test to force deterministic coverage.
3+/** This function runs CPubkey::Verify with parameters that result in all
4+ * branches of the function ecdsa_signature_parse_der_lax to run. Namely, the
5+ * signature literal in vch_sig_str in the following function is comprised of R
6+ * and S values which both contain leading zeroes, forcing some branches of said
7+ * function to run which would otherwise not. This function is called at the end
8+ * of the DoS_mapOrphans test to force deterministic coverage.
9  */

jonatack commented at 5:59 pm on June 11, 2020: member

ACK ffd3953757051ac50cea4129edbd9e9d5b tested rebased on current master

Feel free to ignore the nits below.

davereikher force-pushed on Jun 14, 2020

davereikher commented at 11:39 am on June 14, 2020: contributor

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.

in src/test/denialofservice_tests.cpp:426 in d90308badb outdated

418@@ -391,7 +419,14 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
419     // ... and 50 that depend on other orphans:
420     for (int i = 0; i < 50; i++)
421     {
422-        CTransactionRef txPrev = RandomOrphan();
423+        CTransactionRef txPrev;
424+        if (i == 0) {
425+            // This block makes sure that the condition "if (it == mapOrphanTransactions.end())" in OrphanByIndex() gets called at least once.
426+            // Otherwise test coverage is non-deterministic.
427+            txPrev = OrphanByIndex(ArithToUint256(arith_uint256(std::numeric_limits<uint64_t>::max())));

MarcoFalke commented at 12:28 pm on June 14, 2020:

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.

davereikher commented at 6:11 pm on June 17, 2020:

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?

MarcoFalke commented at 6:21 pm on June 17, 2020:

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?

davereikher commented at 2:01 pm on July 9, 2020:

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.

MarcoFalke changes_requested

MarcoFalke commented at 12:37 pm on June 14, 2020: member

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.

davereikher commented at 6:46 am on July 16, 2020: contributor

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 https://github.com/bitcoin/bitcoin/blob/9a714c51dc79993506718617e383294cf6bff0b4/src/checkqueue.h#L99 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: https://github.com/bitcoin/bitcoin/blob/9a714c51dc79993506718617e383294cf6bff0b4/contrib/devtools/test_deterministic_coverage.sh#L16 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?

MarcoFalke commented at 7:49 am on July 16, 2020: member

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

practicalswift commented at 8:22 am on July 16, 2020: contributor

@davereikher

@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?

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 commented at 9:00 am on July 16, 2020: contributor

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 force-pushed on Jul 19, 2020

davereikher commented at 2:09 pm on July 19, 2020: contributor

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.

in src/test/denialofservice_tests.cpp:325 in 5059382317 outdated

320+{
321+    std::vector<unsigned char> keydata;
322+    do {
323+        keydata = g_insecure_rand_ctx.randbytes(32);
324+        key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true);
325+    } while (!key.IsValid());

MarcoFalke commented at 6:29 am on July 20, 2020:

nit: This can simply say

0    assert(key.IsValid());

davereikher commented at 4:07 pm on July 20, 2020:

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

MarcoFalke commented at 6:29 pm on July 20, 2020:

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

MarcoFalke approved

MarcoFalke commented at 6:29 am on July 20, 2020: member

ACK

Make test DoS_mapOrphans 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.

4455949d6f

davereikher force-pushed on Jul 21, 2020

MarcoFalke commented at 7:05 am on July 21, 2020: member

ACK 4455949d6f0218b40d33d7fe6de6555f8f62192f

MarcoFalke merged this on Jul 21, 2020

MarcoFalke closed this on Jul 21, 2020

jonatack commented at 7:50 am on July 21, 2020: member

ACK 4455949d6f0218b40d33d7fe6de6555f8f62192f

Oops, too late for the merge.

random-zebra referenced this in commit 93f43f0f81 on Apr 14, 2021

DrahtBot locked this on Feb 15, 2022

Fix non-deterministic coverage of test DoS_mapOrphans #16878

Conflicts