Disallow non-segwit scripts in close API

We shouldn't use non-segwit scripts anymore as they can in theory mess with the dust limits (and we should encourage migration to segwit).
ACINQ · Sep 15, 2021 · 705cc91 · 705cc91
1 parent cded0e4
commit 705cc91
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 4 deletions.
diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala
@@ -18,7 +18,7 @@ package fr.acinq.eclair.api.handlers
 
 import akka.http.scaladsl.server.{MalformedFormFieldRejection, Route}
 import akka.util.Timeout
-import fr.acinq.bitcoin.Satoshi
+import fr.acinq.bitcoin.{Satoshi, Script}
 import fr.acinq.eclair.MilliSatoshi
 import fr.acinq.eclair.api.Service
 import fr.acinq.eclair.api.directives.EclairDirectives
@@ -63,7 +63,11 @@ trait Channel {
             val maxFeerate = maxFeerate_opt.map(feerate => FeeratePerKw(feerate)).getOrElse(preferredFeerate * 2)
             ClosingFeerates(preferredFeerate, minFeerate, maxFeerate)
           })
-          complete(eclairApi.close(channels, scriptPubKey_opt, closingFeerates))
+          if (scriptPubKey_opt.forall(Script.isNativeWitnessScript)) {
+            complete(eclairApi.close(channels, scriptPubKey_opt, closingFeerates))
+          } else {
+            reject(MalformedFormFieldRejection("scriptPubKey", "Non-segwit scripts are not allowed"))
+          }
       }
     }
   }

diff --git a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala
@@ -524,6 +524,21 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM
       }
   }
 
+  test("'close' rejects non-segwit scripts") {
+    val shortChannelId = "1701x42x3"
+    val eclair = mock[Eclair]
+    val mockService = new MockService(eclair)
+
+    Post("/close", FormData("shortChannelId" -> shortChannelId, "scriptPubKey" -> "a914748284390f9e263a4b766a75d0633c50426eb87587").toEntity) ~>
+      addCredentials(BasicHttpCredentials("", mockApi().password)) ~>
+      addHeader("Content-Type", "application/json") ~>
+      Route.seal(mockService.close) ~>
+      check {
+        assert(handled)
+        assert(status == BadRequest)
+      }
+  }
+
   test("'connect' method should accept a nodeId") {
     val remoteNodeId = PublicKey(hex"030bb6a5e0c6b203c7e2180fb78c7ba4bdce46126761d8201b91ddac089cdecc87")
 
@@ -560,7 +575,6 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM
       }
   }
 
-
   test("'send' method should handle payment failures") {
     val eclair = mock[Eclair]
     eclair.send(any, any, any, any, any, any)(any[Timeout]) returns Future.failed(new IllegalArgumentException("invoice has expired"))
@@ -982,7 +996,7 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM
     eclair.findRoute(any, any, any, any)(any[Timeout]) returns Future.successful(Router.RouteResponse(Seq(Router.Route(456.msat, mockHops))))
 
     // invalid format
-    Post("/findroute", FormData("format"-> "invalid-output-format", "invoice" -> invoice, "amountMsat" -> "456")) ~>
+    Post("/findroute", FormData("format" -> "invalid-output-format", "invoice" -> invoice, "amountMsat" -> "456")) ~>
       addCredentials(BasicHttpCredentials("", mockApi().password)) ~>
       addHeader("Content-Type", "application/json") ~>
       Route.seal(mockService.findRoute) ~>