#35598 test: cover feeThreshold = MAX_MONEY in interface_ipc_mining.py

ReviewabilityReviewsAgreementSize
#35598 Sjors test: cover feeThreshold = MAX_MONEY in interface_ipc_mining.pyReady0 (+1)Strong0 + 26 tests
  • Prevent regressions when mining IPC clients ignore mempool fee increases and wait only for new blocks
  • Ready to review
  • Substantive support for adding dedicated test coverage (enirox001)
  • Concept approval without detailed review (jeanpablojp)
  • Past test timing issues and assertion suggestions addressed (davidgumberg, jeanpablojp, enirox001)
  • 0 lines added or modified outside tests
  • 26 lines added or modified in tests
  • 12 lines removed in total
  • 2 files, 1 commits
Mining#14 of 17
P3 · test coverage
  • P3 because it only adds functional test coverage for an existing mining IPC interface option
  • Validates that template recreation is skipped until a tip update without altering node logic
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

This pull request adds functional test coverage in `interface_ipc_mining.py` for `BlockWaitOptions` when `feeThreshold` is set to `MAX_MONEY`. It exercises the mining IPC interface scenario where the client waits exclusively for chain tip updates rather than mempool fee changes, verifying that template recreation is skipped until a new block arrives.

Problem: The mining IPC interface allows clients to ignore mempool fee increases by setting `feeThreshold` to `MAX_MONEY`, but this behavior lacked dedicated functional test coverage to prevent regressions.

Evidence of importance:

  • The PR author notes that this test coverage "ended up in #33922 at some point... but doesn't really belong there," separating the test addition into a standalone PR.

Risk of a mistake: low (tests). The patch only modifies functional test code and test framework helper functions.

Changed since the previous assessment: The PR was rebased and refined to parametrize sleep duration in `wait_and_do` and assert via `assert_debug_log` that `CreateNewBlock()` is not invoked while timing out with `MAX_MONEY`.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

Agreement

Strong

substantive support, no open objection (enirox001, enirox001)

Thread notes: The author rebased the branch after #36446.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
enirox001 (contributor)suggestion, styleAdd a comment explaining the assertion when waitNext returnsfixed · fix pushed · added suggested commentansweredyes2026-06-26: 'Would be nice if there were a comment explaining the assertion here.'
Status from: 2026-06-26: Sjors: 'Added the suggested comment.'
davidgumberg (member)objection, correctnessThe test passed even with feeThreshold = 1 because WaitAndCreateNewBlock slept through the wait_and_do window, failing to verify that MAX_MONEY prevents fee updatesfixed · fix pushed · author took the suggested fix with adjustmentschangedyes2026-07-02: 'The test still passes with waitoptions.feeThreshold = 1 That is because WaitAndCreateNewBlock() checks for a tip update immediately and then goes to sleep'
Status from: 2026-07-02: Sjors: 'Took your suggestion with some adjustments.'
davidgumberg (member)suggestion, correctnessAdd another transaction to check in an additional way that MAX_MONEY works to prevent fee-based template updatesfixed · fix pushed · author accepted and pushed suggestionansweredyes2026-07-02: 'Should probably add a transaction here to check in another way that MAX_MONEY works to prevent fee-based template updates'
Status from: 2026-07-02: Sjors: 'Also taken'
jeanpablojp (contributor)suggestion, styleFix the comment because with feeThreshold at MAX_MONEY nothing is checked at the deadline eitherfixed · fix pushed · author addressed feedback in rebaseansweredyes2026-08-27: 'With feeThreshold at MAX_MONEY the fee block in WaitAndCreateNewBlock is skipped, so nothing is checked at the deadline either. The assert is right, the comment isn't.'
Status from: 2026-08-28: Sjors: 'addressed feedback from @jeanpablojp and @enirox001.'
jeanpablojp (contributor)suggestion, correctnessAssert with assert_debug_log that CreateNewBlock() is not logged to ensure template building is actually skippedfixed · fix pushed · author addressed feedback in rebaseansweredyes2026-08-27: 'What do you think about covering that here too? with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["CreateNewBlock()"])'
Status from: 2026-08-28: Sjors: 'addressed feedback from @jeanpablojp and @enirox001.'
enirox001 (contributor)suggestion, correctnessConsume the existing mempool fee increase directly without an unnecessary send_self_transfer lambdafixed · fix pushed · author addressed feedback in rebase and enirox001 confirmed in reACKansweredyes2026-08-28: 'Would it be clearer to consume the existing fee increase directly here?'
Status from: 2026-10-13: enirox001: 'Previous feedback has been addressed, Changes LGTM.'

Support:

  • enirox001 (contributor) (ACK, 2026-06-26) [read]: The change updates the functional test to explicitly ignore mempool fee increases while waiting for a tip update, then restores the lower fee threshold for subsequent checks.
  • jeanpablojp (contributor) (Approach ACK, 2026-08-27): Approach ACK [not substantive]
  • enirox001 (contributor) (ACK, 2026-08-28) [read]: It is useful to have this coverage, and it makes sense as a standalone pr.
  • enirox001 (contributor) (ACK, 2026-10-13): Previous feedback has been addressed, Changes LGTM. [not substantive]

Participants:

  • enirox001 (contributor, support, 5 statements 2026-06-26 to 2026-10-13): ACKed, suggested explanatory comment and test cleanup, reACKed twice
  • davidgumberg (member, objection, 2 statements 2026-07-02 to 2026-07-02): pointed out timing issue in test coverage where feeThreshold = 1 still passed, and suggested second check
  • jeanpablojp (contributor, support, 3 statements 2026-08-27 to 2026-08-27): Approach ACK, suggested checking that CreateNewBlock is not logged

Files

38 lines under test/bench/ci.

  • test/functional/interface_ipc_mining.py +24/-10
  • test/functional/test_framework/ipc_util.py +2/-2

Card

The short text the ranking pass compares across PRs.

This pull request adds functional test coverage to interface_ipc_mining.py for BlockWaitOptions when feeThreshold is set to MAX_MONEY. It verifies that mining IPC clients can wait purely for chain tip updates while ignoring mempool fee accumulation without redundantly triggering block template generation. Originally extracted from work on #33922, it provides test-only verification for existing mining interface behavior.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-17T01:15, confidence high, input hash f44ec760558bd3c1
thread read through 2026-10-15 (16 statements), head 7dfc7ef17c
labels: Tests

Categories

Mining

P3 · test coverage

Adds functional test coverage for the mining IPC interface when feeThreshold is set to MAX_MONEY. As described, it verifies that template recreation is skipped until a new block arrives, preventing regressions in client tip-wait behavior.

Membership: Changes test/functional/interface_ipc_mining.py to test BlockWaitOptions behavior on the mining interface.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0