multi: upgrade new taproot TLVs to use tlv.OptionalRecordT

In this commit, we update new Taproot related TLVs (nonces, partial sig,
sig with nonce, etc). Along the way we were able to get rid of some
boiler plate, but most importantly, we're able to better protect against
API misuse (using a nonce that isn't initialized, etc) with the new
options API. In some areas this introduces a bit of extra boiler plate,
and where applicable I used some new helper functions to help cut down
on the noise.

Note to reviewers: this is done as a single commit, as changing the API
breaks all callers, so if we want things to compile it needs to be in a
wumbo commit.

lnwallet/chancloser/chancloser.go

@@ -49,6 +49,10 @@ var (
 	// ErrInvalidShutdownScript is returned when we receive an address from
 	// a peer that isn't either a p2wsh or p2tr address.
 	ErrInvalidShutdownScript = fmt.Errorf("invalid shutdown script")
+
+	// errNoShutdownNonce is returned when a shutdown message is received
+	// w/o a nonce for a taproot channel.
+	errNoShutdownNonce = fmt.Errorf("shutdown nonce not populated")
 )
 
 // closeState represents all the possible states the channel closer state
@@ -337,8 +341,8 @@ func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) {
 			return nil, err
 		}
 
-		shutdown.ShutdownNonce = (*lnwire.ShutdownNonce)(
-			&firstClosingNonce.PubNonce,
+		shutdown.ShutdownNonce = lnwire.SomeShutdownNonce(
+			firstClosingNonce.PubNonce,
 		)
 
 		chancloserLog.Infof("Initiating shutdown w/ nonce: %v",
@@ -548,13 +552,15 @@ func (c *ChanCloser) ReceiveShutdown(msg lnwire.Shutdown) (
 		// remote nonces so we can properly create a new musig
 		// session for signing.
 		if c.cfg.Channel.ChanType().IsTaproot() {
-			if msg.ShutdownNonce == nil {
-				return noShutdown, fmt.Errorf("shutdown " +
-					"nonce not populated")
+			shutdownNonce, err := msg.ShutdownNonce.UnwrapOrErrV(
+				errNoShutdownNonce,
+			)
+			if err != nil {
+				return noShutdown, err
 			}
 
 			c.cfg.MusigSession.InitRemoteNonce(&musig2.Nonces{
-				PubNonce: *msg.ShutdownNonce,
+				PubNonce: shutdownNonce,
 			})
 		}
 
@@ -594,13 +600,15 @@ func (c *ChanCloser) ReceiveShutdown(msg lnwire.Shutdown) (
 		// local+remote nonces so we can properly create a new musig
 		// session for signing.
 		if c.cfg.Channel.ChanType().IsTaproot() {
-			if msg.ShutdownNonce == nil {
-				return noShutdown, fmt.Errorf("shutdown " +
-					"nonce not populated")
+			shutdownNonce, err := msg.ShutdownNonce.UnwrapOrErrV(
+				errNoShutdownNonce,
+			)
+			if err != nil {
+				return noShutdown, err
 			}
 
 			c.cfg.MusigSession.InitRemoteNonce(&musig2.Nonces{
-				PubNonce: *msg.ShutdownNonce,
+				PubNonce: shutdownNonce,
 			})
 		}
 
@@ -683,10 +691,10 @@ func (c *ChanCloser) BeginNegotiation() (fn.Option[lnwire.ClosingSigned],
 }
 
 // ReceiveClosingSigned is a method that should be called whenever we receive a
-// ClosingSigned message from the wire. It may or may not return a ClosingSigned
-// of our own to send back to the remote.
-func (c *ChanCloser) ReceiveClosingSigned(msg lnwire.ClosingSigned) (
-	fn.Option[lnwire.ClosingSigned], error) {
+// ClosingSigned message from the wire. It may or may not return a
+// ClosingSigned of our own to send back to the remote.
+func (c *ChanCloser) ReceiveClosingSigned( //nolint:funlen
+	msg lnwire.ClosingSigned) (fn.Option[lnwire.ClosingSigned], error) {
 
 	noClosing := fn.None[lnwire.ClosingSigned]()
 
@@ -702,7 +710,7 @@ func (c *ChanCloser) ReceiveClosingSigned(msg lnwire.ClosingSigned) (
 		// If this is a taproot channel, then it MUST have a partial
 		// signature set at this point.
 		isTaproot := c.cfg.Channel.ChanType().IsTaproot()
-		if isTaproot && msg.PartialSig == nil {
+		if isTaproot && msg.PartialSig.IsNone() {
 			return noClosing,
 				fmt.Errorf("partial sig not set " +
 					"for taproot chan")
@@ -807,12 +815,23 @@ func (c *ChanCloser) ReceiveClosingSigned(msg lnwire.ClosingSigned) (
 		)
 		matchingSig := c.priorFeeOffers[remoteProposedFee]
 		if c.cfg.Channel.ChanType().IsTaproot() {
+			localWireSig, err := matchingSig.PartialSig.UnwrapOrErrV( //nolint:lll
+				fmt.Errorf("none local sig"),
+			)
+			if err != nil {
+				return noClosing, err
+			}
+			remoteWireSig, err := msg.PartialSig.UnwrapOrErrV(
+				fmt.Errorf("none remote sig"),
+			)
+			if err != nil {
+				return noClosing, err
+			}
+
 			muSession := c.cfg.MusigSession
-			localSig, remoteSig, closeOpts, err =
-				muSession.CombineClosingOpts(
-					*matchingSig.PartialSig,
-					*msg.PartialSig,
-				)
+			localSig, remoteSig, closeOpts, err = muSession.CombineClosingOpts( //nolint:lll
+				localWireSig, remoteWireSig,
+			)
 			if err != nil {
 				return noClosing, err
 			}
@@ -952,7 +971,9 @@ func (c *ChanCloser) proposeCloseSigned(fee btcutil.Amount) (
 	// over a partial signature which'll be combined once our offer is
 	// accepted.
 	if partialSig != nil {
-		closeSignedMsg.PartialSig = &partialSig.PartialSig
+		closeSignedMsg.PartialSig = lnwire.SomePartialSig(
+			partialSig.PartialSig,
+		)
 	}
 
 	// We'll also save this close signed, in the case that the remote party

lnwallet/chancloser/chancloser_test.go

@@ -541,11 +541,9 @@ func TestTaprootFastClose(t *testing.T) {
 	require.True(t, oShutdown.IsSome())
 	require.True(t, oClosingSigned.IsNone())
 
-	bobShutdown := oShutdown.UnsafeFromSome()
-
 	// Alice should process the shutdown message, and create a closing
 	// signed of her own.
-	oShutdown, err = aliceCloser.ReceiveShutdown(bobShutdown)
+	oShutdown, err = aliceCloser.ReceiveShutdown(oShutdown.UnwrapOrFail(t))
 	require.NoError(t, err)
 	oClosingSigned, err = aliceCloser.BeginNegotiation()
 	require.NoError(t, err)
@@ -554,7 +552,7 @@ func TestTaprootFastClose(t *testing.T) {
 	require.True(t, oShutdown.IsNone())
 	require.True(t, oClosingSigned.IsSome())
 
-	aliceClosingSigned := oClosingSigned.UnsafeFromSome()
+	aliceClosingSigned := oClosingSigned.UnwrapOrFail(t)
 
 	// Next, Bob will process the closing signed message, and send back a
 	// new one that should match exactly the offer Alice sent.
@@ -564,7 +562,7 @@ func TestTaprootFastClose(t *testing.T) {
 	require.NotNil(t, tx)
 	require.True(t, oClosingSigned.IsSome())
 
-	bobClosingSigned := oClosingSigned.UnsafeFromSome()
+	bobClosingSigned := oClosingSigned.UnwrapOrFail(t)
 
 	// At this point, Bob has accepted the offer, so he can broadcast the
 	// closing transaction, and considers the channel closed.
@@ -597,7 +595,7 @@ func TestTaprootFastClose(t *testing.T) {
 	require.NotNil(t, tx)
 	require.True(t, oClosingSigned.IsSome())
 
-	aliceClosingSigned = oClosingSigned.UnsafeFromSome()
+	aliceClosingSigned = oClosingSigned.UnwrapOrFail(t)
 
 	// Alice should now also broadcast her closing transaction.
 	_, err = lnutils.RecvOrTimeout(broadcastSignal, time.Second*1)

lnwallet/channel.go

@@ -127,6 +127,13 @@ var (
 	// we've likely lost data ourselves.
 	ErrForceCloseLocalDataLoss = errors.New("cannot force close " +
 		"channel with local data loss")
+
+	// errNoNonce is returned when a nonce is required, but none is found.
+	errNoNonce = errors.New("no nonce found")
+
+	// errNoPartialSig is returned when a partial signature is required,
+	// but none is found.
+	errNoPartialSig = errors.New("no partial signature found")
 )
 
 // ErrCommitSyncLocalDataLoss is returned in the case that we receive a valid
@@ -4128,7 +4135,7 @@ type CommitSigs struct {
 
 	// PartialSig is the musig2 partial signature for taproot commitment
 	// transactions.
-	PartialSig *lnwire.PartialSigWithNonce
+	PartialSig lnwire.OptPartialSigWithNonceTLV
 }
 
 // NewCommitState wraps the various signatures needed to properly
@@ -4341,7 +4348,7 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) {
 		CommitSigs: &CommitSigs{
 			CommitSig:  sig,
 			HtlcSigs:   htlcSigs,
-			PartialSig: partialSig,
+			PartialSig: lnwire.MaybePartialSigWithNonce(partialSig),
 		},
 		PendingHTLCs: commitDiff.Commitment.Htlcs,
 	}, nil
@@ -4416,19 +4423,24 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 	// bail out, otherwise we'll init our local session then continue as
 	// normal.
 	switch {
-	case lc.channelState.ChanType.IsTaproot() && msg.LocalNonce == nil:
+	case lc.channelState.ChanType.IsTaproot() && msg.LocalNonce.IsNone():
 		return nil, nil, nil, fmt.Errorf("remote verification nonce " +
 			"not sent")
 
-	case lc.channelState.ChanType.IsTaproot() && msg.LocalNonce != nil:
+	case lc.channelState.ChanType.IsTaproot() && msg.LocalNonce.IsSome():
 		if lc.opts.skipNonceInit {
 			// Don't call InitRemoteMusigNonces if we have already
 			// done so.
 			break
 		}
 
-		err := lc.InitRemoteMusigNonces(&musig2.Nonces{
-			PubNonce: *msg.LocalNonce,
+		nextNonce, err := msg.LocalNonce.UnwrapOrErrV(errNoNonce)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+
+		err = lc.InitRemoteMusigNonces(&musig2.Nonces{
+			PubNonce: nextNonce,
 		})
 		if err != nil {
 			return nil, nil, nil, fmt.Errorf("unable to init "+
@@ -5227,6 +5239,13 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error {
 	if lc.channelState.ChanType.IsTaproot() {
 		localSession := lc.musigSessions.LocalSession
 
+		partialSig, err := commitSigs.PartialSig.UnwrapOrErrV(
+			errNoPartialSig,
+		)
+		if err != nil {
+			return err
+		}
+
 		// As we want to ensure we never write nonces to disk, we'll
 		// use the shachain state to generate a nonce for our next
 		// local state. Similar to generateRevocation, we do height + 2
@@ -5236,7 +5255,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error {
 			nextHeight+1, lc.taprootNonceProducer,
 		)
 		nextVerificationNonce, err := localSession.VerifyCommitSig(
-			localCommitTx, commitSigs.PartialSig, localCtrNonce,
+			localCommitTx, &partialSig, localCtrNonce,
 		)
 		if err != nil {
 			close(cancelChan)
@@ -5352,9 +5371,16 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error {
 	// serialize the ECDSA sig. For taproot channels, we'll serialize the
 	// partial sig that includes the nonce that was used for signing.
 	if lc.channelState.ChanType.IsTaproot() {
+		partialSig, err := commitSigs.PartialSig.UnwrapOrErrV(
+			errNoPartialSig,
+		)
+		if err != nil {
+			return err
+		}
+
 		var sigBytes [lnwire.PartialSigWithNonceLen]byte
 		b := bytes.NewBuffer(sigBytes[0:0])
-		if err := commitSigs.PartialSig.Encode(b); err != nil {
+		if err := partialSig.Encode(b); err != nil {
 			return err
 		}
 
@@ -5783,19 +5809,21 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
 	// Now that we have a new verification nonce from them, we can refresh
 	// our remote musig2 session which allows us to create another state.
 	if lc.channelState.ChanType.IsTaproot() {
-		if revMsg.LocalNonce == nil {
-			return nil, nil, nil, nil, fmt.Errorf("next " +
-				"revocation nonce not set")
+		localNonce, err := revMsg.LocalNonce.UnwrapOrErrV(errNoNonce)
+		if err != nil {
+			return nil, nil, nil, nil, err
 		}
-		newRemoteSession, err := lc.musigSessions.RemoteSession.Refresh(
+
+		session, err := lc.musigSessions.RemoteSession.Refresh(
 			&musig2.Nonces{
-				PubNonce: *revMsg.LocalNonce,
+				PubNonce: localNonce,
 			},
 		)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
-		lc.musigSessions.RemoteSession = newRemoteSession
+
+		lc.musigSessions.RemoteSession = session
 	}
 
 	// At this point, the revocation has been accepted, and we've rotated
@@ -8506,8 +8534,8 @@ func (lc *LightningChannel) generateRevocation(height uint64) (*lnwire.RevokeAnd
 		if err != nil {
 			return nil, err
 		}
-		revocationMsg.LocalNonce = (*lnwire.Musig2Nonce)(
-			&nextVerificationNonce.PubNonce,
+		revocationMsg.LocalNonce = lnwire.SomeMusig2Nonce(
+			nextVerificationNonce.PubNonce,
 		)
 	}
 

lnwallet/channel_test.go

@@ -2875,10 +2875,10 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel,
 	// nonces.
 	if aliceChannel.channelState.ChanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceChanSyncMsg.LocalNonce,
+			PubNonce: aliceChanSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobChanSyncMsg.LocalNonce,
+			PubNonce: bobChanSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 	}
 
@@ -3486,10 +3486,10 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) {
 	// nonces.
 	if chanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceSyncMsg.LocalNonce,
+			PubNonce: aliceSyncMsg.LocalNonce.UnwrapOrFail(t).Val,
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobSyncMsg.LocalNonce,
+			PubNonce: bobSyncMsg.LocalNonce.UnwrapOrFail(t).Val,
 		}
 	}
 
@@ -3548,10 +3548,10 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) {
 	// nonces.
 	if chanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceSyncMsg.LocalNonce,
+			PubNonce: aliceSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobSyncMsg.LocalNonce,
+			PubNonce: bobSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 	}
 
@@ -3671,10 +3671,10 @@ func testChanSyncOweRevocationAndCommit(t *testing.T,
 	// nonces.
 	if chanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceSyncMsg.LocalNonce,
+			PubNonce: aliceSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobSyncMsg.LocalNonce,
+			PubNonce: bobSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 	}
 
@@ -3751,10 +3751,10 @@ func testChanSyncOweRevocationAndCommit(t *testing.T,
 	// nonces.
 	if chanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceSyncMsg.LocalNonce,
+			PubNonce: aliceSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobSyncMsg.LocalNonce,
+			PubNonce: bobSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 	}
 
@@ -3888,10 +3888,10 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T,
 	// nonces.
 	if chanType.IsTaproot() {
 		aliceChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *aliceSyncMsg.LocalNonce,
+			PubNonce: aliceSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 		bobChannel.pendingVerificationNonce = &musig2.Nonces{
-			PubNonce: *bobSyncMsg.LocalNonce,
+			PubNonce: bobSyncMsg.LocalNonce.UnwrapOrFailV(t),
 		}
 	}