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.

Author: Roasbeef

channeldb/channel.go

@@ -1551,7 +1551,7 @@ func (c *OpenChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) {
 	// If this is a taproot channel, then we'll need to generate our next
 	// verification nonce to send to the remote party. They'll use this to
 	// sign the next update to our commitment transaction.
-	var nextTaprootNonce *lnwire.Musig2Nonce
+	var nextTaprootNonce lnwire.OptMusig2NonceTLV
 	if c.ChanType.IsTaproot() {
 		taprootRevProducer, err := DeriveMusig2Shachain(
 			c.RevocationProducer,
@@ -1569,7 +1569,7 @@ func (c *OpenChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) {
 				"nonce: %w", err)
 		}
 
-		nextTaprootNonce = (*lnwire.Musig2Nonce)(&nextNonce.PubNonce)
+		nextTaprootNonce = lnwire.SomeMusig2Nonce(nextNonce.PubNonce)
 	}
 
 	return &lnwire.ChannelReestablish{

funding/manager.go

@@ -48,6 +48,14 @@ var (
 	//
 	// NOTE: for itest, this value is changed to 10ms.
 	checkPeerChannelReadyInterval = 1 * time.Second
+
+	// errNoLocalNonce is returned when a local nonce is not found in the
+	// expected TLV.
+	errNoLocalNonce = fmt.Errorf("local nonce not found")
+
+	// errNoPartialSig is returned when a partial sig is not found in the
+	// expected TLV.
+	errNoPartialSig = fmt.Errorf("partial sig not found")
 )
 
 // WriteOutpoint writes an outpoint to an io.Writer. This is not the same as
@@ -1801,17 +1809,17 @@ func (f *Manager) fundeeProcessOpenChannel(peer lnpeer.Peer,
 	}
 
 	if resCtx.reservation.IsTaproot() {
-		if msg.LocalNonce == nil {
-			err := fmt.Errorf("local nonce not set for taproot " +
-				"chan")
-			log.Error(err)
-			f.failFundingFlow(
-				resCtx.peer, cid, err,
-			)
+		localNonce, err := msg.LocalNonce.UnwrapOrErrV(errNoLocalNonce)
+		if err != nil {
+			log.Error(errNoLocalNonce)
+
+			f.failFundingFlow(resCtx.peer, cid, errNoLocalNonce)
+
+			return
 		}
 
 		remoteContribution.LocalNonce = &musig2.Nonces{
-			PubNonce: *msg.LocalNonce,
+			PubNonce: localNonce,
 		}
 	}
 
@@ -1827,13 +1835,6 @@ func (f *Manager) fundeeProcessOpenChannel(peer lnpeer.Peer,
 	log.Debugf("Remote party accepted commitment constraints: %v",
 		spew.Sdump(remoteContribution.ChannelConfig.ChannelConstraints))
 
-	var localNonce *lnwire.Musig2Nonce
-	if commitType.IsTaproot() {
-		localNonce = (*lnwire.Musig2Nonce)(
-			&ourContribution.LocalNonce.PubNonce,
-		)
-	}
-
 	// With the initiator's contribution recorded, respond with our
 	// contribution in the next message of the workflow.
 	fundingAccept := lnwire.AcceptChannel{
@@ -1854,7 +1855,12 @@ func (f *Manager) fundeeProcessOpenChannel(peer lnpeer.Peer,
 		UpfrontShutdownScript: ourContribution.UpfrontShutdown,
 		ChannelType:           chanType,
 		LeaseExpiry:           msg.LeaseExpiry,
-		LocalNonce:            localNonce,
+	}
+
+	if commitType.IsTaproot() {
+		fundingAccept.LocalNonce = lnwire.SomeMusig2Nonce(
+			ourContribution.LocalNonce.PubNonce,
+		)
 	}
 
 	if err := peer.SendMessage(true, &fundingAccept); err != nil {
@@ -2044,15 +2050,17 @@ func (f *Manager) funderProcessAcceptChannel(peer lnpeer.Peer,
 	}
 
 	if resCtx.reservation.IsTaproot() {
-		if msg.LocalNonce == nil {
-			err := fmt.Errorf("local nonce not set for taproot " +
-				"chan")
-			log.Error(err)
-			f.failFundingFlow(resCtx.peer, cid, err)
+		localNonce, err := msg.LocalNonce.UnwrapOrErrV(errNoLocalNonce)
+		if err != nil {
+			log.Error(errNoLocalNonce)
+
+			f.failFundingFlow(resCtx.peer, cid, errNoLocalNonce)
+
+			return
 		}
 
 		remoteContribution.LocalNonce = &musig2.Nonces{
-			PubNonce: *msg.LocalNonce,
+			PubNonce: localNonce,
 		}
 	}
 
@@ -2263,7 +2271,9 @@ func (f *Manager) continueFundingAccept(resCtx *reservationWithCtx,
 			return
 		}
 
-		fundingCreated.PartialSig = partialSig.ToWireSig()
+		fundingCreated.PartialSig = lnwire.MaybePartialSigWithNonce(
+			partialSig.ToWireSig(),
+		)
 	} else {
 		fundingCreated.CommitSig, err = lnwire.NewSigFromSignature(sig)
 		if err != nil {
@@ -2317,14 +2327,15 @@ func (f *Manager) fundeeProcessFundingCreated(peer lnpeer.Peer,
 	// our internal input.Signature type.
 	var commitSig input.Signature
 	if resCtx.reservation.IsTaproot() {
-		if msg.PartialSig == nil {
-			log.Errorf("partial sig not included: %v", err)
+		partialSig, err := msg.PartialSig.UnwrapOrErrV(errNoPartialSig)
+		if err != nil {
 			f.failFundingFlow(peer, cid, err)
+
 			return
 		}
 
 		commitSig = new(lnwallet.MusigPartialSig).FromWireSig(
-			msg.PartialSig,
+			&partialSig,
 		)
 	} else {
 		commitSig, err = msg.CommitSig.ToSignature()
@@ -2408,7 +2419,9 @@ func (f *Manager) fundeeProcessFundingCreated(peer lnpeer.Peer,
 			return
 		}
 
-		fundingSigned.PartialSig = partialSig.ToWireSig()
+		fundingSigned.PartialSig = lnwire.MaybePartialSigWithNonce(
+			partialSig.ToWireSig(),
+		)
 	} else {
 		fundingSigned.CommitSig, err = lnwire.NewSigFromSignature(sig)
 		if err != nil {
@@ -2565,14 +2578,15 @@ func (f *Manager) funderProcessFundingSigned(peer lnpeer.Peer,
 	// our internal input.Signature type.
 	var commitSig input.Signature
 	if resCtx.reservation.IsTaproot() {
-		if msg.PartialSig == nil {
-			log.Errorf("partial sig not included: %v", err)
+		partialSig, err := msg.PartialSig.UnwrapOrErrV(errNoPartialSig)
+		if err != nil {
 			f.failFundingFlow(peer, cid, err)
+
 			return
 		}
 
 		commitSig = new(lnwallet.MusigPartialSig).FromWireSig(
-			msg.PartialSig,
+			&partialSig,
 		)
 	} else {
 		commitSig, err = msg.CommitSig.ToSignature()
@@ -3153,8 +3167,8 @@ func (f *Manager) sendChannelReady(completeChan *channeldb.OpenChannel,
 		}
 		f.nonceMtx.Unlock()
 
-		channelReadyMsg.NextLocalNonce = (*lnwire.Musig2Nonce)(
-			&localNonce.PubNonce,
+		channelReadyMsg.NextLocalNonce = lnwire.SomeMusig2Nonce(
+			localNonce.PubNonce,
 		)
 	}
 
@@ -3824,11 +3838,9 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, //nolint:funlen
 			channelReadyMsg.AliasScid = &alias
 
 			if firstVerNonce != nil {
-				wireNonce := (*lnwire.Musig2Nonce)(
-					&firstVerNonce.PubNonce,
+				channelReadyMsg.NextLocalNonce = lnwire.SomeMusig2Nonce( //nolint:lll
+					firstVerNonce.PubNonce,
 				)
-
-				channelReadyMsg.NextLocalNonce = wireNonce
 			}
 
 			err = peer.SendMessage(true, channelReadyMsg)
@@ -3873,16 +3885,21 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, //nolint:funlen
 		log.Infof("ChanID(%v): applying local+remote musig2 nonces",
 			chanID)
 
-		if msg.NextLocalNonce == nil {
-			log.Errorf("remote nonces are nil")
+		remoteNonce, err := msg.NextLocalNonce.UnwrapOrErrV(
+			errNoLocalNonce,
+		)
+		if err != nil {
+			cid := newChanIdentifier(msg.ChanID)
+			f.failFundingFlow(peer, cid, err)
+
 			return
 		}
 
 		chanOpts = append(
 			chanOpts,
 			lnwallet.WithLocalMusigNonces(localNonce),
 			lnwallet.WithRemoteMusigNonces(&musig2.Nonces{
-				PubNonce: *msg.NextLocalNonce,
+				PubNonce: remoteNonce,
 			}),
 		)
 	}
@@ -4714,13 +4731,6 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
 	log.Infof("Starting funding workflow with %v for pending_id(%x), "+
 		"committype=%v", msg.Peer.Address(), chanID, commitType)
 
-	var localNonce *lnwire.Musig2Nonce
-	if commitType.IsTaproot() {
-		localNonce = (*lnwire.Musig2Nonce)(
-			&ourContribution.LocalNonce.PubNonce,
-		)
-	}
-
 	fundingOpen := lnwire.OpenChannel{
 		ChainHash:             *f.cfg.Wallet.Cfg.NetParams.GenesisHash,
 		PendingChannelID:      chanID,
@@ -4743,8 +4753,14 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
 		UpfrontShutdownScript: shutdown,
 		ChannelType:           chanType,
 		LeaseExpiry:           leaseExpiry,
-		LocalNonce:            localNonce,
 	}
+
+	if commitType.IsTaproot() {
+		fundingOpen.LocalNonce = lnwire.SomeMusig2Nonce(
+			ourContribution.LocalNonce.PubNonce,
+		)
+	}
+
 	if err := msg.Peer.SendMessage(true, &fundingOpen); err != nil {
 		e := fmt.Errorf("unable to send funding request message: %v",
 			err)

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