Add support for option_shutdown_anysegwit (#1801)

Opt-in to allow any future segwit script in shutdown as long as it complies with BIP 141 (see lightning/bolts#672).
ACINQ · May 17, 2021 · 5a92f84 · 5a92f84
1 parent 1fbede7
commit 5a92f84
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 20 deletions.
diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf
@@ -50,6 +50,7 @@ eclair {
     basic_mpp = optional
     option_support_large_channel = optional
     option_anchor_outputs = disabled
+    option_shutdown_anysegwit = optional
     trampoline_payment = disabled
     keysend = disabled
   }

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala
@@ -188,6 +188,11 @@ object Features {
     val mandatory = 20
   }
 
+  case object ShutdownAnySegwit extends Feature {
+    val rfcName = "option_shutdown_anysegwit"
+    val mandatory = 26
+  }
+
   // TODO: @t-bast: update feature bits once spec-ed (currently reserved here: https://github.com/lightningnetwork/lightning-rfc/issues/605)
   // We're not advertising these bits yet in our announcements, clients have to assume support.
   // This is why we haven't added them yet to `areSupported`.
@@ -213,6 +218,7 @@ object Features {
     TrampolinePayment,
     StaticRemoteKey,
     AnchorOutputs,
+    ShutdownAnySegwit,
     KeySend
   )
 

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
@@ -863,14 +863,15 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
 
     case Event(c: CMD_CLOSE, d: DATA_NORMAL) =>
       val localScriptPubKey = c.scriptPubKey.getOrElse(d.commitments.localParams.defaultFinalScriptPubKey)
+      val allowAnySegwit = Features.canUseFeature(d.commitments.localParams.features, d.commitments.remoteParams.features, Features.ShutdownAnySegwit)
       if (d.localShutdown.isDefined) {
         handleCommandError(ClosingAlreadyInProgress(d.channelId), c)
       } else if (Commitments.localHasUnsignedOutgoingHtlcs(d.commitments)) {
         // NB: simplistic behavior, we could also sign-then-close
         handleCommandError(CannotCloseWithUnsignedOutgoingHtlcs(d.channelId), c)
       } else if (Commitments.localHasUnsignedOutgoingUpdateFee(d.commitments)) {
         handleCommandError(CannotCloseWithUnsignedOutgoingUpdateFee(d.channelId), c)
-      } else if (!Closing.isValidFinalScriptPubkey(localScriptPubKey)) {
+      } else if (!Closing.isValidFinalScriptPubkey(localScriptPubKey, allowAnySegwit)) {
         handleCommandError(InvalidFinalScript(d.channelId), c)
       } else {
         val shutdown = Shutdown(d.channelId, localScriptPubKey)
@@ -892,7 +893,8 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
       //      we did not send a shutdown message
       //        there are pending signed changes  => go to SHUTDOWN
       //        there are no htlcs                => go to NEGOTIATING
-      if (!Closing.isValidFinalScriptPubkey(remoteScriptPubKey)) {
+      val allowAnySegwit = Features.canUseFeature(d.commitments.localParams.features, d.commitments.remoteParams.features, Features.ShutdownAnySegwit)
+      if (!Closing.isValidFinalScriptPubkey(remoteScriptPubKey, allowAnySegwit)) {
         handleLocalError(InvalidFinalScript(d.channelId), d, Some(remoteShutdown))
       } else if (Commitments.remoteHasUnsignedOutgoingHtlcs(d.commitments)) {
         handleLocalError(CannotCloseWithUnsignedOutgoingHtlcs(d.channelId), d, Some(remoteShutdown))

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -410,12 +410,13 @@ object Helpers {
     // used only to compute tx weights and estimate fees
     lazy val dummyPublicKey = PrivateKey(ByteVector32(ByteVector.fill(32)(1))).publicKey
 
-    def isValidFinalScriptPubkey(scriptPubKey: ByteVector): Boolean = {
+    def isValidFinalScriptPubkey(scriptPubKey: ByteVector, allowAnySegwit: Boolean): Boolean = {
       Try(Script.parse(scriptPubKey)) match {
         case Success(OP_DUP :: OP_HASH160 :: OP_PUSHDATA(pubkeyHash, _) :: OP_EQUALVERIFY :: OP_CHECKSIG :: Nil) if pubkeyHash.size == 20 => true
         case Success(OP_HASH160 :: OP_PUSHDATA(scriptHash, _) :: OP_EQUAL :: Nil) if scriptHash.size == 20 => true
         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 _ => false
       }
     }
@@ -449,8 +450,9 @@ object Helpers {
 
     def makeClosingTx(keyManager: ChannelKeyManager, commitments: Commitments, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, closingFee: Satoshi)(implicit log: LoggingAdapter): (ClosingTx, ClosingSigned) = {
       import commitments._
-      require(isValidFinalScriptPubkey(localScriptPubkey), "invalid localScriptPubkey")
-      require(isValidFinalScriptPubkey(remoteScriptPubkey), "invalid remoteScriptPubkey")
+      val allowAnySegwit = Features.canUseFeature(commitments.localParams.features, commitments.remoteParams.features, Features.ShutdownAnySegwit)
+      require(isValidFinalScriptPubkey(localScriptPubkey, allowAnySegwit), "invalid localScriptPubkey")
+      require(isValidFinalScriptPubkey(remoteScriptPubkey, allowAnySegwit), "invalid remoteScriptPubkey")
       log.debug("making closing tx with closingFee={} and commitments:\n{}", closingFee, Commitments.specs2String(commitments))
       val dustLimitSatoshis = localParams.dustLimit.max(remoteParams.dustLimit)
       val closingTx = Transactions.makeClosingTx(commitInput, localScriptPubkey, remoteScriptPubkey, localParams.isFunder, dustLimitSatoshis, closingFee, localCommit.spec)

diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala
@@ -209,7 +209,7 @@ class FeaturesSpec extends AnyFunSuite {
       hex"" -> Features.empty,
       hex"0100" -> Features(VariableLengthOnion -> Mandatory),
       hex"028a8a" -> Features(OptionDataLossProtect -> Optional, InitialRoutingSync -> Optional, ChannelRangeQueries -> Optional, VariableLengthOnion -> Optional, ChannelRangeQueriesExtended -> Optional, PaymentSecret -> Optional, BasicMultiPartPayment -> Optional),
-      hex"09004200" -> Features(Map[Feature, FeatureSupport](VariableLengthOnion -> Optional, PaymentSecret -> Mandatory), Set(UnknownFeature(24), UnknownFeature(27))),
+      hex"09004200" -> Features(Map[Feature, FeatureSupport](VariableLengthOnion -> Optional, PaymentSecret -> Mandatory, ShutdownAnySegwit -> Optional), Set(UnknownFeature(24))),
       hex"52000000" -> Features(Map.empty[Feature, FeatureSupport], Set(UnknownFeature(25), UnknownFeature(28), UnknownFeature(30)))
     )
 

diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala
@@ -62,6 +62,8 @@ object StateTestsTags {
   val StaticRemoteKey = "static_remotekey"
   /** If set, channels will use option_anchor_outputs. */
   val AnchorOutputs = "anchor_outputs"
+  /** If set, channels will use option_shutdown_anysegwit. */
+  val ShutdownAnySegwit = "shutdown_anysegwit"
   /** If set, channels will be public (otherwise we don't announce them by default). */
   val ChannelsPublic = "channels_public"
   /** If set, no amount will be pushed when opening a channel (by default we push a small amount). */
@@ -111,6 +113,7 @@ trait StateTestsHelperMethods extends TestKitBase {
       .modify(_.features.activated).usingIf(tags.contains(StateTestsTags.Wumbo))(_.updated(Features.Wumbo, FeatureSupport.Optional))
       .modify(_.features.activated).usingIf(tags.contains(StateTestsTags.StaticRemoteKey))(_.updated(Features.StaticRemoteKey, FeatureSupport.Optional))
       .modify(_.features.activated).usingIf(tags.contains(StateTestsTags.AnchorOutputs))(_.updated(Features.StaticRemoteKey, FeatureSupport.Mandatory).updated(Features.AnchorOutputs, FeatureSupport.Optional))
+      .modify(_.features.activated).usingIf(tags.contains(StateTestsTags.ShutdownAnySegwit))(_.updated(Features.ShutdownAnySegwit, FeatureSupport.Optional))
   }
 
   def reachNormal(setup: SetupFixture, tags: Set[String] = Set.empty): Unit = {

diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
@@ -1737,23 +1737,24 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     relayerA.expectNoMsg(1 seconds)
   }
 
-  def testCmdClose(f: FixtureParam): Unit = {
+  def testCmdClose(f: FixtureParam, script_opt: Option[ByteVector]): Unit = {
     import f._
     val sender = TestProbe()
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].localShutdown.isEmpty)
-    alice ! CMD_CLOSE(sender.ref, None)
+    alice ! CMD_CLOSE(sender.ref, script_opt)
     sender.expectMsgType[RES_SUCCESS[CMD_CLOSE]]
-    alice2bob.expectMsgType[Shutdown]
+    val shutdown = alice2bob.expectMsgType[Shutdown]
+    script_opt.foreach(script => assert(script === shutdown.scriptPubKey))
     awaitCond(alice.stateName == NORMAL)
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].localShutdown.isDefined)
   }
 
-  test("recv CMD_CLOSE (no pending htlcs)") {
-    testCmdClose _
+  test("recv CMD_CLOSE (no pending htlcs)") { f =>
+    testCmdClose(f, None)
   }
 
-  test("recv CMD_CLOSE (no pending htlcs) (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) {
-    testCmdClose _
+  test("recv CMD_CLOSE (no pending htlcs) (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f =>
+    testCmdClose(f, None)
   }
 
   test("recv CMD_CLOSE (with noSender)") { f =>
@@ -1794,6 +1795,17 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     sender.expectMsgType[RES_FAILURE[CMD_CLOSE, InvalidFinalScript]]
   }
 
+  test("recv CMD_CLOSE (with unsupported native segwit script)") { f =>
+    import f._
+    val sender = TestProbe()
+    alice ! CMD_CLOSE(sender.ref, Some(hex"51050102030405"))
+    sender.expectMsgType[RES_FAILURE[CMD_CLOSE, InvalidFinalScript]]
+  }
+
+  test("recv CMD_CLOSE (with native segwit script)", Tag(StateTestsTags.ShutdownAnySegwit)) { f =>
+    testCmdClose(f, Some(hex"51050102030405"))
+  }
+
   test("recv CMD_CLOSE (with signed sent htlcs)") { f =>
     import f._
     val sender = TestProbe()
@@ -1849,22 +1861,22 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     awaitCond(alice.stateName == NORMAL)
   }
 
-  def testShutdown(f: FixtureParam): Unit = {
+  def testShutdown(f: FixtureParam, script_opt: Option[ByteVector]): Unit = {
     import f._
-    alice ! Shutdown(ByteVector32.Zeroes, Bob.channelParams.defaultFinalScriptPubKey)
+    alice ! Shutdown(ByteVector32.Zeroes, script_opt.getOrElse(Bob.channelParams.defaultFinalScriptPubKey))
     alice2bob.expectMsgType[Shutdown]
     alice2bob.expectMsgType[ClosingSigned]
     awaitCond(alice.stateName == NEGOTIATING)
     // channel should be advertised as down
     assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === alice.stateData.asInstanceOf[DATA_NEGOTIATING].channelId)
   }
 
-  test("recv Shutdown (no pending htlcs)") {
-    testShutdown _
+  test("recv Shutdown (no pending htlcs)") { f =>
+    testShutdown(f, None)
   }
 
-  test("recv Shutdown (no pending htlcs) (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) {
-    testShutdown _
+  test("recv Shutdown (no pending htlcs) (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f =>
+    testShutdown(f, None)
   }
 
   test("recv Shutdown (with unacked sent htlcs)") { f =>
@@ -1938,6 +1950,18 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     awaitCond(bob.stateName == CLOSING)
   }
 
+  test("recv Shutdown (with unsupported native segwit script)") { f =>
+    import f._
+    bob ! Shutdown(ByteVector32.Zeroes, hex"51050102030405")
+    bob2alice.expectMsgType[Error]
+    bob2blockchain.expectMsgType[PublishTx]
+    awaitCond(bob.stateName == CLOSING)
+  }
+
+  test("recv Shutdown (with native segwit script)", Tag(StateTestsTags.ShutdownAnySegwit)) { f =>
+    testShutdown(f, Some(hex"51050102030405"))
+  }
+
   test("recv Shutdown (with invalid final script and signed htlcs, in response to a Shutdown)") { f =>
     import f._
     val sender = TestProbe()

diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala
@@ -399,7 +399,7 @@ class PaymentRequestSpec extends AnyFunSuite {
       // those are useful for nonreg testing of the areSupported method (which needs to be updated with every new supported mandatory bit)
       PaymentRequestFeatures(bin"          0010000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = false),
       PaymentRequestFeatures(bin"     000001000000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = false),
-      PaymentRequestFeatures(bin"     000100000000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = false),
+      PaymentRequestFeatures(bin"     000100000000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = true),
       PaymentRequestFeatures(bin"00000010000000000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = false),
       PaymentRequestFeatures(bin"00001000000000000000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = false)
     )