Initial PublicKey Combine API #439

csjones · 2023-10-08T23:08:17Z

Adding Swift APIs for based on secp256k1_ec_pubkey_combine #397

TODO:

Unit Tests (source from libsecp256k1/cashu)

Swift API Usage:

let privateKey = try! secp256k1.Signing.PrivateKey()
let publicKey = privateKey.publicKey

// The Combine API arguments are an array of PublicKey objects and an optional format 
publicKey.combine([publicKey], format: .uncompressed)

what-the-diff · 2023-10-08T23:08:40Z

PR Summary

New File Addition
A new file named Combine.swift has been added under the secp256k1 folder. This is in line with our aim to enhance our code organization and make it more modular.
Extension to Public Key
An extension to secp256k1.Signing.PublicKey has been created inside Combine.swift. This extension allows for a new PublicKey to be created by merging the current public key with other public keys.
Combine Function Introduction
The new combine function has been designed to take an array of the current public keys (self) and other public keys (pubkeys). The function is designed to return a unified PublicKey object.
Internal Function Calls
To streamline the process of combining and transforming public keys, the combine function calls two internal functions secp256k1_ec_pubkey_combine and secp256k1_ec_pubkey_serialize. This allows for efficient and accurate combination and serialization operations.
Return of Combined Key
After all necessary validations are carried out, the combine function returns the combined PublicKey object ensuring enhanced collective key generation.

This pull request is designed to enhance our existing functionality by instituting improved key management and facilitating easier and more efficient public key combinations.

Sources/zkp/Combine.swift

zeugmaster · 2023-10-29T18:15:44Z

First of all, thank you so much for putting this together! Super useful for what I am trying to build.
I have been testing the code for a couple of days and been getting some unexpected behaviour, but that might be more due to me being a beginner dev. Seemingly unrelated code changes lead to the function giving me underlyingCryptoError. Also using try! instead of a do - catch block gives me the same error and I can't really figure out why.

When I try to test combining two public keys, whenever it does work, it returns a different result than the python wrapper for secp256k1, although I was careful to make sure of the right order and the fact that the python implementation only combines the keys within the list/array and not the key the function was called on.

This is how I tested in Swift:
(hashToCurvesimply takes a string and hashes/rehashes it until the hash lies on the curve)

func test() {
    let publicKey = hashToCurve(message: "test1")
    let publicKey2 = hashToCurve(message: "test2")

    print("1: " + bytesToHexString(bytes: publicKey.dataRepresentation))
    print("2: " + bytesToHexString(bytes: publicKey2.dataRepresentation))
    
    let new:secp256k1.Signing.PublicKey
        
    do {
        new = try publicKey.combine([publicKey2], format: .compressed)
        print("RESULT " + bytesToHexString(bytes: new.dataRepresentation))
    } catch {
        print(error)
    }
}

1: 021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014
2: 0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752
RESULT 034662b7e2fd30907601f3b16439ddb636792df7166ff0fe88275965eba37c5406

And this is the Python version:

point1 = hash_to_curve("test1".encode("utf-8"))
point2 = hash_to_curve("test2".encode("utf-8"))

print("1: " + point1.serialize().hex())
print("2: " + point2.serialize().hex())

new = PublicKey()
new.combine([point1.public_key, point2.public_key])

print("RESULT " + new.serialize().hex())

1: 021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014
2: 0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752
RESULT 03d6a3a9d62c7650fcac18f9ee68c7a004ebad71b7581b683062213ad9f37ddb28

Notice how the public keys 1 & 2 are equal, but the result does not match.
So far I have tried replacing pubkeys.count with allPubKeys.count which lead to the result looking like this: RESULT 02000000016fdfe810000000016fdfeca800000001e461e270000000016fdfe780.
Unfortunately that is about as far as my troubleshooting abilities allow me to go without breaking everything 😅.
I am also just starting to learn Git and Github, so if there is anything I can do to help with this PR, please let me know.
Thanks again for you help and patience!

csjones · 2023-10-30T20:12:49Z

First of all, thank you so much for putting this together! Super useful for what I am trying to build. I have been testing the code for a couple of days and been getting some unexpected behaviour, but that might be more due to me being a beginner dev. Seemingly unrelated code changes lead to the function giving me underlyingCryptoError. Also using try! instead of a do - catch block gives me the same error and I can't really figure out why.

