ACINQ / bitcoin-kmp

Kotlin Multiplatform Bitcoin Library
Apache License 2.0
68 stars 15 forks source link

Add high-level types for musig2 primitives provided by secp256k1-kmp #107

Closed sstone closed 8 months ago

sstone commented 10 months ago

This PR is based on https://github.com/ACINQ/bitcoin-kmp/pull/108 because of https://github.com/ACINQ/secp256k1-kmp/pull/93

t-bast commented 9 months ago

Why did you keep the prototype musig2 implementation (in the test folders)? Shouldn't we get rid of it entirely, we want to test the "real" implementation and have no use for the prototype one anymore?

It's dangerous to keep it around, as some of the tests in Musig2TestsCommon.kt only test that prototype implementation instead of testing the "real" code.

Or am I too early reviewing this, and it's not ready yet?

sstone commented 9 months ago

I kept it for reference but it's not supposed to be used in Musig2TestsCommon.kt I'll fix that. I don't think it's too soon to start reviewing this PR as the API in https://github.com/ACINQ/secp256k1-kmp/pull/93 should not change much (though we still have a few weeks before it gets merged).