From 71968d06167bb88f7960b6a7891a62d744dc4b35 Mon Sep 17 00:00:00 2001 From: Pierre-Marie Padiou Date: Thu, 25 May 2023 18:03:16 +0200 Subject: [PATCH] More robust channels timestamps (#2674) Previous implementation had the advantage of being all in one place, but it left holes: - `last_connected_timestamp` was only set after the first disconnection - in some corner cases the `closed_timestamp` was never set (nothing at stake, funding tx timeout, post-restart) --- .../fr/acinq/eclair/db/pg/PgChannelsDb.scala | 17 ++++++++--------- .../eclair/db/sqlite/SqliteChannelsDb.scala | 11 ++++++----- .../fr/acinq/eclair/db/ChannelsDbSpec.scala | 12 +++++------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala index ea152ae20f..a29b51d108 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala @@ -171,8 +171,8 @@ class PgChannelsDb(implicit ds: DataSource, lock: PgLock) extends ChannelsDb wit val encoded = channelDataCodec.encode(data).require.toByteArray using(pg.prepareStatement( """ - | INSERT INTO local.channels (channel_id, remote_node_id, data, json, is_closed) - | VALUES (?, ?, ?, ?::JSONB, FALSE) + | INSERT INTO local.channels (channel_id, remote_node_id, data, json, created_timestamp, last_connected_timestamp, is_closed) + | VALUES (?, ?, ?, ?::JSONB, ?, ?, FALSE) | ON CONFLICT (channel_id) | DO UPDATE SET data = EXCLUDED.data, json = EXCLUDED.json ; | """.stripMargin)) { statement => @@ -180,6 +180,8 @@ class PgChannelsDb(implicit ds: DataSource, lock: PgLock) extends ChannelsDb wit statement.setString(2, data.remoteNodeId.toHex) statement.setBytes(3, encoded) statement.setString(4, serialization.write(data)) + statement.setTimestamp(5, Timestamp.from(Instant.now())) + statement.setTimestamp(6, Timestamp.from(Instant.now())) statement.executeUpdate() } } @@ -194,9 +196,7 @@ class PgChannelsDb(implicit ds: DataSource, lock: PgLock) extends ChannelsDb wit } } - /** - * Helper method to factor updating timestamp columns - */ + /** Helper method to factor updating timestamp columns */ private def updateChannelMetaTimestampColumn(channelId: ByteVector32, columnName: String): Unit = { inTransaction(IsolationLevel.TRANSACTION_READ_UNCOMMITTED) { pg => using(pg.prepareStatement(s"UPDATE local.channels SET $columnName=? WHERE channel_id=?")) { statement => @@ -209,11 +209,9 @@ class PgChannelsDb(implicit ds: DataSource, lock: PgLock) extends ChannelsDb wit override def updateChannelMeta(channelId: ByteVector32, event: ChannelEvent.EventType): Unit = { val timestampColumn_opt = event match { - case ChannelEvent.EventType.Created => Some("created_timestamp") case ChannelEvent.EventType.Connected => Some("last_connected_timestamp") case ChannelEvent.EventType.PaymentReceived => Some("last_payment_received_timestamp") case ChannelEvent.EventType.PaymentSent => Some("last_payment_sent_timestamp") - case _: ChannelEvent.EventType.Closed => Some("closed_timestamp") case _ => None } timestampColumn_opt.foreach(updateChannelMetaTimestampColumn(channelId, _)) @@ -231,8 +229,9 @@ class PgChannelsDb(implicit ds: DataSource, lock: PgLock) extends ChannelsDb wit statement.executeUpdate() } - using(pg.prepareStatement("UPDATE local.channels SET is_closed=TRUE WHERE channel_id=?")) { statement => - statement.setString(1, channelId.toHex) + using(pg.prepareStatement("UPDATE local.channels SET is_closed=TRUE, closed_timestamp=? WHERE channel_id=?")) { statement => + statement.setTimestamp(1, Timestamp.from(Instant.now())) + statement.setString(2, channelId.toHex) statement.executeUpdate() } } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala index c39ee4f68d..4648bf73df 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala @@ -106,9 +106,11 @@ class SqliteChannelsDb(val sqlite: Connection) extends ChannelsDb with Logging { update.setBytes(1, encoded) update.setBytes(2, data.channelId.toArray) if (update.executeUpdate() == 0) { - using(sqlite.prepareStatement("INSERT INTO local_channels (channel_id, data, is_closed) VALUES (?, ?, 0)")) { statement => + using(sqlite.prepareStatement("INSERT INTO local_channels (channel_id, data, created_timestamp, last_connected_timestamp, is_closed) VALUES (?, ?, ?, ?, 0)")) { statement => statement.setBytes(1, data.channelId.toArray) statement.setBytes(2, encoded) + statement.setLong(3, TimestampMilli.now().toLong) + statement.setLong(4, TimestampMilli.now().toLong) statement.executeUpdate() } } @@ -135,11 +137,9 @@ class SqliteChannelsDb(val sqlite: Connection) extends ChannelsDb with Logging { override def updateChannelMeta(channelId: ByteVector32, event: ChannelEvent.EventType): Unit = { val timestampColumn_opt = event match { - case ChannelEvent.EventType.Created => Some("created_timestamp") case ChannelEvent.EventType.Connected => Some("last_connected_timestamp") case ChannelEvent.EventType.PaymentReceived => Some("last_payment_received_timestamp") case ChannelEvent.EventType.PaymentSent => Some("last_payment_sent_timestamp") - case _: ChannelEvent.EventType.Closed => Some("closed_timestamp") case _ => None } timestampColumn_opt.foreach(updateChannelMetaTimestampColumn(channelId, _)) @@ -156,8 +156,9 @@ class SqliteChannelsDb(val sqlite: Connection) extends ChannelsDb with Logging { statement.executeUpdate() } - using(sqlite.prepareStatement("UPDATE local_channels SET is_closed=1 WHERE channel_id=?")) { statement => - statement.setBytes(1, channelId.toArray) + using(sqlite.prepareStatement("UPDATE local_channels SET is_closed=1, closed_timestamp=? WHERE channel_id=?")) { statement => + statement.setLong(1, TimestampMilli.now().toLong) + statement.setBytes(2, channelId.toArray) statement.executeUpdate() } } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelsDbSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelsDbSpec.scala index 549798afed..4bd985808d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelsDbSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelsDbSpec.scala @@ -127,11 +127,10 @@ class ChannelsDbSpec extends AnyFunSuite { db.addOrUpdateChannel(channel1) db.addOrUpdateChannel(channel2) - // make sure initially all metadata are empty - assert(getTimestamp(dbs, channel1.channelId, "created_timestamp").isEmpty) + assert(getTimestamp(dbs, channel1.channelId, "created_timestamp").nonEmpty) assert(getTimestamp(dbs, channel1.channelId, "last_payment_sent_timestamp").isEmpty) assert(getTimestamp(dbs, channel1.channelId, "last_payment_received_timestamp").isEmpty) - assert(getTimestamp(dbs, channel1.channelId, "last_connected_timestamp").isEmpty) + assert(getTimestamp(dbs, channel1.channelId, "last_connected_timestamp").nonEmpty) assert(getTimestamp(dbs, channel1.channelId, "closed_timestamp").isEmpty) db.updateChannelMeta(channel1.channelId, ChannelEvent.EventType.Created) @@ -146,14 +145,13 @@ class ChannelsDbSpec extends AnyFunSuite { db.updateChannelMeta(channel1.channelId, ChannelEvent.EventType.Connected) assert(getTimestamp(dbs, channel1.channelId, "last_connected_timestamp").nonEmpty) - db.updateChannelMeta(channel1.channelId, ChannelEvent.EventType.Closed(null)) + db.removeChannel(channel1.channelId) assert(getTimestamp(dbs, channel1.channelId, "closed_timestamp").nonEmpty) - // make sure all metadata are still empty for channel 2 - assert(getTimestamp(dbs, channel2.channelId, "created_timestamp").isEmpty) + assert(getTimestamp(dbs, channel2.channelId, "created_timestamp").nonEmpty) assert(getTimestamp(dbs, channel2.channelId, "last_payment_sent_timestamp").isEmpty) assert(getTimestamp(dbs, channel2.channelId, "last_payment_received_timestamp").isEmpty) - assert(getTimestamp(dbs, channel2.channelId, "last_connected_timestamp").isEmpty) + assert(getTimestamp(dbs, channel2.channelId, "last_connected_timestamp").nonEmpty) assert(getTimestamp(dbs, channel2.channelId, "closed_timestamp").isEmpty) } }