From 2a8e01cc9f9ca77e75991bd584d6752e120c9db6 Mon Sep 17 00:00:00 2001
From: Alex Gherghisan <alexghr@users.noreply.github.com>
Date: Fri, 3 Jan 2025 09:56:21 +0000
Subject: [PATCH] fix: use explicit read transactions (#10911)

Open and hold a read txn open until the entire iterator is consumed.

---------

Co-authored-by: ludamad <domuradical@gmail.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
---
 .../src/e2e_fees/private_payments.test.ts     |  3 +-
 yarn-project/kv-store/src/lmdb/array.ts       | 22 +++--
 yarn-project/kv-store/src/lmdb/map.ts         | 80 +++++++++++--------
 3 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts
index c1fa2d4fbec..39420fd44b8 100644
--- a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts
+++ b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts
@@ -325,8 +325,7 @@ describe('e2e_fees private_payment', () => {
   });
 
   // TODO(#7694): Remove this test once the lacking feature in TXE is implemented.
-  // TODO(#10775): Reenable, hit e.g. https://github.com/AztecProtocol/aztec-packages/actions/runs/12419409370/job/34675397831
-  it.skip('insufficient funded amount is correctly handled', async () => {
+  it('insufficient funded amount is correctly handled', async () => {
     // We call arbitrary `private_get_name(...)` function just to check the correct error is triggered.
     await expect(
       bananaCoin.methods.private_get_name().prove({
diff --git a/yarn-project/kv-store/src/lmdb/array.ts b/yarn-project/kv-store/src/lmdb/array.ts
index 19537dd7598..08aa0decda5 100644
--- a/yarn-project/kv-store/src/lmdb/array.ts
+++ b/yarn-project/kv-store/src/lmdb/array.ts
@@ -90,14 +90,20 @@ export class LmdbAztecArray<T> implements AztecArray<T>, AztecAsyncArray<T> {
   }
 
   *entries(): IterableIterator<[number, T]> {
-    const values = this.#db.getRange({
-      start: this.#slot(0),
-      limit: this.length,
-    });
-
-    for (const { key, value } of values) {
-      const index = key[3];
-      yield [index, value];
+    const transaction = this.#db.useReadTransaction();
+    try {
+      const values = this.#db.getRange({
+        start: this.#slot(0),
+        limit: this.length,
+        transaction,
+      });
+
+      for (const { key, value } of values) {
+        const index = key[3];
+        yield [index, value];
+      }
+    } finally {
+      transaction.done();
     }
   }
 
diff --git a/yarn-project/kv-store/src/lmdb/map.ts b/yarn-project/kv-store/src/lmdb/map.ts
index 4458c3c3539..dd0d9aef064 100644
--- a/yarn-project/kv-store/src/lmdb/map.ts
+++ b/yarn-project/kv-store/src/lmdb/map.ts
@@ -40,9 +40,16 @@ export class LmdbAztecMap<K extends Key, V> implements AztecMultiMap<K, V>, Azte
   }
 
   *getValues(key: K): IterableIterator<V> {
-    const values = this.db.getValues(this.slot(key));
-    for (const value of values) {
-      yield value?.[1];
+    const transaction = this.db.useReadTransaction();
+    try {
+      const values = this.db.getValues(this.slot(key), {
+        transaction,
+      });
+      for (const value of values) {
+        yield value?.[1];
+      }
+    } finally {
+      transaction.done();
     }
   }
 
@@ -88,38 +95,45 @@ export class LmdbAztecMap<K extends Key, V> implements AztecMultiMap<K, V>, Azte
   }
 
   *entries(range: Range<K> = {}): IterableIterator<[K, V]> {
-    const { reverse = false, limit } = range;
-    // LMDB has a quirk where it expects start > end when reverse=true
-    // in that case, we need to swap the start and end sentinels
-    const start = reverse
-      ? range.end
-        ? this.slot(range.end)
-        : this.endSentinel
-      : range.start
-      ? this.slot(range.start)
-      : this.startSentinel;
-
-    const end = reverse
-      ? range.start
+    const transaction = this.db.useReadTransaction();
+
+    try {
+      const { reverse = false, limit } = range;
+      // LMDB has a quirk where it expects start > end when reverse=true
+      // in that case, we need to swap the start and end sentinels
+      const start = reverse
+        ? range.end
+          ? this.slot(range.end)
+          : this.endSentinel
+        : range.start
         ? this.slot(range.start)
-        : this.startSentinel
-      : range.end
-      ? this.slot(range.end)
-      : this.endSentinel;
+        : this.startSentinel;
 
-    const lmdbRange: RangeOptions = {
-      start,
-      end,
-      reverse,
-      limit,
-    };
-
-    const iterator = this.db.getRange(lmdbRange);
-
-    for (const {
-      value: [key, value],
-    } of iterator) {
-      yield [key, value];
+      const end = reverse
+        ? range.start
+          ? this.slot(range.start)
+          : this.startSentinel
+        : range.end
+        ? this.slot(range.end)
+        : this.endSentinel;
+
+      const lmdbRange: RangeOptions = {
+        start,
+        end,
+        reverse,
+        limit,
+        transaction,
+      };
+
+      const iterator = this.db.getRange(lmdbRange);
+
+      for (const {
+        value: [key, value],
+      } of iterator) {
+        yield [key, value];
+      }
+    } finally {
+      transaction.done();
     }
   }