Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Use unsigned integer reading methods for PUSHDATA opcode lengths #19

Merged
merged 2 commits into from
Feb 28, 2025

Conversation

ALiberalVoluntarist
Copy link
Collaborator

Fix PUSHDATA opcode length parsing

Problem

The Script._build_chunks method was using read_int to parse the length bytes for
PUSHDATA1, PUSHDATA2, and PUSHDATA4 opcodes. Since these values should always be treated
as unsigned integers, using a method that interprets them as signed could result in
negative values when the high bit is set. This would cause incorrect parsing of the
script chunks, potentially leading to runtime errors or incorrect script execution.

Solution

This PR modifies the _build_chunks method to use the appropriate unsigned integer
reading methods:

  • read_uint8() for PUSHDATA1
  • read_uint16_le() for PUSHDATA2
  • read_uint32_le() for PUSHDATA4

This ensures that length values are always treated as unsigned, allowing for
the correct handling of large data chunks.

Testing

Added comprehensive tests that verify:

  1. Proper handling of PUSHDATA opcodes with large length values
  2. Correct parsing of edge cases (length 0, incomplete length specification)
  3. Serialization/deserialization consistency with various PUSHDATA operations

The tests ensure that scripts with PUSHDATA opcodes correctly maintain their integrity
through serialization and deserialization, and that chunk parsing works as expected.Retry

Checklist:

  • [ x] I have performed a self-review of my own code
  • [ x] My changes generate no new warnings
  • [ x] I have run the linter

This commit fixes a bug in the Script._build_chunks method where signed integer
reading methods were being used for PUSHDATA opcode length values. Since these
values should always be treated as unsigned, using signed integer methods could
result in negative values when high bits are set, causing incorrect chunk parsing.

Changed to use read_uint8, read_uint16_le, and read_uint32_le methods instead
of read_int for PUSHDATA1, PUSHDATA2, and PUSHDATA4 opcodes respectively.

Added tests to verify correct handling of large data lengths for all PUSHDATA opcodes.
Copy link
Collaborator

@voyager1708 voyager1708 left a comment

Choose a reason for hiding this comment

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

This pull request introduces several improvements and fixes.

@ALiberalVoluntarist ALiberalVoluntarist merged commit 6dcb56d into master Feb 28, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants