signrpc: Upgrade to MuSig2 BIP draft v1.0.0rc2, add version flag to RPC #7171
Conversation
d378ccf to
a9a4d1e
Compare
There was a problem hiding this comment.
Changes lgtm!
While I wasn't a big fan of multiple api versions, it totally makes sense as in the future we might have more breaking version updates.
However now it becomes crucial for application developers to keep track of their musig version. Maybe docs/musig2.md could have a hint at that.
a9a4d1e to
d71ae53
Compare
Good idea, added the section to the docs. |
There was a problem hiding this comment.
LGTM 🦜
One small comment re if some of the other related calls need the version or not. Also do we need to hold off on merging this until a release for loop/pool? Or will they already detect (strict versioning?) that a newer lnd version is being run so they can't continue forward?
| // don't specify the version. Since this API is still declared to be | ||
| // experimental this should be the approach that leads to the least | ||
| // amount of unexpected behavior. | ||
| version, err := UnmarshalMuSig2Version(in.Version) |
There was a problem hiding this comment.
What about the Sign call? The main breaking change in musig2 1.0 was that the challenge hash ends up being changed, so the signing call should be versioned as well afaict.
Or is the idea that the context itself will now implicitly specify the version?
There was a problem hiding this comment.
Because you pass in the session ID to the Sign call, it calls it on the session struct in memory, which is of the correct type. So you only need to specify the version when creating the session (or when combining keys outside of a session).
6d6ef28 to
4e3decd
Compare
4e3decd to
7585e61
Compare
|
@guggero, remember to re-request review from reviewers when ready |
Before we change anything in the MuSig2 API, we first add an integration test that makes sure the current version of the API combines keys according to the v0.4.0 test vectors.
7585e61 to
c5e54cd
Compare
|
Rebased. Also, I was able to verify that these RPCs work in Pool to upgrade existing Taproot accounts to use this updated MuSig2 |
With this commit we copy the exact code of the MuSig2 code as found in github.com/btcsuite/btcec/v2/schnorr/musig2 at the tag btcec/v2.2.2. This corresponds to the MuSig2 BIP specification version of v0.4.0.
Since we explicitly keep an old version of a library in lnd for backward compatibility we want to make sure the purpose and version of it is clear and not misleading.
As a preparation for making it possible to version switch calls to the MuSig2 API, we move some of the calls to the input package where in a future commit we'll call the corresponding code in the correct package.
To allow us to properly test all test vectors, we can't default to true on key sorting. Instead we add a parameter to the input package and move the default value to the RPC server.
We put the calls that don't use musig2 package specific types as parameters or return values behind an interface so we can easily call those directly in the RPC without needing to know the underlying implementation version. Some calls can't be used in the interface because they use the specific package version's types. These calls are implemented in helper functions in the input package instead that do the necessary type switches.
With this commit we bump the github.com/btcd/btcec/v2 library to v2.3.2 which implements the MuSig2 BIP version v1.0.0rc2. With this the github.com/btcsuite/btcd/btcec/v2/schnorr/musig2 package becomes v1.0.0rc2 and the github.com/lightningnetwork/lnd/internal/musig2v040 stays at the old v0.4.0 version.
c5e54cd to
ad3e77c
Compare
ad3e77c to
1e13bb8
Compare
Fixes #6974.
This PR upgrades the experimental MuSig2 RPC in the
signrpcpackage to the BIP draft versionv1.0.0rc2.To remain backward compatible with applications that have on-chain funds on
keys that were created with the previous version of the MuSig2 BIP draft
v0.4.0(such as Pool accounts) a version flag was added to theMuSig2CombineKeysandMuSig2CreateSessionRPC calls. That version flag ismandatory, which means software using MuSig2 (such as Pool or Loop) must
update in order to use the new versioned RPC and upgrade any on-chain outputs
to the new version.
Upgrade/compatibility matrix
lnd v0.15.xlnd v0.15.x(e.g. Poolv0.6.1): Full compatibility ✔️lnd v0.16.x(e.g. future Pool version): Uselndversion to determine what RPC to use ✔️lnd v0.16.xlnd v0.15.x(e.g. Poolv0.6.1):lndwill throw error with hint to upgrade client version 🔼lnd v0.16.x(e.g. future Pool version): Uselndversion to determine what RPC to use ✔️