feat(flashblocks): improve test coverage for revamped flashblocks RPC layer

[brendontan03]

Summary

This PR improves test coverage for the revamped flashblocks RPC layer #176 to prevent regressions and verify new functionality.

Tests

1. Unit tests for FlashblockStateCacheInner and FlashblockStateCache which guarantees the caching and eviction mechanism works as expected.
2. Flashblocks e2e tests to include newly supported RPC methods
   1. eth_getRawTransactionByBlockNumberAndIndex
   2. sendRawTransactionSync
   3. getBlockByNumber using current pending height
3. fb_negative_test - Test for edge cases and ensure error handling
4. Cache test - Compare between data in confirmed cache and actual block data after canonical height has caught up

[sieniven]

@brendontan03 Take a look at the latest commit on the revamp PR - bb05216. Lets also add unit test coverage on the flashblocks sequence validator (flashblocks/src/execution/validator.rs) to validate that the the executed block outputs matches the default payload validator executed block outputs.

You can use the helper debug function to do a full state comparison. Perhaps we can run on a few test cases:

1. Block with legacy transactions
2. Block with multiple ERC20 transactions
3. Block with a tx that calls the selfdestruct opcode
4. Block with some initial ERC20 transactions, then a tx that calls the selfdestruct opcode, then somemore transactions

[sieniven]

@claude review the tests added in the PR.

1. Ensure that the test coverage added to the flashblocks state cache fully covers all functionalities. each test should be responsible for one responsibility / functionality only and all edge cases are considered
2. All the added new e2e test coverages cover the newly supported eth jsonrpc APIs on the flashblocks rpc layer. tests are deterministic (non-flaky), and correctly test the flashblocks low latency feature

[github-actions]

Claude finished @sieniven's task in 2m 55s —— View job

---

Code Review: Flashblocks Test Coverage

Tasks:

• Gather context and read changed files
• Review flashblocks state cache unit tests
• Review e2e test coverage for new RPC APIs
• Post final review

---

1. Cache Unit Tests (crates/flashblocks/src/cache/mod.rs)

Critical: target_index > 0 code path is entirely untested

The handle_pending_sequence method has two distinct branches when pending_height == expected_height:

1. target_index > 0 && last_flashblock_index >= target_index → promotes directly to confirm, clears pending
2. Otherwise → replaces pending in-place

Every single test calls handle_pending_sequence(seq, 0), meaning branch (1) has zero coverage. This is a core flashblocks behavior — the "target flashblock triggers a full sequence commit" — and needs its own tests. Suggested tests:

• Sequence with target_index > 0 when last_flashblock_index >= target_index → should be in confirm, pending cleared
• Sequence with target_index > 0 when last_flashblock_index < target_index → should still be in pending (not yet reached target)

Fix this →

Single-responsibility violations

test_get_tx_info_checks_pending_then_confirm (line 785) tests three distinct behaviors in one function: pending lookup, confirm lookup, and missing-tx returns None. Per the review guidelines, each test should have one responsibility. Split into:

• test_get_tx_info_found_in_pending
• test_get_tx_info_found_in_confirm
• test_get_tx_info_returns_none_for_unknown_hash

Missing edge cases

get_block_by_hash confirm fallback not tested — test_get_block_by_hash_pending_priority (line 748) only checks the pending-cache path. There is no test verifying get_block_by_hash returns a block from the confirm cache when the hash belongs to a confirmed (non-pending) block.

get_rpc_block(BlockNumberOrTag::Number(num)) path untested — The get_rpc_block match arm for Number(num) delegates to get_block_by_number. No test exercises this path via get_rpc_block.

get_rpc_block returns None on empty cache — No test checks that get_rpc_block(Latest) or get_rpc_block(Pending) returns None when the cache is empty (regression guard for the initial state).

get_executed_blocks_up_to_height with target > confirm_height + 1 — The guard target_height > self.confirm_height + 1 returns Ok(None), but this guard is not tested.

get_executed_blocks_up_to_height with pending-only state — No test covers the case where only a pending block exists (no confirm entries) and the target is the pending height.

