Test: followups to #27823 #28612

1. 
   L0laL33tz commented at 3:41 pm on October 8, 2023: contributor

   Fixes #28603

   Added suggested simplifications and implemented randomization

2. test: simplify feature_init 64b80d5c5b
3. test: randomized perturbing in feature_init 5ab6419f38
4. 
   DrahtBot commented at 3:41 pm on October 8, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	theStack, maflcko, achow101
   Concept ACK	fjahr

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

5. DrahtBot added the label Tests on Oct 8, 2023
6. fanquake requested review from mzumsande on Oct 8, 2023
7. in test/functional/feature_init.py:141 in 5ab6419f38

   137@@ -137,8 +138,8 @@ def check_clean_start():
   138                     # Since the genesis block is not checked by -checkblocks, the
   139                     # perturbation window must be chosen such that a higher block
   140                     # in blk*.dat is affected.
   141-                    tf.seek(150)
   142-                    tf.write(b'1' * 200)
   143+                    tf.seek(randint (150, 15000))
   

   ---

   ---

   

   brunoerg commented at 11:51 am on October 10, 2023:

   In 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae: What is the rationale of the range 150~15000?

   ---

   

   L0laL33tz commented at 1:09 pm on October 10, 2023:

   I tried to pick a range large enough for randomization to be efficient, but the range itself is arbitrary and could be adjusted.

   ---

   

   L0laL33tz commented at 4:09 pm on October 12, 2023:

   For more clarification @brunoerg the 150 starting point was chosen in the previous test to exclude the genesis block iiuc. The 15000 range should be large enough to provide efficient randomization but small enough to not end outside of the blockfile (200 blocks in the test with ~160 characters each). Could also shorten or enlarge the range ((200*160 = 32000 characters) but I thought 15000 would a good middle ground.

   Let me know if you have any suggestions. Can also add a line of documentation if necessary.

8. 
   fjahr commented at 9:37 pm on October 10, 2023: contributor

   Concept ACK, better if someone else does a full review though since part of the changes are from me
9. fanquake requested review from theStack on Nov 1, 2023
10. 
    theStack commented at 3:08 pm on November 6, 2023: contributor

    Light ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae

    The refactoring changes in the first commit look correct and randomizing offset/size of the perturbation data seems to be an improvement for discovering more issues. Can’t say much about the concrete random ranges chosen though, it might make sense to have someone review this that has more insight into the file format or specific cases that were in mind to be triggered (on the other hand, perfect is the enemy of good…).

11. DrahtBot removed review request from theStack on Nov 6, 2023
12. DrahtBot requested review from fjahr on Nov 6, 2023
13. 
    maflcko commented at 3:19 pm on November 6, 2023: member

    lgtm ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae
14. 
    achow101 commented at 9:46 pm on November 6, 2023: member

    ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae
15. achow101 merged this on Nov 6, 2023
16. achow101 closed this on Nov 6, 2023

    ---

17. 
    mzumsande commented at 1:46 am on November 7, 2023: contributor

    I ran this locally a couple 100 times and the test failed for me maybe 1% of the time - haven’t done any deeper analysis though.
18. bitcoin locked this on Nov 19, 2024

Contributors
L0laL33tz DrahtBot brunoerg fjahr theStack maflcko achow101 mzumsande

Review Requested
fjahr

Labels
Tests