Use FastRandomContext for all tests #10321

pull sipa wants to merge 10 commits into bitcoin:master from sipa:fast_rand_tests changing 26 files +202 −190
  1. sipa commented at 11:03 pm on May 2, 2017: member
    Currently, the unit tests rely on a mix of an encapsulated FastRandomContext, and some of the Rand*() functions for randomness. This is inefficient, and will become more inefficient when the Rand* functions are changed to always provide strong randomness.
  2. laanwj commented at 5:25 am on May 3, 2017: member
    Concept ACK, also been meaning to do this. There is no need for strong randomness in tests and speed is very important.
  3. laanwj added the label Tests on May 3, 2017
  4. in src/random.cpp:307 in 7666694cfe outdated 
    246@@ -247,6 +247,27 @@ void FastRandomContext::RandomSeed()
247     requires_seed = false;
248 }
249 
250+uint256 FastRandomContext::rand256()
    jnewbery commented at 1:59 pm on May 3, 2017:
    Any particular reason why this is defined in random.cpp, and rand64(), which is almost identical, is defined in the header file?
    sipa commented at 6:57 pm on May 5, 2017:
    It doesn’t feel worthwhile to inline rand256() to me.
  5. jnewbery commented at 3:23 pm on May 3, 2017: member

    Tested ACK https://github.com/bitcoin/bitcoin/pull/10321/commits/7666694cfe35ffbfeefcf92c0c36026ee50e6eb0 with one question.

    I’ve completely unscientifically benchmarked this locally by running the tests which were updated (excluding the wallet tests). This seems to marginally speed up test run time:

    master:

     0→ for i in {1..10}; do test_bitcoin --run_test=DoS_tests,blockencodings_tests,bloom_tests,checkqueue_tests,coins_tests,dbwrapper_tests,miner_tests,pow_tests,sighash_tests --log_level=test_suite 2>/dev/null | grep "testing time" | cut -d' ' -f7 | rev | cut -c 4- | rev | paste -sd+ - | bc;done
 19805962
 210284966
 39880339
 49470773
 59775485
 610186809
 710072789
 89522348
 99297190
109433797

    avg: 9.773045.8s

    PR10321:

     0→ for i in {1..10}; do test_bitcoin --run_test=DoS_tests,blockencodings_tests,bloom_tests,checkqueue_tests,coins_tests,dbwrapper_tests,miner_tests,pow_tests,sighash_tests --log_level=test_suite 2>/dev/null | grep "testing time" | cut -d' ' -f7 | rev | cut -c 4- | rev | paste -sd+ - | bc;done
 19597130
 29677749
 39644620
 49597136
 59393690
 69542741
 79417484
 89128269
 99600153
109382188

    avg: 9.498116s

  6. TheBlueMatt commented at 9:18 pm on May 5, 2017: member
    Can you scripted-diff this? should be rather trivial to s/GetRandHash/insecure_rand256/?
  7. sipa force-pushed on May 23, 2017
  8. sipa commented at 11:33 pm on May 23, 2017: member
    Significant update: the scope is expanded (some more constructions are replaced with more efficient ones), test_random.h was merged into test_bitcoin.h (why was it separate? the code for both was in test_bitcoin.cpp), and some of the changes are done as scripted diffs.
  9. sipa force-pushed on May 23, 2017
  10. sipa force-pushed on May 23, 2017
  11. sipa force-pushed on May 24, 2017
  12. in src/random.cpp:322 in faeb6319cc outdated 
    317+
