Split commit nonces from funding nonce in tx_complete (#3145)

The commit nonces and funding nonce provided in `tx_complete` are
actually completely orthogonal:

- the commit nonces must be provided whenever the *next* commitment
  format is using taproot
- the funding nonce must be provided whenever the *previous* commitment
  format is using taproot

It thus makes more sense to split them into separate TLVs.

Author: t-bast

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -722,7 +722,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
           replyTo ! RemoteFailure(cause)
           unlockAndStop(session)
         case Right(completeTx) =>
-          signCommitTx(completeTx, session.txCompleteReceived.flatMap(_.nonces_opt))
+          signCommitTx(completeTx, session.txCompleteReceived.flatMap(_.fundingNonce_opt), session.txCompleteReceived.flatMap(_.commitNonces_opt))
       }
       case _: WalletFailure =>
         replyTo ! RemoteFailure(UnconfirmedInteractiveTxInputs(fundingParams.channelId))
@@ -799,7 +799,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
         case _: SegwitV0CommitmentFormat => ()
         case _: SimpleTaprootChannelCommitmentFormat =>
           // If we're spending a taproot channel, our peer must provide a nonce for the shared input.
-          val remoteFundingNonce_opt: Option[IndividualNonce] = session.txCompleteReceived.flatMap(_.nonces_opt).flatMap(_.fundingNonce_opt)
+          val remoteFundingNonce_opt = session.txCompleteReceived.flatMap(_.fundingNonce_opt)
           if (remoteFundingNonce_opt.isEmpty) return Left(MissingFundingNonce(fundingParams.channelId, sharedInput.info.outPoint.txid))
       }
       sharedInputs.headOption match {
@@ -821,7 +821,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
     fundingParams.commitmentFormat match {
       case _: SegwitV0CommitmentFormat => ()
       case _: SimpleTaprootChannelCommitmentFormat =>
-        val remoteCommitNonces_opt = session.txCompleteReceived.flatMap(_.nonces_opt)
+        val remoteCommitNonces_opt = session.txCompleteReceived.flatMap(_.commitNonces_opt)
         if (remoteCommitNonces_opt.isEmpty) return Left(MissingCommitNonce(fundingParams.channelId, tx.txid, purpose.remoteCommitIndex))
     }
 
@@ -890,7 +890,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
     Right(sharedTx)
   }
 
