Address @sstone comments

Author: t-bast

docs/release-notes/eclair-vnext.md

@@ -6,7 +6,7 @@
 
 ### Simplified mutual close
 
-This release includes support for the latest [mutual close protocol](https://github.com/lightning/bolts/pull/1096).
+This release includes support for the latest [mutual close protocol](https://github.com/lightning/bolts/pull/1205).
 This protocol allows both channel participants to decide exactly how much fees they're willing to pay to close the channel.
 Each participant obtains a channel closing transaction where they are paying the fees.
 

eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

@@ -635,7 +635,7 @@ object Helpers {
           case Success(OP_0 :: OP_PUSHDATA(pubkeyHash, _) :: Nil) if pubkeyHash.size == 20 => true
           case Success(OP_0 :: OP_PUSHDATA(scriptHash, _) :: Nil) if scriptHash.size == 32 => true
           case Success((OP_1 | OP_2 | OP_3 | OP_4 | OP_5 | OP_6 | OP_7 | OP_8 | OP_9 | OP_10 | OP_11 | OP_12 | OP_13 | OP_14 | OP_15 | OP_16) :: OP_PUSHDATA(program, _) :: Nil) if allowAnySegwit && 2 <= program.length && program.length <= 40 => true
-          case Success(OP_RETURN :: _) if allowOpReturn => true
+          case Success(OP_RETURN :: OP_PUSHDATA(data, code) :: Nil) if allowOpReturn => OP_PUSHDATA.isMinimal(data, code) && data.size >= 6 && data.size <= 80
           case _ => false
         }
       }

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala

@@ -752,7 +752,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
             if (d.commitments.hasNoPendingHtlcsOrFeeUpdate) {
               // there are no pending signed changes, let's directly negotiate a closing transaction
               if (Features.canUseFeature(d.commitments.params.localParams.initFeatures, d.commitments.params.remoteParams.initFeatures, Features.SimpleClose)) {
-                startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates, sendList)
+                val (d1, closingComplete_opt) = startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates)
+                goto(NEGOTIATING_SIMPLE) using d1 storing() sending sendList ++ closingComplete_opt.toSeq
               } else if (d.commitments.params.localParams.paysClosingFees) {
                 // we pay the closing fees, so we initiate the negotiation by sending the first closing_signed
                 val (closingTx, closingSigned) = MutualClose.makeFirstClosingTx(keyManager, d.commitments.latest, localShutdown.scriptPubKey, remoteShutdownScript, nodeParams.currentFeeratesForFundingClosing, nodeParams.onChainFeeConf, d.closingFeerates)
@@ -1536,7 +1537,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
               context.system.eventStream.publish(ChannelSignatureReceived(self, commitments1))
               if (commitments1.hasNoPendingHtlcsOrFeeUpdate) {
                 if (Features.canUseFeature(d.commitments.params.localParams.initFeatures, d.commitments.params.remoteParams.initFeatures, Features.SimpleClose)) {
-                  startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates, revocation :: Nil)
+                  val (d1, closingComplete_opt) = startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates)
+                  goto(NEGOTIATING_SIMPLE) using d1 storing() sending revocation +: closingComplete_opt.toSeq
                 } else if (d.commitments.params.localParams.paysClosingFees) {
                   // we pay the closing fees, so we initiate the negotiation by sending the first closing_signed
                   val (closingTx, closingSigned) = MutualClose.makeFirstClosingTx(keyManager, commitments1.latest, localShutdown.scriptPubKey, remoteShutdown.scriptPubKey, nodeParams.currentFeeratesForFundingClosing, nodeParams.onChainFeeConf, closingFeerates)
@@ -1580,7 +1582,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
           if (commitments1.hasNoPendingHtlcsOrFeeUpdate) {
             log.debug("switching to NEGOTIATING spec:\n{}", commitments1.latest.specs2String)
             if (Features.canUseFeature(d.commitments.params.localParams.initFeatures, d.commitments.params.remoteParams.initFeatures, Features.SimpleClose)) {
-              startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates, Nil)
+              val (d1, closingComplete_opt) = startSimpleClose(d.commitments, localShutdown, remoteShutdown, d.closingFeerates)
+              goto(NEGOTIATING_SIMPLE) using d1 storing() sending closingComplete_opt.toSeq
             } else if (d.commitments.params.localParams.paysClosingFees) {
               // we pay the closing fees, so we initiate the negotiation by sending the first closing_signed
               val (closingTx, closingSigned) = MutualClose.makeFirstClosingTx(keyManager, commitments1.latest, localShutdown.scriptPubKey, remoteShutdown.scriptPubKey, nodeParams.currentFeeratesForFundingClosing, nodeParams.onChainFeeConf, closingFeerates)

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/CommonHandlers.scala

@@ -23,7 +23,7 @@ import fr.acinq.eclair.channel.Helpers.Closing.MutualClose
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.db.PendingCommandsDb
 import fr.acinq.eclair.io.Peer
-import fr.acinq.eclair.wire.protocol.{HtlcSettlementMessage, LightningMessage, Shutdown, UpdateMessage}
+import fr.acinq.eclair.wire.protocol.{ClosingComplete, HtlcSettlementMessage, LightningMessage, Shutdown, UpdateMessage}
 import scodec.bits.ByteVector
 
 import scala.concurrent.duration.DurationInt
@@ -132,19 +132,19 @@ trait CommonHandlers {
     finalScriptPubKey
   }
 
-  def startSimpleClose(commitments: Commitments, localShutdown: Shutdown, remoteShutdown: Shutdown, closingFeerates: Option[ClosingFeerates], toSend: List[LightningMessage]) = {
+  def startSimpleClose(commitments: Commitments, localShutdown: Shutdown, remoteShutdown: Shutdown, closingFeerates: Option[ClosingFeerates]): (DATA_NEGOTIATING_SIMPLE, Option[ClosingComplete]) = {
     val localScript = localShutdown.scriptPubKey
     val remoteScript = remoteShutdown.scriptPubKey
     val closingFeerate = closingFeerates.map(_.preferred).getOrElse(nodeParams.onChainFeeConf.getClosingFeerate(nodeParams.currentBitcoinCoreFeerates))
     MutualClose.makeSimpleClosingTx(nodeParams.currentBlockHeight, keyManager, commitments.latest, localScript, remoteScript, closingFeerate) match {
       case Left(f) =>
         log.warning("cannot create local closing txs, waiting for remote closing_complete: {}", f.getMessage)
         val d = DATA_NEGOTIATING_SIMPLE(commitments, closingFeerate, localScript, remoteScript, Nil, Nil)
-        goto(NEGOTIATING_SIMPLE) using d storing() sending toSend
+        (d, None)
       case Right((closingTxs, closingComplete)) =>
         log.debug("signing local mutual close transactions: {}", closingTxs)
         val d = DATA_NEGOTIATING_SIMPLE(commitments, closingFeerate, localScript, remoteScript, closingTxs :: Nil, Nil)
-        goto(NEGOTIATING_SIMPLE) using d storing() sending toSend :+ closingComplete
+        (d, Some(closingComplete))
     }
   }
 

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

@@ -923,9 +923,9 @@ object Transactions {
     (toLocalOutput_opt, toRemoteOutput_opt) match {
       case (Some(toLocalOutput), Some(toRemoteOutput)) =>
         val txLocalAndRemote = LexicographicalOrdering.sort(txNoOutput.copy(txOut = Seq(toLocalOutput, toRemoteOutput)))
-        val toLocalOutputInfo = findPubKeyScriptIndex(txLocalAndRemote, localScriptPubKey).map(index => OutputInfo(index, toLocalOutput.amount, localScriptPubKey)).toOption
+        val Right(toLocalOutputInfo) = findPubKeyScriptIndex(txLocalAndRemote, localScriptPubKey).map(index => OutputInfo(index, toLocalOutput.amount, localScriptPubKey))
         ClosingTxs(
-          localAndRemote_opt = Some(ClosingTx(input, txLocalAndRemote, toLocalOutputInfo)),
+          localAndRemote_opt = Some(ClosingTx(input, txLocalAndRemote, Some(toLocalOutputInfo))),
           // We also provide a version of the transaction without the remote output, which they may want to omit if not economical to spend.
           localOnly_opt = Some(ClosingTx(input, txNoOutput.copy(txOut = Seq(toLocalOutput)), Some(OutputInfo(0, toLocalOutput.amount, localScriptPubKey)))),
           remoteOnly_opt = None

eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/AsyncPaymentTriggererSpec.scala

@@ -10,7 +10,7 @@ import com.typesafe.config.ConfigFactory
 import fr.acinq.bitcoin.scalacompat.ByteVector32
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.eclair.blockchain.CurrentBlockHeight
-import fr.acinq.eclair.channel.{NEGOTIATING, NEGOTIATING_SIMPLE}
+import fr.acinq.eclair.channel.NEGOTIATING
 import fr.acinq.eclair.io.Switchboard.GetPeerInfo
 import fr.acinq.eclair.io.{Peer, PeerConnected, PeerReadyManager, Switchboard}
 import fr.acinq.eclair.payment.relay.AsyncPaymentTriggerer._
@@ -166,7 +166,7 @@ class AsyncPaymentTriggererSpec extends ScalaTestWithActorTestKit(ConfigFactory.
     system.eventStream ! EventStream.Publish(PeerConnected(peer.ref.toClassic, remoteNodeId, null))
     val request2 = switchboard.expectMessageType[Switchboard.GetPeerInfo]
     request2.replyTo ! Peer.PeerInfo(peer.ref.toClassic, remoteNodeId, Peer.CONNECTED, None, None, Set(TestProbe().ref.toClassic))
-    peer.expectMessageType[Peer.GetPeerChannels].replyTo ! Peer.PeerChannels(remoteNodeId, Seq(Peer.ChannelInfo(null, NEGOTIATING_SIMPLE, null)))
+    peer.expectMessageType[Peer.GetPeerChannels].replyTo ! Peer.PeerChannels(remoteNodeId, Seq(Peer.ChannelInfo(null, NEGOTIATING, null)))
     probe.expectNoMessage(100 millis)
     probe2.expectMessage(AsyncPaymentTriggered)
   }