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

pull theStack wants to merge 4 commits into bitcoin-core:master from theStack:split-up-eckey_pubkey_serialize-void changing 7 files +39 −75
  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. 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
  12. furszy commented at 10:10 pm on November 18, 2025: member
    Looks good, just left a minor nit. No need to tackle it.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-11-24 13:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me