318+std::vector<unsigned char> FastRandomContext::randbytes(size_t len)
319+{
320+    std::vector<unsigned char> ret;
321+    if (len > 0) {
322+        ret.resize(len);
    TheBlueMatt commented at 7:51 pm on May 29, 2017:
    its an unsigned type - just create the vector with the size constructor?
  13. in src/test/cuckoocache_tests.cpp:26 in 6c41d59afe outdated 
    22@@ -23,18 +23,18 @@
23  *  using BOOST_CHECK_CLOSE to fail.
24  *
25  */
26-FastRandomContext insecure_rand(true);
27+static FastRandomContext local_rand_ctx(true);
    TheBlueMatt commented at 7:54 pm on May 29, 2017:
    In “Rename cuckoo tests’ local rand context”: Note that you need to make it “scripted-diff” not “script-diff”, and also note that on this line its not scripted, should probably separate the static or rename the commit to avoid confusing review.
  14. sipa force-pushed on May 30, 2017
  15. sipa commented at 9:43 pm on May 30, 2017: member
    Addressed @TheBlueMatt’s comments.
  16. laanwj added this to the "Blockers" column in a project
  17. sipa force-pushed on Jun 2, 2017
  18. sipa force-pushed on Jun 2, 2017
  19. Add FastRandomContext::rand256() and ::randbytes() 
    FastRandomContext now provides all functionality that the real Rand* functions
provide.
    37e864eb9f
  20. scripted-diff: Rename cuckoo tests' local rand context 
    -BEGIN VERIFY SCRIPT-
sed -i 's/insecure_rand/local_rand_ctx/g' src/test/cuckoocache_tests.cpp
-END VERIFY SCRIPT-
    90620d66c9
  21. Merge test_random.h into test_bitcoin.h 124d13a58c
  22. Add various insecure_rand wrappers for tests 1119927df0
  23. scripted-diff: use insecure_rand256/randrange more 
    -BEGIN VERIFY SCRIPT-
sed -i "s/\<GetRandHash(/insecure_rand256(/" src/test/*_tests.cpp
sed -i "s/\<GetRand(/insecure_randrange(/" src/test/*_tests.cpp src/test/test_bitcoin.cpp
sed -i 's/\<insecure_rand() % \([0-9]\+\)/insecure_randrange(\1)/g' src/test/*_tests.cpp
-END VERIFY SCRIPT-
    efee1db21a
  24. sipa force-pushed on Jun 5, 2017
  25. sipa commented at 7:46 pm on June 5, 2017: member
    Rebased after merge of #10514.
  26. in src/test/test_bitcoin.h:36 in 1119927df0 outdated 
    35+static inline uint32_t insecure_rand() { return insecure_rand_ctx.rand32(); }
36+static inline uint256 insecure_rand256() { return insecure_rand_ctx.rand256(); }
37+static inline uint64_t insecure_randbits(int bits) { return insecure_rand_ctx.randbits(bits); }
38+static inline uint64_t insecure_randrange(uint64_t range) { return insecure_rand_ctx.randrange(range); }
39+static inline bool insecure_randbool() { return insecure_rand_ctx.randbool(); }
40+static inline std::vector<unsigned char> insecure_randbytes(size_t len) { return insecure_rand_ctx.randbytes(len); }
    ryanofsky commented at 5:50 pm on June 7, 2017:
    The new functions could be made camelcase to conform to style guide.
    sipa commented at 8:57 pm on June 7, 2017:
    Done.
  27. in src/test/coins_tests.cpp:158 in afc80b8a2d outdated 
    154@@ -155,11 +155,11 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test)
155                 newcoin.out.nValue = insecure_rand();
156                 newcoin.nHeight = 1;
157                 if (insecure_randrange(16) == 0 && coin.IsSpent()) {
158-                    newcoin.out.scriptPubKey.assign(1 + (insecure_rand() & 0x3F), OP_RETURN);
159+                    newcoin.out.scriptPubKey.assign(1 + insecure_randbits(6), OP_RETURN);
    ryanofsky commented at 5:54 pm on June 7, 2017:

    In commit “Replace more rand() % NUM by randranges”

    There are a few randbits replacements mixed up in this commit.

    sipa commented at 8:57 pm on June 7, 2017:
    Moved to the next commit.
  28. in src/test/prevector_tests.cpp:204 in 3f6ac4b77d outdated 
    202-            if ((r % 4) == 0) {
203+            if (insecure_randbits(2) == 0) {
204                 test.insert(insecure_randrange(test.size() + 1), insecure_rand());
205             }
206-            if (test.size() > 0 && ((r >> 2) % 4) == 1) {
207+            if (test.size() > 0 && insecure_randbits(2) == 1) {
    ryanofsky commented at 6:15 pm on June 7, 2017:

    In commit “Use randbits instead of ad-hoc emulation in prevector tests”

    Can all these == 1, == 2, == 3 checks be replaced with == 0? That would seem like a simpler style.

    sipa commented at 8:58 pm on June 7, 2017:
    Let’s do that some other time.
  29. ryanofsky commented at 6:20 pm on June 7, 2017: member
    utACK b09bd02e3cbbb03758b1467fc2a4bb847142c35d
  30. Replace more rand() % NUM by randranges 3ecabae363
  31. Replace rand() & ((1 << N) - 1) with randbits(N) 5f0b04eedc
  32. Use randbits instead of ad-hoc emulation in prevector tests 2ada678521
  33. scripted-diff: Use randbits/bool instead of randrange where possible 
    -BEGIN VERIFY SCRIPT-
sed -i 's/insecure_randbits(1)/insecure_randbool()/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(2)/insecure_randbool()/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(4)/insecure_randbits(2)/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(32)/insecure_randbits(5)/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(256)/insecure_randbits(8)/g' src/test/*_tests.cpp
-END VERIFY SCRIPT-
    2fcd9cc86b
  34. scripted-diff: Use new naming style for insecure_rand* functions 
    -BEGIN VERIFY SCRIPT-
sed -i 's/\<insecure_randbits(/InsecureRandBits(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randbool(/InsecureRandBool(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randrange(/InsecureRandRange(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randbytes(/InsecureRandBytes(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_rand256(/InsecureRand256(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_rand(/InsecureRand32(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<seed_insecure_rand(/SeedInsecureRand(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
-END VERIFY SCRIPT-
    e945848582
  35. sipa force-pushed on Jun 7, 2017
  36. TheBlueMatt commented at 8:09 pm on June 7, 2017: member
    utACK b09bd02e3cbbb03758b1467fc2a4bb847142c35d
  37. jtimon commented at 8:46 pm on June 7, 2017: contributor
    Concept ACK
  38. ryanofsky commented at 9:09 pm on June 7, 2017: member
    utACK e945848582160b23fa0bc6f797e2bd8ac676ee0e. Changes since last review were moving insecure_randbits changes to appropriate commit, and adding a new scripted diff commit to update style of function names.
  39. sipa merged this on Jun 7, 2017
  40. sipa closed this on Jun 7, 2017
  41. sipa referenced this in commit e801084dec on Jun 7, 2017
  42. laanwj removed this from the "Blockers" column in a project
  43. sipa deleted the branch on Jun 23, 2017
  44. sipa referenced this in commit 424be03305 on Oct 12, 2017
  45. PastaPastaPasta referenced this in commit 932fd432c3 on Jul 5, 2019
  46. PastaPastaPasta referenced this in commit 4b3d1057fc on Jul 5, 2019
  47. PastaPastaPasta referenced this in commit 83e55d23bb on Jul 6, 2019
  48. Warrows referenced this in commit 509e311d7f on Jul 6, 2019
  49. Warrows referenced this in commit 091ce255cf on Jul 8, 2019
  50. PastaPastaPasta referenced this in commit 57c5cfb0f6 on Jul 8, 2019
  51. Warrows referenced this in commit a4fd5eac90 on Jul 9, 2019
  52. Warrows referenced this in commit 40fb15537e on Jul 9, 2019
  53. Warrows referenced this in commit 8e19443e41 on Jul 31, 2019
  54. CaveSpectre11 referenced this in commit 37b6a226e8 on Dec 14, 2019
  55. PastaPastaPasta referenced this in commit 67513d3df3 on Dec 22, 2019
  56. PastaPastaPasta referenced this in commit b85ffe17e5 on Jan 2, 2020
  57. PastaPastaPasta referenced this in commit 84a3603b87 on Jan 4, 2020
  58. PastaPastaPasta referenced this in commit 84ab39d400 on Jan 12, 2020
  59. PastaPastaPasta referenced this in commit 962becc9eb on Jan 12, 2020
  60. PastaPastaPasta referenced this in commit 69991c3764 on Jan 12, 2020
  61. PastaPastaPasta referenced this in commit 4d0475b592 on Jan 12, 2020
  62. PastaPastaPasta referenced this in commit 920e3e709d on Jan 12, 2020
  63. PastaPastaPasta referenced this in commit 40f559ebbb on Jan 12, 2020
  64. PastaPastaPasta referenced this in commit 1e2ab7633f on Jan 16, 2020
  65. barrystyle referenced this in commit 7244ce8864 on Jan 22, 2020
  66. wqking referenced this in commit 9f6412a364 on Apr 22, 2020
  67. Kokary referenced this in commit 03d792fbb2 on May 26, 2020
  68. Kokary referenced this in commit 306c924533 on Nov 13, 2020
  69. Kokary referenced this in commit 2f1705261a on Nov 17, 2020
  70. Kokary referenced this in commit a080c3f945 on Nov 17, 2020
  71. KolbyML referenced this in commit fff25346dc on Nov 21, 2020
  72. Kokary referenced this in commit e971485b18 on Nov 24, 2020
  73. Cryptarchist referenced this in commit fa8dfabb4c on Nov 30, 2020
  74. ckti referenced this in commit 72d0e25cdd on Mar 28, 2021
  75. PastaPastaPasta referenced this in commit 87a9465e20 on Jul 17, 2021
  76. UdjinM6 referenced this in commit 4d80e2865e on Jul 19, 2021
  77. MarcoFalke locked this on Sep 8, 2021


sipa laanwj jnewbery TheBlueMatt ryanofsky jtimon

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-04 22:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me