refactor: split up internal pubkey serialization function into compressed/uncompressed variants #1774

1. 
   theStack commented at 6:57 pm on November 17, 2025: contributor

   This PR splits up the pubkey serialization function secp256k1_eckey_pubkey_serialize into two variants for the compressed (33 bytes) and uncompressed (65 bytes) public key output format each, where only non-infinity group elements as input are allowed. The motivation is to simplify call-sites significantly, as they currently need to introduce two variables and a VERIFY_CHECKs on the return value and the in/out size parameter within a pre-processor block, typically leading to 8 lines of code. By using the new functions, the code is reduced to a single line of code that just calls the function (see #1773). This is helpful for already existing modules on master (ellswift, musig) and upcoming ones (silentpayments, see #1765).

   One drawback is that the public API function secp256k1_ec_pubkey_serialize is now slightly more complex (we now call one of two functions instead of a single one, depending on whether the compressed flag is set or not), but that should hopefully not be a problem.

   The commits are intentionally kept small to ease review, happy to squash them if that is preferred.

   (Kudos to w0xlt for the initial idea (https://github.com/bitcoin-core/secp256k1/pull/1765#pullrequestreview-3462461331) and to real-or-random for the suggestion to split the already existing function (https://github.com/bitcoin-core/secp256k1/issues/1773#issuecomment-3540461718).)

2. introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions fc7458ca3e
3. use new `_eckey_pubkey_serialize{33,65}` functions in public API adb76f82ea
4. use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) 0d3659c547
5. remove secp256k1_eckey_pubkey_serialize function f5e815f430
6. 
   w0xlt commented at 9:57 pm on November 17, 2025: none

   Approach ACK
7. real-or-random approved
8. 
   real-or-random commented at 7:09 am on November 18, 2025: contributor

   utACK f5e815f4306ea1763d27d83646b4f06e3d2f0b94
9. real-or-random added the label tweak/refactor on Nov 18, 2025
10. w0xlt approved
11. 
    w0xlt commented at 7:17 pm on November 18, 2025: none

    ACK https://github.com/bitcoin-core/secp256k1/pull/1774/commits/f5e815f4306ea1763d27d83646b4f06e3d2f0b94
12. in src/secp256k1.c:292 in adb76f82ea outdated

    290+            secp256k1_eckey_pubkey_serialize65(&Q, output);
    291+            *outputlen = 65;
    292         }
    293+        ret = 1;
    294     }
    295     return ret;
    

    ---

    ---

    

    furszy commented at 9:52 pm on November 18, 2025:

    In adb76f82eadadf4f9df4cd958f7c266b551b50c6:

    nit: could remove ret if you write it as:

    0if (!secp256k1_pubkey_load(ctx, &Q, pubkey)) return 0;
    1
    2if (flags & SECP256K1_FLAGS_BIT_COMPRESSION) {
    3    secp256k1_eckey_pubkey_serialize33(&Q, output);
    4    *outputlen = 33;
    5} else {
    6    secp256k1_eckey_pubkey_serialize65(&Q, output);
    7    *outputlen = 65;
    8}
    9return 1;
    

    ---

    

    theStack commented at 12:08 pm on November 19, 2025:

    Makes sense yeah, will do if I have to retouch
13. 
    furszy commented at 10:10 pm on November 18, 2025: member

    Looks good, just left a minor nit. No need to tackle it.
14. real-or-random merged this on Nov 27, 2025
15. real-or-random closed this on Nov 27, 2025

    ---

16. theStack deleted the branch on Nov 27, 2025
17. Eunovo referenced this in commit d66ec60d1e on Dec 1, 2025
18. 
    fanquake commented at 10:27 am on December 10, 2025: member

    Note that our downstream fuzzing of secp256k1 is currently broken until we land changes to cryptofuzz to adapt to the API changes here. We are using https://github.com/fanquake/cryptofuzz, however there is some activity in https://github.com/MozillaSecurity/cryptofuzz, which could be(come?) the new upstream (https://github.com/MozillaSecurity/cryptofuzz/issues/3).
19. 
    real-or-random commented at 10:54 am on December 10, 2025: contributor

    Note that our downstream fuzzing of secp256k1 is currently broken until we land changes to cryptofuzz to adapt to the API changes here. We are using fanquake/cryptofuzz, however there is some activity in MozillaSecurity/cryptofuzz, which could be(come?) the new upstream (MozillaSecurity/cryptofuzz#3).

    working on a PR to your repo

Contributors
theStack w0xlt real-or-random furszy fanquake

Labels
tweak/refactor