commit e07929117f2c9493442cf52a31b0123e3463115a
parent 375c75212cc6e04f3c9c024b7e08c09b4bdb505d
Author: Florian Dold <dold@taler.net>
Date: Sun, 19 Jul 2026 22:33:55 +0200
db: count rows read on the sqlite backend
Conformance cases assert a query is bounded. Without this the sqlite
backend couldn't be checked.
Diffstat:
3 files changed, 119 insertions(+), 32 deletions(-)
diff --git a/packages/taler-wallet-core/src/dbtx-conformance-cases.ts b/packages/taler-wallet-core/src/dbtx-conformance-cases.ts
@@ -92,6 +92,7 @@ import {
WalletExchangeSignkeys,
} from "./db-common.js";
import { ConformanceCase } from "./dbtx-conformance.js";
+import { WalletDbTransaction } from "./dbtx.js";
/**
* Deterministic Crockford base32 value derived from a readable label.
@@ -2845,4 +2846,69 @@ export const conformanceCases: ConformanceCase[] = [
);
},
},
+ {
+ name: "bounded queries do not scan: limit, cursor and point lookup",
+ // These are the other places where a correct-looking implementation can
+ // quietly read the whole table and filter afterwards. The row counter
+ // is the only way to see it: every one of these returns the right
+ // answer either way.
+ async run(t, runner) {
+ const N = 40;
+ await runner.runTx(async (tx) => {
+ for (let i = 0; i < N; i++) {
+ const c = makeCoin(`scan-${i}`);
+ c.exchangeBaseUrl = "https://scan/";
+ c.denomPubHash = ckh("scan-denom");
+ c.maxAge = 0;
+ c.status = CoinStatus.Fresh;
+ await tx.upsertCoin(c);
+ await tx.upsertTransactionMeta({
+ transactionId: `txn:scan:${String(i).padStart(3, "0")}`,
+ timestamp: tsPrecise(1000 + i),
+ status: WithdrawalGroupStatus.Done,
+ currency: "TESTKUDOS",
+ exchanges: [],
+ });
+ }
+ });
+
+ const measure = async (
+ label: string,
+ limit: number,
+ f: (tx: WalletDbTransaction) => Promise<unknown>,
+ ): Promise<void> => {
+ const before = runner.recordsRead?.();
+ await runner.runTx(f);
+ const after = runner.recordsRead?.();
+ if (before === undefined || after === undefined) {
+ return;
+ }
+ t.ok(
+ after - before <= limit,
+ `${label}: read ${after - before} records, expected at most ${limit}`,
+ );
+ };
+
+ await measure("getCoin point lookup", 3, (tx) =>
+ tx.getCoin(ck("scan-7")),
+ );
+ await measure("getFreshCoinsByDenomAndAge with limit 5", 8, (tx) =>
+ tx.getFreshCoinsByDenomAndAge("https://scan/", ckh("scan-denom"), 0, 5),
+ );
+ await measure("listTransactionMetaByTimestamp with limit 5", 8, (tx) =>
+ tx.listTransactionMetaByTimestamp({ limit: 5 }),
+ );
+ await measure("getTransactionMetaAfter", 3, (tx) =>
+ tx.getTransactionMetaAfter(tsPrecise(1010)),
+ );
+
+ // getTransactionMetaBefore is deliberately left out. The sqlite
+ // implementation is bounded (ORDER BY ... DESC LIMIT 1, one row read),
+ // but the IndexedDB one reads the whole range below the bound and
+ // takes the last entry -- an accepted cost, documented there, because
+ // it is only reached when a pagination offset transaction was deleted.
+ // Asserting a bound here would encode an expectation one backend does
+ // not meet by design; the DAL contract is the result, not the cost.
+ },
+ },
];
diff --git a/packages/taler-wallet-core/src/dbtx-runners.ts b/packages/taler-wallet-core/src/dbtx-runners.ts
@@ -31,9 +31,8 @@ import { DbAccessImpl } from "./query.js";
import { DbTxRunner } from "./dbtx-conformance.js";
import { IdbWalletTransaction } from "./dbtx-indexeddb.js";
import {
- initSqliteWalletDb,
- SqliteTxControl,
- SqliteWalletTransaction,
+ openNativeSqliteWalletDb,
+ runNativeSqliteWalletTx,
} from "./dbtx-sqlite.js";
const logger = new Logger("dbtx-runners.ts");
@@ -100,36 +99,25 @@ export async function makeSqliteRunner(
const sqlite3Impl = await createNodeHelperSqlite3Impl({
enableTracing: false,
});
- const db = await sqlite3Impl.open(filename);
- await initSqliteWalletDb(db);
- const txc = await SqliteTxControl.create(db);
+ const ndb = await openNativeSqliteWalletDb(await sqlite3Impl.open(filename));
return {
name: "SqliteWalletTransaction",
async runTx<T>(f: (tx: any) => Promise<T>): Promise<T> {
- const tx = new SqliteWalletTransaction(db);
- await txc.begin();
- let res: T;
- try {
- res = await f(tx);
- } catch (e) {
- // Roll back best-effort: if ROLLBACK itself fails the original error
- // is the interesting one and must not be masked by it.
- try {
- await txc.rollback();
- } catch (rollbackErr) {
- logger.warn(`rollback failed: ${rollbackErr}`);
- }
- throw e;
- }
- await txc.commit();
- // Notifications and commit hooks are released only after COMMIT.
- for (const h of tx.afterCommitHandlers) {
- h();
- }
- return res;
+ // Goes through the same helper the wallet uses, rather than a copy of
+ // it: otherwise the suite would not exercise transaction
+ // serialisation, the shared statement cache, checkpoint-on-idle or
+ // post-commit notification delivery.
+ return await runNativeSqliteWalletTx(
+ ndb,
+ () => {},
+ async (tx) => f(tx),
+ );
+ },
+ recordsRead() {
+ return ndb.stats.rowsRead;
},
async close() {
- await db.close();
+ await ndb.db.close();
},
};
}
diff --git a/packages/taler-wallet-core/src/dbtx-sqlite.ts b/packages/taler-wallet-core/src/dbtx-sqlite.ts
@@ -384,11 +384,18 @@ export class SqliteWalletTransaction implements WalletDbTransaction {
*/
private stmtCache: Map<string, Sqlite3Statement>;
+ /**
+ * Row counter shared with the connection, when one was supplied.
+ */
+ private stats: SqliteAccessStats;
+
constructor(
private db: Sqlite3Database,
stmtCache?: Map<string, Sqlite3Statement>,
+ stats?: SqliteAccessStats,
) {
this.stmtCache = stmtCache ?? new Map();
+ this.stats = stats ?? { rowsRead: 0 };
}
private async prep(sql: string): Promise<Sqlite3Statement> {
@@ -408,14 +415,20 @@ export class SqliteWalletTransaction implements WalletDbTransaction {
sql: string,
params: Record<string, any> = {},
): Promise<ResultRow | undefined> {
- return await (await this.prep(sql)).getFirst(params);
+ const row = await (await this.prep(sql)).getFirst(params);
+ if (row !== undefined) {
+ this.stats.rowsRead++;
+ }
+ return row;
}
private async all(
sql: string,
params: Record<string, any> = {},
): Promise<ResultRow[]> {
- return await (await this.prep(sql)).getAll(params);
+ const rows = await (await this.prep(sql)).getAll(params);
+ this.stats.rowsRead += rows.length;
+ return rows;
}
// Bound as an instance property for the same reason as the IndexedDB
@@ -4735,9 +4748,23 @@ export class SqliteWalletTransaction implements WalletDbTransaction {
* statements, because those must be prepared once per connection and must not
* go through `exec` (see {@link SqliteTxControl}).
*/
+/**
+ * Rows handed back by the statement layer, counted per connection.
+ *
+ * Exists so a test can assert that a query is *bounded* rather than scanning.
+ * Counting callbacks is not enough: a backend can materialise a whole range
+ * and still invoke a predicate once, which is exactly the regression the
+ * denomination keyset scan is guarded against.
+ */
+export interface SqliteAccessStats {
+ rowsRead: number;
+}
+
export interface NativeSqliteWalletDb {
db: Sqlite3Database;
txc: SqliteTxControl;
+ /** Cumulative across transactions on this connection. */
+ stats: SqliteAccessStats;
/** Shared across transactions; see SqliteWalletTransaction.stmtCache. */
stmtCache: Map<string, Sqlite3Statement>;
/**
@@ -4809,7 +4836,13 @@ export async function openNativeSqliteWalletDb(
): Promise<NativeSqliteWalletDb> {
await initSqliteWalletDb(db);
const txc = await SqliteTxControl.create(db);
- return { db, txc, lock: new TxQueue(), stmtCache: new Map() };
+ return {
+ db,
+ txc,
+ lock: new TxQueue(),
+ stmtCache: new Map(),
+ stats: { rowsRead: 0 },
+ };
}
/**
@@ -4833,7 +4866,7 @@ async function runNativeSqliteWalletTxLocked<T>(
notifyFn: (n: WalletNotification) => void,
f: (tx: SqliteWalletTransaction) => Promise<T>,
): Promise<T> {
- const tx = new SqliteWalletTransaction(ndb.db, ndb.stmtCache);
+ const tx = new SqliteWalletTransaction(ndb.db, ndb.stmtCache, ndb.stats);
await ndb.txc.begin();
let res: T;
try {