When I try to test combining two public keys, whenever it does work, it returns a different result than the python wrapper for secp256k1, although I was careful to make sure of the right order and the fact that the python implementation only combines the keys within the list/array and not the key the function was called on.

This is how I tested in Swift: (hashToCurvesimply takes a string and hashes/rehashes it until the hash lies on the curve)
func test() {
    let publicKey = hashToCurve(message: "test1")
    let publicKey2 = hashToCurve(message: "test2")

    print("1: " + bytesToHexString(bytes: publicKey.dataRepresentation))
    print("2: " + bytesToHexString(bytes: publicKey2.dataRepresentation))
    
    let new:secp256k1.Signing.PublicKey
        
    do {
        new = try publicKey.combine([publicKey2], format: .compressed)
        print("RESULT " + bytesToHexString(bytes: new.dataRepresentation))
    } catch {
        print(error)
    }
}
1: 021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014
2: 0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752
RESULT 034662b7e2fd30907601f3b16439ddb636792df7166ff0fe88275965eba37c5406
And this is the Python version:
point1 = hash_to_curve("test1".encode("utf-8"))
point2 = hash_to_curve("test2".encode("utf-8"))

print("1: " + point1.serialize().hex())
print("2: " + point2.serialize().hex())

new = PublicKey()
new.combine([point1.public_key, point2.public_key])

print("RESULT " + new.serialize().hex())
1: 021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014
2: 0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752
RESULT 03d6a3a9d62c7650fcac18f9ee68c7a004ebad71b7581b683062213ad9f37ddb28
Notice how the public keys 1 & 2 are equal, but the result does not match. So far I have tried replacing pubkeys.count with allPubKeys.count which lead to the result looking like this: RESULT 02000000016fdfe810000000016fdfeca800000001e461e270000000016fdfe780. Unfortunately that is about as far as my troubleshooting abilities allow me to go without breaking everything 😅. I am also just starting to learn Git and Github, so if there is anything I can do to help with this PR, please let me know. Thanks again for you help and patience!

This is great feedback, thank you! 😊

I'll push up some small changes that should help figure out what's going wrong with the current implementation.

csjones · 2023-10-30T21:45:02Z

@zeugmaster I was able to re-create the issue you described, my first thought is the .map is off. Alternatively, my understanding of how to use libsecp256k1's combine pubkey API could be incorrect. I need to spend more time researching. I've pushed a small test in the meantime that's basically the issue you detailed above (thanks again for for the detailed feedback!).

zeugmaster · 2023-11-05T16:13:52Z

@csjones I've tried a few things in the meantime to narrow down the potential causes and would love to hear what you think about them:

Simplified the function to combine two static pubkeys to avoid using .map for the test. Seems to work somewhat, although the result is still not what I was expecting:

func simpleCombine(format: secp256k1.Format = .compressed) {
        let context = secp256k1.Context.rawRepresentation
        var publicKey = secp256k1_pubkey()
        var pubKeyLen = format.length
        var pubBytes = [UInt8](repeating: 0, count: pubKeyLen)

        let publicKey1 = try! secp256k1.Signing.PublicKey(dataRepresentation: "021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014".bytes, format: .compressed)
        let publicKey2 = try! secp256k1.Signing.PublicKey(dataRepresentation: "0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752".bytes, format: .compressed)
        
        print(publicKey1)
        print(publicKey2)
        
        var newPubKey1 = secp256k1_pubkey()
        publicKey1.dataRepresentation.copyToUnsafeMutableBytes(of: &newPubKey1.data)
        let pointerKey1: UnsafePointer<secp256k1_pubkey>? = withUnsafePointer(to: &newPubKey1) { $0 }
        
        var newPubKey2 = secp256k1_pubkey()
        publicKey2.dataRepresentation.copyToUnsafeMutableBytes(of: &newPubKey2.data)
        let pointerKey2: UnsafePointer<secp256k1_pubkey>? = withUnsafePointer(to: &newPubKey2) { $0 }
        
        let keys = [pointerKey1, pointerKey2]
        print(keys)
        
        let ret1 = secp256k1_ec_pubkey_combine(context, &publicKey, keys, 2)
        print(ret1)         // prints 1, according to docs the return value for valid key combination
        print(publicKey)    // prints valid secp256k1_pubkey but does not match what the python implementation produces
        
        let ret2 = secp256k1_ec_pubkey_serialize(context, &pubBytes, &pubKeyLen, &publicKey, format.rawValue)
        print(ret2)         // prints 1
        
        print(pubBytes)
}