-  private def signCommitTx(completeTx: SharedTransaction, remoteNonces_opt: Option[TxCompleteTlv.Nonces]): Behavior[Command] = {
+  private def signCommitTx(completeTx: SharedTransaction, remoteFundingNonce_opt: Option[IndividualNonce], remoteCommitNonces_opt: Option[TxCompleteTlv.CommitNonces]): Behavior[Command] = {
     val fundingTx = completeTx.buildUnsignedTx()
     val fundingOutputIndex = fundingTx.txOut.indexWhere(_.publicKeyScript == fundingPubkeyScript)
     val liquidityFee = fundingParams.liquidityFees(liquidityPurchase_opt)
@@ -916,7 +916,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
         val localSigOfRemoteTx = fundingParams.commitmentFormat match {
           case _: SegwitV0CommitmentFormat => Right(remoteCommitTx.sign(localFundingKey, fundingParams.remoteFundingPubKey))
           case _: SimpleTaprootChannelCommitmentFormat =>
-            remoteNonces_opt match {
+            remoteCommitNonces_opt match {
               case Some(remoteNonces) =>
                 val localNonce = NonceGenerator.signingNonce(localFundingKey.publicKey, fundingParams.remoteFundingPubKey, fundingTx.txid)
                 remoteCommitTx.partialSign(localFundingKey, fundingParams.remoteFundingPubKey, localNonce, Seq(localNonce.publicNonce, remoteNonces.commitNonce)) match {
@@ -935,13 +935,13 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
             val localCommitSig = CommitSig(fundingParams.channelId, localSigOfRemoteTx, htlcSignatures, batchSize = 1)
             val localCommit = UnsignedLocalCommit(purpose.localCommitIndex, localSpec, localCommitTx.tx.txid)
             val remoteCommit = RemoteCommit(purpose.remoteCommitIndex, remoteSpec, remoteCommitTx.tx.txid, purpose.remotePerCommitmentPoint)
-            signFundingTx(completeTx, remoteNonces_opt, localCommitSig, localCommit, remoteCommit)
+            signFundingTx(completeTx, remoteFundingNonce_opt, remoteCommitNonces_opt.map(_.nextCommitNonce), localCommitSig, localCommit, remoteCommit)
         }
     }
   }
 
-  private def signFundingTx(completeTx: SharedTransaction, remoteNonces_opt: Option[TxCompleteTlv.Nonces], commitSig: CommitSig, localCommit: UnsignedLocalCommit, remoteCommit: RemoteCommit): Behavior[Command] = {
-    signTx(completeTx, remoteNonces_opt.flatMap(_.fundingNonce_opt))
+  private def signFundingTx(completeTx: SharedTransaction, remoteFundingNonce_opt: Option[IndividualNonce], nextRemoteCommitNonce_opt: Option[IndividualNonce], commitSig: CommitSig, localCommit: UnsignedLocalCommit, remoteCommit: RemoteCommit): Behavior[Command] = {
+    signTx(completeTx, remoteFundingNonce_opt)
     Behaviors.receiveMessagePartial {
       case SignTransactionResult(signedTx) =>
         log.info(s"interactive-tx txid=${signedTx.txId} partially signed with {} local inputs, {} remote inputs, {} local outputs and {} remote outputs", signedTx.tx.localInputs.length, signedTx.tx.remoteInputs.length, signedTx.tx.localOutputs.length, signedTx.tx.remoteOutputs.length)
@@ -978,8 +978,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
           remoteCommit,
           liquidityPurchase_opt.map(_.basicInfo(isBuyer = fundingParams.isInitiator))
         )
-        val nextRemoteCommitNonce_opt = remoteNonces_opt.map(n => signedTx.txId -> n.nextCommitNonce)
-        replyTo ! Succeeded(signingSession, commitSig, liquidityPurchase_opt, nextRemoteCommitNonce_opt)
+        replyTo ! Succeeded(signingSession, commitSig, liquidityPurchase_opt, nextRemoteCommitNonce_opt.map(n => signedTx.txId -> n))
         Behaviors.stopped
       case WalletFailure(t) =>
         log.error("could not sign funding transaction: ", t)

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/InteractiveTxTlv.scala

@@ -76,21 +76,20 @@ sealed trait TxCompleteTlv extends Tlv
 
 object TxCompleteTlv {
   /**
-   * Musig2 nonces exchanged during an interactive tx session, when using a taproot channel or upgrading a channel to
-   * use taproot.
+   * Musig2 nonces for the commitment transaction(s), exchanged during an interactive tx session, when using a taproot
+   * channel or upgrading a channel to use taproot.
    *
-   * @param commitNonce      the sender's verification nonce for the current commit tx spending the interactive tx.
-   * @param nextCommitNonce  the sender's verification nonce for the next commit tx spending the interactive tx.
-   * @param fundingNonce_opt when splicing a taproot channel, the sender's random signing nonce for the previous funding output.
+   * @param commitNonce     the sender's verification nonce for the current commit tx spending the interactive tx.
+   * @param nextCommitNonce the sender's verification nonce for the next commit tx spending the interactive tx.
    */
-  case class Nonces(commitNonce: IndividualNonce, nextCommitNonce: IndividualNonce, fundingNonce_opt: Option[IndividualNonce]) extends TxCompleteTlv
+  case class CommitNonces(commitNonce: IndividualNonce, nextCommitNonce: IndividualNonce) extends TxCompleteTlv
 
-  object Nonces {
-    val codec: Codec[Nonces] = tlvField((publicNonce :: publicNonce :: optional(bitsRemaining, publicNonce)).as[Nonces])
-  }
+  /** When splicing a taproot channel, the sender's random signing nonce for the previous funding output. */
+  case class FundingInputNonce(nonce: IndividualNonce) extends TxCompleteTlv
 
   val txCompleteTlvCodec: Codec[TlvStream[TxCompleteTlv]] = tlvStream(discriminated[TxCompleteTlv].by(varint)
-    .typecase(UInt64(4), Nonces.codec)
+    .typecase(UInt64(4), tlvField[CommitNonces, CommitNonces]((publicNonce :: publicNonce).as[CommitNonces]))
+    .typecase(UInt64(6), tlvField[FundingInputNonce, FundingInputNonce](publicNonce.as[FundingInputNonce]))
   )
 }
 

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala

@@ -122,12 +122,18 @@ case class TxRemoveOutput(channelId: ByteVector32,
 
 case class TxComplete(channelId: ByteVector32,
                       tlvStream: TlvStream[TxCompleteTlv] = TlvStream.empty) extends InteractiveTxConstructionMessage with HasChannelId {
-  val nonces_opt: Option[TxCompleteTlv.Nonces] = tlvStream.get[TxCompleteTlv.Nonces]
+  val commitNonces_opt: Option[TxCompleteTlv.CommitNonces] = tlvStream.get[TxCompleteTlv.CommitNonces]
+  val fundingNonce_opt: Option[IndividualNonce] = tlvStream.get[TxCompleteTlv.FundingInputNonce].map(_.nonce)
 }
 
 object TxComplete {
-  def apply(channelId: ByteVector32, commitNonce: IndividualNonce, nextCommitNonce: IndividualNonce, fundingNonce_opt: Option[IndividualNonce]): TxComplete =
-    TxComplete(channelId, TlvStream(TxCompleteTlv.Nonces(commitNonce, nextCommitNonce, fundingNonce_opt)))
+  def apply(channelId: ByteVector32, commitNonce: IndividualNonce, nextCommitNonce: IndividualNonce, fundingNonce_opt: Option[IndividualNonce]): TxComplete = {
+    val tlvs = Set(
+      Some(TxCompleteTlv.CommitNonces(commitNonce, nextCommitNonce)),
+      fundingNonce_opt.map(TxCompleteTlv.FundingInputNonce(_)),
+    ).flatten[TxCompleteTlv]
+    TxComplete(channelId, TlvStream(tlvs))
+  }
 }
 
 case class TxSignatures(channelId: ByteVector32,

eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala

@@ -503,14 +503,14 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
       val txCompleteB3 = fwd.forwardBob2Alice[TxComplete]
       // Alice --- tx_complete --> Bob
       val txCompleteA = fwd.forwardAlice2Bob[TxComplete]
-      assert(txCompleteA.nonces_opt.nonEmpty)
-      assert(txCompleteA.nonces_opt.flatMap(_.fundingNonce_opt).isEmpty)
+      assert(txCompleteA.commitNonces_opt.nonEmpty)
+      assert(txCompleteA.fundingNonce_opt.isEmpty)
       Seq(txCompleteB1, txCompleteB2, txCompleteB3).foreach(txCompleteB => {
-        assert(txCompleteB.nonces_opt.nonEmpty)
-        assert(txCompleteB.nonces_opt.flatMap(_.fundingNonce_opt).isEmpty)
+        assert(txCompleteB.commitNonces_opt.nonEmpty)
+        assert(txCompleteB.fundingNonce_opt.isEmpty)
       })
       // Nonces change every time the shared transaction changes.
-      assert(Seq(txCompleteB1, txCompleteB2, txCompleteB3).flatMap(_.nonces_opt).flatMap(n => Seq(n.commitNonce, n.nextCommitNonce)).toSet.size == 6)
+      assert(Seq(txCompleteB1, txCompleteB2, txCompleteB3).flatMap(_.commitNonces_opt).flatMap(n => Seq(n.commitNonce, n.nextCommitNonce)).toSet.size == 6)
 
       // Alice is responsible for adding the shared output.
       assert(aliceParams.fundingAmount == fundingA)
@@ -523,8 +523,8 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
       val successB = bob2alice.expectMsgType[Succeeded]
       assert(successB.commitSig.sigOrPartialSig.isInstanceOf[PartialSignatureWithNonce])
       val (txA, _, txB, _) = fixtureParams.exchangeSigsBobFirst(bobParams, successA, successB)
-      assert(successA.nextRemoteCommitNonce_opt.contains((txA.txId, txCompleteB3.nonces_opt.get.nextCommitNonce)))
-      assert(successB.nextRemoteCommitNonce_opt.contains((txB.txId, txCompleteA.nonces_opt.get.nextCommitNonce)))
+      assert(successA.nextRemoteCommitNonce_opt.contains((txA.txId, txCompleteB3.commitNonces_opt.get.nextCommitNonce)))
+      assert(successB.nextRemoteCommitNonce_opt.contains((txB.txId, txCompleteA.commitNonces_opt.get.nextCommitNonce)))
       // The resulting transaction is valid and has the right feerate.
       assert(txA.txId == txB.txId)
       assert(txA.signedTx.lockTime == aliceParams.lockTime)
@@ -1175,9 +1175,9 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
       // Alice --- tx_complete --> Bob
       val txCompleteA = fwdSplice.forwardAlice2Bob[TxComplete]
       Seq(txCompleteA, txCompleteB).foreach(txComplete => {
-        assert(txComplete.nonces_opt.nonEmpty)
-        assert(txComplete.nonces_opt.flatMap(_.fundingNonce_opt).isEmpty) // the previous commitment didn't use taproot
-        assert(txComplete.nonces_opt.map(n => Seq(n.commitNonce, n.nextCommitNonce)).get.size == 2)
+        assert(txComplete.commitNonces_opt.nonEmpty)
+        assert(txComplete.fundingNonce_opt.isEmpty) // the previous commitment didn't use taproot
+        assert(txComplete.commitNonces_opt.map(n => Seq(n.commitNonce, n.nextCommitNonce)).get.size == 2)
       })
 
       val successA2 = alice2bob.expectMsgType[Succeeded]
@@ -1189,8 +1189,8 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
       assert(successB2.signingSession.fundingTx.localSigs.previousFundingTxPartialSig_opt.isEmpty)
       assert(successB2.commitSig.sigOrPartialSig.isInstanceOf[PartialSignatureWithNonce])
       val (spliceTxA, commitmentA2, spliceTxB, commitmentB2) = fixtureParams.exchangeSigsBobFirst(spliceFixtureParams.fundingParamsB, successA2, successB2)
-      assert(successA2.nextRemoteCommitNonce_opt.contains((spliceTxA.txId, txCompleteB.nonces_opt.get.nextCommitNonce)))
-      assert(successB2.nextRemoteCommitNonce_opt.contains((spliceTxB.txId, txCompleteA.nonces_opt.get.nextCommitNonce)))
+      assert(successA2.nextRemoteCommitNonce_opt.contains((spliceTxA.txId, txCompleteB.commitNonces_opt.get.nextCommitNonce)))
+      assert(successB2.nextRemoteCommitNonce_opt.contains((spliceTxB.txId, txCompleteA.commitNonces_opt.get.nextCommitNonce)))
       assert(spliceTxA.tx.localAmountIn > spliceTxA.tx.remoteAmountIn)
       assert(spliceTxA.signedTx.txIn.exists(_.outPoint == commitmentA1.fundingInput))
       assert(0.msat < spliceTxA.tx.localFees)
@@ -2948,13 +2948,13 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
     // Alice --- tx_add_output --> Bob
     bob ! ReceiveMessage(alice2bob.expectMsgType[SendMessage].msg.asInstanceOf[TxAddOutput])
     val txCompleteBob = bob2alice.expectMsgType[SendMessage].msg.asInstanceOf[TxComplete]
-    assert(txCompleteBob.nonces_opt.nonEmpty)
+    assert(txCompleteBob.commitNonces_opt.nonEmpty)
     alice ! ReceiveMessage(txCompleteBob)
     // Alice --- tx_complete --> Bob
     bob ! ReceiveMessage(alice2bob.expectMsgType[SendMessage].msg.asInstanceOf[TxComplete])
     // Alice <-- commit_sig --- Bob
     val successA1 = alice2bob.expectMsgType[Succeeded]
-    val invalidCommitSig = CommitSig(params.channelId, PartialSignatureWithNonce(randomBytes32(), txCompleteBob.nonces_opt.get.commitNonce), Nil, batchSize = 1)
+    val invalidCommitSig = CommitSig(params.channelId, PartialSignatureWithNonce(randomBytes32(), txCompleteBob.commitNonces_opt.get.commitNonce), Nil, batchSize = 1)
     val Left(error) = successA1.signingSession.receiveCommitSig(params.channelParamsA, params.channelKeysA, invalidCommitSig, params.nodeParamsA.currentBlockHeight)(akka.event.NoLogging)
     assert(error.isInstanceOf[InvalidCommitmentSignature])
   }

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingCreatedStateSpec.scala

@@ -124,8 +124,8 @@ class WaitForDualFundingCreatedStateSpec extends TestKitBaseClass with FixtureAn
     alice2bob.expectMsgType[TxAddOutput]
     alice2bob.forward(bob)
     val txComplete = bob2alice.expectMsgType[TxComplete]
-    assert(txComplete.nonces_opt.isDefined)
-    bob2alice.forward(alice, txComplete.copy(tlvStream = txComplete.tlvStream.copy(records = txComplete.tlvStream.records.filterNot(_.isInstanceOf[TxCompleteTlv.Nonces]))))
+    assert(txComplete.commitNonces_opt.isDefined)
+    bob2alice.forward(alice, txComplete.copy(tlvStream = txComplete.tlvStream.copy(records = txComplete.tlvStream.records.filterNot(_.isInstanceOf[TxCompleteTlv.CommitNonces]))))
     aliceListener.expectMsgType[ChannelAborted]
     awaitCond(alice.stateName == CLOSED)
   }

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForDualFundingConfirmedStateSpec.scala

@@ -993,8 +993,8 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
           assert(channelReestablish.nextCommitNonces.contains(fundingTxId))
           assert(channelReestablish.nextCommitNonces.contains(rbfTxId))
         })
-        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.nonces_opt.get.nextCommitNonce))
-        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.nonces_opt.get.nextCommitNonce))
+        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.commitNonces_opt.get.nextCommitNonce))
+        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.commitNonces_opt.get.nextCommitNonce))
     }
 
     // Alice retransmits commit_sig, and they exchange tx_signatures afterwards.
