feat(icp-rosetta): DEFI-2950: Migrate block store to async tokio-rusqlite by mbjorkqvist · Pull Request #10817 · dfinity/ic

mbjorkqvist · 2026-07-17T14:33:06Z

Purpose

ICP Rosetta serves its endpoints from async handlers on a multi-threaded
tokio runtime, but its block store's query methods were blocking
(synchronous SQLite behind a mutex). Under concurrent load these calls occupied
all tokio worker threads for the duration of each query, starving every other
async task — health checks, metrics, the block-sync watchdog — which is the
runtime-starvation bottleneck analysed under DEFI-2491.

This PR removes that starvation by moving the ICP block store to async
tokio-rusqlite (queries run on a dedicated background DB thread), mirroring
what ICRC Rosetta already does. The block store's API becomes async and its
callers await it, so a slow query no longer blocks the runtime.

A new deterministic regression test demonstrates the fix: it hammers the store
from several concurrent tasks on a 2-worker runtime while a lightweight canary
task only sleeps and counts ticks. On master the blocking store starved the
canary (~1/200 ticks); with this change it ticks ~180/200. Red on master, green
here.

PR stack — 3 ICP Rosetta PRs, in merge order

DEFI-2950 async tokio-rusqlite block store ← this PR
DEFI-2951 async-store hardening (panic-safety + shorten lock hold) — lands before the axum migration
DEFI-2952 actix → axum migration

Move the ICP Rosetta block store off a blocking `std::sync::Mutex<rusqlite::Connection>` and onto async `tokio-rusqlite`, so the store's query methods no longer occupy the tokio worker threads for the whole duration of each SQLite call and starve the async runtime. The low-level SQL stays as synchronous free functions in `database_access`; each public `Blocks` method (and the constructors) is now `async` and wraps its work in `connection.call(...).await`, mapping the outer `tokio_rusqlite::Error` back into `BlockStoreError`. Callers in the synchronizer and request handler add `.await`. Includes a deterministic runtime-responsiveness regression test that was red on master (the store starved a lightweight canary task) and is green after the migration. PR 1 of 2 under DEFI-2950; the actix->axum migration follows in PR 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR migrates the ICP Rosetta block store from a synchronous rusqlite connection behind a Mutex to an async tokio-rusqlite connection to prevent Tokio runtime worker-thread starvation during SQLite queries.

Changes:

Reworked Blocks to use tokio_rusqlite::Connection and made its constructors and query/mutation APIs async.
Updated request handler, synchronizer, and tests to await block-store operations and adjusted LedgerAccess::read_blocks to return a Send guard.
Added a regression test that detects runtime starvation under concurrent store load.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rs/rosetta-api/icp/tests/test_utils/mod.rs	Makes `TestLedger` construction async and awaits `Blocks` operations.
rs/rosetta-api/icp/tests/rosetta_cli_tests.rs	Awaits async `TestLedger::new()` in CLI integration tests.
rs/rosetta-api/icp/src/request_handler/tests.rs	Updates mock `LedgerAccess::read_blocks` signature to include `Send`.
rs/rosetta-api/icp/src/request_handler.rs	Awaits `Blocks` methods and adapts custom-query parameter passing for async store calls.
rs/rosetta-api/icp/src/ledger_client.rs	Updates `LedgerAccess::read_blocks` return type to be `Send`.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/tests/store_tests.rs	Converts store tests/helpers to async and awaits store methods.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/tests/runtime_responsiveness_test.rs	Adds deterministic regression test for Tokio runtime starvation.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/src/ledger_blocks_sync.rs	Awaits async `Blocks` constructors/methods during synchronization and initialization.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/src/blocks.rs	Core migration to `tokio-rusqlite`, async API surface, error conversions, and updated unit tests.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/Cargo.toml	Adds `tokio-rusqlite` dependency.
rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/BUILD.bazel	Adds Bazel dependency for `tokio-rusqlite`.
Cargo.lock	Records the new dependency in the lockfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mbjorkqvist · 2026-07-17T15:15:57Z

 pub trait LedgerAccess {
    // Maybe we should just return RwLockReadGuard explicitly and drop the Box
-    async fn read_blocks<'a>(&'a self) -> Box<dyn Deref<Target = Blocks> + 'a>;
+    async fn read_blocks<'a>(&'a self) -> Box<dyn Deref<Target = Blocks> + Send + 'a>;
    async fn sync_blocks(&self, stopped: Arc<AtomicBool>) -> Result<(), ApiError>;


Good point. The guard here is a tokio::sync::RwLock read guard, and tokio's RwLock is write-preferring — a waiting writer (block sync / prune) blocks new readers rather than being starved — so this isn't a live starvation bug. That said, holding the guard across .await isn't ideal: we're tracking shortening the hold (cloning the needed data out before awaiting) and the deadlock-discipline point (not re-acquiring the lock under a guard) in DEFI-2951, which will land before the actix→axum migration.

Addresses review finding 1 (transaction rollback safety). `push` and `prune` opened a transaction on the long-lived tokio-rusqlite connection but returned early on error without closing it, leaving the transaction open so every subsequent write failed with "cannot start a transaction within a transaction". Both now roll back explicitly on every error path, mirroring `push_batch`. `rusqlite::Transaction` does not implement `DerefMut`, so the RAII approach used by `create_tables` does not compose with the `&mut Connection` signatures of the `database_access::*` helpers; the explicit ROLLBACK approach was chosen for that reason. Adds a regression test that a duplicate push errors and a subsequent valid push still succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review finding 5 (Copilot): "forein" -> "foreign". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review finding 2. The runtime responsiveness test builds a fixed 2-worker tokio runtime and asserts a wall-clock tick ratio, so it can under-tick and flake on an oversubscribed CI runner. Extract it into its own `rust_test` target tagged `cpu:4` (excluded from the tests/** suite glob so it is not double-registered) and document the CPU-count assumption in the test file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review finding 3. `from_blockchain` is now called by `new()`, so the `#[allow(dead_code)]` above it is stale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…tions Addresses review finding 4. The `x as i64` casts on the u64 block index and limit params silently bit-reinterpret; add a one-line comment noting that indices/limits are well within the i64 range. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/src/blocks.rs:1678

Inside the tokio_rusqlite::Connection::call closure, several prepare_cached(...).unwrap() calls can panic. With a long-lived, single-threaded DB worker this can be more damaging than before (it can surface as a panic/connection-closed error and potentially break the store for the rest of the process).

It’s safer to propagate these as BlockStoreErrors instead of panicking.

        let sql_tx = connection
            .transaction()
            .map_err(|e| format!("Unable to initialize a transaction: {e:?}"))?;

        let mut insert_rosetta_block_stmt = sql_tx
            .prepare_cached(
                r#"INSERT INTO rosetta_blocks (rosetta_block_idx, hash, timestamp)
                VALUES (:idx, :hash, :timestamp)"#,

mbjorkqvist · 2026-07-20T09:53:02Z

+        parameters.push((
+            // `as i64` bit-reinterprets the u64; block indices are well within
+            // the i64 range, so this is lossless in practice.
+            ":max_block_idx".to_string(),
+            rusqlite::types::Value::from(start_idx as i64),
+        ));


Good catch — the comment was misleading. Addressed in a follow-up commit: start_idx derives from max_block, which comes from the i64 max_block request field and is validated via try_into::<u64>() (negatives rejected), so the value provably fits in i64 and the u64 → i64 cast round-trips losslessly. Reworded to say that, instead of implying the account-balance bit-reinterpretation.

mbjorkqvist · 2026-07-20T09:53:03Z

        command.push_str("LIMIT :limit ");
-        parameters.push((":limit", Box::new(limit)));
+        parameters.push((
+            // `as i64` bit-reinterprets the u64; the limit is well within the
+            // i64 range, so this is lossless in practice.
+            ":limit".to_string(),
+            rusqlite::types::Value::from(limit as i64),
+        ));


Same fix in the same commit: limit originates from the i64 limit request field (try_into::<u64>()-validated), so the cast back to i64 is guaranteed lossless. Comment reworded to state the real reason rather than referencing bit-reinterpretation.

mbjorkqvist · 2026-07-20T09:53:03Z

        if range.end > range.start
-            && database_access::contains_block(&mut connection, &range.start).unwrap_or(false)
+            && database_access::contains_block(connection, &range.start).unwrap_or(false)
        {


Agreed — addressed in a follow-up commit: contains_block(...).unwrap_or(false) is now contains_block(...)?, so real DB errors propagate as BlockStoreError instead of being masked as "range not allowed or not found".

…elds Reword the max_block_idx and limit cast comments in search_transactions. The values originate from the request's i64 max_block/limit fields and are try_into()-validated non-negative, so the u64 -> i64 cast is lossless; this is unrelated to the account-balance bit-reinterpretation convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace contains_block(...).unwrap_or(false) with `?` so prepare/query failures surface as the real BlockStoreError instead of being masked as a "range not allowed or not found" result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

mbjorkqvist · 2026-07-20T12:16:13Z

+        // We fetch the first icp block to extract the timestamp, the first icp transaction and the first parent_hash of the first rosetta block created
+        let Block {
+            parent_hash,
+            timestamp,
+            ..
+        } = Block::decode(
+            self.get_hashed_block(&next_block_indices.first_block_index)
+                .await?
+                .block,
+        )?;
+


Good catch, and correct. Note this is pre-existing (identical on master) and unrelated to the async-SQLite migration in this PR: in Rosetta Blocks mode, when already caught up (first_block_index == certified_tip_index + 1) the empty work range should be a no-op, but the first block is pre-fetched and returns NotFound. Tracking it as a separate defect — DEFI-2953 — rather than expanding this PR's scope.

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

+                connection
+                    .execute_batch("COMMIT TRANSACTION;")
+                    .map_err(|e| BlockStoreError::Other(format!("{e}")))?;


+                con.execute_batch("COMMIT TRANSACTION;")
+                    .map_err(|e| BlockStoreError::Other(format!("{e}")))?;


        connection
            .execute_batch("COMMIT TRANSACTION;")
            .map_err(|e| BlockStoreError::Other(format!("{e}")))?;


+                        // with no `.await`, so it monopolises the worker thread.
+                        // After the async-SQLite migration this becomes
+                        // `store.get_hashed_block(&block_idx).await`.
+                        let _ = store.get_hashed_block(&block_idx).await;


mbjorkqvist requested a review from Copilot July 17, 2026 14:33

github-actions Bot added the feat label Jul 17, 2026

Copilot started reviewing on behalf of mbjorkqvist July 17, 2026 14:33 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

mbjorkqvist and others added 5 commits July 17, 2026 15:21

docs(icp-rosetta): fix "forein" typo in blocks.rs comment

ecb8e7e

Addresses review finding 5 (Copilot): "forein" -> "foreign". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chore(icp-rosetta): drop stale allow(dead_code) on from_blockchain

04f5edf

Addresses review finding 3. `from_blockchain` is now called by `new()`, so the `#[allow(dead_code)]` above it is stale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mbjorkqvist requested a review from Copilot July 17, 2026 16:54

Copilot started reviewing on behalf of mbjorkqvist July 17, 2026 16:54 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

mbjorkqvist and others added 2 commits July 20, 2026 09:54

mbjorkqvist changed the title ~~feat(icp-rosetta): migrate block store to async tokio-rusqlite [DEFI-2950]~~ feat(icp-rosetta): DEFI-2950: Migrate block store to async tokio-rusqlite Jul 20, 2026

mbjorkqvist requested a review from Copilot July 20, 2026 11:02

Copilot started reviewing on behalf of mbjorkqvist July 20, 2026 11:02 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

mbjorkqvist requested a review from Copilot July 20, 2026 12:26

Copilot started reviewing on behalf of mbjorkqvist July 20, 2026 12:26 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Navigation Menu

feat(icp-rosetta): DEFI-2950: Migrate block store to async tokio-rusqlite - #10817

feat(icp-rosetta): DEFI-2950: Migrate block store to async tokio-rusqlite#10817
mbjorkqvist wants to merge 8 commits into
masterfrom
DEFI-2950-icp-rosetta-async-sqlite

mbjorkqvist commented Jul 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

mbjorkqvist Jul 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mbjorkqvist Jul 20, 2026

Uh oh!

mbjorkqvist Jul 20, 2026

Uh oh!

mbjorkqvist Jul 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mbjorkqvist Jul 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		con.execute_batch("COMMIT TRANSACTION;")
		.map_err(\|e\| BlockStoreError::Other(format!("{e}")))?;

Conversation

mbjorkqvist commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

PR stack — 3 ICP Rosetta PRs, in merge order

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

mbjorkqvist Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mbjorkqvist Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

mbjorkqvist Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

mbjorkqvist Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mbjorkqvist Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mbjorkqvist commented Jul 17, 2026 •

edited

Loading