Had ChatGPT whip up some C and slightly modified it to check how secp256k1 combination works in it's native language. This works exactly as expected and produces the same public key as the python wrapper:

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include "/opt/homebrew/Cellar/secp256k1/0.4.0/include/secp256k1.h"

int main() {
    // Initialize the secp256k1 context
    secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);

    // Two static secp256k1 public keys in serialized form (compressed format, 33 bytes)
    unsigned char pubKey1[] = {
        0x02, 0x1b, 0x4f, 0x0e, 0x98, 0x51, 0x97, 0x19, 0x98, 0xe7, 0x32, 0x07, 0x85, 0x44, 0xc9, 0x6b,
        0x36, 0xc3, 0xd0, 0x1c, 0xed, 0xf7, 0xca, 0xa3, 0x32, 0x35, 0x9d, 0x6f, 0x1d, 0x83, 0x56, 0x70, 0x14
    };

    unsigned char pubKey2[] = {
        0x02, 0x60, 0x30, 0x3a, 0xe2, 0x2b, 0x99, 0x88, 0x61, 0xbc, 0xe3, 0xb2, 0x8f, 0x33, 0xee, 0xc1,
        0xbe, 0x75, 0x8a, 0x21, 0x3c, 0x86, 0xc9, 0x3c, 0x07, 0x6d, 0xbe, 0x9f, 0x55, 0x8c, 0x11, 0xc7, 0x52
    };

    // Parse the public keys
    secp256k1_pubkey pubkey1, pubkey2;
    int ret1 = secp256k1_ec_pubkey_parse(ctx, &pubkey1, pubKey1, sizeof(pubKey1));
    int ret2 = secp256k1_ec_pubkey_parse(ctx, &pubkey2, pubKey2, sizeof(pubKey2));
    
    assert(ret1 == 1);
    assert(ret2 == 1);

    // Combine the two public keys
    const secp256k1_pubkey *pubkeys[] = { &pubkey1, &pubkey2 };
    secp256k1_pubkey combined_pubkey;
    int ret_combine = secp256k1_ec_pubkey_combine(ctx, &combined_pubkey, pubkeys, 2);

    assert(ret_combine == 1);

    // Serialize the combined public key
    unsigned char combined_serialized_pubkey[33];
    size_t combined_pubkey_len = sizeof(combined_serialized_pubkey);
    int ret_serialize = secp256k1_ec_pubkey_serialize(ctx, combined_serialized_pubkey, &combined_pubkey_len, &combined_pubkey, SECP256K1_EC_COMPRESSED);

    assert(ret_serialize == 1);

    // Do something with the combined public key, like printing it out
    printf("Combined Public Key: ");
    for (size_t i = 0; i < combined_pubkey_len; i++) {
        printf("%02x", combined_serialized_pubkey[i]);
    }
    printf("\n");

    // Destroy the secp256k1 context
    secp256k1_context_destroy(ctx);

    return 0;
}

After many hours of tinkering and staring at results at byte-level 🤦‍♂️ I am once again at my wits' end. 😅
Could it maybe have something to do with the Context?

csjones · 2023-11-09T08:04:01Z

@csjones I've tried a few things in the meantime to narrow down the potential causes and would love to hear what you think about them:

Simplified the function to combine two static pubkeys to avoid using .map for the test. Seems to work somewhat, although the result is still not what I was expecting:

