fix(rpc): report syncing status when starting block header is pruned

[Ehsan-saradar]

Summary

Fixes #3791.

In pruned mode, Handler.Syncing (rpc v8/v9/v10) could return false ("not syncing") while the node was in fact still syncing.

It read the starting block's header only to obtain that block's hash:

startingBlockHeader, err := h.bcReader.BlockHeaderByNumber(startingBlockNumber)
if err != nil {
    return defaultSyncState, nil // <-- pruned starting header lands here
}

With history pruning enabled, once the starting block falls outside the retained window its header no longer exists in the DB. The lookup errors, and the handler falls through to the default "not syncing" response — so a node that is actively syncing reports itself as fully synced.

Fix

The sync decision only depends on head vs highest block, so make that decision first. The starting block hash is purely informational metadata (starting_block_hash), so it is now read best-effort afterwards:

• if the starting header is available, starting_block_hash is set as before;
• if it has been pruned, the field is simply omitted instead of the node being reported as synced.

starting_block_num now comes from StartingBlockNumber() (already in memory) rather than the header, so it no longer depends on a DB read that pruning can invalidate.

Applied identically to rpc/v8, rpc/v9 and rpc/v10.

Testing

• Reworked TestSyncing in all three versions to match the new call order.
• Added a syncing with pruned starting block header case asserting the node still reports syncing when BlockHeaderByNumber fails.
• make lint and the rpc/v8, rpc/v9, rpc/v10 test suites pass locally.

[Copilot]

Pull request overview

This PR fixes incorrect starknet_syncing reporting in history-pruning mode by ensuring the syncing decision is made without depending on the starting block header (which may be pruned), and by fetching starting_block_hash only on a best-effort basis afterward.

Changes:

• Reorders Syncing() logic in rpc v8/v9/v10 to decide syncing from head vs highest first.
• Makes starting_block_hash optional (best-effort DB lookup) while always returning starting_block_num from StartingBlockNumber().
• Updates and extends tests to reflect the new call order and pruned-header behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
rpc/v8/sync.go	Makes starting header lookup best-effort so pruning doesn’t cause false “not syncing”.
rpc/v8/sync_test.go	Reworks expectations for new call order; adds pruned-header syncing case.
rpc/v9/sync.go	Same best-effort starting header lookup and corrected sync decision order.
rpc/v9/sync_test.go	Updates expectations; adds pruned-header syncing case.
rpc/v10/sync.go	Same logic update as v8/v9.
rpc/v10/sync_test.go	Updates expectations; adds pruned-header syncing case.

Comments suppressed due to low confidence (3)

rpc/v9/sync_test.go:55

• This subtest is named as if it exercises the highestBlockHeader.Number <= head.Number branch, but HighestBlockHeader() is currently mocked to return nil (via the .Times(2) expectation above), so the handler returns early at highestBlockHeader == nil. This means the head>highest condition isn’t covered and the test name is misleading. Mock a non-nil highest header with a lower number (and adjust the nil expectation count) so the intended branch is actually tested.

	t.Run("block height is greater than highest block", func(t *testing.T) {
		mockReader.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)

		syncing, err := handler.Syncing()
		assert.Nil(t, err)
		assert.Equal(t, &rpc.Sync{Syncing: &defaultSyncState}, syncing)
	})

rpc/v8/sync_test.go:55

• This subtest is named as if it exercises the highestBlockHeader.Number <= head.Number branch, but HighestBlockHeader() is currently mocked to return nil (via the .Times(2) expectation above), so the handler returns early at highestBlockHeader == nil. This means the head>highest condition isn’t covered and the test name is misleading. Mock a non-nil highest header with a lower number (and adjust the nil expectation count) so the intended branch is actually tested.

	t.Run("block height is greater than highest block", func(t *testing.T) {
		mockReader.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)

		syncing, err := handler.Syncing()
		assert.Nil(t, err)
		assert.Equal(t, &rpc.Sync{Syncing: &defaultSyncState}, syncing)
	})

rpc/v10/sync_test.go:55

• This subtest is named as if it exercises the highestBlockHeader.Number <= head.Number branch, but HighestBlockHeader() is currently mocked to return nil (via the .Times(2) expectation above), so the handler returns early at highestBlockHeader == nil. This means the head>highest condition isn’t covered and the test name is misleading. Mock a non-nil highest header with a lower number (and adjust the nil expectation count) so the intended branch is actually tested.

	t.Run("block height is greater than highest block", func(t *testing.T) {
		mockReader.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)

		syncing, err := handler.Syncing()
		assert.Nil(t, err)
		assert.Equal(t, &rpc.Sync{Syncing: &defaultSyncState}, syncing)
	})

---

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