-
Notifications
You must be signed in to change notification settings - Fork 282
Store more payment info for sent payments #1048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a51f705
b72722b
09bb355
198d307
fa25fd7
d974626
8472d93
9c7567c
7c2e8c9
6c71ea2
738216a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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._ | ||
|
|
@@ -25,6 +25,7 @@ import fr.acinq.eclair.payment.PaymentRequest | |
| import grizzled.slf4j.Logging | ||
| import scala.collection.immutable.Queue | ||
| import OutgoingPaymentStatus._ | ||
| import fr.acinq.bitcoin.Crypto.PublicKey | ||
| import concurrent.duration._ | ||
| import scala.compat.Platform | ||
|
|
||
|
|
@@ -33,25 +34,50 @@ 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 migration12(statement: Statement) = { | ||
| 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("CREATE INDEX IF NOT EXISTS payment_hash_idx ON sent_payments(payment_hash)") | ||
| setVersion(statement, DB_NAME, CURRENT_VERSION) | ||
| } | ||
|
|
||
| def migration23(statement: Statement) = { | ||
| statement.executeUpdate("ALTER TABLE sent_payments ADD payment_request TEXT") | ||
| statement.executeUpdate("ALTER TABLE sent_payments ADD description BLOB") | ||
| statement.executeUpdate(s"ALTER TABLE sent_payments ADD target_node_id BLOB") | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having a default node id for the payment destination does not sound right. I think we should either make this field nullable, meaning that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, i've made a fix in 6c71ea2 where the |
||
|
|
||
| using(sqlite.createStatement()) { statement => | ||
| getVersion(statement, DB_NAME, CURRENT_VERSION) match { | ||
| case 1 => | ||
| logger.warn(s"migrating $DB_NAME from version 1 to $CURRENT_VERSION") | ||
| migration12(statement) | ||
| migration23(statement) | ||
| setVersion(statement, DB_NAME, CURRENT_VERSION) | ||
| case 2 => | ||
| logger.warn(s"migrating $DB_NAME from version 2 to $CURRENT_VERSION") | ||
| 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)") | ||
| 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, payment_request TEXT, description BLOB, target_node_id BLOB)") | ||
| statement.executeUpdate("CREATE INDEX IF NOT EXISTS payment_hash_idx ON sent_payments(payment_hash)") | ||
| setVersion(statement, DB_NAME, CURRENT_VERSION) | ||
| } | ||
| } | ||
|
|
||
| override def addOutgoingPayment(sent: OutgoingPayment): Unit = { | ||
| using(sqlite.prepareStatement("INSERT INTO sent_payments (id, payment_hash, amount_msat, created_at, status) VALUES (?, ?, ?, ?, ?)")) { statement => | ||
| using(sqlite.prepareStatement("INSERT INTO sent_payments (id, payment_hash, amount_msat, created_at, status, payment_request, description, target_node_id) VALUES (?, ?, ?, ?, ?, ?, ?, ?)")) { statement => | ||
| statement.setString(1, sent.id.toString) | ||
| statement.setBytes(2, sent.paymentHash.toArray) | ||
| statement.setLong(3, sent.amountMsat) | ||
| statement.setLong(4, sent.createdAt) | ||
| statement.setString(5, sent.status.toString) | ||
| val res = statement.executeUpdate() | ||
| logger.debug(s"inserted $res payment=${sent.paymentHash} into payment DB") | ||
| statement.setString(6, sent.paymentRequest_opt.orNull) | ||
| statement.setBytes(7, sent.description_opt.map(_.getBytes).orNull) | ||
| statement.setBytes(8, sent.targetNodeId.map(_.value.toArray).orNull) | ||
| statement.executeUpdate() | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -68,18 +94,21 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { | |
| } | ||
|
|
||
| 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, payment_request, description, target_node_id FROM sent_payments WHERE id = ?")) { statement => | ||
| statement.setString(1, id.toString) | ||
| val rs = statement.executeQuery() | ||
| if (rs.next()) { | ||
| Some(OutgoingPayment( | ||
| UUID.fromString(rs.getString("id")), | ||
| rs.getByteVector32("payment_hash"), | ||
| rs.getByteVector32Nullable("preimage"), | ||
| rs.getByteVectorNullable("target_node_id").map(PublicKey(_)), | ||
| rs.getLong("amount_msat"), | ||
| rs.getLong("created_at"), | ||
| getNullableLong(rs, "completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")) | ||
| rs.getNullableLong("completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")), | ||
| rs.getStringNullable("payment_request"), | ||
| rs.getByteVectorNullable("description").map(bytes => new String(bytes.toArray)) | ||
| )) | ||
| } else { | ||
| None | ||
|
|
@@ -88,7 +117,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, payment_request, description, target_node_id FROM sent_payments WHERE payment_hash = ?")) { statement => | ||
| statement.setBytes(1, paymentHash.toArray) | ||
| val rs = statement.executeQuery() | ||
| var q: Queue[OutgoingPayment] = Queue() | ||
|
|
@@ -97,10 +126,13 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging { | |
| UUID.fromString(rs.getString("id")), | ||
| rs.getByteVector32("payment_hash"), | ||
| rs.getByteVector32Nullable("preimage"), | ||
| rs.getByteVectorNullable("target_node_id").map(PublicKey(_)), | ||
| rs.getLong("amount_msat"), | ||
| rs.getLong("created_at"), | ||
| getNullableLong(rs, "completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")) | ||
| rs.getNullableLong("completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")), | ||
| rs.getStringNullable("payment_request"), | ||
| rs.getByteVectorNullable("description").map(bytes => new String(bytes.toArray)) | ||
| ) | ||
| } | ||
| q | ||
|
|
@@ -109,17 +141,20 @@ 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, payment_request, description, target_node_id FROM sent_payments") | ||
| var q: Queue[OutgoingPayment] = Queue() | ||
| while (rs.next()) { | ||
| q = q :+ OutgoingPayment( | ||
| UUID.fromString(rs.getString("id")), | ||
| rs.getByteVector32("payment_hash"), | ||
| rs.getByteVector32Nullable("preimage"), | ||
| rs.getByteVectorNullable("target_node_id").map(PublicKey(_)), | ||
| rs.getLong("amount_msat"), | ||
| rs.getLong("created_at"), | ||
| getNullableLong(rs, "completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")) | ||
| rs.getNullableLong("completed_at"), | ||
| OutgoingPaymentStatus.withName(rs.getString("status")), | ||
| rs.getStringNullable("payment_request"), | ||
| rs.getByteVectorNullable("description").map(bytes => new String(bytes.toArray)) | ||
| ) | ||
| } | ||
| q | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,11 +17,9 @@ | |
| package fr.acinq.eclair.db.sqlite | ||
|
|
||
| import java.sql.{Connection, ResultSet, Statement} | ||
|
|
||
| import fr.acinq.bitcoin.ByteVector32 | ||
| import scodec.Codec | ||
| import scodec.bits.{BitVector, ByteVector} | ||
|
|
||
| import scala.collection.immutable.Queue | ||
|
|
||
| object SqliteUtils { | ||
|
|
@@ -92,19 +90,6 @@ object SqliteUtils { | |
| q | ||
| } | ||
|
|
||
| /** | ||
| * This helper retrieves the value from a nullable integer column and interprets it as an option. This is needed | ||
| * because `rs.getLong` would return `0` for a null value. | ||
| * It is used on Android only | ||
| * | ||
| * @param label | ||
| * @return | ||
| */ | ||
| def getNullableLong(rs: ResultSet, label: String) : Option[Long] = { | ||
| val result = rs.getLong(label) | ||
| if (rs.wasNull()) None else Some(result) | ||
| } | ||
|
|
||
| /** | ||
| * Obtain an exclusive lock on a sqlite database. This is useful when we want to make sure that only one process | ||
| * accesses the database file (see https://www.sqlite.org/pragma.html). | ||
|
|
@@ -127,9 +112,21 @@ object SqliteUtils { | |
|
|
||
| def getByteVector32(columnLabel: String): ByteVector32 = ByteVector32(ByteVector(rs.getBytes(columnLabel))) | ||
|
|
||
| def getByteVector32Nullable(columnLabel: String): Option[ByteVector32] = { | ||
| def getByteVectorNullable(columnLabel: String): Option[ByteVector] = { | ||
| val bytes = rs.getBytes(columnLabel) | ||
| if(rs.wasNull()) None else Some(ByteVector32(ByteVector(bytes))) | ||
| if(rs.wasNull()) None else Some(ByteVector(bytes)) | ||
| } | ||
|
|
||
| def getByteVector32Nullable(columnLabel: String): Option[ByteVector32] = getByteVectorNullable(columnLabel).map(ByteVector32(_)) | ||
|
|
||
| def getStringNullable(columnLabel: String): Option[String] = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is We should move
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, done in 7c2e8c9 |
||
| val str = rs.getString(columnLabel) | ||
| if(rs.wasNull()) None else Some(str) | ||
| } | ||
|
|
||
| def getNullableLong(label: String) : Option[Long] = { | ||
| val result = rs.getLong(label) | ||
| if (rs.wasNull()) None else Some(result) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation is a bit weird, it should be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7c2e8c9