func simpleCombine(format: secp256k1.Format = .compressed) {
        let context = secp256k1.Context.rawRepresentation
        var publicKey = secp256k1_pubkey()
        var pubKeyLen = format.length
        var pubBytes = [UInt8](repeating: 0, count: pubKeyLen)

        let publicKey1 = try! secp256k1.Signing.PublicKey(dataRepresentation: "021b4f0e9851971998e732078544c96b36c3d01cedf7caa332359d6f1d83567014".bytes, format: .compressed)
        let publicKey2 = try! secp256k1.Signing.PublicKey(dataRepresentation: "0260303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752".bytes, format: .compressed)
        
        print(publicKey1)
        print(publicKey2)
        
        var newPubKey1 = secp256k1_pubkey()
        publicKey1.dataRepresentation.copyToUnsafeMutableBytes(of: &newPubKey1.data)
        let pointerKey1: UnsafePointer<secp256k1_pubkey>? = withUnsafePointer(to: &newPubKey1) { $0 }
        
        var newPubKey2 = secp256k1_pubkey()
        publicKey2.dataRepresentation.copyToUnsafeMutableBytes(of: &newPubKey2.data)
        let pointerKey2: UnsafePointer<secp256k1_pubkey>? = withUnsafePointer(to: &newPubKey2) { $0 }
        
        let keys = [pointerKey1, pointerKey2]
        print(keys)
        
        let ret1 = secp256k1_ec_pubkey_combine(context, &publicKey, keys, 2)
        print(ret1)         // prints 1, according to docs the return value for valid key combination
        print(publicKey)    // prints valid secp256k1_pubkey but does not match what the python implementation produces
        
        let ret2 = secp256k1_ec_pubkey_serialize(context, &pubBytes, &pubKeyLen, &publicKey, format.rawValue)
        print(ret2)         // prints 1
        
        print(pubBytes)
}

Had ChatGPT whip up some C and slightly modified it to check how secp256k1 combination works in it's native language. This works exactly as expected and produces the same public key as the python wrapper:

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include "/opt/homebrew/Cellar/secp256k1/0.4.0/include/secp256k1.h"

int main() {
    // Initialize the secp256k1 context
    secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);

    // Two static secp256k1 public keys in serialized form (compressed format, 33 bytes)
    unsigned char pubKey1[] = {
        0x02, 0x1b, 0x4f, 0x0e, 0x98, 0x51, 0x97, 0x19, 0x98, 0xe7, 0x32, 0x07, 0x85, 0x44, 0xc9, 0x6b,
        0x36, 0xc3, 0xd0, 0x1c, 0xed, 0xf7, 0xca, 0xa3, 0x32, 0x35, 0x9d, 0x6f, 0x1d, 0x83, 0x56, 0x70, 0x14
    };

    unsigned char pubKey2[] = {
        0x02, 0x60, 0x30, 0x3a, 0xe2, 0x2b, 0x99, 0x88, 0x61, 0xbc, 0xe3, 0xb2, 0x8f, 0x33, 0xee, 0xc1,
        0xbe, 0x75, 0x8a, 0x21, 0x3c, 0x86, 0xc9, 0x3c, 0x07, 0x6d, 0xbe, 0x9f, 0x55, 0x8c, 0x11, 0xc7, 0x52
    };

    // Parse the public keys
    secp256k1_pubkey pubkey1, pubkey2;
    int ret1 = secp256k1_ec_pubkey_parse(ctx, &pubkey1, pubKey1, sizeof(pubKey1));
    int ret2 = secp256k1_ec_pubkey_parse(ctx, &pubkey2, pubKey2, sizeof(pubKey2));
    
    assert(ret1 == 1);
    assert(ret2 == 1);

    // Combine the two public keys
    const secp256k1_pubkey *pubkeys[] = { &pubkey1, &pubkey2 };
    secp256k1_pubkey combined_pubkey;
    int ret_combine = secp256k1_ec_pubkey_combine(ctx, &combined_pubkey, pubkeys, 2);

    assert(ret_combine == 1);

    // Serialize the combined public key
    unsigned char combined_serialized_pubkey[33];
    size_t combined_pubkey_len = sizeof(combined_serialized_pubkey);
    int ret_serialize = secp256k1_ec_pubkey_serialize(ctx, combined_serialized_pubkey, &combined_pubkey_len, &combined_pubkey, SECP256K1_EC_COMPRESSED);

    assert(ret_serialize == 1);

    // Do something with the combined public key, like printing it out
    printf("Combined Public Key: ");
    for (size_t i = 0; i < combined_pubkey_len; i++) {
        printf("%02x", combined_serialized_pubkey[i]);
    }
    printf("\n");

    // Destroy the secp256k1 context
    secp256k1_context_destroy(ctx);

    return 0;
}

After many hours of tinkering and staring at results at byte-level 🤦‍♂️ I am once again at my wits' end. 😅 Could it maybe have something to do with the Context?

