From 0292fb5e604e48b7ed838ee7f8a922124b18ed1d Mon Sep 17 00:00:00 2001 From: anton Date: Thu, 30 May 2019 12:27:46 +0300 Subject: [PATCH 1/6] Correctly decode requests without multipliers --- .../main/scala/fr/acinq/eclair/payment/PaymentRequest.scala | 1 + .../scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala | 6 ++++++ 2 files changed, 7 insertions(+) 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 c1b5894128..0f57fc48a6 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 @@ -399,6 +399,7 @@ object PaymentRequest { case a if a.last == 'n' => Some(MilliSatoshi(a.dropRight(1).toLong * 100L)) case a if a.last == 'u' => Some(MilliSatoshi(a.dropRight(1).toLong * 100000L)) case a if a.last == 'm' => Some(MilliSatoshi(a.dropRight(1).toLong * 100000000L)) + case a => Some(MilliSatoshi(a.toLong * 100000000000L)) } def encode(amount: Option[MilliSatoshi]): String = { 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 96a633557c..c2342029fa 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 @@ -262,6 +262,12 @@ class PaymentRequestSpec extends FunSuite { assert(PaymentRequest.write(PaymentRequest.read(input.toUpperCase())) == input) } + test("Pay 1 BTC without multiplier") { + val ref = "lnbc11pdkmqhupp5n2ees808r98m0rh4472yyth0c5fptzcxmexcjznrzmq8xald0cgqdqsf4ujqarfwqsxymmccqp2xvtsv5tc743wgctlza8k3zlpxucl7f3kvjnjptv7xz0nkaww307sdyrvgke2w8kmq7dgz4lkasfn0zvplc9aa4gp8fnhrwfjny0j59sq42x9gp" + val pr = PaymentRequest.read(ref) + assert(pr.amount.contains(MilliSatoshi(100000000000L))) + } + test("nonreg") { val requests = List( "lnbc40n1pw9qjvwpp5qq3w2ln6krepcslqszkrsfzwy49y0407hvks30ec6pu9s07jur3sdpstfshq5n9v9jzucm0d5s8vmm5v5s8qmmnwssyj3p6yqenwdencqzysxqrrss7ju0s4dwx6w8a95a9p2xc5vudl09gjl0w2n02sjrvffde632nxwh2l4w35nqepj4j5njhh4z65wyfc724yj6dn9wajvajfn5j7em6wsq2elakl", From cf22c6e3999a2b85cbb8ed040c791048f44ba51f Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 12 Jul 2019 10:51:42 +0300 Subject: [PATCH 2/6] Add `feeMsat` field to `OutgoingPayment` --- .../scala/fr/acinq/eclair/db/PaymentsDb.scala | 4 +- .../eclair/db/sqlite/SqlitePaymentsDb.scala | 45 ++++++++++++++----- .../eclair/payment/PaymentLifecycle.scala | 17 +++---- .../fr/acinq/eclair/payment/Relayer.scala | 8 ++-- .../eclair/db/SqlitePaymentsDbSpec.scala | 23 +++++----- 5 files changed, 62 insertions(+), 35 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 b13e190446..03d469f173 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 @@ -26,7 +26,7 @@ trait PaymentsDb { def addOutgoingPayment(outgoingPayment: OutgoingPayment) // updates the status of the payment, if the newStatus is SUCCEEDED you must supply a preimage - def updateOutgoingPayment(id: UUID, newStatus: OutgoingPaymentStatus.Value, preimage: Option[ByteVector32] = None) + def updateOutgoingPayment(id: UUID, newStatus: OutgoingPaymentStatus.Value, preimage: Option[ByteVector32], feeMsat: Long) def getOutgoingPayment(id: UUID): Option[OutgoingPayment] @@ -77,7 +77,7 @@ case class IncomingPayment(paymentHash: ByteVector32, amountMsat: Long, received * @param completedAt absolute time in seconds since UNIX epoch when the payment succeeded. * @param status current status of the payment. */ -case class OutgoingPayment(id: UUID, paymentHash: ByteVector32, preimage:Option[ByteVector32], amountMsat: Long, createdAt: Long, completedAt: Option[Long], status: OutgoingPaymentStatus.Value) +case class OutgoingPayment(id: UUID, paymentHash: ByteVector32, preimage: Option[ByteVector32], amountMsat: Long, feeMsat: Long, createdAt: Long, completedAt: Option[Long], status: OutgoingPaymentStatus.Value) object OutgoingPaymentStatus extends Enumeration { val PENDING = Value(1, "PENDING") 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..a18c6495d0 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 @@ -16,7 +16,7 @@ package fr.acinq.eclair.db.sqlite -import java.sql.Connection +import java.sql.{Connection, Statement} import java.util.UUID import fr.acinq.bitcoin.ByteVector32 import fr.acinq.eclair.db.sqlite.SqliteUtils._ @@ -33,14 +33,33 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { import SqliteUtils.ExtendedResultSet._ val DB_NAME = "payments" - val CURRENT_VERSION = 2 + val CURRENT_VERSION = 3 - using(sqlite.createStatement()) { statement => - require(getVersion(statement, DB_NAME, CURRENT_VERSION) <= CURRENT_VERSION, s"incompatible version of $DB_NAME DB found") // version 2 is "backward compatible" in the sense that it uses separate tables from version 1. There is no migration though + def migration123(statement: Statement): Unit = { + // 1 -> 2 was backwards compatible so this method is used to migrate both 1 and 2 to 3 statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)") statement.executeUpdate("CREATE TABLE IF NOT EXISTS sent_payments (id TEXT NOT NULL PRIMARY KEY, payment_hash BLOB NOT NULL, preimage BLOB, amount_msat INTEGER NOT NULL, created_at INTEGER NOT NULL, completed_at INTEGER, status VARCHAR NOT NULL)") + statement.executeUpdate("ALTER TABLE sent_payments ADD COLUMN fee_msat INTEGER DEFAULT 0 NOT NULL") statement.executeUpdate("CREATE INDEX IF NOT EXISTS payment_hash_idx ON sent_payments(payment_hash)") - setVersion(statement, DB_NAME, CURRENT_VERSION) + } + + using(sqlite.createStatement()) { statement => + getVersion(statement, DB_NAME, CURRENT_VERSION) match { + case 1 => // previous version let's migrate + logger.warn(s"migrating db $DB_NAME, found version=1 current=$CURRENT_VERSION") + migration123(statement) + setVersion(statement, DB_NAME, CURRENT_VERSION) + case 2 => + logger.warn(s"migrating db $DB_NAME, found version=2 current=$CURRENT_VERSION") + migration123(statement) + setVersion(statement, DB_NAME, CURRENT_VERSION) + case CURRENT_VERSION => + statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)") + statement.executeUpdate("CREATE TABLE IF NOT EXISTS sent_payments (id TEXT NOT NULL PRIMARY KEY, payment_hash BLOB NOT NULL, preimage BLOB, amount_msat INTEGER NOT NULL, created_at INTEGER NOT NULL, completed_at INTEGER, status VARCHAR NOT NULL, fee_msat INTEGER DEFAULT 0 NOT NULL)") + statement.executeUpdate("CREATE INDEX IF NOT EXISTS payment_hash_idx ON sent_payments(payment_hash)") + case unknownVersion => + throw new RuntimeException(s"Unknown version of DB $DB_NAME found, version=$unknownVersion") + } } override def addOutgoingPayment(sent: OutgoingPayment): Unit = { @@ -55,20 +74,21 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { } } - override def updateOutgoingPayment(id: UUID, newStatus: OutgoingPaymentStatus.Value, preimage: Option[ByteVector32] = None) = { + override def updateOutgoingPayment(id: UUID, newStatus: OutgoingPaymentStatus.Value, preimage: Option[ByteVector32], feeMsat: Long): Unit = { require((newStatus == SUCCEEDED && preimage.isDefined) || (newStatus == FAILED && preimage.isEmpty), "Wrong combination of state/preimage") - using(sqlite.prepareStatement("UPDATE sent_payments SET (completed_at, preimage, status) = (?, ?, ?) WHERE id = ? AND completed_at IS NULL")) { statement => + using(sqlite.prepareStatement("UPDATE sent_payments SET (completed_at, preimage, status, fee_msat) = (?, ?, ?,?) WHERE id = ? AND completed_at IS NULL")) { statement => statement.setLong(1, Platform.currentTime) statement.setBytes(2, if (preimage.isEmpty) null else preimage.get.toArray) statement.setString(3, newStatus.toString) - statement.setString(4, id.toString) + statement.setLong(4, feeMsat) + statement.setString(5, id.toString) if (statement.executeUpdate() == 0) throw new IllegalArgumentException(s"Tried to update an outgoing payment (id=$id) already in final status with=$newStatus") } } override def getOutgoingPayment(id: UUID): Option[OutgoingPayment] = { - using(sqlite.prepareStatement("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status FROM sent_payments WHERE id = ?")) { statement => + using(sqlite.prepareStatement("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status, fee_msat FROM sent_payments WHERE id = ?")) { statement => statement.setString(1, id.toString) val rs = statement.executeQuery() if (rs.next()) { @@ -77,6 +97,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { rs.getByteVector32("payment_hash"), rs.getByteVector32Nullable("preimage"), rs.getLong("amount_msat"), + rs.getLong("fee_msat"), rs.getLong("created_at"), getNullableLong(rs, "completed_at"), OutgoingPaymentStatus.withName(rs.getString("status")) @@ -88,7 +109,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { } override def getOutgoingPayments(paymentHash: ByteVector32): Seq[OutgoingPayment] = { - using(sqlite.prepareStatement("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status FROM sent_payments WHERE payment_hash = ?")) { statement => + using(sqlite.prepareStatement("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status, fee_msat FROM sent_payments WHERE payment_hash = ?")) { statement => statement.setBytes(1, paymentHash.toArray) val rs = statement.executeQuery() var q: Queue[OutgoingPayment] = Queue() @@ -98,6 +119,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { rs.getByteVector32("payment_hash"), rs.getByteVector32Nullable("preimage"), rs.getLong("amount_msat"), + rs.getLong("fee_msat"), rs.getLong("created_at"), getNullableLong(rs, "completed_at"), OutgoingPaymentStatus.withName(rs.getString("status")) @@ -109,7 +131,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { override def listOutgoingPayments(): Seq[OutgoingPayment] = { using(sqlite.createStatement()) { statement => - val rs = statement.executeQuery("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status FROM sent_payments") + val rs = statement.executeQuery("SELECT id, payment_hash, preimage, amount_msat, created_at, completed_at, status, fee_msat FROM sent_payments") var q: Queue[OutgoingPayment] = Queue() while (rs.next()) { q = q :+ OutgoingPayment( @@ -117,6 +139,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { rs.getByteVector32("payment_hash"), rs.getByteVector32Nullable("preimage"), rs.getLong("amount_msat"), + rs.getLong("fee_msat"), rs.getLong("created_at"), getNullableLong(rs, "completed_at"), OutgoingPaymentStatus.withName(rs.getString("status")) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala index cf878ed627..bc8a4edd50 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala @@ -49,13 +49,13 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis when(WAITING_FOR_REQUEST) { case Event(c: SendPaymentToRoute, WaitingForRequest) => val send = SendPayment(c.amountMsat, c.paymentHash, c.hops.last, finalCltvExpiry = c.finalCltvExpiry, maxAttempts = 1) - paymentsDb.addOutgoingPayment(OutgoingPayment(id, c.paymentHash, None, c.amountMsat, Platform.currentTime, None, OutgoingPaymentStatus.PENDING)) + paymentsDb.addOutgoingPayment(OutgoingPayment(id, c.paymentHash, None, c.amountMsat, feeMsat = 0L, Platform.currentTime, None, OutgoingPaymentStatus.PENDING)) router ! FinalizeRoute(c.hops) goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, send, failures = Nil) case Event(c: SendPayment, WaitingForRequest) => router ! RouteRequest(nodeParams.nodeId, c.targetNodeId, c.amountMsat, c.assistedRoutes, routeParams = c.routeParams) - paymentsDb.addOutgoingPayment(OutgoingPayment(id, c.paymentHash, None, c.amountMsat, Platform.currentTime, None, OutgoingPaymentStatus.PENDING)) + paymentsDb.addOutgoingPayment(OutgoingPayment(id, c.paymentHash, None, c.amountMsat, feeMsat = 0L, Platform.currentTime, None, OutgoingPaymentStatus.PENDING)) goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, c, failures = Nil) } @@ -72,7 +72,7 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis case Event(Status.Failure(t), WaitingForRoute(s, c, failures)) => reply(s, PaymentFailed(id, c.paymentHash, failures = failures :+ LocalFailure(t))) - paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) stop(FSM.Normal) } @@ -80,9 +80,10 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis case Event("ok", _) => stay() case Event(fulfill: UpdateFulfillHtlc, WaitingForComplete(s, c, cmd, _, _, _, _, hops)) => - paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.SUCCEEDED, preimage = Some(fulfill.paymentPreimage)) + val feesPaid = MilliSatoshi(cmd.amountMsat - c.amountMsat) + paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.SUCCEEDED, preimage = Some(fulfill.paymentPreimage), feeMsat = feesPaid.amount) reply(s, PaymentSucceeded(id, cmd.amountMsat, c.paymentHash, fulfill.paymentPreimage, hops)) - context.system.eventStream.publish(PaymentSent(id, MilliSatoshi(c.amountMsat), MilliSatoshi(cmd.amountMsat - c.amountMsat), cmd.paymentHash, fulfill.paymentPreimage, fulfill.channelId)) + context.system.eventStream.publish(PaymentSent(id, MilliSatoshi(c.amountMsat), feesPaid, cmd.paymentHash, fulfill.paymentPreimage, fulfill.channelId)) stop(FSM.Normal) case Event(fail: UpdateFailHtlc, WaitingForComplete(s, c, _, failures, sharedSecrets, ignoreNodes, ignoreChannels, hops)) => @@ -91,7 +92,7 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis // if destination node returns an error, we fail the payment immediately log.warning(s"received an error message from target nodeId=$nodeId, failing the payment (failure=$failureMessage)") reply(s, PaymentFailed(id, c.paymentHash, failures = failures :+ RemoteFailure(hops, e))) - paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) stop(FSM.Normal) case res if failures.size + 1 >= c.maxAttempts => // otherwise we never try more than maxAttempts, no matter the kind of error returned @@ -105,7 +106,7 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis } log.warning(s"too many failed attempts, failing the payment") reply(s, PaymentFailed(id, c.paymentHash, failures = failures :+ failure)) - paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) stop(FSM.Normal) case Failure(t) => log.warning(s"cannot parse returned error: ${t.getMessage}") @@ -170,7 +171,7 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis case Event(Status.Failure(t), WaitingForComplete(s, c, _, failures, _, ignoreNodes, ignoreChannels, hops)) => if (failures.size + 1 >= c.maxAttempts) { - paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) reply(s, PaymentFailed(id, c.paymentHash, failures :+ LocalFailure(t))) stop(FSM.Normal) } else { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala index 1a75ee2083..b86f1f6c73 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala @@ -137,7 +137,7 @@ class Relayer(nodeParams: NodeParams, register: ActorRef, paymentHandler: ActorR case Local(id, None) => // we sent the payment, but we probably restarted and the reference to the original sender was lost, // we publish the failure on the event stream and update the status in paymentDb - nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) context.system.eventStream.publish(PaymentFailed(id, paymentHash, Nil)) case Local(_, Some(sender)) => sender ! Status.Failure(addFailed) @@ -162,7 +162,7 @@ class Relayer(nodeParams: NodeParams, register: ActorRef, paymentHandler: ActorR context.system.eventStream.publish(PaymentSent(id, MilliSatoshi(add.amountMsat), feesPaid, add.paymentHash, fulfill.paymentPreimage, fulfill.channelId)) // we sent the payment, but we probably restarted and the reference to the original sender was lost, // we publish the failure on the event stream and update the status in paymentDb - nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.SUCCEEDED, Some(fulfill.paymentPreimage)) + nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.SUCCEEDED, Some(fulfill.paymentPreimage), feeMsat = 0L) context.system.eventStream.publish(PaymentSucceeded(id, add.amountMsat, add.paymentHash, fulfill.paymentPreimage, Nil)) // case Local(_, Some(sender)) => sender ! fulfill @@ -177,7 +177,7 @@ class Relayer(nodeParams: NodeParams, register: ActorRef, paymentHandler: ActorR case Local(id, None) => // we sent the payment, but we probably restarted and the reference to the original sender was lost // we publish the failure on the event stream and update the status in paymentDb - nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) context.system.eventStream.publish(PaymentFailed(id, add.paymentHash, Nil)) case Local(_, Some(sender)) => sender ! fail @@ -191,7 +191,7 @@ class Relayer(nodeParams: NodeParams, register: ActorRef, paymentHandler: ActorR case Local(id, None) => // we sent the payment, but we probably restarted and the reference to the original sender was lost // we publish the failure on the event stream and update the status in paymentDb - nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED) + nodeParams.db.payments.updateOutgoingPayment(id, OutgoingPaymentStatus.FAILED, preimage = None, feeMsat = 0L) context.system.eventStream.publish(PaymentFailed(id, add.paymentHash, Nil)) case Local(_, Some(sender)) => sender ! fail 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..dcd5826a24 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 @@ -38,7 +38,7 @@ class SqlitePaymentsDbSpec extends FunSuite { val db2 = new SqlitePaymentsDb(sqlite) } - test("handle version migration 1->2") { + test("handle version migration v1 -> v3") { val connection = TestConstants.sqliteInMemory() @@ -64,14 +64,14 @@ class SqlitePaymentsDbSpec extends FunSuite { val preMigrationDb = new SqlitePaymentsDb(connection) using(connection.createStatement()) { statement => - assert(getVersion(statement, "payments", 1) == 2) // version has changed from 1 to 2! + assert(getVersion(statement, "payments", 1) == 3) // version has changed from 1 to 3! } // the existing received payment can NOT be queried anymore assert(preMigrationDb.getIncomingPayment(oldReceivedPayment.paymentHash).isEmpty) // add a few rows - val ps1 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"0f059ef9b55bb70cc09069ee4df854bf0fab650eee6f2b87ba26d1ad08ab114f"), None, amountMsat = 12345, createdAt = 12345, None, PENDING) + val ps1 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"0f059ef9b55bb70cc09069ee4df854bf0fab650eee6f2b87ba26d1ad08ab114f"), None, amountMsat = 12345, feeMsat = 0L, createdAt = 12345, None, PENDING) val i1 = PaymentRequest.read("lnbc10u1pw2t4phpp5ezwm2gdccydhnphfyepklc0wjkxhz0r4tctg9paunh2lxgeqhcmsdqlxycrqvpqwdshgueqvfjhggr0dcsry7qcqzpgfa4ecv7447p9t5hkujy9qgrxvkkf396p9zar9p87rv2htmeuunkhydl40r64n5s2k0u7uelzc8twxmp37nkcch6m0wg5tvvx69yjz8qpk94qf3") val pr1 = IncomingPayment(i1.paymentHash, 12345678, 1513871928275L) @@ -86,12 +86,15 @@ class SqlitePaymentsDbSpec extends FunSuite { val postMigrationDb = new SqlitePaymentsDb(connection) using(connection.createStatement()) { statement => - assert(getVersion(statement, "payments", 2) == 2) // version still to 2 + assert(getVersion(statement, "payments", 2) == 3) // version still to 3 } assert(postMigrationDb.listIncomingPayments() == Seq(pr1)) assert(postMigrationDb.listOutgoingPayments() == Seq(ps1)) assert(preMigrationDb.listPaymentRequests(0, (Platform.currentTime.milliseconds + 1.minute).toSeconds) == Seq(i1)) + + preMigrationDb.updateOutgoingPayment(ps1.id, OutgoingPaymentStatus.SUCCEEDED, preimage = Some(ByteVector32.Zeroes), feeMsat = 1000L) + assert(preMigrationDb.getOutgoingPayment(ps1.id).head.feeMsat === 1000L) } test("add/list received payments/find 1 payment that exists/find 1 payment that does not exist") { @@ -121,8 +124,8 @@ class SqlitePaymentsDbSpec extends FunSuite { val db = new SqlitePaymentsDb(TestConstants.sqliteInMemory()) - val s1 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"0f059ef9b55bb70cc09069ee4df854bf0fab650eee6f2b87ba26d1ad08ab114f"), None, amountMsat = 12345, createdAt = 12345, None, PENDING) - val s2 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"08d47d5f7164d4b696e8f6b62a03094d4f1c65f16e9d7b11c4a98854707e55cf"), None, amountMsat = 12345, createdAt = 12345, None, PENDING) + val s1 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"0f059ef9b55bb70cc09069ee4df854bf0fab650eee6f2b87ba26d1ad08ab114f"), None, amountMsat = 12345, feeMsat = 0L, createdAt = 12345, None, PENDING) + val s2 = OutgoingPayment(id = UUID.randomUUID(), paymentHash = ByteVector32(hex"08d47d5f7164d4b696e8f6b62a03094d4f1c65f16e9d7b11c4a98854707e55cf"), None, amountMsat = 12345, feeMsat = 0L, createdAt = 12345, None, PENDING) assert(db.listOutgoingPayments().isEmpty) db.addOutgoingPayment(s1) @@ -138,17 +141,18 @@ class SqlitePaymentsDbSpec extends FunSuite { val s3 = s2.copy(id = UUID.randomUUID(), amountMsat = 88776655) db.addOutgoingPayment(s3) - db.updateOutgoingPayment(s3.id, FAILED) + db.updateOutgoingPayment(s3.id, FAILED, preimage = None, feeMsat = 0L) assert(db.getOutgoingPayment(s3.id).get.status == FAILED) assert(db.getOutgoingPayment(s3.id).get.preimage.isEmpty) // failed sent payments don't have a preimage assert(db.getOutgoingPayment(s3.id).get.completedAt.isDefined) // can't update again once it's in a final state - assertThrows[IllegalArgumentException](db.updateOutgoingPayment(s3.id, SUCCEEDED)) + assertThrows[IllegalArgumentException](db.updateOutgoingPayment(s3.id, SUCCEEDED, preimage = None, feeMsat = 10000L)) - db.updateOutgoingPayment(s1.id, SUCCEEDED, Some(ByteVector32.One)) + db.updateOutgoingPayment(s1.id, SUCCEEDED, preimage = Some(ByteVector32.One), feeMsat = 20000L) assert(db.getOutgoingPayment(s1.id).get.preimage.isDefined) assert(db.getOutgoingPayment(s1.id).get.completedAt.isDefined) + assert(db.getOutgoingPayment(s1.id).get.feeMsat === 20000) } test("add/retrieve payment requests") { @@ -183,5 +187,4 @@ class SqlitePaymentsDbSpec extends FunSuite { val to = (someTimestamp + 100).seconds.toSeconds assert(db.listPaymentRequests(from, to) == Seq(i1)) } - } From cbfc9a1b45e8da68b1a805bd94aed8c4bb64c232 Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 12 Jul 2019 16:40:25 +0300 Subject: [PATCH 3/6] Minor fixes --- .../src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala | 1 + .../scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala | 4 ++-- 2 files changed, 3 insertions(+), 2 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 03d469f173..644d04227c 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 @@ -73,6 +73,7 @@ case class IncomingPayment(paymentHash: ByteVector32, amountMsat: Long, received * @param paymentHash payment_hash * @param preimage the preimage of the payment_hash, known if the outgoing payment was successful * @param amountMsat amount of the payment, in milli-satoshis + * @param feeMsat amount of fees paid, in milli-satoshis * @param createdAt absolute time in seconds since UNIX epoch when the payment was created. * @param completedAt absolute time in seconds since UNIX epoch when the payment succeeded. * @param status current status of the payment. 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 a18c6495d0..51ddbacc77 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 @@ -75,9 +75,9 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { } override def updateOutgoingPayment(id: UUID, newStatus: OutgoingPaymentStatus.Value, preimage: Option[ByteVector32], feeMsat: Long): Unit = { - require((newStatus == SUCCEEDED && preimage.isDefined) || (newStatus == FAILED && preimage.isEmpty), "Wrong combination of state/preimage") + require((newStatus == SUCCEEDED && preimage.isDefined) || (newStatus != SUCCEEDED && preimage.isEmpty && feeMsat == 0L), "Wrong combination of state/preimage") - using(sqlite.prepareStatement("UPDATE sent_payments SET (completed_at, preimage, status, fee_msat) = (?, ?, ?,?) WHERE id = ? AND completed_at IS NULL")) { statement => + using(sqlite.prepareStatement("UPDATE sent_payments SET (completed_at, preimage, status, fee_msat) = (?, ?, ?, ?) WHERE id = ? AND completed_at IS NULL")) { statement => statement.setLong(1, Platform.currentTime) statement.setBytes(2, if (preimage.isEmpty) null else preimage.get.toArray) statement.setString(3, newStatus.toString) From 22cfea47ccf6821217abc6a1dc37a3508bd2df29 Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 19 Jul 2019 13:53:16 +0300 Subject: [PATCH 4/6] Fix migration methods naming --- .../fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 51ddbacc77..83de4bb282 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 @@ -35,8 +35,10 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { val DB_NAME = "payments" val CURRENT_VERSION = 3 - def migration123(statement: Statement): Unit = { - // 1 -> 2 was backwards compatible so this method is used to migrate both 1 and 2 to 3 + // 1 -> 2 was backwards compatible so this method is noop + def migration12(statement: Statement): Unit = () + + def migration23(statement: Statement): Unit = { statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)") statement.executeUpdate("CREATE TABLE IF NOT EXISTS sent_payments (id TEXT NOT NULL PRIMARY KEY, payment_hash BLOB NOT NULL, preimage BLOB, amount_msat INTEGER NOT NULL, created_at INTEGER NOT NULL, completed_at INTEGER, status VARCHAR NOT NULL)") statement.executeUpdate("ALTER TABLE sent_payments ADD COLUMN fee_msat INTEGER DEFAULT 0 NOT NULL") @@ -47,11 +49,12 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { getVersion(statement, DB_NAME, CURRENT_VERSION) match { case 1 => // previous version let's migrate logger.warn(s"migrating db $DB_NAME, found version=1 current=$CURRENT_VERSION") - migration123(statement) + migration12(statement) + migration23(statement) setVersion(statement, DB_NAME, CURRENT_VERSION) case 2 => logger.warn(s"migrating db $DB_NAME, found version=2 current=$CURRENT_VERSION") - migration123(statement) + migration23(statement) setVersion(statement, DB_NAME, CURRENT_VERSION) case CURRENT_VERSION => statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)") From c3273f2258367af92c6f3a02e7055038a756b1f9 Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 19 Jul 2019 15:51:39 +0300 Subject: [PATCH 5/6] Split db updates across migration versions --- .../fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 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 83de4bb282..16525d7b79 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 @@ -35,16 +35,17 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { val DB_NAME = "payments" val CURRENT_VERSION = 3 - // 1 -> 2 was backwards compatible so this method is noop - def migration12(statement: Statement): Unit = () - - def migration23(statement: Statement): Unit = { + def migration12(statement: Statement): Unit = { + // version 2 is "backward compatible" in the sense that it uses separate tables from version 1. There is no migration though statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)") statement.executeUpdate("CREATE TABLE IF NOT EXISTS sent_payments (id TEXT NOT NULL PRIMARY KEY, payment_hash BLOB NOT NULL, preimage BLOB, amount_msat INTEGER NOT NULL, created_at INTEGER NOT NULL, completed_at INTEGER, status VARCHAR NOT NULL)") - statement.executeUpdate("ALTER TABLE sent_payments ADD COLUMN fee_msat INTEGER DEFAULT 0 NOT NULL") statement.executeUpdate("CREATE INDEX IF NOT EXISTS payment_hash_idx ON sent_payments(payment_hash)") } + def migration23(statement: Statement): Unit = { + statement.executeUpdate("ALTER TABLE sent_payments ADD COLUMN fee_msat INTEGER DEFAULT 0 NOT NULL") + } + using(sqlite.createStatement()) { statement => getVersion(statement, DB_NAME, CURRENT_VERSION) match { case 1 => // previous version let's migrate From 33ac71ecd5d39f051320715811b874f494bd8803 Mon Sep 17 00:00:00 2001 From: anton Date: Sat, 14 Sep 2019 14:52:07 +0300 Subject: [PATCH 6/6] Minor fix --- .../main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala index 914ad94183..255be36b04 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala @@ -79,7 +79,7 @@ class PaymentLifecycle(nodeParams: NodeParams, id: UUID, router: ActorRef, regis val feesPaid = cmd.amount - c.finalPayload.amount paymentsDb.updateOutgoingPayment(id, OutgoingPaymentStatus.SUCCEEDED, preimage = Some(fulfill.paymentPreimage), fee = feesPaid) reply(s, PaymentSucceeded(id, cmd.amount, c.paymentHash, fulfill.paymentPreimage, hops)) - context.system.eventStream.publish(PaymentSent(id, c.finalPayload.amount, cmd.amount - c.finalPayload.amount, cmd.paymentHash, fulfill.paymentPreimage, fulfill.channelId)) + context.system.eventStream.publish(PaymentSent(id, c.finalPayload.amount, feesPaid, cmd.paymentHash, fulfill.paymentPreimage, fulfill.channelId)) stop(FSM.Normal) case Event(fail: UpdateFailHtlc, WaitingForComplete(s, c, _, failures, sharedSecrets, ignoreNodes, ignoreChannels, hops)) =>