Skip to content

Conversation

@billymcbip
Copy link
Contributor

@billymcbip billymcbip commented Jan 14, 2026

  1. Remove a comment referencing a file that no longer exists in the codebase: script_invalid.json.

  2. Fix a test that isn't implemented as intended. The idea is to test execution order by providing a signature that would cause script failure when parsed. An empty signature does not cause script failure in CHECKMULTISIG. Use OP_1 for the second signature instead of OP_0.

  3. Copy existing STRICTENC tests and change the flag to DERSIG. DERSIG is a consensus flag (unlike STRICTENC), so it'd be good to have dedicated test cases.

script_tests pass on my end.

@DrahtBot DrahtBot added the Tests label Jan 14, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 14, 2026

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/34295.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK darosior

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

  • "to provide a compact way to provide" -> "to provide a compact way to present" [duplicate "provide" makes the phrase ungrammatical/awkward and hinders comprehension]

2026-01-30 14:34:16

Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 63ff7c4

Could you add the context given in OP to the commit messages? It's useful to be able to have the context for a change (especially one that isn't immediate such as this one) when git blaming or searching through the git history, without having to rely on Github.

script_invalid.json no longer exists.
Fix a test that isn't implemented as intended. The idea is to test execution order by providing a signature that would cause script failure when parsed. An empty signature does not cause script failure in CHECKMULTISIG. Use OP_1 for the second signature instead of OP_0.
Copy existing STRICTENC tests and change the flag to DERSIG.
Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reACK 4dfb6ee

@fanquake fanquake requested a review from sipa January 30, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants