Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions crates/blockchain/smoke_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,85 @@ mod blockchain_integration_test {
assert_eq!(latest_canonical_block_hash(&store).await.unwrap(), hash_b);
}

#[tokio::test]
async fn test_state_not_reachable_on_reorg_beyond_pruning_range() {
// This test simulates a reorg where the common ancestor's state has been pruned.
//
// Real-world scenario:
// - Canonical chain: genesis → 1 → 2 → ... → 130 (head)
// - Fork branch: genesis → 1 → fork_block (created early, not canonical)
// - Block 1 is the "link" between fork_block and the canonical chain
// - Block 1's state has been pruned (it's 129 blocks behind head, beyond 128-block window)
// - Attempting to reorg to fork_block fails with StateNotReachable
//
// In this test, we simulate state pruning by clearing the trie cache for block 1's
// state root, since the InMemory backend doesn't actually prune state.

const PRUNING_THRESHOLD: u64 = 128;
const CHAIN_LENGTH: u64 = PRUNING_THRESHOLD + 2; // 130 blocks

let store = test_store().await;
let genesis_header = store.get_block_header(0).unwrap().unwrap();
let genesis_hash = genesis_header.hash();
let blockchain = Blockchain::default_with_store(store.clone());

// Build block 1 on the canonical chain
let block_1 = new_block(&store, &genesis_header).await;
let hash_1 = block_1.hash();
let block_1_state_root = block_1.header.state_root;
blockchain.add_block(block_1.clone()).unwrap();
apply_fork_choice(&store, hash_1, genesis_hash, genesis_hash)
.await
.unwrap();

// Create a fork block at height 2, child of block 1 (will be non-canonical)
let fork_block = new_block(&store, &block_1.header).await;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop builds 129 blocks, each calling new_block (which creates a Blockchain, builds a payload) + add_block + apply_fork_choice. This is likely slow — consider reducing CHAIN_LENGTH and using a smaller threshold, or documenting that this test is intentionally slow.

Alternatively, since clear_trie_cache_for_testing nukes all cached state regardless of how many blocks were built, the 128-block threshold is only conceptually relevant (it motivates why state would be pruned). You could use a much smaller chain (e.g., 5 blocks) and the test would still verify the same code path.

let fork_hash = fork_block.hash();
blockchain.add_block(fork_block).unwrap();
// Don't make it canonical - it sits as an alternative branch

// Extend the canonical chain to 130 blocks (beyond pruning threshold)
let mut current_header = block_1.header.clone();
for _ in 2..=CHAIN_LENGTH {
let block = new_block(&store, &current_header).await;
let hash = block.hash();
blockchain.add_block(block.clone()).unwrap();
apply_fork_choice(&store, hash, genesis_hash, genesis_hash)
.await
.unwrap();
current_header = block.header;
}

// Verify chain length
assert_eq!(store.get_latest_block_number().await.unwrap(), CHAIN_LENGTH);

// Verify fork block exists but is not canonical
assert!(!is_canonical(&store, 2, fork_hash).await.unwrap());
assert!(store.get_block_header_by_hash(fork_hash).unwrap().is_some());

// Simulate state pruning: clear block 1's state from the trie cache.
// In production, this happens automatically when state is beyond the 128-block window.
// The link block for the reorg is block 1 (parent of fork_block).
store.clear_trie_cache_for_testing();

// Verify state is no longer reachable (only works after cache is cleared
// and state was never committed to disk due to InMemory threshold being 10000)
assert!(
!store.has_state_root(block_1_state_root).unwrap(),
"Block 1's state should not be reachable after clearing cache"
);

// Now attempt to reorg to the fork block.
// The link block is block 1, whose state has been "pruned".
let result = apply_fork_choice(&store, fork_hash, genesis_hash, genesis_hash).await;

assert!(
matches!(result, Err(InvalidForkChoice::StateNotReachable)),
"Expected StateNotReachable when link block's state is pruned, got {:?}",
result
);
}

async fn new_block(store: &Store, parent: &BlockHeader) -> Block {
let args = BuildPayloadArgs {
parent: parent.hash(),
Expand Down
17 changes: 17 additions & 0 deletions crates/storage/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,23 @@ impl Store {
let last_computed_flatkeyvalue = self.last_written()?;
Ok(&last_computed_flatkeyvalue[0..64] > account_nibbles.as_ref())
}

/// Clears the in-memory trie layer cache.
///
/// # Warning
/// This is a test helper that simulates state pruning. **Do not use in production.**
///
/// In production, state older than 128 blocks is pruned from the diff layers and
/// may not be available on disk if using InMemory backend (which has a 10000 block
/// threshold for disk commits).
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub visibility on a test-only method exposes it in the public API of ethrex-storage. Since this crate is #[cfg(test)]-incompatible (the test is in a different crate), consider either:

  1. #[doc(hidden)] pub fn clear_trie_cache_for_testing(...) — hides from docs but still accessible
  2. A pub fn clear_trie_cache_for_testing(...) behind a #[cfg(feature = "test-utils")] feature flag
  3. Or simply accept the pub with a doc comment warning (which you already have) — this is test tooling and the method name makes intent clear

Option 3 is fine for now, but worth considering for the rebase.

/// Use this to test scenarios where a reorg's common ancestor state is unavailable.
pub fn clear_trie_cache_for_testing(&self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silently ignoring lock poisoning: if another thread panicked while holding trie_cache, lock() returns Err and this does nothing. In a test helper this is probably fine, but an expect("trie_cache lock poisoned") would surface the issue rather than silently passing through.

use crate::layering::TrieLayerCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TrieLayerCache::default() sets commit_threshold: 128, but the store may have been initialized with a different value (e.g., TrieLayerCache::new(10000) for InMemory). This doesn't break the test since clearing the cache is the goal, but if the threshold matters for subsequent operations, the replacement cache will behave differently.

Consider TrieLayerCache::new(cache.commit_threshold) to preserve the original threshold (you'd need to add a getter or read it before replacing), or simply *cache = Arc::new(TrieLayerCache::default()); is fine if the test doesn't execute more blocks after clearing.

if let Ok(mut cache) = self.trie_cache.lock() {
*cache = Arc::new(TrieLayerCache::default());
}
}
}

type TrieNodesUpdate = Vec<(Nibbles, Vec<u8>)>;
Expand Down