SQLite lease cleanup joins token_locks on the wrong column, expiring locks by the locked token's tx instead of the consumer's #2018

Summary

The token_locks lease cleanup is implemented separately per backend, and the two implementations join on different columns. The SQLite version joins token_locks to token_requests on tl.tx_id = tr.tx_id; the Postgres version correlates on tl.consumer_tx_id = tr.tx_id.

token/services/storage/db/sql/sqlite/tokenlock.go:36-46:

q.Select().
    Fields(tokenLocks.Field("tx_id")).
    From(tokenLocks.Join(tokenRequests, cond.Cmp(tokenLocks.Field("tx_id"), "=", tokenRequests.Field("tx_id")))).
    Where(common4.IsExpiredToken(tokenRequests, tokenLocks, c.leaseExpiry)).

token/services/storage/db/sql/postgres/tokenlock.go:72-80:

cond.Cmp(tokenRequests.Field("tx_id"), "=", tokenLocks.Field("consumer_tx_id")),
cond.FieldIn(tokenRequests.Field("status"), driver.Deleted, driver.Orphan),

The two columns mean different things. In token_locks, (tx_id, idx) identifies the locked token — i.e. the transaction that created it — while consumer_tx_id identifies the transaction that is trying to spend it. A lease should expire when the consuming transaction is Deleted/Orphan, which is what Postgres does. SQLite instead expires the lock based on the status of the transaction that produced the token.

Consequences on SQLite

• Locks leak. When a consuming transaction becomes Deleted or Orphan, its locks are not released by the status branch. They are only reclaimed once created_at passes leaseExpiry (default 3m), so tokens stay unspendable for the remainder of the lease.
• Locks are released too early. If the transaction that created the token is Deleted/Orphan, the lock is deleted even though the consuming transaction may be in flight — dropping a lock that is still live.

Second defect: the DELETE is scoped to the wrong key

The subquery selects only tl.tx_id, and the outer statement is DELETE FROM <token_locks> WHERE tx_id IN (...) (sqlite/tokenlock.go:39-45, :48-51). Since the primary key is (tx_id, idx), matching on tx_id alone deletes locks for every index of that transaction, not just the expired row. One expired lock on output 0 evicts live locks on outputs 1..n of the same transaction. The Postgres version has no such issue: it deletes by correlated predicate rather than by a partial-key IN.

Why it is not caught by tests

token/services/storage/db/sql/sqlite/tokenlock_test.go:36-51 asserts the generated SQL string rather than the behaviour, so it encodes the current (incorrect) join as expected output:

Expect(query).To(Equal("DELETE FROM TokenLocks WHERE tx_id IN (" +
    "SELECT tl.tx_id " +
    "FROM TokenLocks AS tl " +
    "LEFT JOIN Requests AS tr " +
    "ON tl.tx_id = tr.tx_id " + ...

There is no test asserting that a lock held by a Deleted consumer is actually released, nor that sibling indices survive cleanup. Note the join also renders as LEFT JOIN, so rows with no matching request participate with NULL status.

Expected behaviour

Both backends should expire a lock when the consumer transaction is Deleted/Orphan or the lease has aged out, and should delete only the matching (tx_id, idx) rows. The shared IsExpiredToken predicate (token/services/storage/db/sql/common/tokenlock.go:111-116) already exists and can back a single implementation for both backends.

Behavioural tests should replace the string-equality assertions: insert locks for a Deleted consumer and assert release; insert multiple indices under one tx_id, expire one, and assert the siblings survive.

Impact

SQLite is the default local/dev and in-memory backend, so selector behaviour differs between development and Postgres production deployments. Both symptoms — leaked locks and prematurely dropped locks — manifest as spurious SelectorSufficientButLockedFunds or as double-spend attempts surfacing later in validation.

Related: #2017.