Hey @zeugmaster, this information was super helpful, thank you! I've spent time recently playing with the code you shared and confirmed it's not the Context (though I was thinking it was sus at one point 😉 ).

Now, I am confident this code (copyToUnsafeMutableBytes in particular ) is what's causing the issue:

        var keys = allPubKeys.map {
            var newPubKey = secp256k1_pubkey()
            $0.dataRepresentation.copyToUnsafeMutableBytes(of: &newPubKey.data)
            let pointerKey: UnsafePointer<secp256k1_pubkey>? = withUnsafePointer(to: &newPubKey) { $0 }
            return pointerKey
        }

I pushed a new unit test just using the binding but replacing Context with .none raw values. Getting closer!

csjones · 2023-11-09T10:23:37Z

Hey @zeugmaster, pushed an update were the Combine API works! Let me know what your thoughts are 😁

zeugmaster · 2023-11-13T11:01:06Z

@csjones Works perfecty now!
I can not thank you enough! This was a major roadblock and I couldn't have solved or implemented it myself. These are my first steps toward working in open source and becoming a dev and your kind support with this means a lot. If we ever cross paths in real life, remind me that I owe you a beer (or coffee)! :)

csjones · 2023-11-14T14:58:48Z

@csjones Works perfecty now!

I can not thank you enough! This was a major roadblock and I couldn't have solved or implemented it myself. These are my first steps toward working in open source and becoming a dev and your kind support with this means a lot. If we ever cross paths in real life, remind me that I owe you a beer (or coffee)! :)

You're welcome, I am thrilled to hear that the API is working perfectly for you now! 🎉 No need to worry about owing me a beer or coffee, but if we do cross paths, it would be great to connect and talk Bitcoin! 😄 I appreciate the great feedback you gave, keep up the great work! 👏🚀

Initial PublicKey Combine API

5563243

csjones added the 🔏 enhancement New feature or request label Oct 8, 2023

csjones mentioned this pull request Oct 8, 2023

How to use secp256k1_ec_pubkey_combine #397

Closed

zeugmaster reviewed Oct 29, 2023

View reviewed changes

Sources/zkp/Combine.swift Outdated Show resolved Hide resolved

Adding unit test

fba5ace

Bindings test

80520fb

Initial Working API

aae9431

csjones marked this pull request as ready for review November 9, 2023 10:26

csjones linked an issue Nov 9, 2023 that may be closed by this pull request

How to use secp256k1_ec_pubkey_combine #397

Closed

Simplify secp256k1_pubkey usage

2191438

csjones force-pushed the COMBINE branch from 0da1940 to 2191438 Compare November 13, 2023 07:17

Update Readme

d23f1f8

csjones merged commit 0f5f60e into main Nov 14, 2023
2 checks passed

csjones deleted the COMBINE branch November 14, 2023 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial PublicKey Combine API #439

Initial PublicKey Combine API #439

csjones commented Oct 8, 2023 •

edited

Loading

what-the-diff bot commented Oct 8, 2023

zeugmaster commented Oct 29, 2023 •

edited

Loading

csjones commented Oct 30, 2023

csjones commented Oct 30, 2023 •

edited

Loading

zeugmaster commented Nov 5, 2023 •

edited

Loading

csjones commented Nov 9, 2023

csjones commented Nov 9, 2023

zeugmaster commented Nov 13, 2023

csjones commented Nov 14, 2023

Initial PublicKey Combine API #439

Initial PublicKey Combine API #439

Conversation

csjones commented Oct 8, 2023 • edited Loading

what-the-diff bot commented Oct 8, 2023

PR Summary

zeugmaster commented Oct 29, 2023 • edited Loading

csjones commented Oct 30, 2023

csjones commented Oct 30, 2023 • edited Loading

zeugmaster commented Nov 5, 2023 • edited Loading

csjones commented Nov 9, 2023

csjones commented Nov 9, 2023

zeugmaster commented Nov 13, 2023

csjones commented Nov 14, 2023

csjones commented Oct 8, 2023 •

edited

Loading

zeugmaster commented Oct 29, 2023 •

edited

Loading

csjones commented Oct 30, 2023 •

edited

Loading

zeugmaster commented Nov 5, 2023 •

edited

Loading