Musig2: add new MuSig2 RPC methods to `signrpc` #6361

guggero · 2022-03-23T21:05:02Z

Adds a new MuSig2 API for cooperatively producing Schnorr signatures that can be used either for Taproot internal keys or for OP_CHECKSIG signatures within a tap script tree.

Roasbeef

The final component!

Completed an early initial pass focusing mainly on the protocol and client flow, which matches up pretty closely with what I had in mind.

lnrpc/signrpc/signer.proto

Roasbeef · 2022-03-23T22:00:10Z

lnrpc/signrpc/signer.proto

+    Indicates whether all nonces required to start the signing process are known
+    now.
+    */
+    bool have_all_nonces = 4;


Won't this always be false the first time a session is created?

We can send a set of already known nonces. So for the last signer, we might already have all other nonces and therefore it can be true.

Roasbeef · 2022-03-23T22:00:35Z

lnrpc/signrpc/signer.proto

+    of 66 bytes. Can be split into the two 33-byte points to get the individual
+    nonces.
+    */
+    bytes local_public_nonces = 3;


FWIW this is only actually needed for incremental verification, but maybe it's also useful on the caller side of things?

We need to communicate the nonce to the other party, so we need to know it at some point, don't we? Or how would the nonce exchange work?

Roasbeef · 2022-03-23T22:02:05Z

lnrpc/signrpc/signer_server.go

+	// MuSig2SessionIDSize is the size of a MuSig2 session ID. It is the
+	// length of the combined public key and the two public nonces, all of
+	// them being compressed points.
+	MuSig2SessionIDSize = 99


Ah I see this sort of combines the notions of the session+context into one. If we wanted to get away with a smaller value there, then we can just hash these items and use a single 32 byte value here.

Good idea with the hashing, added that.

Roasbeef · 2022-04-12T23:32:41Z

What needs to be done to take this out of the draft stage? Merging of the upstream btcd PR?

guggero · 2022-04-13T15:24:03Z

What needs to be done to take this out of the draft stage? Merging of the upstream btcd PR?

Mostly adding the MuSig2 methods to the signer interface so they can also be forwarded to the remote signer. And cleaning up some user input validation TODOs. So not a big diff, working on it today.

guggero · 2022-04-14T16:17:01Z

Took the PR out of draft state. Everything works now, including remote signing!

While working on remote signing support for MuSig2 I noticed that the standalone Taproot functionality does not work in remote mode. Turns out we need #6268 to enable that.
I'm going to work on that in a separate PR, commented out the failing remote mode itests for now.

bhandras

The only thing remaining is to make the linter happy and fix the mobile build.

Other than those it's a super clean PR, really enjoyed reviewing! Awesome addition to LND 🚀 Great work @guggero 🥇

lntest/itest/lnd_taproot_test.go

lnrpc/signrpc/signer.proto

lightninglabs-deploy · 2022-04-22T15:17:22Z

@Roasbeef: review reminder

guggero · 2022-04-25T17:11:04Z

I added a convenience RPC called MuSig2CombineKeys to make it possible to just get the combined key and Taproot public key without creating a session.

I also went ahead and added a short doc explaining the current MuSig2 implementation and examples.

guggero · 2022-04-27T20:20:40Z

I updated to the latest version of btcsuite/btcd#1820 and added an itest for using a MuSig2 combined key in a tap leaf with a simple OP_CHECKSIG script.

I also found a small bug in the helper function TapscriptPartialReveal that is used for the fee calculation. A proof for a tree with just a single leaf can be empty, so the inclusion proof can be variable size.

Roasbeef · 2022-04-28T23:46:45Z

The btcd PR has been merged, so this can be rebased!

Roasbeef · 2022-04-28T23:52:28Z

A proof for a tree with just a single leaf can be empty, so the inclusion proof can be variable size.

Yes in this case, the verifier just takes the leaf, computes one level of tapLeaf hashing and uses that directly. You can see this interaction here if you pretend the loop below doesn't actually exist: https://github.com/btcsuite/btcd/blob/master/txscript/taproot.go#L155-L177

Roasbeef

LGTM 🌂

Roasbeef · 2022-04-28T23:53:16Z

go.mod