@@ -1054,8 +1054,8 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
           assert(channelReestablish.nextCommitNonces.contains(fundingTxId))
           assert(channelReestablish.nextCommitNonces.contains(rbfTxId))
         })
-        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.nonces_opt.get.nextCommitNonce))
-        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.nonces_opt.get.nextCommitNonce))
+        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.commitNonces_opt.get.nextCommitNonce))
+        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.commitNonces_opt.get.nextCommitNonce))
     }
 
     // Bob retransmits commit_sig and tx_signatures, then Alice sends her tx_signatures.
@@ -1149,8 +1149,8 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
           assert(channelReestablish.nextCommitNonces.contains(fundingTxId))
           assert(channelReestablish.nextCommitNonces.contains(rbfTx.txId))
         })
-        assert(channelReestablishAlice.nextCommitNonces.get(rbfTx.txId).contains(txCompleteAlice.nonces_opt.get.nextCommitNonce))
-        assert(channelReestablishBob.nextCommitNonces.get(rbfTx.txId).contains(txCompleteBob.nonces_opt.get.nextCommitNonce))
+        assert(channelReestablishAlice.nextCommitNonces.get(rbfTx.txId).contains(txCompleteAlice.commitNonces_opt.get.nextCommitNonce))
+        assert(channelReestablishBob.nextCommitNonces.get(rbfTx.txId).contains(txCompleteBob.commitNonces_opt.get.nextCommitNonce))
     }
 
     // Alice and Bob exchange tx_signatures and complete the RBF attempt.
@@ -1252,8 +1252,8 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
           assert(channelReestablish.nextCommitNonces.contains(currentFundingTxId))
           assert(channelReestablish.nextCommitNonces.contains(rbfTxId))
         })
-        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.nonces_opt.get.nextCommitNonce))
-        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.nonces_opt.get.nextCommitNonce))
+        assert(channelReestablishAlice.nextCommitNonces.get(rbfTxId).contains(txCompleteAlice.commitNonces_opt.get.nextCommitNonce))
+        assert(channelReestablishBob.nextCommitNonces.get(rbfTxId).contains(txCompleteBob.commitNonces_opt.get.nextCommitNonce))
     }
 
     // Alice and Bob exchange signatures and complete the RBF attempt.