test: cover invalid codesep positions for signature in taproot #32301

1. 
   instagibbs commented at 8:18 PM on April 17, 2025: member

   There is some basic coverage, but I felt like adding some boundary conditions where the only issue is the codesep value would be nice.

2. 
   DrahtBot commented at 8:18 PM on April 17, 2025: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ajtowns, TheCharlatan
   Stale ACK	sipa

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

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   No conflicts as of last run.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

3. DrahtBot added the label Tests on Apr 17, 2025
4. 
   sipa commented at 9:51 PM on April 17, 2025: member

   ACK 175740991e0081c35f471cd7f1ad067e3e1f978f

5. fanquake requested review from ajtowns on Apr 25, 2025
6. DrahtBot added the label Needs rebase on May 6, 2025
7. instagibbs force-pushed on May 30, 2025
8. instagibbs force-pushed on May 30, 2025
9. bitcoin deleted a comment on May 30, 2025
10. 
    instagibbs commented at 2:11 PM on May 30, 2025: member

    rebased due to merge conflict

11. DrahtBot removed the label Needs rebase on May 30, 2025
12. in test/functional/test_framework/script.py:850 in 89646cbd82 outdated

     848 | @@ -849,7 +849,7 @@ def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index=0, *, scriptpa
     849 |      if scriptpath:
     850 |          ss += TaggedHash("TapLeaf", bytes([leaf_ver]) + ser_string(leaf_script))
     851 |          ss += bytes([0])
     852 | -        ss += codeseparator_pos.to_bytes(4, "little", signed=True)
     853 | +        ss += codeseparator_pos.to_bytes(4, "little", signed=False)
    

    ---

    ---

    

    ajtowns commented at 5:48 AM on June 16, 2025:

    Why this change? Having a code separator at position 2**31 or more doesn't seem very plausible given the block weight limit is lower than 2**22, and writing 0xffff_ffff as -1 and 0xffff_fffe as -2 seems fine?

    ---

    

    instagibbs commented at 1:27 PM on June 16, 2025:

    Having a code separator at position 231 or more doesn't seem very plausible given the block weight limit is lower than 222

    This might be an argument for more coverage, but not against adding coverage. Obviously the case will never make it into the "success" column without a a hardfork.

    writing 0xffff_ffff as -1 and 0xffff_fffe as -2 seems fine?

    I think writing test code to be obvious is better, all else equal.

13. DrahtBot added the label Needs rebase on Aug 11, 2025
14. instagibbs force-pushed on Aug 14, 2025
15. 
    instagibbs commented at 11:50 AM on August 14, 2025: member

    rebased

16. DrahtBot added the label CI failed on Aug 14, 2025
17. DrahtBot removed the label Needs rebase on Aug 14, 2025
18. 
    maflcko commented at 12:04 PM on August 18, 2025: member

    CI failure:

    [08:01:28.655] �[0;36m                                   OverflowError: can't convert negative int to unsigned�[0m
    

19. test: cover invalid codesep positions for signature in taproot 81e5c8385b
20. instagibbs force-pushed on Aug 18, 2025
21. DrahtBot removed the label CI failed on Aug 18, 2025
22. ajtowns approved
23. 
    ajtowns commented at 9:41 PM on November 3, 2025: contributor

    ACK 81e5c8385b9ec170c97190a97c560a39ccfc544a

24. DrahtBot requested review from sipa on Nov 3, 2025
25. TheCharlatan approved
26. 
    TheCharlatan commented at 10:08 PM on November 3, 2025: contributor

    ACK 81e5c8385b9ec170c97190a97c560a39ccfc544a

27. fanquake merged this on Nov 4, 2025
28. fanquake closed this on Nov 4, 2025

    ---

Contributors

instagibbsDrahtBotsipaajtownsmaflckoTheCharlatan

Review Requested
sipa