From 72a591b48348b015ae29014a2cab4d3aba27fef1 Mon Sep 17 00:00:00 2001 From: notTanveer Date: Wed, 3 Jun 2026 11:45:18 +0530 Subject: [PATCH 1/2] =?UTF-8?q?refactor(sp):=20audit=20fixes=20=E2=80=94?= =?UTF-8?q?=20version=20guard,=20error=20propagation,=20memory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four targeted fixes from the post-implementation code audit: - SPOutputScope.write_to: add `if version == 2` guard so sp_data / sp_label are never written into a PSBTv0 (or version=None) stream. Without it the round-trip raises PSBTError on parse. - _sign_with_sp: remove silent `except SPFieldError: continue`. We only reach share computation when we've already resolved the private key for that input; swallowing the error hides RNG or key-op failures and returns success with an incomplete PSBT. - create_outputs: replace signing_keys list + sum() with a running accumulator, eliminating the O(n) bigint allocation before the sum. - derive_silent_payment_outputs: drop the dead `shared_secret` parameter (no caller ever passed it; body always fell through to `= ecdh_share`). --- src/embit/silent_payments/bip352.py | 26 +++++++++---------------- src/embit/silent_payments/psbt.py | 30 ++++++++++++++--------------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/embit/silent_payments/bip352.py b/src/embit/silent_payments/bip352.py index 384ff19a..55e3bc67 100644 --- a/src/embit/silent_payments/bip352.py +++ b/src/embit/silent_payments/bip352.py @@ -124,19 +124,17 @@ def create_outputs(input_privkeys, outpoints, recipients): if not input_privkeys: return {} - signing_keys = [] + a_sum = 0 for sec, is_xonly in input_privkeys: if not ec_seckey_verify(sec): raise ValueError("Invalid private key") - if is_xonly: pub = ec_pubkey_create(sec) ser = ec_pubkey_serialize(pub) if ser[0] == 0x03: sec = ec_privkey_negate(sec) - signing_keys.append(int.from_bytes(sec, "big")) + a_sum = (a_sum + int.from_bytes(sec, "big")) % SECP256K1_ORDER - a_sum = sum(signing_keys) % SECP256K1_ORDER if a_sum == 0: return {} @@ -183,7 +181,7 @@ def create_outputs(input_privkeys, outpoints, recipients): return result -def derive_silent_payment_outputs(ecdh_share, recipients, shared_secret=None): +def derive_silent_payment_outputs(ecdh_share, recipients): """ Derive silent payment outputs for recipients from a precomputed ECDH share. @@ -193,16 +191,14 @@ def derive_silent_payment_outputs(ecdh_share, recipients, shared_secret=None): The spend key of each recipient is the final per-output spend key as carried in PSBT_OUT_SP_V0_INFO (already label-tweaked when the address was labeled), - so no label tweak is applied here — the label requires the private scan key - and cannot be recomputed by a validator. ``k`` is the recipient's position - in ``recipients`` (the caller fixes the ordering). + so no label tweak is applied here. ``k`` is the recipient's position in + ``recipients`` (the caller fixes the ordering). Args: - ecdh_share: ECDH shared-secret point, serialized compressed (33 bytes), - already multiplied by input_hash. + ecdh_share: 33-byte compressed shared-secret point, already multiplied + by input_hash (validator does this before calling). recipients: ordered list of (scan_key, spend_key, label) tuples; label - is informational and unused. - shared_secret: precomputed compressed shared secret (for efficiency). + is informational and unused here. Returns: Dict mapping recipient position k to output pubkey x-only (32 bytes). @@ -210,16 +206,12 @@ def derive_silent_payment_outputs(ecdh_share, recipients, shared_secret=None): if not recipients: return {} - # Use precomputed or default to the full 33-byte compressed point (BIP-352 §1) - if shared_secret is None: - shared_secret = ecdh_share - result = {} for k, (scan_key, spend_key, _label) in enumerate(recipients): t_k = tagged_hash( "BIP0352/SharedSecret", - shared_secret + k.to_bytes(4, "big"), + ecdh_share + k.to_bytes(4, "big"), ) # P_k = B_spend + t_k·G diff --git a/src/embit/silent_payments/psbt.py b/src/embit/silent_payments/psbt.py index 46acc805..5a42af6a 100644 --- a/src/embit/silent_payments/psbt.py +++ b/src/embit/silent_payments/psbt.py @@ -178,12 +178,13 @@ def read_value(self, stream, k, version=None): def write_to(self, stream, skip_separator=False, version=None, **kwargs) -> int: r = super().write_to(stream, skip_separator=True, version=version, **kwargs) - if self.sp_data is not None: - r += ser_string(stream, b"\x09") - r += ser_string(stream, self.sp_data.serialize()) - if self.sp_label is not None: - r += ser_string(stream, b"\x0a") - r += ser_string(stream, self.sp_label.to_bytes(4, "little")) + if version == 2: + if self.sp_data is not None: + r += ser_string(stream, b"\x09") + r += ser_string(stream, self.sp_data.serialize()) + if self.sp_label is not None: + r += ser_string(stream, b"\x0a") + r += ser_string(stream, self.sp_label.to_bytes(4, "little")) if not skip_separator: r += stream.write(b"\x00") return r @@ -535,16 +536,13 @@ def _sign_with_sp(self, root, aux_rand=None) -> int: for sk_bytes, scan_key in scan_keys.items(): if sk_bytes in inp.sp_ecdh_shares: continue - try: - share = compute_ecdh_share(priv_bytes, scan_key) - proof = compute_dleq_proof( - priv_bytes, scan_key, share, aux_rand=aux_rand - ) - inp.sp_ecdh_shares[sk_bytes] = share - inp.sp_dleq_proofs[sk_bytes] = proof - counter += 1 - except SPFieldError: - continue + share = compute_ecdh_share(priv_bytes, scan_key) + proof = compute_dleq_proof( + priv_bytes, scan_key, share, aux_rand=aux_rand + ) + inp.sp_ecdh_shares[sk_bytes] = share + inp.sp_dleq_proofs[sk_bytes] = proof + counter += 1 return counter From aff9a6704e86089fb7e3c903cefd5d680fb1526e Mon Sep 17 00:00:00 2001 From: notTanveer Date: Wed, 3 Jun 2026 12:19:12 +0530 Subject: [PATCH 2/2] refactor(sp): replace local NUMS_H with ec.NUMS_PUBKEY.xonly() NUMS_PUBKEY is already defined in ec.py. NUMS_H was a redundant copy of the same 32-byte value, with its own unhexlify import. Replace both with a module-level _NUMS_XONLY = ec.NUMS_PUBKEY.xonly() so the canonical definition lives in one place. --- src/embit/silent_payments/ecdh.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/embit/silent_payments/ecdh.py b/src/embit/silent_payments/ecdh.py index 4fd868b8..678e1a80 100644 --- a/src/embit/silent_payments/ecdh.py +++ b/src/embit/silent_payments/ecdh.py @@ -2,8 +2,6 @@ BIP-375 ECDH share and DLEQ proof computation, plus input eligibility. """ -from binascii import unhexlify - from .. import ec from ..hashes import hash160 from ..misc import urandom @@ -154,12 +152,7 @@ def pubkey_hash_from_script(script, redeem_script=None): return None -# BIP-341 nothing-up-my-sleeve (NUMS) internal key. A taproot output that -# commits to H as its internal key is script-path-only and is NOT eligible for -# silent-payment shared-secret derivation (BIP-352). -NUMS_H = unhexlify( - "50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" -) +_NUMS_XONLY = ec.NUMS_PUBKEY.xonly() def witness_version(script): @@ -257,7 +250,7 @@ def get_eligible_inputs(inputs, has_sp_outputs: bool = False): # NUMS internal key -> script-path-only, not eligible. if ( inp.taproot_internal_key is not None - and inp.taproot_internal_key.xonly() == NUMS_H + and inp.taproot_internal_key.xonly() == _NUMS_XONLY ): continue eligible.append(i)