Skip to content

Initial BIP-352 support#64

Open
kdmukai wants to merge 2 commits intodiybitcoinhardware:masterfrom
kdmukai:initial_bip352
Open

Initial BIP-352 support#64
kdmukai wants to merge 2 commits intodiybitcoinhardware:masterfrom
kdmukai:initial_bip352

Conversation

@kdmukai
Copy link
Copy Markdown
Contributor

@kdmukai kdmukai commented Aug 26, 2024

Very basic initial implementation of BIP-352 Silent Payments.

This small start only generates the shareable, reusable Silent Payment address.

Rough TODO list for full SP support is included in the python comments, some of which are dependant on not-yet-settled discussions. Other parts that could be implemented now are, to be honest, beyond my abilities. Hopefully this starting point will encourage others to start filling in those holes.

@miketlk
Copy link
Copy Markdown
Collaborator

miketlk commented Mar 26, 2026

AI review of BIP-352 implementation against the spec

(Take it with a grain of salt.)

  1. Non-spec labels accepted (high): Code and tests allow str/bytes labels, but BIP-352 labels are ser_32(m) integers. This can break interoperability/recovery across compliant wallets.
    src/embit/bip352.py:26 src/embit/bip352.py:35 tests/tests/test_bip352.py:74 bips/bip-0352.mediawiki:201 bips/bip-0352.mediawiki:139

  2. No compressed-key / 66-byte enforcement for v0 address payload (high): Address generation uses raw sec() concatenation without checking compressed keys, so uncompressed pubkeys can produce non-compliant v0 addresses.
    src/embit/bip352.py:20 bips/bip-0352.mediawiki:141 bips/bip-0352.mediawiki:180

  3. BIP352 tests not in default suite (high): test_bip352.py is not imported by tests/tests/__init__.py; it also depends on pytest, so it can be skipped in normal unittest runs.
    tests/tests/__init__.py:1 tests/tests/test_bip352.py:9

  4. Network argument handling can produce wrong HRP (medium): Logic treats only literal "main" as mainnet; passing network dicts (common elsewhere in repo) yields testnet HRP (tsp) even for mainnet.
    src/embit/bip352.py:21 src/embit/script.py:15 bips/bip-0352.mediawiki:204

  5. Missing explicit scalar-validity checks for label tweak (medium): Spec requires failure on invalid scalars (0 or >=n). Current label tweak path relies on backend behavior; consistency risk across ctypes vs pure-python secp backends.
    src/embit/bip352.py:42 src/embit/util/ctypes_secp256k1.py:677 src/embit/util/py_secp256k1.py:229 bips/bip-0352.mediawiki:302

  6. Version semantics under-specified (medium): Function accepts arbitrary version but does not enforce/reject according to BIP-352 v0 rules (including v31 fail behavior).
    src/embit/bip352.py:16 bips/bip-0352.mediawiki:179 bips/bip-0352.mediawiki:182

  7. Test coverage is much narrower than spec vectors/functional requirements (medium): Current tests only cover address construction basics and label samples, missing sender/receiver protocol behaviors and edge cases from official vectors.
    tests/tests/test_bip352.py:15 bips/bip-0352.mediawiki:379 bips/bip-0352.mediawiki:444

  8. Architectural incompleteness vs full BIP scope (medium): Module currently implements only address construction; sender output derivation, receiver scanning, and spending flows are not implemented.
    src/embit/bip352.py:5 bips/bip-0352.mediawiki:283 bips/bip-0352.mediawiki:333 bips/bip-0352.mediawiki:362

@kdmukai , could you please review this report?

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