test_handle_canonical_evicts_confirm missing assertion (line 676) — confirm_height should remain 4 after a non-flush canonical update (the max preserves it), but this is not asserted. Adding assert_eq!(inner.confirm_height, 4) would guard against regressions in the max logic.

Minor: misleading assertion

test_handle_pending_replace_same_height (line 618) — assert_ne!(seq1_hash, seq2_hash) verifies the test helper, not production code. The production assertion is that pending_cache.block_hash == seq2_hash. The assert_ne is noise; remove it or move it to the helper's own test.

---

2. E2E Tests (crates/tests/flashblocks-tests/main.rs)

Single-responsibility violation in fb_smoke_test

fb_smoke_test exercises 10+ distinct RPC methods in a single test function. When this test fails in CI, it provides no immediate signal about which API regressed. Each newly supported API (eth_getRawTransactionByBlockNumberAndIndex, eth_sendRawTransactionSync, block-by-current-pending-height) should be its own test.

Flashblocks low-latency feature not actually tested

The core flashblocks guarantee is sub-block-time visibility: a tx is visible in the pending flashblock before it is included in a canonical block. The smoke test defeats this by calling native_balance_transfer(..., wait_for_confirmation: true) (line 48-55) before querying the tx. By the time any API is called, the tx is already canonical — the test is exercising the canonical RPC path, not the flashblocks path.

To test the low-latency property, the tx should be sent without waiting for confirmation, and the flashblocks endpoint should be queried immediately afterward while the tx is still only in the pending flashblock. Example:

let tx_hash = operations::native_balance_transfer(
    operations::DEFAULT_L2_NETWORK_URL_FB,
    U256::from(operations::GWEI),
    test_address,
    false, // do NOT wait for confirmation
).await?;
// immediately check flashblocks pending
let tx = operations::eth_get_transaction_by_hash(&fb_client, &tx_hash).await?;
assert!(!tx.is_null(), "tx must be visible in pending flashblock before canonical inclusion");

Potential flakiness: initial eth_getBlockTransactionCountByNumber call

Lines 37–46 call eth_getBlockTransactionCountByNumber with Pending tag at test startup and assert != 0. This assumes the sequencer has already built a pending block with at least one transaction, which is not guaranteed at startup and is a source of flakiness.

eth_getRawTransactionByBlockNumberAndIndex tests canonical path, not flashblocks path

Lines 204–215 query by the pending block's number (fb_block["number"]), but only after the tx was already confirmed. The query will be served by the canonical chain, not the flashblocks cache. To test the flashblocks code path, query with the Pending tag directly, or with the block number while the block is still ahead of canonical.

eth_sendRawTransactionSync doesn't verify sync semantics

The test (lines 218–244) verifies that after calling sendRawTransactionSync, the tx is visible via eth_getTransactionByHash. However, the defining contract of sendRawTransactionSync is that the response itself already contains the pending tx's inclusion data — the tx should be visible synchronously within the same call, without a subsequent round-trip. The test should verify that fb_send_raw_transaction_sync["blockNumber"] (or equivalent inclusion field) is already populated in the response.

fb_negative_test missing edge cases

• No test for eth_getRawTransactionByBlockNumberAndIndex with a non-existent block number or index — should return null, not an error.
• No test for submitting a tx with a duplicate nonce via eth_sendRawTransactionSync — should the second submission error, or replace the first?

---

Summary

Area	Severity	Issue
Cache	Critical	target_index > 0 promotion path has no coverage
Cache	Medium	test_get_tx_info tests 3 behaviors; split required
Cache	Medium	get_block_by_hash confirm fallback not tested
Cache	Medium	get_rpc_block(Number(n)) path not tested
Cache	Low	get_executed_blocks target > confirm+1 guard untested
Cache	Low	Missing assert_eq!(confirm_height, 4) in eviction test
E2E	Critical	Low-latency (pending) property is not actually tested
E2E	Medium	fb_smoke_test violates single responsibility
E2E	Medium	eth_sendRawTransactionSync sync semantics unverified
E2E	Low	Flaky startup assertion on block tx count
E2E	Low	eth_getRawTransactionByBlockNumberAndIndex hits canonical, not flashblocks