secp256k1: implement ec_pubkey_tweak_mul and ec_pubkey_combine fallbacks

[kdmukai]

Summary

The pure-Python py_secp256k1 backend was missing implementations of ec_pubkey_tweak_mul and ec_pubkey_combine (commented out as unfinished stubs since the file's introduction). Now that prebuilt libsecp256k1 binaries are no longer shipped (#115), callers that lack a system libsecp256k1 fall back to py_secp256k1 and hit AttributeError when these functions are needed.

This PR adds working implementations of both functions, built on the existing _key.SECP256K1.mul / add primitives, plus tests.

Changes

• src/embit/util/py_secp256k1.py:
  • Implement ec_pubkey_tweak_mul(pub, tweak) — mutates pub in place (requires bytearray), matching the existing ec_pubkey_tweak_add convention. Rejects zero and out-of-range tweaks per the libsecp256k1 spec.
  • Implement ec_pubkey_combine(*pubkeys) — returns the combined pubkey. Rejects empty input and combinations summing to the point at infinity.
• tests/tests/test_bindings.py:
  • 7 standalone tests for ec_pubkey_tweak_mul: identity, generator-equivalence, scalar composition, zero/overflow/length rejection, and 10 canonical BIP-47 ECDH spec vectors.
  • 6 standalone tests for ec_pubkey_combine: single-arg no-op, doubling identity (combine(P, P) == tweak_mul(P, 2)), three-arg associativity, infinity rejection, arg validation.
  • Parity assertions added to the existing test_cross (skipped when libsecp256k1 is unavailable, per the existing pattern).

References

• libsecp256k1 API: secp256k1_ec_pubkey_tweak_mul, secp256k1_ec_pubkey_combine (header docs).
• Algebraic identities mirror upstream property tests in bitcoin-core/secp256k1/src/tests.c.
• BIP-47 test vectors: BIP-47 spec, vectors at https://gist.github.com/SamouraiDev/6aad669604c5930864bd.

Test results

• Without libsecp256k1: 14 new tests pass; 4 parity tests skip.
• With libsecp256k1 available: all 18 tests pass, including byte-for-byte parity against the C implementation.

[Copilot]

Pull request overview

This PR adds pure-Python fallback support for secp256k1 public-key tweak multiplication and public-key combination, addressing missing functionality now that native libsecp256k1 binaries are no longer bundled.

Changes:

• Implements ec_pubkey_tweak_mul and ec_pubkey_combine in py_secp256k1.
• Adds standalone algebraic/vector tests for both functions.
• Adds ctypes parity checks when native libsecp256k1 is available.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/embit/util/py_secp256k1.py	Adds pure-Python implementations for public-key tweak multiplication and public-key combination.
tests/tests/test_bindings.py	Adds coverage for the new fallback functions and parity checks against ctypes bindings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lucasbalieiro]

@qlrd, I think this makes sense.

According to PEP 3137:

- bytes is an immutable array of bytes (PyString)
- bytearray is a mutable array of bytes (PyBytes)
- memoryview is a bytes view on another object (PyMemory)

So at runtime this loop may raise a TypeError:

for i in range(len(pub)):
    pub[i] = res[i]

because pub may be a bytes object, which is immutable. In that case the assignment would fail with something like:

TypeError: 'bytes' object does not support item assignment

Looking at the suggested code paths, ec_pubkey_parse() returns a bytes object:

embit/src/embit/util/ctypes_secp256k1.py

Lines 590 to 594 in 49d9004

	pub = bytes(64)
	r = _secp.secp256k1_ec_pubkey_parse(context, pub, sec, len(sec))
	if r == 0:
	raise ValueError("Failed parsing public key")
	return pub

and that value is passed directly to ec_pubkey_tweak_mul():

embit/src/embit/liquid/transaction.py

Lines 85 to 86 in 49d9004

	pub = secp256k1.ec_pubkey_parse(pubkey)
	secp256k1.ec_pubkey_tweak_mul(pub, blinding_key)

So this will fail because we are trying to mutate an immutable bytes object.

[miketlk]

Lucas, thank you for your analysis and explanations. I addressed this issue in my commit 7673240. Your feedback is very welcome.

[lucasbalieiro]

@miketlk, seems like a nice patch adding _ensure_mutable_key() helper that raises TypeError.

Is way batter then silently just doing pub = bytearray(pub) inside of ec_pubkey_tweak_mul

I also cloned the repo and executed the tests locally and everything seems to be working properly.

[odudex]

AI bros and me reached the same conclusion as Copilot.
I was able to reproduce the same compatibility issue under the pure-Python backend.

The new implementation is correct for mutable buffers, and the standalone tests cover that path well. The remaining problem is that the production Liquid paths pass the direct bytes result of ec_pubkey_parse(...):

• src/embit/liquid/transaction.py: pub = secp256k1.ec_pubkey_parse(pubkey) then ec_pubkey_tweak_mul(pub, blinding_key)
• src/embit/liquid/pset.py: same pattern in reblind

With pure Python forced, this raises:

TypeError: 'bytes' object does not support item assignment

So the fix probably needs to preserve the existing wrapper-level API for callers, not only the low-level libsecp256k1 “mutable output” convention. A good regression test would exercise the real call shape:

pub = py_secp256k1.ec_pubkey_parse(serialized_pubkey)
py_secp256k1.ec_pubkey_tweak_mul(pub, tweak)

Suggested fix options: either update the relevant call sites to explicitly use bytearray(pub), or make the fallback implementation tolerate immutable bytes consistently with the ctypes wrapper behavior. The latter seems less likely to leave external callers with the same surprise.