[basvandijk]

Problem

ICP Rosetta keeps its blocks in LedgerBlocksSynchronizer::blockchain, a tokio::sync::RwLock<Blocks> that every API request that reads blocks takes for reading. The sync thread calls sync_blocks once a second, and sync_blocks:

• took the write lock on every call, before it knew whether there were new blocks;
• held it while sync_range_of_blocks fetched the new blocks from the ledger and its archives, with retries.

So API requests that read blocks queued behind every sync, and waited for the network round trips of every sync with new blocks. During the initial sync, which is a single sync_blocks call, they waited until it finished.

This was reported in #11962.

Fix

• sync_blocks reads the local tip under a read lock, and returns without taking the write lock when there are no new blocks.
• sync_range_of_blocks fetches and checks blocks without a lock. It takes the write lock only to store each batch of 500k blocks, and once at the end to store the remaining blocks, which always include the last one, and verify them. It returns that write guard, and sync_blocks makes the Rosetta blocks and prunes under it, as before.
• A new sync_lock mutex makes sync_blocks calls wait for each other, which the write lock used to do. Rosetta runs a single sync task on a current-thread runtime, so its syncs don't overlap, but sync_blocks is public, and two overlapping calls would fetch and store the same blocks.
• The batch size is now a field, so tests can lower it.
• Until the initial sync is complete, /account/balance, /block, /block/transaction, /search/transactions and the query_block_range method of /call return the retriable "still syncing" error of /network/status (feat(ICP_Rosetta): FI-1876: Return errors immediately during initial sync #7203) instead of reading the store. The check runs before each endpoint's first block read, so the request checks that come before that read still answer right away, as before. The other /call methods and the construction endpoints don't read blocks and keep working.

Why it needs #11962

This PR doesn't touch the code #11962 changed, but it relies on its fix. Before #11962, some request handlers took a second read lock on the blockchain while holding one, which deadlocks with a write lock request queued in between. Before this PR, the initial sync held the write lock throughout, so no request held a read lock while it ran. Now requests run while the sync fetches, and every write lock the sync takes to store blocks can land in one of those windows. The watchdog only restarts a stuck sync after the first successful one, so without #11962 a deadlock during the initial sync would last until Rosetta is restarted. This PR is stacked on #11964 only because GitHub stacks are linear; it doesn't depend on it.

What readers see

• A sync of up to 500k blocks still stores, verifies, makes Rosetta blocks and prunes under one write lock, so readers see the same states as before. While it fetches, they see the store as it was before the sync.
• In a sync of more than 500k blocks that isn't the initial sync, such as a catch-up after the ledger was unreachable for days, readers can now see stored but unverified blocks between batches. They could already see that state after such a sync failed midway. The endpoints that return blocks serve only verified ones, except /call query_block_range, which can return the first unverified block and mark the response idempotent.
• During the initial sync, the endpoints that read blocks now answer right away with the retriable "still syncing" error instead of hanging until the sync ends. Without that check they would read a store with no verified blocks yet: they would mostly return a non-retriable Blockchain is empty error, and looking up the latest verified block would read every stored block, since the verified column has no index. The initial sync is the first successful sync after Rosetta starts, so like /network/status they return the error until then even if the store has blocks from an earlier run, for example while Rosetta can't reach the ledger.

Testing

New tests in ledger_blocks_sync.rs use a mock ledger whose fetches wait until the test lets them through:

Test	master's locking	this PR without sync_lock	this PR
reading_blocks_does_not_wait_for_fetches: at every fetch of a 3-block sync, a reader gets the read lock and doesn't see the new blocks yet	fails	passes	passes
sync_without_new_blocks_does_not_wait_for_readers	fails (10 s timeout)	passes	passes
sync_stores_blocks_in_batches_and_verifies_them_at_the_end: batch size 2, 5 blocks	fails	passes	passes
sync_verifies_its_last_blocks_when_it_stores_them: batch size 2, 4 blocks; a reader that gets the read lock right after the sync stores its last block finds it verified	fails	passes	passes
concurrent_syncs_store_each_block_once	passes	fails: the second sync's insert violates the block_idx primary key	passes
sync_dropped_while_fetching_does_not_block_the_next_one: like the watchdog aborting the sync task	passes	passes	passes

A variant that stores the blocks of every fetch right away fails reading_blocks_does_not_wait_for_fetches. A variant that stores the last blocks of a sync in a batch of their own, before it takes the write lock again to verify them, fails sync_verifies_its_last_blocks_when_it_stores_them: if the sync were cancelled in between, the stored tip would stay unverified until the ledger moves on. sync_dropped_while_fetching_does_not_block_the_next_one guards that a sync the watchdog aborts releases sync_lock.

test_endpoints_that_read_blocks_return_error_during_initial_sync in request_handler/tests.rs calls each endpoint that reads blocks while the initial sync isn't complete and the blockchain is write-locked: each returns the "still syncing" error without waiting for the lock. Once the initial sync is complete, they all succeed. Without the check in any one of the endpoints, the test fails for that endpoint. test_invalid_requests_get_their_own_errors_during_initial_sync checks that during the initial sync, the request checks before the first block read still return their own errors, and a /call method that doesn't read blocks isn't affected. It fails if the check in /account/balance, /search/transactions or /call comes before those request checks.

The test targets that depend on the changed files pass locally, including icp_rosetta_system_tests. So do rosetta-integration_tests, which run Rosetta's sync against PocketIC, and the _local variants of rosetta_test and rosetta_neuron_follow_test in rs/tests/financial_integrations/rosetta. PR CI doesn't run those system tests, because they're tagged long_test.

Not changed here

/network/options lists error 702 (NotAvailableOffline) as not retriable, but the "still syncing" error is retriable. /network/status has returned it since #7203, and this PR returns it from more endpoints. Offline mode returns 702 as not retriable, so fixing this needs a separate error code for "still syncing".

This PR fixes an issue reported in #11962, which was created following the steps in .claude/skills/fix-flaky-tests/SKILL.md.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Cancellation between the final batch write and verification can leave the stored tip unverified across subsequent syncs.

1 open issue

• Prevent unverified tip after cancellation at exact batch boundary · New

What changed in this PR

This PR reduces ICP Rosetta API blocking during ledger synchronization by moving network fetches outside the blockchain write lock, building on the earlier nested-read-lock fix.

Changes:

• Uses a read lock to check the local tip and write locks to store batches and finalize synchronization.
• Adds a mutex to serialize sync calls.
• Adds gated-fetch tests covering reader access, batching, concurrent syncs, and cancellation.

File	Description
rs/​rosetta-api/​icp/​ledger_canister_blocks_synchronizer/​src/​ledger_blocks_sync.rs	Narrows write-lock scope, serializes syncs, and adds locking regression tests.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No unresolved issues remain; sync serialization and atomic final-batch verification are preserved and covered by targeted regression tests.

No open issues 🎉

1 resolved since last review

• Prevent unverified tip after cancellation at exact batch boundary

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues are reported for the ledger synchronization locking changes.

No open issues 🎉

_{Review effort: Balanced}

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 6f8f068.

Security Overview

• 🔎 Scanned files: 3 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/bd30fac0-c6f5-4bec-83e2-480d2b345946?scanId=57a8c3e8-106d-4076-8ba4-aa6addfadf85&codeScanTypes=PrScan&tab=issues&detectionTypes=traditional,businessLogic,customRule,IAC,SECRETS,CICD,EOL,uncategorized

Detected Code Changes

Change Type

Relevant files

Enhancement

► rs/rosetta-api/icp/ledger_canister_blocks_synchronizer/src/ledger_blocks_sync.rs and associated tests modifications describing synchronization locking, batching, and block storage behavior ► rs/rosetta-api/icp/src/request_handler.rs and associated tests additions to check initial sync behavior and retriable errors for endpoints before initial sync completion ► rs/rosetta-api/icp/src/request_handler/tests.rs tests for initial sync behavior and endpoints that read blocks returning retriable errors during sync

[Copilot]

🟢 Approval recommended

The synchronization lock-scope changes have no active review issues.

No open issues 🎉

_{Review effort: Balanced}

[mbjorkqvist]

Thanks @basvandijk! Just one comment regarding new behavior during initial sync that this PR introduces.

[Copilot]

🟡 Changes recommended

The sync check precedes storage-independent validation, causing malformed account addresses and negative pagination values to return a retriable syncing error instead of InvalidRequest.

1 open issue

• Validate request fields before initial sync check · New

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues remain for the synchronization-locking and initial-sync endpoint changes.

No open issues 🎉

1 resolved since last review

• Validate request fields before initial sync check

_{Review effort: Balanced}