#36184 test: fix TaprootSignatureMsg default codeseparator_pos

full analysis

https://github.com/bitcoin/bitcoin/pull/36184 · fametrano · +1/-1 in 1 files, 1 commits · labels: Tests

Goal

  • Stop a taproot test helper from raising an exception when callers omit optional arguments
  • Prevents errors for developers writing new functional tests that rely on helper defaults

Updates the default `codeseparator_pos` parameter in `TaprootSignatureMsg` in `test/functional/test_framework/script.py` from `-1` to `0xFFFFFFFF`. Because the function encodes `codeseparator_pos` as an unsigned 4-byte integer, the existing `-1` default raises an `OverflowError` whenever `scriptpath=True` is used without explicitly providing a position.

Problem: The `TaprootSignatureMsg` helper fails with an exception if a script-path caller omits `codeseparator_pos`. No in-tree functional tests currently hit this failure because existing test cases pass the argument explicitly, but it hinders developers writing new tests that rely on default arguments.

Category: Test infrastructure (#43 of 45)

P4 · bug fix

  • P4 because it fixes a broken default argument in a test helper that has no in-tree callers
  • Only affects future test authors writing new script-path test cases

P4 because it fixes a broken default argument in a test helper where no existing in-tree callers are affected. As the author notes, 'The only in-tree scriptpath caller passes codeseparator_pos explicitly, so CI does not hit it', making the impact limited to potential future test authors.

Membership: Changes a helper function in test/functional/test_framework/script.py used for generating Taproot signature messages.

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

Reviewability: Ready

  • Ready to review

The change is a 1-line test framework fix with green CI and no blocking dependencies.

Author status: active

Open concerns:

  • maflcko questioned the motivation for changing the default to 0xFFFFFFFF, suggesting that throwing an exception with -1 or None may be preferable to enforce explicit test parameters, and suggested either using None or closing the PR

Resolved concerns:

  • fametrano explained that 0xFFFFFFFF is the BIP 342 value for no codeseparator executed rather than a sentinel, avoiding an extra branch

Agreement: Mild

  • Questioned whether enforcing explicit parameters with an exception or None is preferable (maflcko)
  • Suggests using None or closing the PR (maflcko)
  • Author has not yet responded to the latest objection

maflcko questions the value of the PR and recommends closing it or using None; author has not replied to latest comment

maflcko noted that requiring explicit parameters via an exception might be preferable to defaulting to 0xFFFFFFFF, recommending that the PR be closed or changed to use None. The author has not yet responded to that latest comment.

  • maflcko asked whether codeseparator_pos could default to None like leaf_script
  • fametrano argued 0xFFFFFFFF is the BIP342 value for 'no codeseparator executed' and avoids an unnecessary branch
  • maflcko responded that test code should be explicit, noted that -1 or None achieve this by raising an exception, and recommended closing the PR or using None

Review verdicts (DrahtBot): 0

Files

2 lines under test/bench/ci.

  • test/functional/test_framework/script.py +1/-1

Card

This PR updates the default codeseparator_pos in TaprootSignatureMsg from -1 to 0xFFFFFFFF in the functional test framework. The -1 default triggers an OverflowError when serializing unsigned 4-byte integers if a caller enables scriptpath without setting the parameter. However, all in-tree tests currently specify the parameter explicitly, so no CI tests currently fail. maflcko questioned the change, suggesting that raising an exception might be desirable to enforce explicit test code and recommending either closing the PR or defaulting to None.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:58, confidence high, input hash 53a35f31e5482fe6