Skip to content

BIP352: align reference address decoding with silent payment rules - #2288

Open
ginavalent wants to merge 1 commit into
bitcoin:masterfrom
ginavalent:master
Open

ginavalent wants to merge 1 commit into
bitcoin:masterfrom
ginavalent:master

Conversation

@ginavalent

Copy link
Copy Markdown

BIP 352 specifies that silent payment addresses use Bech32m encoding and defines forward-compatible address decoding rules for versions 0 through 30.

The BIP352 reference implementation currently uses the generic SegWit decoder without checking the encoding type, which allows a Bech32 checksum for a silent payment address. It also rejects versions above 16 and passes the complete payload to the spend key parser instead of discarding extension data for future versions.

Update the BIP352-specific decoder to:

  • require Bech32m encoding;
  • accept versions 0 through 30 and reject version 31;
  • require exactly 66 decoded bytes for version 0;
  • accept at least 66 decoded bytes for versions 1 through 30 and consume only the first 66 bytes.

Signed-off-by: ginavalent <ginavalent@outlook.com>

@murchandamus murchandamus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. @theStack, this seems to fix a discrepancy between the specification and the reference implementation. Do you want it?

@murchandamus murchandamus added Proposed BIP modification PR by non-owner to update BIP content Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified Bug fix labels Sep 15, 2026
@theStack

Copy link
Copy Markdown
Contributor

Concept ACK

@fametrano

Copy link
Copy Markdown
Contributor

ACK c39bae2

The decoder now does what the BIP says: bech32m only, exactly 66 bytes for v0, the first 66 bytes for v1 through v30, fail on v31. At c39bae2 the five new tests pass and reference.py send_and_receive_test_vectors.json passes all 28 cases; against master's decoder four of the five new tests fail. I also ran 33 addresses, the ten from the test vectors and 23 constructed edge cases, through the BIP's rules over btclib's bech32m decoder: accept/reject and the decoded keys agree with this PR on all 33, while master differs on nine, among them accepting a v0 address with a bech32 checksum.

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 Proposed BIP modification PR by non-owner to update BIP content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants