fix(db/remote): Fix remote db issues

[brbrr]

User description

1. Disable migrations for the remote DB.
2. Op_FIRST was not implemented. The server's cursor switch fell through to unknown operation, which killed the stream. Every prefixed Scan starts with it.First(), so all of them failed.
3. Iterator bounds were dropped on the wire. db/remote's NewIterator discarded the prefix and the upper-bound flag, so the server opened a whole-database iterator and First returned a key from a foreign bucket - silently wrong scan results, not an error.
4. State reads never released their batch. blockchain/statebackend/deprecated.go opened an IndexedBatch per state read and returned NoopStateCloser. Free on pebble, but on a remote DB that batch is a live stream plus a pinned server-side batch.

---

PR Type

Bug fix, Tests

---

Description

• Disable migrations for remote databases

• Release state read batches via proper closers

• Fix remote iterator bounds and Op_FIRST

• Close remote transactions with context cancellation

---

File Walkthrough

	Relevant files
Bug fix	7 files deprecated.go Release indexed batches after state reads                                +11/-5    db.go Add transaction context and discard helpers                            +34/-17  iterator.go Introduce ownedIterator closing its transaction                    +16/-0    transaction.go Send iterator bounds and release stream context                    +22/-6    handlers.go Handle Op_FIRST and pass cursor bounds                                      +6/-1      tx.go Create cursors with bounds and close transaction                  +3/-3      migration.go Skip migrations when using remote database                              +5/-0
Tests	2 files db_test.go Add remote iterator bounds and first tests                              +65/-0    migration_test.go Test migration skip for remote database                                    +6/-0
Documentation	1 files kv.proto Document Op_FIRST and V flag semantics                                      +2/-1

---

[github-actions]

Claude finished @brbrr's task in 6m 48s —— View job

---

Review: PR #4030 — fix(db/remote): Fix remote db issues

• Gather context (PR diff, CLAUDE.md)
• Review blockchain/statebackend/deprecated.go
• Review db/remote/db.go
• Review db/remote/iterator.go
• Review db/remote/transaction.go
• Review grpc/handlers.go / grpc/tx.go
• Review node/migration.go
• Post findings (inline where GitHub's diff allowed it; rest below)
• Post final summary

The four fixes described in the PR body all check out against the code:

• Op_FIRST is now handled in grpc/handlers.go's cursor switch.
• NewIterator now forwards prefix/withUpperBound over the wire (BucketName + V) and the server passes them into tx.dbTx.NewIterator (grpc/tx.go, grpc/handlers.go).
• Every remote read path (Get, Has, NewIterator, Update, Write) now discards/closes its transaction, including the new ownedIterator wrapper in db/remote/iterator.go that discards the whole transaction instead of doing a redundant Op_CLOSE round trip.
• Migrations are skipped for config.RemoteDB != "".
  go build and go vet are clean on the touched packages; I wasn't able to run go test (sandboxed shell), so please confirm db/remote and grpc tests pass in CI.

Couldn't inline these because they land outside the diff hunks GitHub allows comments on, so noting them here:

Important

• blockchain/statebackend/deprecated.go:140 (GetReverseStateDiff) opens b.database.NewIndexedBatch() but never closes it — the exact same batch/stream leak this PR just fixed in HeadState, StateAtBlockNumber, and StateAtBlockHash a few lines above. Worth adding defer txn.Close() here too. Fix this →
• grpc/handlers.go:44 — Handler.Tx creates dbTx := h.db.NewIndexedBatch() per stream but it's never closed/discarded when the stream ends; tx.cleanup() (grpc/tx.go:52) only closes tracked cursors, not dbTx itself. Pre-existing (not introduced by this PR), but it's the same server-side leak class this PR is fixing on the client, and every remote Tx call pins a batch on the underlying store indefinitely. Worth a follow-up.

Nit

• grpc/kv.proto:31 — the v field's comment (// not used) is now stale since it's repurposed to signal withUpperBound on Op_OPEN (db/remote/transaction.go:37-39). Also worth a one-line note that Op.FIRST = 0 is the enum zero-value, so any cursor message that fails to set Op silently becomes Op_FIRST — not new, but easy to trip over given Op_FIRST handling is brand new.
• No tests were added for either silent-correctness bug the PR fixes. The existing TestRemote/iterate test (db/remote/db_test.go:72) starts iteration with it.Next(), never it.First(), so it wouldn't have caught the missing-Op_FIRST bug, and no test exercises a prefixed/bounded scan, so it wouldn't have caught the dropped-bounds bug either. Given the PR description explicitly calls out that these failed silently rather than erroring, a regression test for both would be valuable.

No blockers — the core logic changes are sound and match the described root causes.
· branch fix/remote-db-migrations

[codecov]

Codecov Report

❌ Patch coverage is 75.51020% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.16%. Comparing base (5a74a35) to head (e8dc186).

Files with missing lines	Patch %	Lines
db/remote/db.go	50.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4030      +/-   ##
==========================================
- Coverage   79.28%   79.16%   -0.12%     
==========================================
  Files         464      464              
  Lines       35793    35815      +22     
==========================================
- Hits        28377    28354      -23     
- Misses       7407     7452      +45     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude finished @brbrr's task in 6m 8s —— View job

---

Review: PR #4030 — fix(db/remote): Fix remote db issues

• Gather context (diff vs origin/main, repo CLAUDE.md)
• Review blockchain/statebackend/deprecated.go
• Review db/remote/db.go
• Review db/remote/iterator.go
• Review db/remote/transaction.go
• Review grpc/handlers.go / grpc/tx.go
• Review node/migration.go
• Review new tests
• Post inline comments
• Post final summary

Summary

This revision (chore: address feedback, 1a2e6ac) resolves the two "Important" findings from the previous automated review round:

• GetReverseStateDiff in blockchain/statebackend/deprecated.go now defers txn.Close().
• grpc/tx.go's cleanup() now closes dbTx in addition to the tracked cursors, fixing the server-side batch leak on Handler.Tx.

I traced every production call site of HeadState/StateAtBlockNumber/StateAtBlockHash (rpc v8/v9/v10, builder, mempool, p2p sync/server, sync/data_source) to check that switching the deprecated backend's StateCloser from NoopStateCloser to a real txn.Close doesn't introduce a leak or double-close now that the closer actually does something on a remote DB — all of them already defer (or otherwise unconditionally invoke) the closer, so this is safe.

Also verified: go build and go vet are clean on db/remote, grpc, blockchain/statebackend, and node. I wasn't able to run go test in this sandbox (command required approval that wasn't grantable), so please confirm db/remote and node tests pass in CI — the new TestRemoteIteratorBounds and first subtests look like solid regression coverage for the two silent-correctness bugs this PR fixes.

Nit

• grpc/handlers.go:98 (inline) — Op_FIRST = 0 is the proto enum's zero-value, so any cursor message with an unset Op now silently behaves as First() instead of hitting the unknown operation error path. Not exploitable given db/remote's client always sets Op, but worth a doc comment since it's new behavior as of this PR (Op_FIRST wasn't handled before).
• grpc/kv.proto:31 — the v field's // not used comment is stale now that it's repurposed as the withUpperBound flag on Op_OPEN (db/remote/transaction.go:37-39). Flagged in the previous review round too; still open. Not inline-commentable since kv.proto isn't part of this diff.

No blockers. The core fixes (missing Op_FIRST handling, dropped iterator bounds, unclosed state-read batches, disabled remote migrations) are all correctly implemented and match the root causes described in the PR body.
· branch fix/remote-db-migrations

[github-actions]

Claude finished @brbrr's task in 4m 20s —— View job

---

Review: PR #4030 — fix(db/remote): Fix remote db issues

• Gather context (diff vs origin/main, prior review rounds)
• Review blockchain/statebackend/deprecated.go
• Review db/remote/db.go, iterator.go, transaction.go
• Review grpc/handlers.go, grpc/tx.go, grpc/kv.proto
• Review node/migration.go
• Review tests
• Post final summary

Summary

This push (address feedback, e8dc186) only touches db/remote/db_test.go (simplifies TestRemoteIteratorBounds to use memDB.NewBatch()/Write() instead of memDB.Update()) and grpc/kv.proto (adds the zero-value doc comment on Op and updates the stale // not used comment on Cursor.v). Both are exactly the two "Nit" items raised in the previous review round — no new logic changed.

I re-reviewed the full diff against origin/main end-to-end (not just the incremental change) to make sure nothing regressed:

• Op_FIRST handling in grpc/handlers.go is correct and matches the pattern of the other cursor ops.
• Iterator prefix/upper-bound are correctly threaded through db/remote/transaction.go → grpc/handlers.go → grpc/tx.go → dbTx.NewIterator.
• Resource cleanup is now consistent end-to-end: db/remote/db.go's Get/Has/NewIterator all discard their transaction, ownedIterator (db/remote/iterator.go) releases the stream on Close, blockchain/statebackend/deprecated.go closes the batch on all StateCloser paths (including the GetReverseStateDiff leak fixed last round), and grpc/tx.go's cleanup() now closes dbTx server-side too.
• node/migration.go correctly skips migrations when config.RemoteDB != "".
• No new goroutines, locks, or shared mutable state were introduced that would create races.

Both previously-flagged Important issues (unclosed batch in GetReverseStateDiff, unclosed dbTx in Handler.Tx) and both Nit issues (stale proto comment, Op_FIRST zero-value footgun) from earlier rounds are now resolved.

go build and go vet are clean on db/remote, grpc, blockchain/statebackend, and node. go test requires approval that isn't grantable in this sandboxed environment, so please confirm CI is green for db/remote and node (the new TestRemoteIteratorBounds and first/TestMigrateIfNeeded_SkipsMigrationsForRemoteDB tests give solid regression coverage for the bugs this PR fixes).

No blockers, no new findings. This PR looks ready to merge pending CI.
· branch fix/remote-db-migrations

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🏅 Score: 82

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Resource Leak Write only closes the batch in the two explicit return paths (fn returning an error, or fn succeeding). If fn panics, batch.Close() is never invoked, leaking the remote transaction's gRPC stream and the server-side pinned batch — exactly the kind of resource leak this PR is meant to fix. Using defer batch.Close() instead of closing inline in each branch would also cover the panic case, consistent with the fix applied to GetReverseStateDiff in this same PR. func (d *DB) Write(fn func(w db.Batch) error) error { defer d.listener.OnCommit(time.Now()) batch := d.NewBatch() if err := fn(batch); err != nil { return errors.Join(err, batch.Close()) } return errors.Join(batch.Write(), batch.Close()) }