From 3e92e6afc490378c8adb31e4903ffd6416c2dca4 Mon Sep 17 00:00:00 2001 From: Andrea Date: Tue, 2 Jul 2019 10:30:15 +0200 Subject: [PATCH 1/4] Add test for rejecting payments for expired invoices --- .../eclair/payment/PaymentHandlerSpec.scala | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala index 90f7c8cbfd..86bc06ef7e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala @@ -149,4 +149,24 @@ class PaymentHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike sender.send(handler, ReceivePayment(Some(MilliSatoshi(42000)), "1 coffee without routing info")) assert(sender.expectMsgType[PaymentRequest].routingInfo === Nil) } + + test("LocalPaymentHandler should reject incoming payments if the payment request is expired") { + val nodeParams = Alice.nodeParams + val handler = TestActorRef[LocalPaymentHandler](LocalPaymentHandler.props(nodeParams)) + val sender = TestProbe() + val eventListener = TestProbe() + system.eventStream.subscribe(eventListener.ref, classOf[PaymentReceived]) + + val amountMsat = MilliSatoshi(42000) + val expiry = Globals.blockCount.get() + 12 + + sender.send(handler, ReceivePayment(Some(amountMsat), "some desc", expirySeconds_opt = Some(0))) + val pr = sender.expectMsgType[PaymentRequest] + + val add = UpdateAddHtlc(ByteVector32(ByteVector.fill(32)(1)), 0, amountMsat.amount, pr.paymentHash, expiry, ByteVector.empty) + sender.send(handler, add) + + sender.expectMsgType[CMD_FAIL_HTLC] + assert(nodeParams.db.payments.getIncomingPayment(pr.paymentHash).isEmpty) + } } From f67efa86b158cbcdef658898edb0ee3bcf05526c Mon Sep 17 00:00:00 2001 From: Andrea Date: Tue, 2 Jul 2019 10:41:40 +0200 Subject: [PATCH 2/4] Filter expired payment requests in 'getPendingPaymentRequestAndPreimage' --- .../acinq/eclair/db/sqlite/SqlitePaymentsDb.scala | 3 ++- .../fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala | 14 +++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala index 8fd6ded566..a409999bfe 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala @@ -155,8 +155,9 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { } override def getPendingPaymentRequestAndPreimage(paymentHash: ByteVector32): Option[(ByteVector32, PaymentRequest)] = { - using(sqlite.prepareStatement("SELECT payment_request, preimage FROM received_payments WHERE payment_hash = ? AND received_at IS NULL")) { statement => + using(sqlite.prepareStatement("SELECT payment_request, preimage FROM received_payments WHERE payment_hash = ? AND received_at IS NULL AND (expire_at > ? OR expire_at IS NULL)")) { statement => statement.setBytes(1, paymentHash.toArray) + statement.setLong(2, Platform.currentTime) val rs = statement.executeQuery() if (rs.next()) { val preimage = rs.getByteVector32("preimage") diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala index b8eec4b784..1263a22f2a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala @@ -158,26 +158,30 @@ class SqlitePaymentsDbSpec extends FunSuite { val bob = Bob.keyManager - val (paymentHash1, paymentHash2) = (randomBytes32, randomBytes32) + val (paymentHash1, paymentHash2, paymentHash3) = (randomBytes32, randomBytes32, randomBytes32) - val i1 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash1, privateKey = bob.nodeKey.privateKey, description = "Some invoice", expirySeconds = None, timestamp = someTimestamp) - val i2 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = None, paymentHash = paymentHash2, privateKey = bob.nodeKey.privateKey, description = "Some invoice", expirySeconds = Some(123456), timestamp = Platform.currentTime.milliseconds.toSeconds) + val i1 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash1, privateKey = bob.nodeKey.privateKey, description = "Some invoice1", expirySeconds = None, timestamp = someTimestamp) + val i2 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = None, paymentHash = paymentHash2, privateKey = bob.nodeKey.privateKey, description = "Some invoice2", expirySeconds = Some(123456), timestamp = Platform.currentTime.milliseconds.toSeconds) + val i3 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash3, privateKey = bob.nodeKey.privateKey, description = "Some invoice3", expirySeconds = Some(0), timestamp = someTimestamp - 500) - // i2 doesn't expire + // i1 doesn't expire assert(i1.expiry.isEmpty && i2.expiry.isDefined) assert(i1.amount.isDefined && i2.amount.isEmpty) db.addPaymentRequest(i1, ByteVector32.Zeroes) db.addPaymentRequest(i2, ByteVector32.One) + db.addPaymentRequest(i3, ByteVector32.One) // order matters, i2 has a more recent timestamp than i1 - assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1)) + assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1, i3)) assert(db.getPaymentRequest(i1.paymentHash) == Some(i1)) assert(db.getPaymentRequest(i2.paymentHash) == Some(i2)) + assert(db.getPaymentRequest(i3.paymentHash) == Some(i3)) assert(db.listPendingPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1)) assert(db.getPendingPaymentRequestAndPreimage(paymentHash1) == Some((ByteVector32.Zeroes, i1))) assert(db.getPendingPaymentRequestAndPreimage(paymentHash2) == Some((ByteVector32.One, i2))) + assert(db.getPendingPaymentRequestAndPreimage(paymentHash3) == None) // expired invoice won't be returned val from = (someTimestamp - 100).seconds.toSeconds val to = (someTimestamp + 100).seconds.toSeconds From cae244e1343297be391ecba63adb0fb36f3160c8 Mon Sep 17 00:00:00 2001 From: Andrea Date: Wed, 3 Jul 2019 12:05:34 +0200 Subject: [PATCH 3/4] Use a more recent timestamp when testing payment request expiry in payment db --- .../scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala index 1263a22f2a..4752371c2b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala @@ -162,7 +162,7 @@ class SqlitePaymentsDbSpec extends FunSuite { val i1 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash1, privateKey = bob.nodeKey.privateKey, description = "Some invoice1", expirySeconds = None, timestamp = someTimestamp) val i2 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = None, paymentHash = paymentHash2, privateKey = bob.nodeKey.privateKey, description = "Some invoice2", expirySeconds = Some(123456), timestamp = Platform.currentTime.milliseconds.toSeconds) - val i3 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash3, privateKey = bob.nodeKey.privateKey, description = "Some invoice3", expirySeconds = Some(0), timestamp = someTimestamp - 500) + val i3 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash3, privateKey = bob.nodeKey.privateKey, description = "Some invoice3", expirySeconds = Some(0), timestamp = Platform.currentTime.milliseconds.toSeconds - 1) // i1 doesn't expire assert(i1.expiry.isEmpty && i2.expiry.isDefined) @@ -172,8 +172,8 @@ class SqlitePaymentsDbSpec extends FunSuite { db.addPaymentRequest(i2, ByteVector32.One) db.addPaymentRequest(i3, ByteVector32.One) - // order matters, i2 has a more recent timestamp than i1 - assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1, i3)) + // order matters, i2 and i3 have a more recent timestamp than i1 + assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i3, i1)) assert(db.getPaymentRequest(i1.paymentHash) == Some(i1)) assert(db.getPaymentRequest(i2.paymentHash) == Some(i2)) assert(db.getPaymentRequest(i3.paymentHash) == Some(i3)) From e128992c50022243b0609662ef7c39ae80cc2c8b Mon Sep 17 00:00:00 2001 From: Andrea Date: Wed, 3 Jul 2019 14:53:59 +0200 Subject: [PATCH 4/4] Do not filter expired payment requests from getPendingPaymentRequestAndPreimage, Implement `isExpired` on PaymentRequest, Check for expired payment request in payment handler --- .../scala/fr/acinq/eclair/db/PaymentsDb.scala | 1 + .../eclair/db/sqlite/SqlitePaymentsDb.scala | 3 +-- .../eclair/payment/LocalPaymentHandler.scala | 2 ++ .../fr/acinq/eclair/payment/PaymentRequest.scala | 12 +++++++++--- .../acinq/eclair/db/SqlitePaymentsDbSpec.scala | 16 ++++++---------- .../eclair/payment/PaymentHandlerSpec.scala | 1 + 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala index 38b1f60e8d..b13e190446 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala @@ -39,6 +39,7 @@ trait PaymentsDb { def getPaymentRequest(paymentHash: ByteVector32): Option[PaymentRequest] + // returns non paid payment request def getPendingPaymentRequestAndPreimage(paymentHash: ByteVector32): Option[(ByteVector32, PaymentRequest)] def listPaymentRequests(from: Long, to: Long): Seq[PaymentRequest] diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala index a409999bfe..8fd6ded566 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala @@ -155,9 +155,8 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { } override def getPendingPaymentRequestAndPreimage(paymentHash: ByteVector32): Option[(ByteVector32, PaymentRequest)] = { - using(sqlite.prepareStatement("SELECT payment_request, preimage FROM received_payments WHERE payment_hash = ? AND received_at IS NULL AND (expire_at > ? OR expire_at IS NULL)")) { statement => + using(sqlite.prepareStatement("SELECT payment_request, preimage FROM received_payments WHERE payment_hash = ? AND received_at IS NULL")) { statement => statement.setBytes(1, paymentHash.toArray) - statement.setLong(2, Platform.currentTime) val rs = statement.executeQuery() if (rs.next()) { val preimage = rs.getByteVector32("preimage") diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/LocalPaymentHandler.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/LocalPaymentHandler.scala index d5daaa8d1f..1070f3ef37 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/LocalPaymentHandler.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/LocalPaymentHandler.scala @@ -65,6 +65,8 @@ class LocalPaymentHandler(nodeParams: NodeParams) extends Actor with ActorLoggin // it must not be greater than two times the requested amount. // see https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#failure-messages paymentRequest.amount match { + case _ if paymentRequest.isExpired => + sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true) case _ if htlc.cltvExpiry < minFinalExpiry => sender ! CMD_FAIL_HTLC(htlc.id, Right(FinalExpiryTooSoon), commit = true) case Some(amount) if MilliSatoshi(htlc.amountMsat) < amount => diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala index 9e8a9323cf..1d31a625ce 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala @@ -16,8 +16,6 @@ package fr.acinq.eclair.payment -import java.math.BigInteger - import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.{MilliSatoshi, _} import fr.acinq.eclair.ShortChannelId @@ -25,7 +23,8 @@ import fr.acinq.eclair.payment.PaymentRequest._ import scodec.Codec import scodec.bits.{BitVector, ByteOrdering, ByteVector} import scodec.codecs.{list, ubyte} - +import scala.concurrent.duration._ +import scala.compat.Platform import scala.util.Try /** @@ -78,6 +77,11 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam case cltvExpiry: PaymentRequest.MinFinalCltvExpiry => cltvExpiry.toLong } + def isExpired: Boolean = expiry match { + case Some(expiryTime) => timestamp + expiryTime <= Platform.currentTime.milliseconds.toSeconds + case None => timestamp + DEFAULT_EXPIRY_SECONDS <= Platform.currentTime.milliseconds.toSeconds + } + /** * * @return the hash of this payment request @@ -106,6 +110,8 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam object PaymentRequest { + val DEFAULT_EXPIRY_SECONDS = 3600 + val prefixes = Map( Block.RegtestGenesisBlock.hash -> "lnbcrt", Block.TestnetGenesisBlock.hash -> "lntb", diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala index 4752371c2b..b8eec4b784 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/db/SqlitePaymentsDbSpec.scala @@ -158,30 +158,26 @@ class SqlitePaymentsDbSpec extends FunSuite { val bob = Bob.keyManager - val (paymentHash1, paymentHash2, paymentHash3) = (randomBytes32, randomBytes32, randomBytes32) + val (paymentHash1, paymentHash2) = (randomBytes32, randomBytes32) - val i1 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash1, privateKey = bob.nodeKey.privateKey, description = "Some invoice1", expirySeconds = None, timestamp = someTimestamp) - val i2 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = None, paymentHash = paymentHash2, privateKey = bob.nodeKey.privateKey, description = "Some invoice2", expirySeconds = Some(123456), timestamp = Platform.currentTime.milliseconds.toSeconds) - val i3 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash3, privateKey = bob.nodeKey.privateKey, description = "Some invoice3", expirySeconds = Some(0), timestamp = Platform.currentTime.milliseconds.toSeconds - 1) + val i1 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = Some(MilliSatoshi(123)), paymentHash = paymentHash1, privateKey = bob.nodeKey.privateKey, description = "Some invoice", expirySeconds = None, timestamp = someTimestamp) + val i2 = PaymentRequest(chainHash = Block.TestnetGenesisBlock.hash, amount = None, paymentHash = paymentHash2, privateKey = bob.nodeKey.privateKey, description = "Some invoice", expirySeconds = Some(123456), timestamp = Platform.currentTime.milliseconds.toSeconds) - // i1 doesn't expire + // i2 doesn't expire assert(i1.expiry.isEmpty && i2.expiry.isDefined) assert(i1.amount.isDefined && i2.amount.isEmpty) db.addPaymentRequest(i1, ByteVector32.Zeroes) db.addPaymentRequest(i2, ByteVector32.One) - db.addPaymentRequest(i3, ByteVector32.One) - // order matters, i2 and i3 have a more recent timestamp than i1 - assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i3, i1)) + // order matters, i2 has a more recent timestamp than i1 + assert(db.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1)) assert(db.getPaymentRequest(i1.paymentHash) == Some(i1)) assert(db.getPaymentRequest(i2.paymentHash) == Some(i2)) - assert(db.getPaymentRequest(i3.paymentHash) == Some(i3)) assert(db.listPendingPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i2, i1)) assert(db.getPendingPaymentRequestAndPreimage(paymentHash1) == Some((ByteVector32.Zeroes, i1))) assert(db.getPendingPaymentRequestAndPreimage(paymentHash2) == Some((ByteVector32.One, i2))) - assert(db.getPendingPaymentRequestAndPreimage(paymentHash3) == None) // expired invoice won't be returned val from = (someTimestamp - 100).seconds.toSeconds val to = (someTimestamp + 100).seconds.toSeconds diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala index 86bc06ef7e..9edf987b85 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala @@ -52,6 +52,7 @@ class PaymentHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike val pr = sender.expectMsgType[PaymentRequest] assert(nodeParams.db.payments.getIncomingPayment(pr.paymentHash).isEmpty) assert(nodeParams.db.payments.getPendingPaymentRequestAndPreimage(pr.paymentHash).isDefined) + assert(!nodeParams.db.payments.getPendingPaymentRequestAndPreimage(pr.paymentHash).get._2.isExpired) val add = UpdateAddHtlc(ByteVector32(ByteVector.fill(32)(1)), 0, amountMsat.amount, pr.paymentHash, expiry, ByteVector.empty) sender.send(handler, add)