test: Add time-timewarp-attack boundary cases #30698

1. 
   instagibbs commented at 4:44 pm on August 22, 2024: member

   Basic addition to test case added in #30681
2. test: Add time-timewarp-attack boundary cases 31378d44f4
3. 
   DrahtBot commented at 4:44 pm on August 22, 2024: 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	fjahr, tdb3, achow101
   Concept ACK	Sjors

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

4. instagibbs marked this as ready for review on Aug 22, 2024
5. DrahtBot added the label Tests on Aug 22, 2024
6. 
   fjahr commented at 7:05 pm on August 22, 2024: contributor

   Code review ACK 31378d44f44cabc576bf92ddd61afde4b528de77
7. fanquake added this to the milestone 28.0 on Aug 23, 2024
8. in test/functional/mining_basic.py:160 in 31378d44f4

   158@@ -159,6 +159,15 @@ def test_timewarp(self):
   159         bad_block.solve()
   160         assert_raises_rpc_error(-25, 'time-timewarp-attack', lambda: node.submitheader(hexdata=CBlockHeader(bad_block).serialize().hex()))
   

   ---

   ---

   

   tdb3 commented at 0:39 am on August 24, 2024:

   I’m not immediately seeing why the previous check (for the bad_block.nTime = t case) is needed anymore now that the check closer to the boundary is being added (bad_block.nTime = t + MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP - 1 case). Maybe I’m overlooking something?

   If not, then could add a commit that removes the check for the bad_block.nTime = t case.

9. tdb3 approved
10. 
    tdb3 commented at 0:47 am on August 24, 2024: contributor

    ACK 31378d44f44cabc576bf92ddd61afde4b528de77

    Great addition (testing the boundary, i.e. MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP). Tested locally (and sanity checked that the assert would catch removing the -1 from bad_block.nTime = t + MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP - 1).

    Left one comment, but it doesn’t block the ACK.

11. in test/functional/mining_basic.py:167 in 31378d44f4

    158@@ -159,6 +159,15 @@ def test_timewarp(self):
    159         bad_block.solve()
    160         assert_raises_rpc_error(-25, 'time-timewarp-attack', lambda: node.submitheader(hexdata=CBlockHeader(bad_block).serialize().hex()))
    161 
    162+        self.log.info("Test timewarp protection boundary")
    163+        bad_block.nTime = t + MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP - 1
    164+        bad_block.solve()
    165+        assert_raises_rpc_error(-25, 'time-timewarp-attack', lambda: node.submitheader(hexdata=CBlockHeader(bad_block).serialize().hex()))
    166+
    167+        bad_block.nTime = t + MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP
    

    ---

    ---

    

    Sjors commented at 9:09 am on August 26, 2024:

    Let’s use block here, since it’s not actually bad.

    Alternatively you could modify the test above to not bother copying block to bad_block.

12. 
    Sjors commented at 9:10 am on August 26, 2024: member

    Concept ACK
13. 
    achow101 commented at 6:28 pm on August 26, 2024: member

    ACK 31378d44f44cabc576bf92ddd61afde4b528de77

    Going to merge this now in the interest of having test coverage of the boundary before the 28.x branch off.

14. DrahtBot requested review from Sjors on Aug 26, 2024
15. achow101 merged this on Aug 26, 2024
16. achow101 closed this on Aug 26, 2024

    ---

Contributors
instagibbs DrahtBot fjahr tdb3 Sjors achow101

Review Requested
Sjors

Labels
Tests

Milestone
28.0