From e960a1f9a9f8a368091121b4526fbf98bcb4f14b Mon Sep 17 00:00:00 2001 From: Bastien Teinturier Date: Tue, 3 Sep 2019 14:47:40 +0200 Subject: [PATCH] Reject expired invoices before payment flow starts --- .../main/scala/fr/acinq/eclair/Eclair.scala | 28 +++++++----- .../fr/acinq/eclair/EclairImplSpec.scala | 21 +++++---- .../scala/fr/acinq/eclair/api/Service.scala | 8 ++-- .../fr/acinq/eclair/api/ApiServiceSpec.scala | 45 +++++++++++-------- 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala index b5a6de6a9e..50dde47091 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala @@ -53,7 +53,6 @@ object TimestampQueryFilters { } } - trait Eclair { def connect(target: Either[NodeURI, PublicKey])(implicit timeout: Timeout): Future[String] @@ -78,7 +77,7 @@ trait Eclair { def receivedInfo(paymentHash: ByteVector32)(implicit timeout: Timeout): Future[Option[IncomingPayment]] - def send(recipientNodeId: PublicKey, amount: MilliSatoshi, paymentHash: ByteVector32, assistedRoutes: Seq[Seq[PaymentRequest.ExtraHop]] = Seq.empty, minFinalCltvExpiryDelta_opt: Option[CltvExpiryDelta] = None, maxAttempts_opt: Option[Int] = None, feeThresholdSat_opt: Option[Satoshi] = None, maxFeePct_opt: Option[Double] = None)(implicit timeout: Timeout): Future[UUID] + def send(recipientNodeId: PublicKey, amount: MilliSatoshi, paymentHash: ByteVector32, invoice_opt: Option[PaymentRequest] = None, maxAttempts_opt: Option[Int] = None, feeThresholdSat_opt: Option[Satoshi] = None, maxFeePct_opt: Option[Double] = None)(implicit timeout: Timeout): Future[UUID] def sentInfo(id: Either[UUID, ByteVector32])(implicit timeout: Timeout): Future[Seq[OutgoingPayment]] @@ -190,7 +189,7 @@ class EclairImpl(appKit: Kit) extends Eclair { (appKit.paymentInitiator ? SendPaymentToRoute(amount, paymentHash, route, finalCltvExpiryDelta)).mapTo[UUID] } - override def send(recipientNodeId: PublicKey, amount: MilliSatoshi, paymentHash: ByteVector32, assistedRoutes: Seq[Seq[PaymentRequest.ExtraHop]] = Seq.empty, minFinalCltvExpiryDelta_opt: Option[CltvExpiryDelta], maxAttempts_opt: Option[Int], feeThreshold_opt: Option[Satoshi], maxFeePct_opt: Option[Double])(implicit timeout: Timeout): Future[UUID] = { + override def send(recipientNodeId: PublicKey, amount: MilliSatoshi, paymentHash: ByteVector32, invoice_opt: Option[PaymentRequest], maxAttempts_opt: Option[Int], feeThreshold_opt: Option[Satoshi], maxFeePct_opt: Option[Double])(implicit timeout: Timeout): Future[UUID] = { val maxAttempts = maxAttempts_opt.getOrElse(appKit.nodeParams.maxPaymentAttempts) val defaultRouteParams = Router.getDefaultRouteParams(appKit.nodeParams.routerConf) @@ -199,11 +198,18 @@ class EclairImpl(appKit: Kit) extends Eclair { maxFeeBase = feeThreshold_opt.map(_.toMilliSatoshi).getOrElse(defaultRouteParams.maxFeeBase) ) - val sendPayment = minFinalCltvExpiryDelta_opt match { - case Some(minCltv) => SendPayment(amount, paymentHash, recipientNodeId, assistedRoutes, finalCltvExpiryDelta = minCltv, maxAttempts = maxAttempts, routeParams = Some(routeParams)) - case None => SendPayment(amount, paymentHash, recipientNodeId, assistedRoutes, maxAttempts = maxAttempts, routeParams = Some(routeParams)) + invoice_opt match { + case Some(invoice) if invoice.isExpired => Future.failed(new IllegalArgumentException("invoice has expired")) + case Some(invoice) => + val sendPayment = invoice.minFinalCltvExpiryDelta match { + case Some(minFinalCltvExpiryDelta) => SendPayment(amount, paymentHash, recipientNodeId, invoice.routingInfo, minFinalCltvExpiryDelta, maxAttempts = maxAttempts, routeParams = Some(routeParams)) + case None => SendPayment(amount, paymentHash, recipientNodeId, invoice.routingInfo, maxAttempts = maxAttempts, routeParams = Some(routeParams)) + } + (appKit.paymentInitiator ? sendPayment).mapTo[UUID] + case None => + val sendPayment = SendPayment(amount, paymentHash, recipientNodeId, maxAttempts = maxAttempts, routeParams = Some(routeParams)) + (appKit.paymentInitiator ? sendPayment).mapTo[UUID] } - (appKit.paymentInitiator ? sendPayment).mapTo[UUID] } override def sentInfo(id: Either[UUID, ByteVector32])(implicit timeout: Timeout): Future[Seq[OutgoingPayment]] = Future { @@ -252,10 +258,10 @@ class EclairImpl(appKit: Kit) extends Eclair { } /** - * Sends a request to a channel and expects a response - * - * @param channelIdentifier either a shortChannelId (BOLT encoded) or a channelId (32-byte hex encoded) - */ + * Sends a request to a channel and expects a response + * + * @param channelIdentifier either a shortChannelId (BOLT encoded) or a channelId (32-byte hex encoded) + */ def sendToChannel(channelIdentifier: Either[ByteVector32, ShortChannelId], request: Any)(implicit timeout: Timeout): Future[Any] = channelIdentifier match { case Left(channelId) => appKit.register ? Forward(channelId, request) case Right(shortChannelId) => appKit.register ? ForwardShortId(shortChannelId, request) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala index faf2a2a745..1802fc5b6e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala @@ -20,15 +20,15 @@ import akka.actor.ActorSystem import akka.testkit.{TestKit, TestProbe} import akka.util.Timeout import fr.acinq.bitcoin.Crypto.PublicKey -import fr.acinq.bitcoin.{ByteVector32, Crypto} +import fr.acinq.bitcoin.{Block, ByteVector32, Crypto} import fr.acinq.eclair.TestConstants._ import fr.acinq.eclair.blockchain.TestWallet import fr.acinq.eclair.channel.{CMD_FORCECLOSE, Register, _} import fr.acinq.eclair.db._ import fr.acinq.eclair.io.Peer.OpenChannel -import fr.acinq.eclair.payment.LocalPaymentHandler import fr.acinq.eclair.payment.PaymentLifecycle.{ReceivePayment, SendPayment, SendPaymentToRoute} import fr.acinq.eclair.payment.PaymentRequest.ExtraHop +import fr.acinq.eclair.payment.{LocalPaymentHandler, PaymentRequest} import fr.acinq.eclair.router.RouteCalculationSpec.makeUpdate import org.mockito.scalatest.IdiomaticMockito import org.scalatest.{Outcome, fixture} @@ -40,7 +40,7 @@ import scala.util.Success class EclairImplSpec extends TestKit(ActorSystem("mySystem")) with fixture.FunSuiteLike with IdiomaticMockito { - implicit val timeout = Timeout(30 seconds) + implicit val timeout: Timeout = Timeout(30 seconds) case class FixtureParam(register: TestProbe, router: TestProbe, paymentInitiator: TestProbe, switchboard: TestProbe, paymentHandler: TestProbe, kit: Kit) @@ -93,7 +93,7 @@ class EclairImplSpec extends TestKit(ActorSystem("mySystem")) with fixture.FunSu val eclair = new EclairImpl(kit) val nodeId = PublicKey(hex"030bb6a5e0c6b203c7e2180fb78c7ba4bdce46126761d8201b91ddac089cdecc87") - eclair.send(recipientNodeId = nodeId, amount = 123 msat, paymentHash = ByteVector32.Zeroes, assistedRoutes = Seq.empty, minFinalCltvExpiryDelta_opt = None) + eclair.send(nodeId, 123 msat, ByteVector32.Zeroes, invoice_opt = None) val send = paymentInitiator.expectMsgType[SendPayment] assert(send.targetNodeId == nodeId) assert(send.amount == 123.msat) @@ -101,8 +101,9 @@ class EclairImplSpec extends TestKit(ActorSystem("mySystem")) with fixture.FunSu assert(send.assistedRoutes == Seq.empty) // with assisted routes - val hints = Seq(Seq(ExtraHop(Bob.nodeParams.nodeId, ShortChannelId("569178x2331x1"), feeBase = 10 msat, feeProportionalMillionths = 1, cltvExpiryDelta = CltvExpiryDelta(12)))) - eclair.send(recipientNodeId = nodeId, amount = 123 msat, paymentHash = ByteVector32.Zeroes, assistedRoutes = hints, minFinalCltvExpiryDelta_opt = None) + val hints = List(List(ExtraHop(Bob.nodeParams.nodeId, ShortChannelId("569178x2331x1"), feeBase = 10 msat, feeProportionalMillionths = 1, cltvExpiryDelta = CltvExpiryDelta(12)))) + val invoice1 = PaymentRequest(Block.RegtestGenesisBlock.hash, Some(123 msat), ByteVector32.Zeroes, randomKey, "description", None, None, hints) + eclair.send(nodeId, 123 msat, ByteVector32.Zeroes, invoice_opt = Some(invoice1)) val send1 = paymentInitiator.expectMsgType[SendPayment] assert(send1.targetNodeId == nodeId) assert(send1.amount == 123.msat) @@ -110,7 +111,8 @@ class EclairImplSpec extends TestKit(ActorSystem("mySystem")) with fixture.FunSu assert(send1.assistedRoutes == hints) // with finalCltvExpiry - eclair.send(recipientNodeId = nodeId, amount = 123 msat, paymentHash = ByteVector32.Zeroes, assistedRoutes = Seq.empty, minFinalCltvExpiryDelta_opt = Some(CltvExpiryDelta(96))) + val invoice2 = PaymentRequest("lntb", Some(123 msat), System.currentTimeMillis() / 1000L, nodeId, List(PaymentRequest.MinFinalCltvExpiry(96), PaymentRequest.PaymentHash(ByteVector32.Zeroes), PaymentRequest.Description("description")), ByteVector.empty) + eclair.send(nodeId, 123 msat, ByteVector32.Zeroes, invoice_opt = Some(invoice2)) val send2 = paymentInitiator.expectMsgType[SendPayment] assert(send2.targetNodeId == nodeId) assert(send2.amount == 123.msat) @@ -118,13 +120,16 @@ class EclairImplSpec extends TestKit(ActorSystem("mySystem")) with fixture.FunSu assert(send2.finalCltvExpiryDelta == CltvExpiryDelta(96)) // with custom route fees parameters - eclair.send(recipientNodeId = nodeId, amount = 123 msat, paymentHash = ByteVector32.Zeroes, assistedRoutes = Seq.empty, minFinalCltvExpiryDelta_opt = None, feeThreshold_opt = Some(123 sat), maxFeePct_opt = Some(4.20)) + eclair.send(nodeId, 123 msat, ByteVector32.Zeroes, invoice_opt = None, feeThreshold_opt = Some(123 sat), maxFeePct_opt = Some(4.20)) val send3 = paymentInitiator.expectMsgType[SendPayment] assert(send3.targetNodeId == nodeId) assert(send3.amount == 123.msat) assert(send3.paymentHash == ByteVector32.Zeroes) assert(send3.routeParams.get.maxFeeBase == 123000.msat) // conversion sat -> msat assert(send3.routeParams.get.maxFeePct == 4.20) + + val expiredInvoice = invoice2.copy(timestamp = 0L) + assertThrows[IllegalArgumentException](Await.result(eclair.send(nodeId, 123 msat, ByteVector32.Zeroes, invoice_opt = Some(expiredInvoice)), 50 millis)) } test("allupdates can filter by nodeId") { f => diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala index 1e12e7f176..20d9609419 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala @@ -31,7 +31,6 @@ import akka.stream.scaladsl.{BroadcastHub, Flow, Keep, Source} import akka.stream.{ActorMaterializer, OverflowStrategy} import akka.util.Timeout import com.google.common.net.HostAndPort -import de.heikoseeberger.akkahttpjson4s.Json4sSupport import fr.acinq.bitcoin.Crypto.PublicKey import fr.acinq.bitcoin.{ByteVector32, Satoshi} import fr.acinq.eclair.api.FormParamExtractors._ @@ -74,6 +73,9 @@ trait Service extends ExtraDirectives with Logging { val paramParsingTimeout = 5 seconds val apiExceptionHandler = ExceptionHandler { + case t: IllegalArgumentException => + logger.error(s"API call failed with cause=${t.getMessage}", t) + complete(StatusCodes.BadRequest, ErrorResponse(t.getMessage)) case t: Throwable => logger.error(s"API call failed with cause=${t.getMessage}", t) complete(StatusCodes.InternalServerError, ErrorResponse(t.getMessage)) @@ -224,9 +226,9 @@ trait Service extends ExtraDirectives with Logging { path("payinvoice") { formFields(invoiceFormParam, amountMsatFormParam.?, "maxAttempts".as[Int].?, "feeThresholdSat".as[Satoshi].?, "maxFeePct".as[Double].?) { case (invoice@PaymentRequest(_, Some(amount), _, nodeId, _, _), None, maxAttempts, feeThresholdSat_opt, maxFeePct_opt) => - complete(eclairApi.send(nodeId, amount, invoice.paymentHash, invoice.routingInfo, invoice.minFinalCltvExpiryDelta, maxAttempts, feeThresholdSat_opt, maxFeePct_opt)) + complete(eclairApi.send(nodeId, amount, invoice.paymentHash, Some(invoice), maxAttempts, feeThresholdSat_opt, maxFeePct_opt)) case (invoice, Some(overrideAmount), maxAttempts, feeThresholdSat_opt, maxFeePct_opt) => - complete(eclairApi.send(invoice.nodeId, overrideAmount, invoice.paymentHash, invoice.routingInfo, invoice.minFinalCltvExpiryDelta, maxAttempts, feeThresholdSat_opt, maxFeePct_opt)) + complete(eclairApi.send(invoice.nodeId, overrideAmount, invoice.paymentHash, Some(invoice), maxAttempts, feeThresholdSat_opt, maxFeePct_opt)) case _ => reject(MalformedFormFieldRejection("invoice", "The invoice must have an amount or you need to specify one using the field 'amountMsat'")) } } ~ 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 index 5b7d26cc04..330b821fd0 100644 --- a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala +++ b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala @@ -25,10 +25,10 @@ import akka.http.scaladsl.model.headers.BasicHttpCredentials import akka.http.scaladsl.server.Route import akka.http.scaladsl.testkit.{RouteTestTimeout, ScalatestRouteTest, WSProbe} import akka.stream.ActorMaterializer -import akka.util.{ByteString, Timeout} +import akka.util.Timeout import de.heikoseeberger.akkahttpjson4s.Json4sSupport +import fr.acinq.bitcoin.ByteVector32 import fr.acinq.bitcoin.Crypto.PublicKey -import fr.acinq.bitcoin.{ByteVector32, Satoshi} import fr.acinq.eclair._ import fr.acinq.eclair.io.NodeURI import fr.acinq.eclair.io.Peer.PeerInfo @@ -38,7 +38,8 @@ import fr.acinq.eclair.wire.NodeAddress import org.mockito.scalatest.IdiomaticMockito import org.scalatest.{FunSuite, Matchers} import scodec.bits._ -import scala.concurrent.{Await, Future} + +import scala.concurrent.Future import scala.concurrent.duration._ import scala.io.Source import scala.reflect.ClassTag @@ -113,7 +114,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'peers' should ask the switchboard for current known peers") { - val eclair = mock[Eclair] val mockService = new MockService(eclair) eclair.peersInfo()(any[Timeout]) returns Future.successful(List( @@ -141,7 +141,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'usablebalances' asks router for current usable balances") { - val eclair = mock[Eclair] val mockService = new MockService(eclair) eclair.usableBalances()(any[Timeout]) returns Future.successful(List( @@ -162,7 +161,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'getinfo' response should include this node ID") { - val eclair = mock[Eclair] val mockService = new MockService(eclair) eclair.getInfoResponse()(any[Timeout]) returns Future.successful(GetInfoResponse( @@ -187,7 +185,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'close' method should accept a channelId and shortChannelId") { - val shortChannelIdSerialized = "42000x27x3" val channelId = "56d7d6eda04d80138270c49709f1eadb5ab4939e5061309ccdacdb98ce637d0e" @@ -223,7 +220,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'connect' method should accept an URI and a triple with nodeId/host/port") { - val remoteNodeId = PublicKey(hex"030bb6a5e0c6b203c7e2180fb78c7ba4bdce46126761d8201b91ddac089cdecc87") val remoteUri = NodeURI.parse("030bb6a5e0c6b203c7e2180fb78c7ba4bdce46126761d8201b91ddac089cdecc87@93.137.102.239:9735") @@ -252,13 +248,30 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } } + test("'send' method should handle payment failures") { + val eclair = mock[Eclair] + eclair.send(any, any, any, any, any, any, any)(any[Timeout]) returns Future.failed(new IllegalArgumentException("invoice has expired")) + val mockService = new MockService(eclair) - test("'send' method should correctly forward amount parameters to EclairImpl") { + val invoice = "lnbc12580n1pw2ywztpp554ganw404sh4yjkwnysgn3wjcxfcq7gtx53gxczkjr9nlpc3hzvqdq2wpskwctddyxqr4rqrzjqwryaup9lh50kkranzgcdnn2fgvx390wgj5jd07rwr3vxeje0glc7z9rtvqqwngqqqqqqqlgqqqqqeqqjqrrt8smgjvfj7sg38dwtr9kc9gg3era9k3t2hvq3cup0jvsrtrxuplevqgfhd3rzvhulgcxj97yjuj8gdx8mllwj4wzjd8gdjhpz3lpqqvk2plh" + Post("/payinvoice", FormData("invoice" -> invoice).toEntity) ~> + addCredentials(BasicHttpCredentials("", mockService.password)) ~> + Route.seal(mockService.route) ~> + check { + assert(handled) + assert(status == BadRequest) + val resp = entityAs[ErrorResponse](Json4sSupport.unmarshaller, ClassTag(classOf[ErrorResponse])) + assert(resp.error == "invoice has expired") + eclair.send(any, 1258000 msat, any, any, any, any, any)(any[Timeout]).wasCalled(once) + } + } + + test("'send' method should correctly forward amount parameters to EclairImpl") { val invoice = "lnbc12580n1pw2ywztpp554ganw404sh4yjkwnysgn3wjcxfcq7gtx53gxczkjr9nlpc3hzvqdq2wpskwctddyxqr4rqrzjqwryaup9lh50kkranzgcdnn2fgvx390wgj5jd07rwr3vxeje0glc7z9rtvqqwngqqqqqqqlgqqqqqeqqjqrrt8smgjvfj7sg38dwtr9kc9gg3era9k3t2hvq3cup0jvsrtrxuplevqgfhd3rzvhulgcxj97yjuj8gdx8mllwj4wzjd8gdjhpz3lpqqvk2plh" val eclair = mock[Eclair] - eclair.send(any, any, any, any, any, any, any, any)(any[Timeout]) returns Future.successful(UUID.randomUUID()) + eclair.send(any, any, any, any, any, any, any)(any[Timeout]) returns Future.successful(UUID.randomUUID()) val mockService = new MockService(eclair) Post("/payinvoice", FormData("invoice" -> invoice).toEntity) ~> @@ -267,23 +280,21 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock check { assert(handled) assert(status == OK) - eclair.send(any, 1258000 msat, any, any, any, any, any, any)(any[Timeout]).wasCalled(once) + eclair.send(any, 1258000 msat, any, any, any, any, any)(any[Timeout]).wasCalled(once) } - Post("/payinvoice", FormData("invoice" -> invoice, "amountMsat" -> "123", "feeThresholdSat" -> "112233", "maxFeePct" -> "2.34").toEntity) ~> addCredentials(BasicHttpCredentials("", mockService.password)) ~> Route.seal(mockService.route) ~> check { assert(handled) assert(status == OK) - eclair.send(any, 123 msat, any, any, any, any, Some(112233 sat), Some(2.34))(any[Timeout]).wasCalled(once) + eclair.send(any, 123 msat, any, any, any, Some(112233 sat), Some(2.34))(any[Timeout]).wasCalled(once) } } test("'getreceivedinfo' method should respond HTTP 404 with a JSON encoded response if the element is not found") { - val eclair = mock[Eclair] eclair.receivedInfo(any[ByteVector32])(any) returns Future.successful(None) val mockService = new MockService(eclair) @@ -301,7 +312,6 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock } test("'sendtoroute' method should accept a both a json-encoded AND comma separaterd list of pubkeys") { - val rawUUID = "487da196-a4dc-4b1e-92b4-3e5e905e9f3f" val paymentUUID = UUID.fromString(rawUUID) val expectedRoute = List(PublicKey(hex"0217eb8243c95f5a3b7d4c5682d10de354b7007eb59b6807ae407823963c7547a9"), PublicKey(hex"0242a4ae0c5bef18048fbecf995094b74bfb0f7391418d71ed394784373f41e4f3"), PublicKey(hex"026ac9fcd64fb1aa1c491fc490634dc33da41d4a17b554e0adf1b32fee88ee9f28")) @@ -319,7 +329,7 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock check { assert(handled) assert(status == OK) - assert(entityAs[String] == "\""+rawUUID+"\"") + assert(entityAs[String] == "\"" + rawUUID + "\"") eclair.sendToRoute(expectedRoute, 1234 msat, ByteVector32.Zeroes, CltvExpiryDelta(190))(any[Timeout]).wasCalled(once) } @@ -331,13 +341,12 @@ class ApiServiceSpec extends FunSuite with ScalatestRouteTest with IdiomaticMock check { assert(handled) assert(status == OK) - assert(entityAs[String] == "\""+rawUUID+"\"") + assert(entityAs[String] == "\"" + rawUUID + "\"") eclair.sendToRoute(expectedRoute, 1234 msat, ByteVector32.One, CltvExpiryDelta(190))(any[Timeout]).wasCalled(once) } } test("the websocket should return typed objects") { - val mockService = new MockService(mock[Eclair]) val fixedUUID = UUID.fromString("487da196-a4dc-4b1e-92b4-3e5e905e9f3f")