Skip to content

Conversation

phrwlk
Copy link

@phrwlk phrwlk commented Sep 16, 2025

Replace (features & 4) with (features & 3) in the four decoding-case checks. The docstring specifies that the lower two bits (f & 3) select which of x1/x2/x3 are valid. Using (features & 4) limited the value to {0,4}, making cases 1 and 2 unreachable and bypassing proper gating when bit 4 (u >= p) was set. This aligns implementation with the documented behavior and ensures all four modes are exercised correctly.

@jonatack jonatack added Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified Bug fix labels Sep 17, 2025
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK e78b332

cc BIP authors @real-or-random @jonasschnelli @sipa @dhruv for sign-off

@real-or-random
Copy link
Contributor

I think that change is correct, but then this PR will also need to update the pregenerated packet_encoding_test_vectors.csv.

@jonatack jonatack added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants