From a46c1aa5c2a04f56a19c0dcaab67a26a14350a92 Mon Sep 17 00:00:00 2001 From: t-bast Date: Fri, 14 May 2021 15:23:11 +0200 Subject: [PATCH] Add support for option_shutdown_anysegwit Opt-in to allow any future segwit script in shutdown as long as it complies with BIP 141 (see https://github.com/lightningnetwork/lightning-rfc/pull/672). --- eclair-core/src/main/resources/reference.conf | 1 + .../main/scala/fr/acinq/eclair/Features.scala | 6 +++ .../fr/acinq/eclair/channel/Channel.scala | 6 ++- .../fr/acinq/eclair/channel/Helpers.scala | 8 +-- .../scala/fr/acinq/eclair/FeaturesSpec.scala | 2 +- .../states/StateTestsHelperMethods.scala | 3 ++ .../channel/states/e/NormalStateSpec.scala | 50 ++++++++++++++----- .../eclair/payment/PaymentRequestSpec.scala | 2 +- 8 files changed, 58 insertions(+), 20 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index c58a8c65ea..dcf06643cf 100644 --- 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 index b712559292..f91aa96435 100644 --- 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 index a1ea6f5b68..ca9d4554c2 100644 --- 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,6 +863,7 @@ 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)) { @@ -870,7 +871,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId 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 index 10876e3014..609522b2bb 100644 --- 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 index 1a835afcf6..95d56f59a6 100644 --- 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 index 4482963e85..76e4821e5b 100644 --- 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 index b5bf05f854..ab50241ccb 100644 --- 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,9 +1861,9 @@ 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) @@ -1859,12 +1871,12 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with 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 index e010ff36e4..c6ddfba4ab 100644 --- 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) )