diff --git a/Cargo.lock b/Cargo.lock index 47f8515ee..f70394cc2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6436,6 +6436,7 @@ dependencies = [ "ream-post-quantum-crypto", "ream-storage", "ream-sync", + "ream-test-utils", "serde", "serde_json", "serde_yaml", diff --git a/crates/common/fork_choice/lean/Cargo.toml b/crates/common/fork_choice/lean/Cargo.toml index 2c91439c0..eb1e6eacb 100644 --- a/crates/common/fork_choice/lean/Cargo.toml +++ b/crates/common/fork_choice/lean/Cargo.toml @@ -17,6 +17,7 @@ devnet5 = [ "ream-network-state-lean/devnet5", "ream-network-spec/devnet5", "ream-storage/devnet5", + "ream-test-utils/devnet5", ] [dependencies] @@ -55,6 +56,7 @@ ream-sync.workspace = true [dev-dependencies] rand.workspace = true +ream-test-utils.workspace = true [lints] workspace = true diff --git a/crates/common/fork_choice/lean/src/store.rs b/crates/common/fork_choice/lean/src/store.rs index 859a22b20..fd2f3fce4 100644 --- a/crates/common/fork_choice/lean/src/store.rs +++ b/crates/common/fork_choice/lean/src/store.rs @@ -1764,9 +1764,13 @@ impl Store { let timer = start_timer(&ATTESTATION_VALIDATION_TIME, &[]); let data = &signed_attestation.message; - let (block_provider, time_provider) = { + let (block_provider, time_provider, latest_finalized_provider) = { let db = self.store.lock().await; - (db.block_provider(), db.time_provider()) + ( + db.block_provider(), + db.time_provider(), + db.latest_finalized_provider(), + ) }; // Validate attestation targets exist in store @@ -1835,6 +1839,13 @@ impl Store { ); } + // Fork choice only ever descends from the finalized block. + ensure!( + self.checkpoint_is_ancestor(&latest_finalized_provider.get()?, &data.head) + .await?, + "Head checkpoint must descend from the finalized block" + ); + ensure!( data.slot >= head_block.block.slot, "Attestation slot precedes head" @@ -2463,6 +2474,9 @@ impl Store { }) } + /// Fork choice only ever descends from the latest finalized block. This is sound only + /// because the finalized checkpoint is re-derived from the head each update; pruning + /// against a finalized checkpoint that drifted off the head chain would be unsound. pub async fn prune_stale_attestation_data(&mut self) -> anyhow::Result<()> { let ( latest_finalized_provider, @@ -2756,3 +2770,165 @@ fn compact_aggregated_proofs( Ok((out_attestations, out_proofs)) } + +#[cfg(test)] +#[cfg(feature = "devnet5")] +mod tests { + use alloy_primitives::B256; + use ream_consensus_lean::{ + attestation::{AttestationData, MultiMessageAggregate, SignedAttestation}, + block::{Block, BlockBody, SignedBlock}, + checkpoint::Checkpoint, + }; + use ream_post_quantum_crypto::leansig::signature::Signature; + use ream_storage::tables::{field::REDBField, table::REDBTable}; + use ream_test_utils::store::sample_store; + use ssz_types::VariableList; + use tree_hash::TreeHash; + + use super::{BlockProductionStrategy, Store}; + + async fn sample_store_as_store(no_of_validators: usize) -> Store { + let test_store = sample_store(no_of_validators).await; + Store { + store: test_store.store, + network_state: test_store.network_state, + tick_interval_duration: None, + block_production_strategy: BlockProductionStrategy::default(), + } + } + + fn fake_signed_block(slot: u64, proposer_index: u64, parent_root: B256) -> SignedBlock { + SignedBlock { + block: Block { + slot, + proposer_index, + parent_root, + state_root: B256::ZERO, + body: BlockBody { + attestations: VariableList::empty(), + }, + }, + proof: MultiMessageAggregate { + proof: VariableList::default(), + }, + } + } + + async fn store_with_finalized_orphaned_branch() -> (Store, B256, B256, B256, B256) { + let store = sample_store_as_store(10).await; + + let genesis_root = { store.store.lock().await.head_provider().get().unwrap() }; + + let canonical_1 = fake_signed_block(1, 0, genesis_root); + let canonical_1_root = canonical_1.block.tree_hash_root(); + let canonical_2 = fake_signed_block(2, 0, canonical_1_root); + let canonical_2_root = canonical_2.block.tree_hash_root(); + + let orphan_1 = fake_signed_block(1, 1, genesis_root); + let orphan_1_root = orphan_1.block.tree_hash_root(); + let orphan_2 = fake_signed_block(2, 1, orphan_1_root); + let orphan_2_root = orphan_2.block.tree_hash_root(); + + { + let db = store.store.lock().await; + let block_provider = db.block_provider(); + block_provider + .insert(canonical_1_root, canonical_1) + .unwrap(); + block_provider + .insert(canonical_2_root, canonical_2) + .unwrap(); + block_provider.insert(orphan_1_root, orphan_1).unwrap(); + block_provider.insert(orphan_2_root, orphan_2).unwrap(); + + db.latest_finalized_provider() + .insert(Checkpoint { + root: canonical_2_root, + slot: 2, + }) + .unwrap(); + db.time_provider().insert(1_000).unwrap(); + } + + ( + store, + canonical_1_root, + canonical_2_root, + orphan_1_root, + orphan_2_root, + ) + } + + #[tokio::test] + async fn test_validate_attestation_rejects_head_on_finalized_orphaned_branch() { + let (store, _, _, orphan_1_root, orphan_2_root) = + store_with_finalized_orphaned_branch().await; + + let genesis_root = { store.store.lock().await.head_provider().get().unwrap() }; + + let attestation_data = AttestationData { + slot: 2, + head: Checkpoint { + root: orphan_2_root, + slot: 2, + }, + target: Checkpoint { + root: orphan_1_root, + slot: 1, + }, + source: Checkpoint { + root: genesis_root, + slot: 0, + }, + }; + + let err = store + .validate_attestation(&SignedAttestation { + validator_id: 0, + message: attestation_data, + signature: Signature::blank(), + }) + .await + .unwrap_err(); + + assert!( + err.to_string() + .contains("Head checkpoint must descend from the finalized block"), + "unexpected error: {err}" + ); + } + + #[tokio::test] + async fn test_validate_attestation_accepts_head_descending_from_finalized() { + let (store, canonical_1_root, canonical_2_root, _, _) = + store_with_finalized_orphaned_branch().await; + + let genesis_root = { store.store.lock().await.head_provider().get().unwrap() }; + + let attestation_data = AttestationData { + slot: 2, + head: Checkpoint { + root: canonical_2_root, + slot: 2, + }, + target: Checkpoint { + root: canonical_1_root, + slot: 1, + }, + source: Checkpoint { + root: genesis_root, + slot: 0, + }, + }; + + store + .validate_attestation(&SignedAttestation { + validator_id: 0, + message: attestation_data, + signature: Signature::blank(), + }) + .await + .unwrap(); + } +} diff --git a/crates/storage/src/tables/lean/latest_finalized.rs b/crates/storage/src/tables/lean/latest_finalized.rs index e803511d7..44b424021 100644 --- a/crates/storage/src/tables/lean/latest_finalized.rs +++ b/crates/storage/src/tables/lean/latest_finalized.rs @@ -12,6 +12,11 @@ pub struct LatestFinalizedField { /// Table definition for the Latest Finalized table /// /// Value: [Checkpoint] +/// +/// Finalization as seen from the canonical head, not irreversible economic finality. +/// +/// Re-derived from the head each update, so it is reorg-mutable and can lower on a reorg. +/// Always an ancestor of the head, never monotone, never a safety guarantee. impl REDBField for LatestFinalizedField { const FIELD_DEFINITION: TableDefinition<'_, &str, SSZEncoding> = TableDefinition::new("lean_latest_finalized"); diff --git a/crates/storage/src/tables/lean/latest_justified.rs b/crates/storage/src/tables/lean/latest_justified.rs index 254902919..c5b128578 100644 --- a/crates/storage/src/tables/lean/latest_justified.rs +++ b/crates/storage/src/tables/lean/latest_justified.rs @@ -15,6 +15,8 @@ pub struct LatestJustifiedField { /// /// NOTE: This table enables O(1) access to the latest justified checkpoint, deviates from /// the original spec which derives it from state dictionary each time it is needed. +/// +/// Highest-slot justified checkpoint observed so far; the head walk starts here. impl REDBField for LatestJustifiedField { const FIELD_DEFINITION: TableDefinition<'_, &str, SSZEncoding> = TableDefinition::new("lean_latest_justified");