+require (
+	github.com/NebulousLabs/fastrand v0.0.0-20181203155948-6fb6489aac4e // indirect
+	github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
+	github.com/aead/siphash v1.0.1 // indirect


None of this goes away with a go mod tidy run w/ Go 1.18?

No, unfortunately not. Go 1.17/1.18 just puts all the indirect dependencies in their own block.

Roasbeef · 2022-04-28T23:54:29Z

input/musig2.go

+
+// MuSig2SessionID is a type for a session ID that is just a hash of the MuSig2
+// combined key and the local public nonces.
+type MuSig2SessionID = [sha256.Size]byte


FWIW this is actually a type alias (the =) sign, but not sure it actually needs to be?

Re the derivation of this: doesn't need to be accounted for yet, but w/ the new mode that lets you generate the nonce before the rest of the keys are known, we may just need to have this be derived using just our local key and the nonce info.

Ah, good point. I thought the alias would make it easier to convert between the type and the hash returned by the sha256 package. But I don't think that's really required.

About your second comment: I'll address that in a follow-up PR. Would like to get this PR in as is to unblock some work down the dependency tree.

Roasbeef · 2022-04-28T23:56:42Z

input/musig2.go

+	// already known, they can be submitted as well to reduce the number of
+	// method calls necessary later on.
+	MuSig2CreateSession(keychain.KeyLocator, []*btcec.PublicKey,
+		*MuSig2Tweaks, [][musig2.PubNonceSize]byte) (*MuSig2SessionInfo,


Hmm so we don't make the distinction between the context vs the session here? Is the param of the set of nonce optional? Looking at just this function signature, I would think no...

Looking at it again, maybe unifying things just makes the API simpler: users call into this once they have all the pubkey and nonce info.

Or is the passed nonce here actually our local nonce? Need to get further in the diff.

No, we don't make the distinction because with the previous design of the API we needed the session to get our local nonce so we could give it to the other parties.

The list of other party public nonces is not optional as in dynamic argument list. But you can pass in nil if you're the first to create a session and don't know any nonces of other participants yet.

I'll see how this looks once I've incorporated the recent API changes in the context/session. I hope I can add that functionality with minimal changes to the RPC API surface.

lntest/itest/lnd_remote_signer_test.go

lntest/itest/lnd_taproot_test.go

docs/musig2.md

Roasbeef · 2022-04-29T00:20:24Z

The new btcec tag: https://github.com/btcsuite/btcd/releases/tag/btcec%2Fv2.2.0

With this commit we add the high-level MuSig2 signing methods to the btcwallet which will later be exposed through an RPC interface.

The inclusion proof field in the TapscriptPartialReveal function was incorrect. An inclusion proof can be zero or more elements of 32-byte slices. So an empty inclusion proof can be valid too for a tree that only consists of a single leaf.

guggero added this to In progress in v0.15.0-beta via automation Mar 23, 2022

guggero added this to the v0.15.0 milestone Mar 23, 2022

Roasbeef reviewed Mar 23, 2022

View reviewed changes

guggero force-pushed the musig2 branch from f71669c to 8e3b1fc Compare March 30, 2022 12:23

carlaKC mentioned this pull request Mar 30, 2022

multi: update to lnd with musig2 lightninglabs/lndclient#97

Closed

guggero force-pushed the musig2 branch 5 times, most recently from 49d2ebb to 096feff Compare April 1, 2022 18:05

guggero force-pushed the musig2 branch from 096feff to 537b3a0 Compare April 13, 2022 13:29

Roasbeef moved this from In progress to Review in progress in v0.15.0-beta Apr 13, 2022

guggero force-pushed the musig2 branch 2 times, most recently from 9ba253e to 892d343 Compare April 14, 2022 16:13

guggero marked this pull request as ready for review April 14, 2022 16:13

guggero requested review from Roasbeef and bhandras April 14, 2022 16:17

bhandras approved these changes Apr 15, 2022

View reviewed changes

lntest/itest/lnd_taproot_test.go Outdated Show resolved Hide resolved

lnrpc/signrpc/signer.proto Outdated Show resolved Hide resolved

guggero force-pushed the musig2 branch from 892d343 to bf2327e Compare April 19, 2022 10:06

Roasbeef moved this from Review in progress to Reviewer approved + active testing in v0.15.0-beta Apr 19, 2022

guggero force-pushed the musig2 branch from bf2327e to fcccf39 Compare April 20, 2022 12:09

Roasbeef mentioned this pull request Apr 22, 2022

bip-taro: implement base asset issuance lightninglabs/taproot-assets#19

Closed

guggero mentioned this pull request Apr 23, 2022

taproot: add PSBT signing support for Taproot, fix remote signing Taproot support #6450

Merged

guggero force-pushed the musig2 branch from fcccf39 to aa8d778 Compare April 25, 2022 17:10

guggero force-pushed the musig2 branch from aa8d778 to 411cb67 Compare April 25, 2022 17:15

guggero force-pushed the musig2 branch 5 times, most recently from e42fc02 to d254573 Compare April 27, 2022 20:20

guggero force-pushed the musig2 branch from d254573 to be8a02f Compare April 28, 2022 10:37

Roasbeef approved these changes Apr 29, 2022

View reviewed changes

guggero added 2 commits April 29, 2022 09:34

mod: bump btcec to MuSig2 version, require go 1.17

be704b0

tools+lint: fix linter for Go 1.18

dfac94b

guggero force-pushed the musig2 branch from be8a02f to 81a4417 Compare April 29, 2022 07:50

guggero added 7 commits April 29, 2022 11:41

itest: refactor taproot test

fee2b28

input+btcwallet: add MuSig2 signing operations

8fc99fb

With this commit we add the high-level MuSig2 signing methods to the btcwallet which will later be exposed through an RPC interface.

multi: implement MuSig2 RPCs and remote signing

f7275c7

multi: fix inclusion proof size

e31aab5

The inclusion proof field in the TapscriptPartialReveal function was incorrect. An inclusion proof can be zero or more elements of 32-byte slices. So an empty inclusion proof can be valid too for a tree that only consists of a single leaf.

itest: add MuSig2 end-to-end tests

3ba6421

itest: add taproot SignOutputRaw Bip86 signing test

63e93ae

docs: add MuSig2 doc and release notes

93e069f

guggero force-pushed the musig2 branch from 81a4417 to 93e069f Compare April 29, 2022 09:42

guggero merged commit 9bbee09 into lightningnetwork:master Apr 29, 2022

v0.15.0-beta automation moved this from Reviewer approved + active testing to Done Apr 29, 2022

guggero deleted the musig2 branch April 29, 2022 10:55

benthecarman mentioned this pull request Apr 29, 2022

May topics austinbitdevs/austinbitdevs.com#5

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Musig2: add new MuSig2 RPC methods to `signrpc` #6361

Musig2: add new MuSig2 RPC methods to `signrpc` #6361

guggero commented Mar 23, 2022 •

edited

Roasbeef left a comment

Roasbeef Mar 23, 2022

guggero Apr 14, 2022

Roasbeef Mar 23, 2022

guggero Apr 14, 2022

Roasbeef Mar 23, 2022

guggero Apr 14, 2022

Roasbeef commented Apr 12, 2022

guggero commented Apr 13, 2022

guggero commented Apr 14, 2022

bhandras left a comment

lightninglabs-deploy commented Apr 22, 2022

guggero commented Apr 25, 2022

guggero commented Apr 27, 2022

Roasbeef commented Apr 28, 2022

Roasbeef commented Apr 28, 2022

Roasbeef left a comment

Roasbeef Apr 28, 2022

guggero Apr 29, 2022

Roasbeef Apr 28, 2022

Roasbeef Apr 28, 2022

guggero Apr 29, 2022

Roasbeef Apr 28, 2022

Roasbeef Apr 28, 2022

guggero Apr 29, 2022

Roasbeef commented Apr 29, 2022

Musig2: add new MuSig2 RPC methods to signrpc #6361

Musig2: add new MuSig2 RPC methods to signrpc #6361

Conversation

guggero commented Mar 23, 2022 • edited

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Apr 12, 2022

guggero commented Apr 13, 2022

guggero commented Apr 14, 2022

bhandras left a comment

Choose a reason for hiding this comment

lightninglabs-deploy commented Apr 22, 2022

guggero commented Apr 25, 2022

guggero commented Apr 27, 2022

Roasbeef commented Apr 28, 2022

Roasbeef commented Apr 28, 2022

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Apr 29, 2022

Musig2: add new MuSig2 RPC methods to `signrpc` #6361

Musig2: add new MuSig2 RPC methods to `signrpc` #6361

guggero commented Mar 23, 2022 •

edited