test: change log rate limit version gate #33612

1. 
   Crypt-iQ commented at 3:33 pm on October 13, 2025: contributor

   Change the version gate from 299900 to 290100 for bypassing the log rate limit in case an explicit version is set in the functional test framework.

   See discussion here: #33225 (review)

2. DrahtBot added the label Tests on Oct 13, 2025
3. 
   DrahtBot commented at 3:33 pm on October 13, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33612.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	janb84, maflcko, stickies-v
   Stale ACK	kevkevinpal

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

4. 
   kevkevinpal commented at 3:41 pm on October 13, 2025: contributor

   ACK 45dda1b

   pretty straightforward and according to #33225 (review) seems like we can still open to master and backport this change

5. fanquake added the label Needs backport (29.x) on Oct 13, 2025
6. fanquake added the label Needs backport (30.x) on Oct 13, 2025
7. in test/functional/test_framework/test_node.py:164 in 45dda1b6c2

   160@@ -161,7 +161,7 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
   161             self.args.append("-logsourcelocations")
   162         if self.version_is_at_least(239000):
   163             self.args.append("-loglevel=trace")
   164-        if self.version_is_at_least(299900):
   165+        if self.version_is_at_least(289900):
   

   ---

   ---

   

   maflcko commented at 7:14 am on October 14, 2025:

   I don’t understand this change. Neither 28.99, nor 29.0 had this change. So appending the arg will lead to a startup error or warning there?

   0$ bitcoin-core.daemon -regtest -nologratelimit 
   1Error: Error parsing command line arguments: Invalid parameter -nologratelimit
   2$ bitcoin-core.daemon -version | grep version  
   3Bitcoin Core daemon version v29.0.0
   

   ---

   

   Crypt-iQ commented at 2:59 pm on October 14, 2025:

   Thanks, I changed this to 29.1.
8. Crypt-iQ force-pushed on Oct 14, 2025
9. test: change log rate limit version gate from 299900 to 290100 7b544341c0
10. Crypt-iQ force-pushed on Oct 14, 2025
11. Crypt-iQ renamed this:
    test: change log rate limit version gate from 299900 to 289900
    test: change log rate limit version gate
    on Oct 14, 2025
12. 
    janb84 commented at 3:07 pm on October 14, 2025: contributor

    ACK 7b544341c0021dd713f05bc439ee190de911930c

    PR changes version gate for test framework to disable the log rate limiting. The log rate limit change was backported to 29.1. This PR sets gate from v29.99 to v29.01 which has the change.

13. 
    maflcko commented at 3:17 pm on October 14, 2025: member

    In the case a user on 29.x runs the functional tests and is confused why logs are rate limited in output.

    This may also help in the future, when a previous release of 29.1 (or later in the 29.x series) is picked.

    lgtm ACK 7b544341c0021dd713f05bc439ee190de911930c

14. 
    Crypt-iQ commented at 3:27 pm on October 14, 2025: contributor

    I was testing out 29.x without this patch and was very confused why the rate limiting was not being applied. Turns out my description in the OP is wrong.

    Unless the version is explicitly set as done in the wallet_backwards_compatibility.py test, the version_is_at_least check will always return True since self.version is None. So, the functional tests in 29.x already ignore the rate limiting, but this is more correct.

15. 
    fanquake commented at 4:18 pm on October 14, 2025: member

    cc @stickies-v
16. stickies-v approved
17. 
    stickies-v commented at 4:24 pm on October 14, 2025: contributor

    ACK 7b544341c0021dd713f05bc439ee190de911930c
18. fanquake merged this on Oct 14, 2025
19. fanquake closed this on Oct 14, 2025

    ---

20. fanquake referenced this in commit ab11e84157 on Oct 14, 2025
21. fanquake removed the label Needs backport (30.x) on Oct 14, 2025
22. 
    fanquake commented at 4:28 pm on October 14, 2025: member

    Backported to 30.x in #33609.
23. 
    fanquake commented at 4:29 pm on October 14, 2025: member

    Backported to 29.x in #33611.
24. fanquake removed the label Needs backport (29.x) on Oct 14, 2025
25. fanquake referenced this in commit 554ff3f7f3 on Oct 14, 2025

Contributors
Crypt-iQ DrahtBot kevkevinpal maflcko janb84 fanquake stickies-v

Labels
Tests