Skip to content

Commit 797ec8e

Browse files
t-bastthomash-acinq
authored andcommitted
Fix SqlitePeersDb storage removal
The confusion between SQL timestamps and long resulted in always removing storage regardless of the `peerRemovedBefore` parameter.
1 parent 3f16d4a commit 797ec8e

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePeersDb.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,15 @@ class SqlitePeersDb(val sqlite: Connection) extends PeersDb with Logging {
4747
}
4848

4949
def migration23(statement: Statement): Unit = {
50-
statement.executeUpdate("CREATE TABLE peer_storage (node_id BLOB NOT NULL PRIMARY KEY, data NOT NULL, last_updated_at INTEGER NOT NULL, removed_peer_at INTEGER)")
50+
statement.executeUpdate("CREATE TABLE peer_storage (node_id BLOB NOT NULL PRIMARY KEY, data BLOB NOT NULL, last_updated_at INTEGER NOT NULL, removed_peer_at INTEGER)")
5151
statement.executeUpdate("CREATE INDEX removed_peer_at_idx ON peer_storage(removed_peer_at)")
5252
}
5353

5454
getVersion(statement, DB_NAME) match {
5555
case None =>
5656
statement.executeUpdate("CREATE TABLE peers (node_id BLOB NOT NULL PRIMARY KEY, data BLOB NOT NULL)")
5757
statement.executeUpdate("CREATE TABLE relay_fees (node_id BLOB NOT NULL PRIMARY KEY, fee_base_msat INTEGER NOT NULL, fee_proportional_millionths INTEGER NOT NULL)")
58-
statement.executeUpdate("CREATE TABLE peer_storage (node_id BLOB NOT NULL PRIMARY KEY, data NOT NULL, last_updated_at INTEGER NOT NULL, removed_peer_at INTEGER)")
58+
statement.executeUpdate("CREATE TABLE peer_storage (node_id BLOB NOT NULL PRIMARY KEY, data BLOB NOT NULL, last_updated_at INTEGER NOT NULL, removed_peer_at INTEGER)")
5959

6060
statement.executeUpdate("CREATE INDEX removed_peer_at_idx ON peer_storage(removed_peer_at)")
6161
case Some(v@(1 | 2)) =>
@@ -101,7 +101,7 @@ class SqlitePeersDb(val sqlite: Connection) extends PeersDb with Logging {
101101

102102
override def removePeerStorage(peerRemovedBefore: TimestampSecond): Unit = withMetrics("peers/remove-storage", DbBackends.Sqlite) {
103103
using(sqlite.prepareStatement("DELETE FROM peer_storage WHERE removed_peer_at < ?")) { statement =>
104-
statement.setTimestamp(1, peerRemovedBefore.toSqlTimestamp)
104+
statement.setLong(1, peerRemovedBefore.toLong)
105105
statement.executeUpdate()
106106
}
107107
}

eclair-core/src/test/scala/fr/acinq/eclair/db/PeersDbSpec.scala

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ package fr.acinq.eclair.db
1818

1919
import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
2020
import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases}
21+
import fr.acinq.eclair._
2122
import fr.acinq.eclair.db.pg.PgPeersDb
2223
import fr.acinq.eclair.db.sqlite.SqlitePeersDb
2324
import fr.acinq.eclair.payment.relay.Relayer.RelayFees
24-
import fr.acinq.eclair._
2525
import fr.acinq.eclair.wire.protocol.{NodeAddress, Tor2, Tor3}
2626
import org.scalatest.funsuite.AnyFunSuite
2727
import scodec.bits.HexStringSyntax
@@ -108,7 +108,7 @@ class PeersDbSpec extends AnyFunSuite {
108108
}
109109
}
110110

111-
test("peer storage") {
111+
test("add/update/remove peer storage") {
112112
forAllDbs { dbs =>
113113
val db = dbs.peers
114114

@@ -126,6 +126,21 @@ class PeersDbSpec extends AnyFunSuite {
126126
db.updateStorage(b, hex"abcd")
127127
assert(db.getStorage(a) == Some(hex"6789"))
128128
assert(db.getStorage(b) == Some(hex"abcd"))
129+
130+
// Actively used storage shouldn't be removed.
131+
db.removePeerStorage(TimestampSecond.now() + 1.hour)
132+
assert(db.getStorage(a) == Some(hex"6789"))
133+
assert(db.getStorage(b) == Some(hex"abcd"))
134+
135+
// After removing the peer, peer storage can be removed.
136+
db.removePeer(a)
137+
assert(db.getStorage(a) == Some(hex"6789"))
138+
db.removePeerStorage(TimestampSecond.now() - 1.hour)
139+
assert(db.getStorage(a) == Some(hex"6789"))
140+
db.removePeerStorage(TimestampSecond.now() + 1.hour)
141+
assert(db.getStorage(a) == None)
142+
assert(db.getStorage(b) == Some(hex"abcd"))
129143
}
130144
}
145+
131146
}

0 commit comments

Comments
 (0)