From 3a00393e0e40e67f69f637d7e174f8aa3e4fa4e0 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 16:44:38 +0200 Subject: [PATCH 1/7] handle errors --- lean_client/Cargo.lock | 49 +-- lean_client/Cargo.toml | 4 +- lean_client/containers/Cargo.toml | 2 + lean_client/containers/src/block.rs | 53 ++-- lean_client/containers/src/config.rs | 9 +- lean_client/containers/src/serde_helpers.rs | 16 +- lean_client/containers/src/state.rs | 56 ++-- lean_client/containers/src/validator.rs | 13 +- .../containers/tests/test_vectors/runner.rs | 299 ++++++++---------- .../tests/unit_tests/state_process.rs | 2 +- .../tests/unit_tests/state_transition.rs | 7 +- lean_client/fork_choice/Cargo.toml | 3 + lean_client/fork_choice/src/handlers.rs | 71 +++-- lean_client/fork_choice/src/store.rs | 5 +- .../tests/fork_choice_test_vectors.rs | 294 ++++++++--------- lean_client/networking/src/enr_ext.rs | 8 +- .../networking/src/gossipsub/message.rs | 8 +- lean_client/networking/src/gossipsub/topic.rs | 22 +- lean_client/src/main.rs | 59 ++-- lean_client/validator/Cargo.toml | 1 + lean_client/validator/src/keys.rs | 45 ++- lean_client/validator/src/lib.rs | 59 ++-- 22 files changed, 553 insertions(+), 532 deletions(-) diff --git a/lean_client/Cargo.lock b/lean_client/Cargo.lock index 5b622b9..68fc02e 100644 --- a/lean_client/Cargo.lock +++ b/lean_client/Cargo.lock @@ -388,7 +388,7 @@ dependencies = [ "nom", "num-traits", "rusticata-macros", - "thiserror 2.0.16", + "thiserror 2.0.18", "time", ] @@ -847,6 +847,7 @@ dependencies = [ name = "containers" version = "0.1.0" dependencies = [ + "anyhow", "hex", "leansig", "pretty_assertions", @@ -1552,12 +1553,14 @@ checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" name = "fork-choice" version = "0.1.0" dependencies = [ + "anyhow", "containers", "serde", "serde_json", "ssz", "ssz_derive", "ssz_rs", + "thiserror 2.0.18", "typenum", ] @@ -1896,7 +1899,7 @@ dependencies = [ "rand 0.9.2", "ring", "socket2 0.5.10", - "thiserror 2.0.16", + "thiserror 2.0.18", "tinyvec", "tokio", "tracing", @@ -1919,7 +1922,7 @@ dependencies = [ "rand 0.9.2", "resolv-conf", "smallvec", - "thiserror 2.0.16", + "thiserror 2.0.18", "tokio", "tracing", ] @@ -2400,6 +2403,7 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" name = "lean_client" version = "0.1.0" dependencies = [ + "anyhow", "chain", "clap", "containers", @@ -2430,7 +2434,7 @@ dependencies = [ "rayon", "serde", "sha3", - "thiserror 2.0.16", + "thiserror 2.0.18", ] [[package]] @@ -2475,7 +2479,7 @@ dependencies = [ "multiaddr 0.18.2", "pin-project", "rw-stream-sink 0.4.0", - "thiserror 2.0.16", + "thiserror 2.0.18", ] [[package]] @@ -2547,7 +2551,7 @@ dependencies = [ "quick-protobuf", "rand 0.8.5", "rw-stream-sink 0.4.0", - "thiserror 2.0.16", + "thiserror 2.0.18", "tracing", "unsigned-varint 0.8.0", "web-time", @@ -2616,7 +2620,7 @@ dependencies = [ "quick-protobuf", "quick-protobuf-codec", "smallvec", - "thiserror 2.0.16", + "thiserror 2.0.18", "tracing", ] @@ -2653,7 +2657,7 @@ dependencies = [ "quick-protobuf", "rand 0.8.5", "sha2 0.10.9 (registry+https://github.com/rust-lang/crates.io-index)", - "thiserror 2.0.16", + "thiserror 2.0.18", "tracing", "zeroize", ] @@ -2729,7 +2733,7 @@ dependencies = [ "rand 0.8.5", "snow", "static_assertions", - "thiserror 2.0.16", + "thiserror 2.0.18", "tracing", "x25519-dalek", "zeroize", @@ -2752,7 +2756,7 @@ dependencies = [ "ring", "rustls", "socket2 0.5.10", - "thiserror 2.0.16", + "thiserror 2.0.18", "tokio", "tracing", ] @@ -2837,7 +2841,7 @@ dependencies = [ "ring", "rustls", "rustls-webpki", - "thiserror 2.0.16", + "thiserror 2.0.18", "x509-parser", "yasna", ] @@ -2866,7 +2870,7 @@ dependencies = [ "either", "futures", "libp2p-core 0.43.1", - "thiserror 2.0.16", + "thiserror 2.0.18", "tracing", "yamux 0.12.1", "yamux 0.13.8", @@ -3142,7 +3146,7 @@ dependencies = [ "log", "netlink-packet-core", "netlink-sys", - "thiserror 2.0.16", + "thiserror 2.0.18", ] [[package]] @@ -3796,7 +3800,7 @@ dependencies = [ "rustc-hash", "rustls", "socket2 0.6.1", - "thiserror 2.0.16", + "thiserror 2.0.18", "tokio", "tracing", "web-time", @@ -3817,7 +3821,7 @@ dependencies = [ "rustls", "rustls-pki-types", "slab", - "thiserror 2.0.16", + "thiserror 2.0.18", "tinyvec", "tracing", "web-time", @@ -4646,7 +4650,7 @@ dependencies = [ "static_assertions", "std_ext", "tap", - "thiserror 2.0.16", + "thiserror 2.0.18", "triomphe", "try_from_iterator", "typenum", @@ -4832,11 +4836,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.16" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3467d614147380f2e4e374161426ff399c91084acd2363eaf549172b3d5e60c0" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl 2.0.16", + "thiserror-impl 2.0.18", ] [[package]] @@ -4852,9 +4856,9 @@ dependencies = [ [[package]] name = "thiserror-impl" -version = "2.0.16" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c5e1be1c48b9172ee610da68fd9cd2770e7a4056cb3fc98710ee6906f0c7960" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", @@ -5252,6 +5256,7 @@ dependencies = [ name = "validator" version = "0.1.0" dependencies = [ + "anyhow", "containers", "fork-choice", "leansig", @@ -5771,7 +5776,7 @@ dependencies = [ "nom", "oid-registry", "rusticata-macros", - "thiserror 2.0.16", + "thiserror 2.0.18", "time", ] diff --git a/lean_client/Cargo.toml b/lean_client/Cargo.toml index 7d98ae7..97bd57c 100644 --- a/lean_client/Cargo.toml +++ b/lean_client/Cargo.toml @@ -28,7 +28,8 @@ libp2p = {version = "0.56.0", default-features = false, features = [ ] } alloy-consensus = "1.0.38" alloy-primitives = "1.4.0" -anyhow = "1.0" +anyhow = "1.0.100" +thiserror = "2.0.18" paste = "1.0.15" serde = { version = "1.0", features = ["derive"] } serde_yaml = "0.9" @@ -67,3 +68,4 @@ tracing-subscriber = { version = "0.3.20", features = ["env-filter"] } tracing = "0.1.41" hex = "0.4" libp2p-identity = { version = "0.2", features = ["secp256k1"] } +anyhow = { workspace = true } diff --git a/lean_client/containers/Cargo.toml b/lean_client/containers/Cargo.toml index 2d5b0ff..7199e27 100644 --- a/lean_client/containers/Cargo.toml +++ b/lean_client/containers/Cargo.toml @@ -11,6 +11,7 @@ name = "containers" path = "src/lib.rs" [dependencies] +anyhow = { workspace = true } ssz = { git = "https://github.com/grandinetech/grandine", package = "ssz", branch = "develop" } ssz_derive = { git = "https://github.com/grandinetech/grandine", package = "ssz_derive", branch = "develop" } typenum = "1" @@ -22,6 +23,7 @@ sha2 = "0.10" leansig = { git = "https://github.com/leanEthereum/leanSig", branch = "main", optional = true } [dev-dependencies] +anyhow = { workspace = true } rstest = "0.18" pretty_assertions = "1.4" serde_json = "1.0" diff --git a/lean_client/containers/src/block.rs b/lean_client/containers/src/block.rs index e4cc998..464783d 100644 --- a/lean_client/containers/src/block.rs +++ b/lean_client/containers/src/block.rs @@ -1,9 +1,9 @@ +use anyhow::{bail, ensure, Context, Result}; use crate::{ Attestation, Attestations, BlockSignatures, Bytes32, Signature, Slot, State, ValidatorIndex, }; use serde::{Deserialize, Serialize}; use ssz_derive::Ssz; - #[cfg(feature = "xmss-verify")] use leansig::signature::generalized_xmss::instantiations_poseidon::lifetime_2_to_the_20::target_sum::SIGTargetSumLifetime20W2NoOff; @@ -114,7 +114,7 @@ impl SignedBlockWithAttestation { /// /// - Spec: /// - XMSS Library: - pub fn verify_signatures(&self, parent_state: State) -> bool { + pub fn verify_signatures(&self, parent_state: State) -> Result<()> { // Unpack the signed block components let block = &self.message.block; let signatures = &self.signature; @@ -157,7 +157,7 @@ impl SignedBlockWithAttestation { // The ordering must be preserved: // 1. Block body attestations, // 2. The proposer attestation. - assert!( + ensure!( signatures_vec.len() == all_attestations.len(), "Number of signatures does not match number of attestations" ); @@ -168,14 +168,19 @@ impl SignedBlockWithAttestation { // Verify each attestation signature for (attestation, signature) in all_attestations.iter().zip(signatures_vec.iter()) { // Ensure validator exists in the active set - assert!( + ensure!( attestation.validator_id.0 < num_validators, "Validator index out of range" ); let validator = validators .get(attestation.validator_id.0) - .expect("validator must exist"); + .with_context(|| { + format!( + "Validator {} not found in state", + attestation.validator_id.0 + ) + })?; // Verify the XMSS signature // @@ -200,13 +205,11 @@ impl SignedBlockWithAttestation { type PubKey = ::PublicKey; let pubkey = match PubKey::from_bytes(pubkey_bytes) { Ok(pk) => pk, - Err(e) => { - eprintln!( - "Failed to deserialize public key at slot {:?}: {:?}", - attestation.data.slot, e - ); - return false; - } + Err(e) => bail!( + "Failed to deserialize public key at slot {:?}: {:?}", + attestation.data.slot, + e + ), }; // Get signature bytes - use as_bytes() method @@ -216,23 +219,19 @@ impl SignedBlockWithAttestation { type Sig = ::Signature; let sig = match Sig::from_bytes(sig_bytes) { Ok(s) => s, - Err(e) => { - eprintln!( - "Failed to deserialize signature at slot {:?}: {:?}", - attestation.data.slot, e - ); - return false; - } + Err(e) => bail!( + "Failed to deserialize signature at slot {:?}: {:?}", + attestation.data.slot, + e + ), }; // Verify the signature - if !SIGTargetSumLifetime20W2NoOff::verify(&pubkey, epoch, &message_bytes, &sig) { - eprintln!( - "XMSS signature verification failed at slot {:?}", - attestation.data.slot - ); - return false; - } + ensure!( + SIGTargetSumLifetime20W2NoOff::verify(&pubkey, epoch, &message_bytes, &sig), + "XMSS signature verification failed at slot {:?}", + attestation.data.slot + ); } #[cfg(not(feature = "xmss-verify"))] @@ -246,6 +245,6 @@ impl SignedBlockWithAttestation { } } - true + Ok(()) } } diff --git a/lean_client/containers/src/config.rs b/lean_client/containers/src/config.rs index fed2b7e..7445c1d 100644 --- a/lean_client/containers/src/config.rs +++ b/lean_client/containers/src/config.rs @@ -1,3 +1,4 @@ +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use ssz_derive::Ssz; use std::fs::File; @@ -20,10 +21,12 @@ pub struct GenesisConfig { } impl GenesisConfig { - pub fn load_from_file>(path: P) -> Result> { - let file = File::open(path)?; + pub fn load_from_file>(path: P) -> Result { + let file = File::open(&path) + .with_context(|| format!("Failed to open genesis config file: {:?}", path.as_ref()))?; let reader = BufReader::new(file); - let config = serde_yaml::from_reader(reader)?; + let config = + serde_yaml::from_reader(reader).context("Failed to parse genesis config YAML")?; Ok(config) } } diff --git a/lean_client/containers/src/serde_helpers.rs b/lean_client/containers/src/serde_helpers.rs index 7cff787..da2fb6d 100644 --- a/lean_client/containers/src/serde_helpers.rs +++ b/lean_client/containers/src/serde_helpers.rs @@ -1,6 +1,6 @@ // Serde helpers for handling test vector JSON format // Test vectors wrap SSZ collections in {"data": [...]} objects - +use anyhow::Result; use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Wrapper for deserializing {"data": T} format @@ -188,6 +188,7 @@ pub mod signature { pub mod block_signatures { use super::*; use crate::{BlockSignatures, Signature}; + use anyhow::Context; use serde_json::Value; use ssz::PersistentList; @@ -204,19 +205,18 @@ pub mod block_signatures { siblings: DataWrapper>>>, } - fn parse_single_signature(value: &Value) -> Result { + fn parse_single_signature(value: &Value) -> Result { // Check if it's a hex string (normal format) if let Value::String(hex_str) = value { let hex_str = hex_str.trim_start_matches("0x"); - let bytes = hex::decode(hex_str).map_err(|e| format!("Invalid hex string: {}", e))?; + let bytes = hex::decode(hex_str).context("Invalid hex string: {}")?; - return Signature::try_from(bytes.as_slice()) - .map_err(|_| "Invalid signature length".to_string()); + return Signature::try_from(bytes.as_slice()).context("Invalid signature length"); } // Otherwise, parse as structured XMSS signature - let xmss_sig: XmssSignature = serde_json::from_value(value.clone()) - .map_err(|e| format!("Failed to parse XMSS signature: {}", e))?; + let xmss_sig: XmssSignature = + serde_json::from_value(value.clone()).context("Failed to parse XMSS signature: {}")?; // Serialize the XMSS signature to bytes // Format: siblings (variable length) + rho (28 bytes) + hashes (variable length) @@ -244,7 +244,7 @@ pub mod block_signatures { // Pad or truncate to 3112 bytes bytes.resize(3112, 0); - Signature::try_from(bytes.as_slice()).map_err(|_| "Failed to create signature".to_string()) + Signature::try_from(bytes.as_slice()).context("Failed to create signature") } pub fn deserialize<'de, D>(deserializer: D) -> Result diff --git a/lean_client/containers/src/state.rs b/lean_client/containers/src/state.rs index 2dccbf3..6381eef 100644 --- a/lean_client/containers/src/state.rs +++ b/lean_client/containers/src/state.rs @@ -7,6 +7,7 @@ use crate::{ use crate::{ HistoricalBlockHashes, JustificationRoots, JustificationsValidators, JustifiedSlots, Validators, }; +use anyhow::{ensure, Context, Result}; use serde::{Deserialize, Serialize}; use ssz::PersistentList as List; use ssz_derive::Ssz; @@ -216,7 +217,7 @@ impl State { &self, signed_block: SignedBlockWithAttestation, valid_signatures: bool, - ) -> Result { + ) -> Result { self.state_transition_with_validation(signed_block, valid_signatures, true) } @@ -226,10 +227,8 @@ impl State { signed_block: SignedBlockWithAttestation, valid_signatures: bool, validate_state_root: bool, - ) -> Result { - if !valid_signatures { - return Err("Block signatures must be valid".to_string()); - } + ) -> Result { + ensure!(valid_signatures, "Block signatures must be valid"); let block = &signed_block.message.block; let mut state = self.process_slots(block.slot)?; @@ -238,18 +237,14 @@ impl State { if validate_state_root { let state_for_hash = state.clone(); let state_root = hash_tree_root(&state_for_hash); - if block.state_root != state_root { - return Err("Invalid block state root".to_string()); - } + ensure!(block.state_root == state_root, "Invalid block state root"); } Ok(state) } - pub fn process_slots(&self, target_slot: Slot) -> Result { - if self.slot >= target_slot { - return Err("Target slot must be in the future".to_string()); - } + pub fn process_slots(&self, target_slot: Slot) -> Result { + ensure!(self.slot < target_slot, "Target slot must be in the future"); let mut state = self.clone(); @@ -279,7 +274,7 @@ impl State { self.clone() } - pub fn process_block(&self, block: &Block) -> Result { + pub fn process_block(&self, block: &Block) -> Result { let state = self.process_block_header(block)?; let state_after_ops = state.process_attestations(&block.body.attestations); @@ -288,23 +283,24 @@ impl State { Ok(state_after_ops) } - pub fn process_block_header(&self, block: &Block) -> Result { - if !(block.slot == self.slot) { - return Err(String::from("Block slot mismatch")); - } - if !(block.slot > self.latest_block_header.slot) { - return Err(String::from("Block is older than latest header")); - } - if !self.is_proposer(block.proposer_index) { - return Err(String::from("Incorrect block proposer")); - } + pub fn process_block_header(&self, block: &Block) -> Result { + ensure!(block.slot == self.slot, "Block slot mismatch"); + ensure!( + block.slot > self.latest_block_header.slot, + "Block is older than latest header" + ); + ensure!( + self.is_proposer(block.proposer_index), + "Incorrect block proposer" + ); // Create a mutable clone for hash computation let latest_header_for_hash = self.latest_block_header.clone(); let parent_root = hash_tree_root(&latest_header_for_hash); - if block.parent_root != parent_root { - return Err(String::from("Block parent root mismatch")); - } + ensure!( + block.parent_root == parent_root, + "Block parent root mismatch" + ); // Build new PersistentList for historical hashes let mut new_historical_hashes = HistoricalBlockHashes::default(); @@ -537,7 +533,7 @@ impl State { initial_attestations: Option>, available_signed_attestations: Option<&[SignedBlockWithAttestation]>, known_block_roots: Option<&std::collections::HashSet>, - ) -> Result<(Block, Self, Vec, BlockSignatures), String> { + ) -> Result<(Block, Self, Vec, BlockSignatures)> { // Initialize empty attestation set for iterative collection let mut attestations = initial_attestations.unwrap_or_default(); let mut signatures = BlockSignatures::default(); @@ -558,7 +554,7 @@ impl State { for att in &attestations { attestations_list .push(att.clone()) - .map_err(|e| format!("Failed to push attestation: {:?}", e))?; + .context("Failed to push attestation")?; } let candidate_block = Block { @@ -643,9 +639,7 @@ impl State { // Add new attestations and continue iteration attestations.extend(new_attestations); for sig in new_signatures { - signatures - .push(sig) - .map_err(|e| format!("Failed to push signature: {:?}", e))?; + signatures.push(sig).context("Failed to push signature")?; } } } diff --git a/lean_client/containers/src/validator.rs b/lean_client/containers/src/validator.rs index 2649f55..dc7c6df 100644 --- a/lean_client/containers/src/validator.rs +++ b/lean_client/containers/src/validator.rs @@ -1,3 +1,4 @@ +use anyhow::{ensure, Context, Result}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use ssz::ByteVector; use ssz_derive::Ssz; @@ -58,12 +59,14 @@ impl<'de> Deserialize<'de> for BlsPublicKey { } impl BlsPublicKey { - pub fn from_hex(s: &str) -> Result { + pub fn from_hex(s: &str) -> Result { let s = s.strip_prefix("0x").unwrap_or(s); - let decoded = hex::decode(s).map_err(|e| e.to_string())?; - if decoded.len() != 52 { - return Err(format!("Expected 52 bytes, got {}", decoded.len())); - } + let decoded = hex::decode(s).context("Failed to decode hex string")?; + ensure!( + decoded.len() == 52, + "Expected 52 bytes, got {}", + decoded.len() + ); let mut byte_vec = ByteVector::default(); unsafe { let dest = &mut byte_vec as *mut ByteVector as *mut u8; diff --git a/lean_client/containers/tests/test_vectors/runner.rs b/lean_client/containers/tests/test_vectors/runner.rs index e2b829e..e5b54a7 100644 --- a/lean_client/containers/tests/test_vectors/runner.rs +++ b/lean_client/containers/tests/test_vectors/runner.rs @@ -1,4 +1,5 @@ use super::*; +use anyhow::{bail, ensure, Context, Result}; use containers::block::hash_tree_root; use std::fs; use std::path::Path; @@ -6,9 +7,7 @@ use std::path::Path; pub struct TestRunner; impl TestRunner { - pub fn run_sequential_block_processing_tests>( - path: P, - ) -> Result<(), Box> { + pub fn run_sequential_block_processing_tests>(path: P) -> Result<()> { let json_content = fs::read_to_string(path)?; // Parse using the new TestVectorFile structure with camelCase @@ -19,7 +18,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -37,14 +36,13 @@ impl TestRunner { let computed_parent_root = hash_tree_root(&state_after_slots.latest_block_header); // Verify the block's parent_root matches what we computed - if block.parent_root != computed_parent_root { - return Err(format!( - "Block {} parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - idx + 1, - block.parent_root, - computed_parent_root - ).into()); - } + ensure!( + block.parent_root == computed_parent_root, + "Block {} parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + idx + 1, + block.parent_root, + computed_parent_root + ); println!(" ✓ Parent root matches: {:?}", computed_parent_root); @@ -59,45 +57,42 @@ impl TestRunner { let computed_state_root = hash_tree_root(&state); // Verify the computed state_root matches the expected one from the vector - if block.state_root != computed_state_root { - return Err(format!( - "Block {} state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - idx + 1, - block.state_root, - computed_state_root - ).into()); - } + ensure!( + block.state_root == computed_state_root, + "Block {} state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + idx + 1, + block.state_root, + computed_state_root + ); println!(" ✓ State root matches: {:?}", computed_state_root); println!(" ✓ Block {} processed successfully", idx + 1); } Err(e) => { - return Err(format!("Block {} processing failed: {:?}", idx + 1, e).into()); + bail!("Block {} processing failed: {:?}", idx + 1, e); } } } // Verify post-state conditions if let Some(post) = test_case.post { - if state.slot != post.slot { - return Err(format!( - "Post-state slot mismatch: expected {:?}, got {:?}", - post.slot, state.slot - ) - .into()); - } + ensure!( + state.slot == post.slot, + "Post-state slot mismatch: expected {:?}, got {:?}", + post.slot, + state.slot + ); // Only check validator count if specified in post-state if let Some(expected_count) = post.validator_count { let mut num_validators: u64 = state.validators.len_u64(); - if num_validators as usize != expected_count { - return Err(format!( - "Post-state validator count mismatch: expected {}, got {}", - expected_count, num_validators - ) - .into()); - } + ensure!( + num_validators as usize == expected_count, + "Post-state validator count mismatch: expected {}, got {}", + expected_count, + num_validators + ); println!("\n✓ All post-state checks passed"); println!(" Final slot: {:?}", state.slot); @@ -114,9 +109,7 @@ impl TestRunner { Ok(()) } - pub fn run_single_block_with_slot_gap_tests>( - path: P, - ) -> Result<(), Box> { + pub fn run_single_block_with_slot_gap_tests>(path: P) -> Result<()> { let json_content = fs::read_to_string(path)?; // Parse using the new TestVectorFile structure with camelCase @@ -127,7 +120,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -150,14 +143,13 @@ impl TestRunner { let computed_parent_root = hash_tree_root(&state_after_slots.latest_block_header); // Verify the block's parent_root matches what we computed - if block.parent_root != computed_parent_root { - return Err(format!( - "Block {} parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - idx + 1, - block.parent_root, - computed_parent_root - ).into()); - } + ensure!( + block.parent_root == computed_parent_root, + "Block {} parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + idx + 1, + block.parent_root, + computed_parent_root + ); println!(" ✓ Parent root matches: {:?}", computed_parent_root); @@ -172,14 +164,13 @@ impl TestRunner { let computed_state_root = hash_tree_root(&state); // Verify the computed state_root matches the expected one from the vector - if block.state_root != computed_state_root { - return Err(format!( - "Block {} state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - idx + 1, - block.state_root, - computed_state_root - ).into()); - } + ensure!( + block.state_root == computed_state_root, + "Block {} state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + idx + 1, + block.state_root, + computed_state_root + ); println!(" ✓ State root matches: {:?}", computed_state_root); println!( @@ -189,20 +180,19 @@ impl TestRunner { ); } Err(e) => { - return Err(format!("Block {} processing failed: {:?}", idx + 1, e).into()); + bail!("Block {} processing failed: {:?}", idx + 1, e); } } } // Verify post-state conditions if let Some(post) = test_case.post { - if state.slot != post.slot { - return Err(format!( - "Post-state slot mismatch: expected {:?}, got {:?}", - post.slot, state.slot - ) - .into()); - } + ensure!( + state.slot == post.slot, + "Post-state slot mismatch: expected {:?}, got {:?}", + post.slot, + state.slot + ); println!("\n✓ All post-state checks passed"); println!(" Final slot: {:?}", state.slot); @@ -214,9 +204,7 @@ impl TestRunner { Ok(()) } - pub fn run_single_empty_block_tests>( - path: P, - ) -> Result<(), Box> { + pub fn run_single_empty_block_tests>(path: P) -> Result<()> { let json_content = fs::read_to_string(path)?; // Parse using the new TestVectorFile structure with camelCase @@ -227,7 +215,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -236,22 +224,22 @@ impl TestRunner { let mut state = test_case.pre.clone(); // Should be exactly one block - if blocks.len() != 1 { - return Err(format!("Expected 1 block, found {}", blocks.len()).into()); - } + ensure!( + blocks.len() == 1, + "Expected 1 block, found {}", + blocks.len() + ); let block = &blocks[0]; println!("\nProcessing single empty block at slot {:?}", block.slot); // Verify it's an empty block (no attestations) let attestation_count = block.body.attestations.len_u64(); - if attestation_count > 0 { - return Err(format!( - "Expected empty block, but found {} attestations", - attestation_count - ) - .into()); - } + ensure!( + attestation_count == 0, + "Expected empty block, but found {} attestations", + attestation_count + ); println!(" ✓ Confirmed: Block has no attestations (empty block)"); // Advance state to the block's slot @@ -261,13 +249,12 @@ impl TestRunner { let computed_parent_root = hash_tree_root(&state_after_slots.latest_block_header); // Verify the block's parent_root matches what we computed - if block.parent_root != computed_parent_root { - return Err(format!( - "Block parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - block.parent_root, - computed_parent_root - ).into()); - } + ensure!( + block.parent_root == computed_parent_root, + "Block parent_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + block.parent_root, + computed_parent_root + ); println!(" ✓ Parent root matches: {:?}", computed_parent_root); @@ -282,31 +269,29 @@ impl TestRunner { let computed_state_root = hash_tree_root(&state); // Verify the computed state_root matches the expected one from the vector - if block.state_root != computed_state_root { - return Err(format!( - "Block state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", - block.state_root, - computed_state_root - ).into()); - } + ensure!( + block.state_root == computed_state_root, + "Block state_root mismatch:\n Expected (from vector): {:?}\n Computed (from state): {:?}", + block.state_root, + computed_state_root + ); println!(" ✓ State root matches: {:?}", computed_state_root); println!(" ✓ Empty block processed successfully"); } Err(e) => { - return Err(format!("Block processing failed: {:?}", e).into()); + bail!("Block processing failed: {:?}", e); } } // Verify post-state conditions if let Some(post) = test_case.post { - if state.slot != post.slot { - return Err(format!( - "Post-state slot mismatch: expected {:?}, got {:?}", - post.slot, state.slot - ) - .into()); - } + ensure!( + state.slot == post.slot, + "Post-state slot mismatch: expected {:?}, got {:?}", + post.slot, + state.slot + ); println!("\n✓ All post-state checks passed"); println!(" Final slot: {:?}", state.slot); @@ -320,9 +305,7 @@ impl TestRunner { /// Generic test runner for block processing test vectors /// Handles all test vectors from test_blocks directory - pub fn run_block_processing_test>( - path: P, - ) -> Result<(), Box> { + pub fn run_block_processing_test>(path: P) -> Result<()> { let json_content = fs::read_to_string(path.as_ref())?; // Parse using the TestVectorFile structure with camelCase @@ -333,7 +316,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("\n{}: {}", test_name, test_case.info.description); @@ -385,7 +368,7 @@ impl TestRunner { println!(" \x1b[31m✗ FAIL: Parent root mismatch\x1b[0m"); println!(" Expected: {:?}", block.parent_root); println!(" Got: {:?}\n", computed_parent_root); - return Err(format!("Block {} parent_root mismatch", idx + 1).into()); + bail!("Block {} parent_root mismatch", idx + 1); } let attestation_count = block.body.attestations.len_u64(); @@ -405,7 +388,7 @@ impl TestRunner { println!(" \x1b[31m✗ FAIL: State root mismatch\x1b[0m"); println!(" Expected: {:?}", block.state_root); println!(" Got: {:?}\n", computed_state_root); - return Err(format!("Block {} state_root mismatch", idx + 1).into()); + bail!("Block {} state_root mismatch", idx + 1); } if attestation_count > 0 { @@ -417,7 +400,7 @@ impl TestRunner { Err(e) => { println!(" \x1b[31m✗ FAIL: Processing failed\x1b[0m"); println!(" Error: {:?}\n", e); - return Err(format!("Block {} processing failed", idx + 1).into()); + bail!("Block {} processing failed", idx + 1); } } } @@ -433,7 +416,7 @@ impl TestRunner { /// Test runner for genesis state test vectors /// Handles test vectors from test_genesis directory - pub fn run_genesis_test>(path: P) -> Result<(), Box> { + pub fn run_genesis_test>(path: P) -> Result<()> { let json_content = fs::read_to_string(path.as_ref())?; // Parse using the TestVectorFile structure @@ -444,7 +427,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("\n{}: {}", test_name, test_case.info.description); @@ -457,32 +440,30 @@ impl TestRunner { ); // Verify it's at genesis (slot 0) - if state.slot.0 != 0 { - return Err(format!("Expected genesis at slot 0, got slot {}", state.slot.0).into()); - } + ensure!( + state.slot.0 == 0, + "Expected genesis at slot 0, got slot {}", + state.slot.0 + ); // Verify checkpoint initialization - if state.latest_justified.slot.0 != 0 { - return Err(format!( - "Expected latest_justified at slot 0, got {}", - state.latest_justified.slot.0 - ) - .into()); - } + ensure!( + state.latest_justified.slot.0 == 0, + "Expected latest_justified at slot 0, got {}", + state.latest_justified.slot.0 + ); - if state.latest_finalized.slot.0 != 0 { - return Err(format!( - "Expected latest_finalized at slot 0, got {}", - state.latest_finalized.slot.0 - ) - .into()); - } + ensure!( + state.latest_finalized.slot.0 == 0, + "Expected latest_finalized at slot 0, got {}", + state.latest_finalized.slot.0 + ); // Verify empty historical data - let has_history = state.historical_block_hashes.get(0).is_ok(); - if has_history { - return Err("Expected empty historical block hashes at genesis".into()); - } + ensure!( + state.historical_block_hashes.get(0).is_err(), + "Expected empty historical block hashes at genesis" + ); println!(" ✓ Genesis state validated"); @@ -497,7 +478,7 @@ impl TestRunner { } /// Helper: Run invalid block test (expecting exception) - fn run_invalid_block_test(test_case: TestCase) -> Result<(), Box> { + fn run_invalid_block_test(test_case: TestCase) -> Result<()> { if let Some(ref blocks) = test_case.blocks { if blocks.is_empty() { println!(" WARNING: Empty blocks array - cannot test invalid block"); @@ -527,9 +508,7 @@ impl TestRunner { break; // Stop at first error } else { println!(" \x1b[31m✗ FAIL: Block processed successfully - but should have failed!\x1b[0m\n"); - return Err( - "Expected block processing to fail, but it succeeded".into() - ); + bail!("Expected block processing to fail, but it succeeded"); } } Err(e) => { @@ -540,16 +519,17 @@ impl TestRunner { } } - if !error_occurred { - return Err("Expected an exception but all blocks processed successfully".into()); - } + ensure!( + error_occurred, + "Expected an exception but all blocks processed successfully" + ); } Ok(()) } /// Helper: Verify genesis state only (no blocks) - fn verify_genesis_state(test_case: TestCase) -> Result<(), Box> { + fn verify_genesis_state(test_case: TestCase) -> Result<()> { let state = &test_case.pre; // Verify post-state if present @@ -559,31 +539,26 @@ impl TestRunner { } /// Helper: Verify post-state conditions - fn verify_post_state( - state: &State, - test_case: &TestCase, - ) -> Result<(), Box> { + fn verify_post_state(state: &State, test_case: &TestCase) -> Result<()> { if let Some(ref post) = test_case.post { // Verify slot - if state.slot != post.slot { - return Err(format!( - "Post-state slot mismatch: expected {:?}, got {:?}", - post.slot, state.slot - ) - .into()); - } + ensure!( + state.slot == post.slot, + "Post-state slot mismatch: expected {:?}, got {:?}", + post.slot, + state.slot + ); // Verify validator count if specified if let Some(expected_count) = post.validator_count { let num_validators: u64 = state.validators.len_u64(); - if num_validators as usize != expected_count { - return Err(format!( - "Post-state validator count mismatch: expected {}, got {}", - expected_count, num_validators - ) - .into()); - } + ensure!( + num_validators as usize == expected_count, + "Post-state validator count mismatch: expected {}, got {}", + expected_count, + num_validators + ); println!( " ✓ Post-state verified: slot {}, {} validators", state.slot.0, num_validators @@ -598,9 +573,7 @@ impl TestRunner { /// Test runner for verify_signatures test vectors /// Tests XMSS signature verification on SignedBlockWithAttestation - pub fn run_verify_signatures_test>( - path: P, - ) -> Result<(), Box> { + pub fn run_verify_signatures_test>(path: P) -> Result<()> { let json_content = fs::read_to_string(path.as_ref())?; // Parse using the VerifySignaturesTestVectorFile structure @@ -611,7 +584,7 @@ impl TestRunner { .tests .into_iter() .next() - .ok_or("No test case found in JSON")?; + .context("No test case found in JSON")?; println!("\n{}: {}", test_name, test_case.info.description); @@ -643,9 +616,9 @@ impl TestRunner { // Verify signatures - we expect this to fail (return false) let result = signed_block.verify_signatures(anchor_state); - if result { + if result.is_ok() { println!(" \x1b[31m✗ FAIL: Signatures verified successfully but should have failed!\x1b[0m\n"); - return Err("Expected signature verification to fail, but it succeeded".into()); + bail!("Expected signature verification to fail, but it succeeded"); } else { println!(" ✓ Correctly rejected: Invalid signatures detected"); println!("\n\x1b[32m✓ PASS\x1b[0m\n"); @@ -654,12 +627,12 @@ impl TestRunner { // Valid test case - signatures should verify successfully let result = signed_block.verify_signatures(anchor_state); - if result { + if result.is_ok() { println!(" ✓ All signatures verified successfully"); println!("\n\x1b[32m✓ PASS\x1b[0m\n"); } else { println!(" \x1b[31m✗ FAIL: Signature verification failed\x1b[0m\n"); - return Err("Signature verification failed".into()); + bail!("Signature verification failed: {:?}", result.unwrap_err()); } } diff --git a/lean_client/containers/tests/unit_tests/state_process.rs b/lean_client/containers/tests/unit_tests/state_process.rs index f423818..976a8cf 100644 --- a/lean_client/containers/tests/unit_tests/state_process.rs +++ b/lean_client/containers/tests/unit_tests/state_process.rs @@ -126,7 +126,7 @@ fn test_process_block_header_invalid( assert!(result.is_err()); let err_msg = result.unwrap_err(); - assert!(err_msg.contains(expected_error)); + assert!(err_msg.to_string().contains(expected_error)); } // This test verifies that attestations correctly justify and finalize slots diff --git a/lean_client/containers/tests/unit_tests/state_transition.rs b/lean_client/containers/tests/unit_tests/state_transition.rs index e530dde..fe64273 100644 --- a/lean_client/containers/tests/unit_tests/state_transition.rs +++ b/lean_client/containers/tests/unit_tests/state_transition.rs @@ -79,7 +79,10 @@ fn test_state_transition_invalid_signatures() { let result = state.state_transition(final_signed_block_with_attestation, false); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Block signatures must be valid"); + assert_eq!( + result.unwrap_err().to_string(), + "Block signatures must be valid" + ); } #[test] @@ -103,5 +106,5 @@ fn test_state_transition_bad_state_root() { let result = state.state_transition(final_signed_block_with_attestation, true); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Invalid block state root"); + assert_eq!(result.unwrap_err().to_string(), "Invalid block state root"); } diff --git a/lean_client/fork_choice/Cargo.toml b/lean_client/fork_choice/Cargo.toml index f906f59..1313ee9 100644 --- a/lean_client/fork_choice/Cargo.toml +++ b/lean_client/fork_choice/Cargo.toml @@ -4,6 +4,8 @@ version = "0.1.0" edition = "2021" [dependencies] +anyhow = { workspace = true } +thiserror = { workspace = true } containers = { path = "../containers" } ssz = { git = "https://github.com/grandinetech/grandine", package = "ssz", branch = "develop"} ssz_derive = { git = "https://github.com/grandinetech/grandine", package = "ssz_derive", branch = "develop" } @@ -11,5 +13,6 @@ typenum = "1.17.0" serde = { version = "1.0", features = ["derive"] } [dev-dependencies] +anyhow = { workspace = true } ssz_rs = "0.9" serde_json = "1.0" diff --git a/lean_client/fork_choice/src/handlers.rs b/lean_client/fork_choice/src/handlers.rs index 25eb50d..27d3b66 100644 --- a/lean_client/fork_choice/src/handlers.rs +++ b/lean_client/fork_choice/src/handlers.rs @@ -1,8 +1,22 @@ use crate::store::*; +use anyhow::{bail, ensure, Context, Result}; use containers::{ attestation::SignedAttestation, block::SignedBlockWithAttestation, Bytes32, ValidatorIndex, }; use ssz::SszHash; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum BlockTransitionError { + #[error("Block queued: parent not yet available (pending: {pending_count} blocks)")] + BlockQueued { pending_count: usize }, + + #[error("No parent state found for block")] + NoParentState, + + #[error(transparent)] + StateTransition(#[from] anyhow::Error), +} #[inline] pub fn on_tick(store: &mut Store, time: u64, has_proposal: bool) { @@ -24,7 +38,7 @@ pub fn on_attestation( store: &mut Store, signed_attestation: SignedAttestation, is_from_block: bool, -) -> Result<(), String> { +) -> Result<()> { let validator_id = ValidatorIndex(signed_attestation.message.validator_id.0); let attestation_slot = signed_attestation.message.data.slot; let source_slot = signed_attestation.message.data.source.slot; @@ -32,20 +46,20 @@ pub fn on_attestation( // Validate attestation is not from future let curr_slot = store.time / INTERVALS_PER_SLOT; - if attestation_slot.0 > curr_slot { - return Err(format!( - "Err: (Fork-choice::Handlers::OnAttestation) Attestation for slot {} has not yet occurred, out of sync. (CURRENT SLOT NUMBER: {})", - attestation_slot.0, curr_slot - )); - } + ensure!( + attestation_slot.0 <= curr_slot, + "Attestation for slot {} has not yet occurred, out of sync (current slot: {})", + attestation_slot.0, + curr_slot + ); // Validate source slot does not exceed target slot (per leanSpec validate_attestation) - if source_slot > target_slot { - return Err(format!( - "Err: (Fork-choice::Handlers::OnAttestation) Source slot {} exceeds target slot {}", - source_slot.0, target_slot.0 - )); - } + ensure!( + source_slot <= target_slot, + "Source slot {} exceeds target slot {}", + source_slot.0, + target_slot.0 + ); if is_from_block { // On-chain attestation processing - immediately becomes "known" @@ -84,7 +98,10 @@ pub fn on_attestation( Ok(()) } -pub fn on_block(store: &mut Store, signed_block: SignedBlockWithAttestation) -> Result<(), String> { +pub fn on_block( + store: &mut Store, + signed_block: SignedBlockWithAttestation, +) -> Result<(), BlockTransitionError> { let block_root = Bytes32(signed_block.message.block.hash_tree_root()); if store.blocks.contains_key(&block_root) { @@ -99,11 +116,9 @@ pub fn on_block(store: &mut Store, signed_block: SignedBlockWithAttestation) -> .entry(parent_root) .or_insert_with(Vec::new) .push(signed_block); - return Err(format!( - "Err: (Fork-choice::Handlers::OnBlock) Block queued: parent {:?} not yet available (pending: {} blocks)", - &parent_root.0.as_bytes()[..4], - store.blocks_queue.values().map(|v| v.len()).sum::() - )); + return Err(BlockTransitionError::BlockQueued { + pending_count: store.blocks_queue.values().map(|v| v.len()).sum(), + }); } process_block_internal(store, signed_block, block_root)?; @@ -116,21 +131,19 @@ fn process_block_internal( store: &mut Store, signed_block: SignedBlockWithAttestation, block_root: Bytes32, -) -> Result<(), String> { +) -> Result<(), BlockTransitionError> { let block = &signed_block.message.block; // Get parent state for validation - let state = match store.states.get(&block.parent_root) { - Some(state) => state, - None => { - return Err( - "Err: (Fork-choice::Handlers::ProcessBlockInternal) No parent state.".to_string(), - ); - } - }; + let state = store + .states + .get(&block.parent_root) + .ok_or(BlockTransitionError::NoParentState)?; // Execute state transition to get post-state - let new_state = state.state_transition_with_validation(signed_block.clone(), true, true)?; + let new_state = state + .state_transition_with_validation(signed_block.clone(), true, true) + .context("State transition failed")?; // Store block and state store.blocks.insert(block_root, signed_block.clone()); diff --git a/lean_client/fork_choice/src/store.rs b/lean_client/fork_choice/src/store.rs index bb54574..7aa871d 100644 --- a/lean_client/fork_choice/src/store.rs +++ b/lean_client/fork_choice/src/store.rs @@ -77,7 +77,10 @@ pub fn get_fork_choice_head( .iter() .min_by_key(|(_, block)| block.message.block.slot) .map(|(r, _)| *r) - .expect("Error: Empty block."); + .unwrap_or_else(|| { + // Should never happen - genesis block should always be present + store.head + }); } let mut vote_weights: HashMap = HashMap::new(); let root_slot = store.blocks[&root].message.block.slot; diff --git a/lean_client/fork_choice/tests/fork_choice_test_vectors.rs b/lean_client/fork_choice/tests/fork_choice_test_vectors.rs index d35c0bf..c713039 100644 --- a/lean_client/fork_choice/tests/fork_choice_test_vectors.rs +++ b/lean_client/fork_choice/tests/fork_choice_test_vectors.rs @@ -1,3 +1,4 @@ +use anyhow::{bail, ensure, Context, Result}; use fork_choice::{ handlers::{on_attestation, on_block, on_tick}, store::{get_forkchoice_store, Store}, @@ -207,30 +208,34 @@ struct TestInfo { fixture_format: String, } -fn parse_root(hex_str: &str) -> Bytes32 { +fn parse_root(hex_str: &str) -> Result { let hex = hex_str.trim_start_matches("0x"); let mut bytes = [0u8; 32]; + ensure!( + hex.len() == 64 || hex.chars().all(|c| c == '0'), + "Invalid root length: {} (expected 64 hex chars)", + hex.len() + ); + if hex.len() == 64 { for i in 0..32 { bytes[i] = u8::from_str_radix(&hex[i * 2..i * 2 + 2], 16) - .unwrap_or_else(|_| panic!("Invalid hex at position {}: {}", i, hex)); + .with_context(|| format!("Invalid hex at position {}: {}", i, hex))?; } - } else if !hex.chars().all(|c| c == '0') { - panic!("Invalid root length: {} (expected 64 hex chars)", hex.len()); } - Bytes32(ssz::H256::from(bytes)) + Ok(Bytes32(ssz::H256::from(bytes))) } -fn convert_test_checkpoint(test_cp: &TestCheckpoint) -> Checkpoint { - Checkpoint { - root: parse_root(&test_cp.root), +fn convert_test_checkpoint(test_cp: &TestCheckpoint) -> Result { + Ok(Checkpoint { + root: parse_root(&test_cp.root)?, slot: Slot(test_cp.slot), - } + }) } -fn convert_test_attestation(test_att: &TestAttestation) -> Attestation { +fn convert_test_attestation(test_att: &TestAttestation) -> Result { let (validator_id, slot, head, target, source) = match test_att { TestAttestation::Nested { validator_id, data } => ( *validator_id, @@ -248,32 +253,34 @@ fn convert_test_attestation(test_att: &TestAttestation) -> Attestation { } => (*validator_id, *slot, head, target, source), }; - Attestation { + Ok(Attestation { validator_id: Uint64(validator_id), data: AttestationData { slot: Slot(slot), - head: convert_test_checkpoint(head), - target: convert_test_checkpoint(target), - source: convert_test_checkpoint(source), + head: convert_test_checkpoint(head)?, + target: convert_test_checkpoint(target)?, + source: convert_test_checkpoint(source)?, }, - } + }) } -fn convert_test_anchor_block(test_block: &TestAnchorBlock) -> SignedBlockWithAttestation { +fn convert_test_anchor_block(test_block: &TestAnchorBlock) -> Result { let mut attestations = ssz::PersistentList::default(); for (i, test_att) in test_block.body.attestations.data.iter().enumerate() { - let signed_vote = convert_test_attestation(test_att); + let signed_vote = convert_test_attestation(test_att)?; attestations .push(signed_vote) - .expect(&format!("Failed to add attestation {}", i)); + .with_context(|| format!("Failed to add attestation {}", i))?; } + let parent_root = parse_root(&test_block.parent_root)?; + let block = Block { slot: Slot(test_block.slot), proposer_index: ValidatorIndex(test_block.proposer_index), - parent_root: parse_root(&test_block.parent_root), - state_root: parse_root(&test_block.state_root), + parent_root, + state_root: parse_root(&test_block.state_root)?, body: BlockBody { attestations }, }; @@ -283,62 +290,62 @@ fn convert_test_anchor_block(test_block: &TestAnchorBlock) -> SignedBlockWithAtt data: AttestationData { slot: Slot(test_block.slot), head: Checkpoint { - root: parse_root(&test_block.parent_root), + root: parent_root, slot: Slot(test_block.slot), }, target: Checkpoint { - root: parse_root(&test_block.parent_root), + root: parent_root, slot: Slot(test_block.slot), }, source: Checkpoint { - root: parse_root(&test_block.parent_root), + root: parent_root, slot: Slot(0), }, }, }; - SignedBlockWithAttestation { + Ok(SignedBlockWithAttestation { message: BlockWithAttestation { block, proposer_attestation, }, signature: BlockSignatures::default(), - } + }) } fn convert_test_block( test_block_with_att: &TestBlockWithAttestation, -) -> SignedBlockWithAttestation { +) -> Result { let test_block = &test_block_with_att.block; let mut attestations = ssz::PersistentList::default(); for (i, test_att) in test_block.body.attestations.data.iter().enumerate() { - let signed_vote = convert_test_attestation(test_att); + let signed_vote = convert_test_attestation(test_att)?; attestations .push(signed_vote) - .expect(&format!("Failed to add attestation {}", i)); + .with_context(|| format!("Failed to add attestation {}", i))?; } let block = Block { slot: Slot(test_block.slot), proposer_index: ValidatorIndex(test_block.proposer_index), - parent_root: parse_root(&test_block.parent_root), - state_root: parse_root(&test_block.state_root), + parent_root: parse_root(&test_block.parent_root)?, + state_root: parse_root(&test_block.state_root)?, body: BlockBody { attestations }, }; - let proposer_attestation = convert_test_attestation(&test_block_with_att.proposer_attestation); + let proposer_attestation = convert_test_attestation(&test_block_with_att.proposer_attestation)?; - SignedBlockWithAttestation { + Ok(SignedBlockWithAttestation { message: BlockWithAttestation { block, proposer_attestation, }, signature: BlockSignatures::default(), - } + }) } -fn initialize_state_from_test(test_state: &TestAnchorState) -> State { +fn initialize_state_from_test(test_state: &TestAnchorState) -> Result { use containers::{ HistoricalBlockHashes, JustificationRoots, JustificationsValidators, JustifiedSlots, }; @@ -351,16 +358,16 @@ fn initialize_state_from_test(test_state: &TestAnchorState) -> State { let latest_block_header = BlockHeader { slot: Slot(test_state.latest_block_header.slot), proposer_index: ValidatorIndex(test_state.latest_block_header.proposer_index), - parent_root: parse_root(&test_state.latest_block_header.parent_root), - state_root: parse_root(&test_state.latest_block_header.state_root), - body_root: parse_root(&test_state.latest_block_header.body_root), + parent_root: parse_root(&test_state.latest_block_header.parent_root)?, + state_root: parse_root(&test_state.latest_block_header.state_root)?, + body_root: parse_root(&test_state.latest_block_header.body_root)?, }; let mut historical_block_hashes = HistoricalBlockHashes::default(); for hash_str in &test_state.historical_block_hashes.data { historical_block_hashes - .push(parse_root(hash_str)) - .expect("within limit"); + .push(parse_root(hash_str)?) + .context("Failed to add historical block hash")?; } let mut justified_slots = JustifiedSlots::new(false, test_state.justified_slots.data.len()); @@ -373,8 +380,8 @@ fn initialize_state_from_test(test_state: &TestAnchorState) -> State { let mut justifications_roots = JustificationRoots::default(); for root_str in &test_state.justifications_roots.data { justifications_roots - .push(parse_root(root_str)) - .expect("within limit"); + .push(parse_root(root_str)?) + .context("Failed to add justification root")?; } let mut justifications_validators = @@ -388,26 +395,28 @@ fn initialize_state_from_test(test_state: &TestAnchorState) -> State { let mut validators = List::default(); for test_validator in &test_state.validators.data { let pubkey = containers::validator::BlsPublicKey::from_hex(&test_validator.pubkey) - .expect("Failed to parse validator pubkey"); + .context("Failed to parse validator pubkey")?; let validator = containers::validator::Validator { pubkey, index: containers::Uint64(test_validator.index), }; - validators.push(validator).expect("Failed to add validator"); + validators + .push(validator) + .context("Failed to add validator")?; } - State { + Ok(State { config, slot: Slot(test_state.slot), latest_block_header, - latest_justified: convert_test_checkpoint(&test_state.latest_justified), - latest_finalized: convert_test_checkpoint(&test_state.latest_finalized), + latest_justified: convert_test_checkpoint(&test_state.latest_justified)?, + latest_finalized: convert_test_checkpoint(&test_state.latest_finalized)?, historical_block_hashes, justified_slots, validators, justifications_roots, justifications_validators, - } + }) } fn verify_checks( @@ -415,7 +424,7 @@ fn verify_checks( checks: &Option, block_labels: &HashMap, step_idx: usize, -) -> Result<(), String> { +) -> Result<()> { // If no checks provided, nothing to verify let checks = match checks { Some(c) => c, @@ -424,35 +433,35 @@ fn verify_checks( if let Some(expected_slot) = checks.head_slot { let actual_slot = store.blocks[&store.head].message.block.slot.0; - if actual_slot != expected_slot { - return Err(format!( - "Step {}: Head slot mismatch - expected {}, got {}", - step_idx, expected_slot, actual_slot - )); - } + ensure!( + actual_slot == expected_slot, + "Step {}: Head slot mismatch - expected {}, got {}", + step_idx, + expected_slot, + actual_slot + ); } if let Some(label) = &checks.head_root_label { let expected_root = block_labels .get(label) - .ok_or_else(|| format!("Step {}: Block label '{}' not found", step_idx, label))?; - if &store.head != expected_root { - let actual_slot = store - .blocks - .get(&store.head) - .map(|b| b.message.block.slot.0) - .unwrap_or(0); - let expected_slot = store - .blocks - .get(expected_root) - .map(|b| b.message.block.slot.0) - .unwrap_or(0); - return Err(format!( - "Step {}: Head root mismatch for label '{}' - expected slot {}, got slot {} (known_attestations: {}, new_attestations: {})", - step_idx, label, expected_slot, actual_slot, - store.latest_known_attestations.len(), store.latest_new_attestations.len() - )); - } + .with_context(|| format!("Step {}: Block label '{}' not found", step_idx, label))?; + let actual_slot = store + .blocks + .get(&store.head) + .map(|b| b.message.block.slot.0) + .unwrap_or(0); + let expected_slot = store + .blocks + .get(expected_root) + .map(|b| b.message.block.slot.0) + .unwrap_or(0); + ensure!( + &store.head == expected_root, + "Step {}: Head root mismatch for label '{}' - expected slot {}, got slot {} (known_attestations: {}, new_attestations: {})", + step_idx, label, expected_slot, actual_slot, + store.latest_known_attestations.len(), store.latest_new_attestations.len() + ); } if let Some(att_checks) = &checks.attestation_checks { @@ -461,35 +470,35 @@ fn verify_checks( match check.location.as_str() { "new" => { - if !store.latest_new_attestations.contains_key(&validator) { - return Err(format!( - "Step {}: Expected validator {} in new attestations, but not found", - step_idx, check.validator - )); - } + ensure!( + store.latest_new_attestations.contains_key(&validator), + "Step {}: Expected validator {} in new attestations, but not found", + step_idx, + check.validator + ); if let Some(target_slot) = check.target_slot { let attestation = &store.latest_new_attestations[&validator]; - if attestation.message.data.target.slot.0 != target_slot { - return Err(format!( - "Step {}: Validator {} new attestation target slot mismatch - expected {}, got {}", - step_idx, check.validator, target_slot, attestation.message.data.target.slot.0 - )); - } + ensure!( + attestation.message.data.target.slot.0 == target_slot, + "Step {}: Validator {} new attestation target slot mismatch - expected {}, got {}", + step_idx, check.validator, target_slot, attestation.message.data.target.slot.0 + ); } } "known" => { - if !store.latest_known_attestations.contains_key(&validator) { - return Err(format!( - "Step {}: Expected validator {} in known attestations, but not found", - step_idx, check.validator - )); - } + ensure!( + store.latest_known_attestations.contains_key(&validator), + "Step {}: Expected validator {} in known attestations, but not found", + step_idx, + check.validator + ); } _ => { - return Err(format!( + bail!( "Step {}: Unknown attestation location: {}", - step_idx, check.location - )); + step_idx, + check.location + ); } } } @@ -498,11 +507,11 @@ fn verify_checks( Ok(()) } -fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { +fn run_single_test(_test_name: &str, test: TestVector) -> Result<()> { println!(" Running: {}", test.info.test_id); - let mut anchor_state = initialize_state_from_test(&test.anchor_state); - let anchor_block = convert_test_anchor_block(&test.anchor_block); + let mut anchor_state = initialize_state_from_test(&test.anchor_state)?; + let anchor_block = convert_test_anchor_block(&test.anchor_block)?; let body_root = hash_tree_root(&anchor_block.message.block.body); anchor_state.latest_block_header = BlockHeader { @@ -526,10 +535,10 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { let test_block = step .block .as_ref() - .ok_or_else(|| format!("Step {}: Missing block data", step_idx))?; + .with_context(|| format!("Step {}: Missing block data", step_idx))?; let result = std::panic::catch_unwind(AssertUnwindSafe(|| { - let signed_block = convert_test_block(test_block); + let signed_block = convert_test_block(test_block)?; let block_root = Bytes32(signed_block.message.block.hash_tree_root()); // Advance time to the block's slot to ensure attestations are processable @@ -542,9 +551,9 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { Ok(block_root) })); - let result = match result { + let result: Result = match result { Ok(inner) => inner, - Err(e) => Err(format!("Panic: {:?}", e)), + Err(e) => bail!("Panic: {:?}", e), }; if let Ok(block_root) = &result { @@ -553,18 +562,17 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { } } - if step.valid && result.is_err() { - return Err(format!( - "Step {}: Block should be valid but processing failed: {:?}", - step_idx, - result.err() - )); - } else if !step.valid && result.is_ok() { - return Err(format!( - "Step {}: Block should be invalid but processing succeeded", - step_idx - )); - } + ensure!( + !(step.valid && result.is_err()), + "Step {}: Block should be valid but processing failed: {:?}", + step_idx, + result.err() + ); + ensure!( + !(!step.valid && result.is_ok()), + "Step {}: Block should be invalid but processing succeeded", + step_idx + ); if step.valid && result.is_ok() { verify_checks(&store, &step.checks, &block_labels, step_idx)?; @@ -574,7 +582,7 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { let time_value = step .tick .or(step.time) - .ok_or_else(|| format!("Step {}: Missing tick/time data", step_idx))?; + .with_context(|| format!("Step {}: Missing tick/time data", step_idx))?; on_tick(&mut store, time_value, false); if step.valid { @@ -585,10 +593,10 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { let test_att = step .attestation .as_ref() - .ok_or_else(|| format!("Step {}: Missing attestation data", step_idx))?; + .with_context(|| format!("Step {}: Missing attestation data", step_idx))?; let result = std::panic::catch_unwind(AssertUnwindSafe(|| { - let attestation = convert_test_attestation(test_att); + let attestation = convert_test_attestation(test_att)?; let signed_attestation = SignedAttestation { message: attestation, signature: Signature::default(), @@ -596,33 +604,29 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { on_attestation(&mut store, signed_attestation, false) })); - let result = match result { + let result: Result<()> = match result { Ok(inner) => inner, - Err(e) => Err(format!("Panic: {:?}", e)), + Err(e) => bail!("Panic: {:?}", e), }; - if step.valid && result.is_err() { - return Err(format!( - "Step {}: Attestation should be valid but processing failed: {:?}", - step_idx, - result.err() - )); - } else if !step.valid && result.is_ok() { - return Err(format!( - "Step {}: Attestation should be invalid but processing succeeded", - step_idx - )); - } + ensure!( + !(step.valid && result.is_err()), + "Step {}: Attestation should be valid but processing failed: {:?}", + step_idx, + result.err() + ); + ensure!( + !(!step.valid && result.is_ok()), + "Step {}: Attestation should be invalid but processing succeeded", + step_idx + ); if step.valid && result.is_ok() { verify_checks(&store, &step.checks, &block_labels, step_idx)?; } } _ => { - return Err(format!( - "Step {}: Unknown step type: {}", - step_idx, step.step_type - )); + bail!("Step {}: Unknown step type: {}", step_idx, step.step_type); } } } @@ -630,12 +634,12 @@ fn run_single_test(_test_name: &str, test: TestVector) -> Result<(), String> { Ok(()) } -fn run_test_vector_file(test_path: &str) -> Result<(), String> { +fn run_test_vector_file(test_path: &str) -> Result<()> { let json_str = std::fs::read_to_string(test_path) - .map_err(|e| format!("Failed to read file {}: {}", test_path, e))?; + .with_context(|| format!("Failed to read file {}", test_path))?; let test_data: TestVectorFile = serde_json::from_str(&json_str) - .map_err(|e| format!("Failed to parse JSON from {}: {}", test_path, e))?; + .with_context(|| format!("Failed to parse JSON from {}", test_path))?; for (test_name, test_vector) in test_data.tests { run_single_test(&test_name, test_vector)?; @@ -648,8 +652,8 @@ fn run_test_vector_file(test_path: &str) -> Result<(), String> { fn test_fork_choice_head_vectors() { let test_dir = "../tests/test_vectors/test_fork_choice/test_fork_choice_head"; - let entries = - std::fs::read_dir(test_dir).expect(&format!("Failed to read test directory: {}", test_dir)); + let entries = std::fs::read_dir(test_dir) + .unwrap_or_else(|e| panic!("Failed to read test directory {}: {}", test_dir, e)); let mut test_count = 0; let mut pass_count = 0; @@ -658,18 +662,24 @@ fn test_fork_choice_head_vectors() { println!("\n=== Fork Choice Head Tests ==="); for entry in entries { - let path = entry.unwrap().path(); + let path = entry.expect("Failed to read directory entry").path(); if path.extension().map_or(false, |ext| ext == "json") { test_count += 1; - println!("\nTest file: {:?}", path.file_name().unwrap()); + let file_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + println!("\nTest file: {}", file_name); - match run_test_vector_file(path.to_str().unwrap()) { + let path_str = path.to_str().expect("Path contains invalid UTF-8"); + + match run_test_vector_file(path_str) { Ok(_) => { println!(" ✓ PASSED"); pass_count += 1; } Err(e) => { - println!(" ✗ FAILED: {}", e); + println!(" ✗ FAILED: {:#}", e); fail_count += 1; } } diff --git a/lean_client/networking/src/enr_ext.rs b/lean_client/networking/src/enr_ext.rs index d7511e4..4932039 100644 --- a/lean_client/networking/src/enr_ext.rs +++ b/lean_client/networking/src/enr_ext.rs @@ -3,7 +3,7 @@ //! ENR extension trait to support libp2p integration. -use anyhow::{Result, bail}; +use anyhow::{Result, anyhow, bail}; use discv5::{ Enr, enr::{CombinedKey, CombinedPublicKey}, @@ -315,13 +315,13 @@ impl CombinedKeyExt for CombinedKey { // helper function to convert a peer_id to a node_id. This is only possible for secp256k1/ed25519 libp2p // peer_ids -pub fn peer_id_to_node_id(peer_id: &PeerId) -> Result { +pub fn peer_id_to_node_id(peer_id: &PeerId) -> anyhow::Result { // A libp2p peer id byte representation should be 2 length bytes + 4 protobuf bytes + compressed pk bytes // if generated from a PublicKey with Identity multihash. let pk_bytes = &peer_id.to_bytes()[2..]; let public_key = PublicKey::try_decode_protobuf(pk_bytes).map_err(|e| { - format!( + anyhow!( " Cannot parse libp2p public key public key from peer id: {}", e ) @@ -353,7 +353,7 @@ pub fn peer_id_to_node_id(peer_id: &PeerId) -> Result Err(format!("Unsupported public key from peer {}", peer_id)), + _ => bail!("Unsupported public key from peer {}", peer_id), } } diff --git a/lean_client/networking/src/gossipsub/message.rs b/lean_client/networking/src/gossipsub/message.rs index 4ac1ae1..95dffba 100644 --- a/lean_client/networking/src/gossipsub/message.rs +++ b/lean_client/networking/src/gossipsub/message.rs @@ -1,5 +1,6 @@ use crate::gossipsub::topic::GossipsubKind; use crate::gossipsub::topic::GossipsubTopic; +use anyhow::{Context, Result}; use containers::SignedAttestation; use containers::SignedBlockWithAttestation; use containers::ssz::SszReadDefault; @@ -11,14 +12,15 @@ pub enum GossipsubMessage { } impl GossipsubMessage { - pub fn decode(topic: &TopicHash, data: &[u8]) -> Result { + pub fn decode(topic: &TopicHash, data: &[u8]) -> Result { match GossipsubTopic::decode(topic)?.kind { GossipsubKind::Block => Ok(Self::Block( SignedBlockWithAttestation::from_ssz_default(data) - .map_err(|e| format!("{:?}", e))?, + .context("Failed to decode SignedBlockWithAttestation")?, )), GossipsubKind::Attestation => Ok(Self::Attestation( - SignedAttestation::from_ssz_default(data).map_err(|e| format!("{:?}", e))?, + SignedAttestation::from_ssz_default(data) + .context("Failed to decode SignedAttestation")?, )), } } diff --git a/lean_client/networking/src/gossipsub/topic.rs b/lean_client/networking/src/gossipsub/topic.rs index 09fcd33..ba68238 100644 --- a/lean_client/networking/src/gossipsub/topic.rs +++ b/lean_client/networking/src/gossipsub/topic.rs @@ -1,3 +1,4 @@ +use anyhow::{Result, bail, ensure}; use libp2p::gossipsub::{IdentTopic, TopicHash}; pub const TOPIC_PREFIX: &str = "leanconsensus"; @@ -32,7 +33,7 @@ pub fn get_topics(fork: String) -> Vec { } impl GossipsubTopic { - pub fn decode(topic: &TopicHash) -> Result { + pub fn decode(topic: &TopicHash) -> Result { let topic_parts = Self::split_topic(topic)?; Self::validate_parts(&topic_parts, topic)?; let fork = Self::extract_fork(&topic_parts); @@ -41,20 +42,19 @@ impl GossipsubTopic { Ok(GossipsubTopic { fork, kind }) } - fn split_topic(topic: &TopicHash) -> Result, String> { + fn split_topic(topic: &TopicHash) -> Result> { let parts: Vec<&str> = topic.as_str().trim_start_matches('/').split('/').collect(); - if parts.len() != 4 { - return Err(format!("Invalid topic part count: {topic:?}")); - } + ensure!(parts.len() == 4, "Invalid topic part count: {topic:?}"); Ok(parts) } - fn validate_parts(parts: &[&str], topic: &TopicHash) -> Result<(), String> { - if parts[0] != TOPIC_PREFIX || parts[3] != SSZ_SNAPPY_ENCODING_POSTFIX { - return Err(format!("Invalid topic parts: {topic:?}")); - } + fn validate_parts(parts: &[&str], topic: &TopicHash) -> Result<()> { + ensure!( + parts[0] == TOPIC_PREFIX && parts[3] == SSZ_SNAPPY_ENCODING_POSTFIX, + "Invalid topic parts: {topic:?}" + ); Ok(()) } @@ -62,11 +62,11 @@ impl GossipsubTopic { parts[1].to_string() } - fn extract_kind(parts: &[&str]) -> Result { + fn extract_kind(parts: &[&str]) -> Result { match parts[2] { BLOCK_TOPIC => Ok(GossipsubKind::Block), ATTESTATION_TOPIC => Ok(GossipsubKind::Attestation), - other => Err(format!("Invalid topic kind: {other:?}")), + other => bail!("Invalid topic kind: {other:?}"), } } } diff --git a/lean_client/src/main.rs b/lean_client/src/main.rs index d5e70f6..7c1d57d 100644 --- a/lean_client/src/main.rs +++ b/lean_client/src/main.rs @@ -1,3 +1,4 @@ +use anyhow::{Context, Error, Result}; use clap::Parser; use containers::ssz::SszHash; use containers::{ @@ -11,7 +12,7 @@ use containers::{ Slot, }; use fork_choice::{ - handlers::{on_attestation, on_block, on_tick}, + handlers::{on_attestation, on_block, on_tick, BlockTransitionError}, store::{get_forkchoice_store, Store, INTERVALS_PER_SLOT}, }; use libp2p_identity::Keypair; @@ -31,10 +32,14 @@ use tokio::{ use tracing::{debug, info, warn}; use validator::{ValidatorConfig, ValidatorService}; -fn load_node_key(path: &str) -> Result> { - let hex_str = std::fs::read_to_string(path)?.trim().to_string(); - let bytes = hex::decode(&hex_str)?; - let secret = libp2p_identity::secp256k1::SecretKey::try_from_bytes(bytes)?; +fn load_node_key(path: &str) -> Result { + let hex_str = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read node key file: {}", path))? + .trim() + .to_string(); + let bytes = hex::decode(&hex_str).context("Failed to decode node key hex")?; + let secret = libp2p_identity::secp256k1::SecretKey::try_from_bytes(bytes) + .context("Failed to parse secp256k1 secret key")?; let keypair = libp2p_identity::secp256k1::Keypair::from(secret); Ok(Keypair::from(keypair)) } @@ -131,7 +136,7 @@ struct Args { } #[tokio::main] -async fn main() { +async fn main() -> Result<()> { tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .init(); @@ -145,21 +150,23 @@ async fn main() { let (genesis_time, validators) = if let Some(genesis_path) = &args.genesis { let genesis_config = containers::GenesisConfig::load_from_file(genesis_path) - .expect("Failed to load genesis config"); + .context("Failed to load genesis config")?; let validators: Vec = genesis_config .genesis_validators .iter() .enumerate() .map(|(i, v_str)| { - let pubkey = containers::validator::BlsPublicKey::from_hex(v_str) - .expect("Invalid genesis validator pubkey"); - containers::validator::Validator { + let pubkey = + containers::validator::BlsPublicKey::from_hex(v_str).with_context(|| { + format!("Invalid genesis validator pubkey at index {}: {}", i, v_str) + })?; + Ok(containers::validator::Validator { pubkey, index: Uint64(i as u64), - } + }) }) - .collect(); + .collect::>>()?; (genesis_config.genesis_time, validators) } else { @@ -303,7 +310,7 @@ async fn main() { keypair, ) .await - .expect("Failed to create network service with custom key") + .context("Failed to create network service with custom key")? } Err(e) => { warn!("Failed to load node key: {}, using random key", e); @@ -314,7 +321,7 @@ async fn main() { peer_count, ) .await - .expect("Failed to create network service") + .context("Failed to create network service")? } } } else { @@ -325,14 +332,10 @@ async fn main() { peer_count, ) .await - .expect("Failed to create network service") + .context("Failed to create network service")? }; - let network_handle = task::spawn(async move { - if let Err(err) = network_service.start().await { - panic!("Network service exited with error: {err}"); - } - }); + let network_handle = task::spawn(async move { network_service.start().await }); let chain_outbound_sender = outbound_p2p_sender.clone(); @@ -497,8 +500,8 @@ async fn main() { } } } - Err(e) if e.starts_with("Err: (Fork-choice::Handlers::OnBlock) Block queued") => { - debug!("Block queued, requesting missing parent: {}", e); + Err(BlockTransitionError::BlockQueued { pending_count: _ }) => { + debug!("Block queued, requesting missing parent"); // Request missing parent block from peers if !parent_root.0.is_zero() { @@ -553,13 +556,17 @@ async fn main() { }); tokio::select! { - _ = network_handle => { - println!("Network service finished."); + res = network_handle => { + res.map_err(Error::from) + .flatten() + .context("Network service failed")?; + info!("Network service exited normally"); } _ = chain_handle => { - println!("Chain service finished."); + info!("Chain service finished"); } } - println!("Main async task exiting..."); + info!("Main async task exiting..."); + Ok(()) } diff --git a/lean_client/validator/Cargo.toml b/lean_client/validator/Cargo.toml index b658c48..349ee17 100644 --- a/lean_client/validator/Cargo.toml +++ b/lean_client/validator/Cargo.toml @@ -8,6 +8,7 @@ default = ["xmss-signing"] xmss-signing = ["leansig"] [dependencies] +anyhow = { workspace = true } serde = { version = "1.0", features = ["derive"] } serde_yaml = "0.9" containers = { path = "../containers" } diff --git a/lean_client/validator/src/keys.rs b/lean_client/validator/src/keys.rs index 392fd95..6d39a6a 100644 --- a/lean_client/validator/src/keys.rs +++ b/lean_client/validator/src/keys.rs @@ -1,3 +1,4 @@ +use anyhow::{ensure, Context, Result}; use containers::attestation::U3112; use containers::ssz::ByteVector; use containers::Signature; @@ -25,12 +26,14 @@ pub struct KeyManager { impl KeyManager { /// Load keys from the hash-sig-keys directory - pub fn new(keys_dir: impl AsRef) -> Result> { + pub fn new(keys_dir: impl AsRef) -> Result { let keys_dir = keys_dir.as_ref().to_path_buf(); - if !keys_dir.exists() { - return Err(format!("Keys directory not found: {:?}", keys_dir).into()); - } + ensure!( + keys_dir.exists(), + "Keys directory not found: {:?}", + keys_dir + ); info!(path = ?keys_dir, "Initializing key manager"); @@ -41,16 +44,15 @@ impl KeyManager { } /// Load a secret key for a specific validator index - pub fn load_key(&mut self, validator_index: u64) -> Result<(), Box> { + pub fn load_key(&mut self, validator_index: u64) -> Result<()> { let sk_path = self .keys_dir .join(format!("validator_{}_sk.ssz", validator_index)); - if !sk_path.exists() { - return Err(format!("Secret key file not found: {:?}", sk_path).into()); - } + ensure!(sk_path.exists(), "Secret key file not found: {:?}", sk_path); - let key_bytes = std::fs::read(&sk_path)?; + let key_bytes = std::fs::read(&sk_path) + .with_context(|| format!("Failed to read secret key file: {:?}", sk_path))?; info!( validator = validator_index, @@ -63,38 +65,31 @@ impl KeyManager { } /// Sign a message with the validator's secret key - pub fn sign( - &self, - validator_index: u64, - epoch: u32, - message: &[u8; 32], - ) -> Result> { + pub fn sign(&self, validator_index: u64, epoch: u32, message: &[u8; 32]) -> Result { #[cfg(feature = "xmss-signing")] { let key_bytes = self .keys .get(&validator_index) - .ok_or_else(|| format!("No key loaded for validator {}", validator_index))?; + .with_context(|| format!("No key loaded for validator {}", validator_index))?; type SecretKey = ::SecretKey; let secret_key = SecretKey::from_bytes(key_bytes) - .map_err(|e| format!("Failed to deserialize secret key: {:?}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to deserialize secret key: {:?}", e))?; let leansig_signature = SIGTopLevelTargetSumLifetime32Dim64Base8::sign(&secret_key, epoch, message) - .map_err(|e| format!("Failed to sign message: {:?}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to sign message: {:?}", e))?; let sig_bytes = leansig_signature.to_bytes(); - if sig_bytes.len() != 3112 { - return Err(format!( - "Invalid signature size: expected 3112, got {}", - sig_bytes.len() - ) - .into()); - } + ensure!( + sig_bytes.len() == 3112, + "Invalid signature size: expected 3112, got {}", + sig_bytes.len() + ); // Convert to ByteVector using unsafe pointer copy (same pattern as BlsPublicKey) let mut byte_vec: ByteVector = ByteVector::default(); diff --git a/lean_client/validator/src/lib.rs b/lean_client/validator/src/lib.rs index 6c6a4a4..f514768 100644 --- a/lean_client/validator/src/lib.rs +++ b/lean_client/validator/src/lib.rs @@ -1,4 +1,5 @@ // Lean validator client with XMSS signing support +use anyhow::{bail, ensure, Context, Result}; use std::collections::HashMap; use std::path::Path; @@ -26,16 +27,16 @@ pub struct ValidatorConfig { impl ValidatorConfig { // load validator index - pub fn load_from_file( - path: impl AsRef, - node_id: &str, - ) -> Result> { - let file = std::fs::File::open(path)?; - let registry: ValidatorRegistry = serde_yaml::from_reader(file)?; + pub fn load_from_file(path: impl AsRef, node_id: &str) -> Result { + let file = std::fs::File::open(&path).with_context(|| { + format!("Failed to open validator config file: {:?}", path.as_ref()) + })?; + let registry: ValidatorRegistry = + serde_yaml::from_reader(file).context("Failed to parse validator registry YAML")?; let indices = registry .get(node_id) - .ok_or_else(|| format!("Node '{}' not found in validator registry", node_id))? + .with_context(|| format!("Node '{}' not found in validator registry", node_id))? .clone(); info!(node_id = %node_id, indices = ?indices, "Validator config loaded..."); @@ -76,7 +77,7 @@ impl ValidatorService { config: ValidatorConfig, num_validators: u64, keys_dir: impl AsRef, - ) -> Result> { + ) -> Result { let mut key_manager = KeyManager::new(keys_dir)?; // Load keys for all assigned validators @@ -118,7 +119,7 @@ impl ValidatorService { store: &mut Store, slot: Slot, proposer_index: ValidatorIndex, - ) -> Result { + ) -> Result { info!( slot = slot.0, proposer = proposer_index.0, @@ -129,22 +130,22 @@ impl ValidatorService { let parent_state = store .states .get(&parent_root) - .ok_or_else(|| format!("Couldn't find parent state {:?}", parent_root))?; + .with_context(|| format!("Couldn't find parent state {:?}", parent_root))?; let vote_target = get_vote_target(store); // Validate that target slot is strictly greater than source slot - if vote_target.slot <= store.latest_justified.slot { - return Err(format!( - "Invalid attestation: target slot {} must be greater than source slot {}", - vote_target.slot.0, store.latest_justified.slot.0 - )); - } + ensure!( + vote_target.slot > store.latest_justified.slot, + "Invalid attestation: target slot {} must be greater than source slot {}", + vote_target.slot.0, + store.latest_justified.slot.0 + ); let head_block = store .blocks .get(&store.head) - .ok_or("Head block not found")?; + .context("Head block not found")?; let head_checkpoint = Checkpoint { root: store.head, slot: head_block.message.block.slot, @@ -197,21 +198,23 @@ impl ValidatorService { ); // Build block with collected attestations (empty body - attestations go to state) - let (block, _post_state, _collected_atts, sigs) = parent_state.build_block( - slot, - proposer_index, - parent_root, - Some(valid_attestations), - None, - None, - )?; + let (block, _post_state, _collected_atts, sigs) = parent_state + .build_block( + slot, + proposer_index, + parent_root, + Some(valid_attestations), + None, + None, + ) + .context("Failed to build block")?; // Collect signatures from the attestations we included let mut signatures = sigs; for signed_att in &valid_signed_attestations { signatures .push(signed_att.signature.clone()) - .map_err(|e| format!("Failed to add attestation signature: {:?}", e))?; + .context("Failed to add attestation signature")?; } info!( @@ -233,11 +236,11 @@ impl ValidatorService { Ok(sig) => { signatures .push(sig) - .map_err(|e| format!("Failed to add proposer signature: {:?}", e))?; + .context("Failed to add proposer signature")?; info!(proposer = proposer_index.0, "Signed proposer attestation"); } Err(e) => { - return Err(format!("Failed to sign proposer attestation: {}", e)); + bail!("Failed to sign proposer attestation") } } } else { From c1cf371cafbbcac67a24ca8e166a89b9d1ce8a22 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 17:38:18 +0200 Subject: [PATCH 2/7] handle errors --- lean_client/containers/src/state.rs | 23 +++++++++++-------- .../containers/tests/test_vectors/runner.rs | 2 +- .../containers/tests/unit_tests/common.rs | 11 +++++---- .../tests/unit_tests/state_justifications.rs | 13 +++++++---- .../tests/unit_tests/state_process.rs | 10 +++++--- .../tests/unit_tests/state_transition.rs | 8 +++++-- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/lean_client/containers/src/state.rs b/lean_client/containers/src/state.rs index 6381eef..16a9767 100644 --- a/lean_client/containers/src/state.rs +++ b/lean_client/containers/src/state.rs @@ -166,7 +166,7 @@ impl State { .collect() } - pub fn with_justifications(mut self, map: BTreeMap>) -> Self { + pub fn with_justifications(mut self, map: BTreeMap>) -> Result { // Use actual validator count, matching leanSpec let num_validators = self.validators.len_usize(); let mut roots: Vec<_> = map.keys().cloned().collect(); @@ -175,7 +175,7 @@ impl State { // Build PersistentList by pushing elements let mut new_roots = JustificationRoots::default(); for r in &roots { - new_roots.push(*r).expect("within limit"); + new_roots.push(*r).context("within limit")?; } // Build BitList: create with length, then set bits @@ -200,16 +200,16 @@ impl State { self.justifications_roots = new_roots; self.justifications_validators = new_validators; - self + Ok(self) } - pub fn with_historical_hashes(mut self, hashes: Vec) -> Self { + pub fn with_historical_hashes(mut self, hashes: Vec) -> Result { let mut new_hashes = HistoricalBlockHashes::default(); for h in hashes { - new_hashes.push(h).expect("within limit"); + new_hashes.push(h).context("within limit")?; } self.historical_block_hashes = new_hashes; - self + Ok(self) } // updated for fork choice tests @@ -280,7 +280,7 @@ impl State { // State root validation is handled by state_transition_with_validation when needed - Ok(state_after_ops) + state_after_ops } pub fn process_block_header(&self, block: &Block) -> Result { @@ -371,7 +371,7 @@ impl State { }) } - pub fn process_attestations(&self, attestations: &Attestations) -> Self { + pub fn process_attestations(&self, attestations: &Attestations) -> Result { let mut justifications = self.get_justifications(); let mut latest_justified = self.latest_justified.clone(); let mut latest_finalized = self.latest_finalized.clone(); @@ -489,7 +489,10 @@ impl State { } } - let mut new_state = self.clone().with_justifications(justifications); + let mut new_state = self + .clone() + .with_justifications(justifications) + .context("Failed to update justifications")?; new_state.latest_justified = latest_justified; new_state.latest_finalized = latest_finalized; @@ -501,7 +504,7 @@ impl State { } new_state.justified_slots = new_justified_slots; - new_state + Ok(new_state) } /// Build a valid block on top of this state. diff --git a/lean_client/containers/tests/test_vectors/runner.rs b/lean_client/containers/tests/test_vectors/runner.rs index e5b54a7..8fc52c1 100644 --- a/lean_client/containers/tests/test_vectors/runner.rs +++ b/lean_client/containers/tests/test_vectors/runner.rs @@ -85,7 +85,7 @@ impl TestRunner { // Only check validator count if specified in post-state if let Some(expected_count) = post.validator_count { - let mut num_validators: u64 = state.validators.len_u64(); + let num_validators: u64 = state.validators.len_u64(); ensure!( num_validators as usize == expected_count, diff --git a/lean_client/containers/tests/unit_tests/common.rs b/lean_client/containers/tests/unit_tests/common.rs index 1535732..88cf45a 100644 --- a/lean_client/containers/tests/unit_tests/common.rs +++ b/lean_client/containers/tests/unit_tests/common.rs @@ -1,3 +1,4 @@ +use anyhow::{Context, Result}; use containers::{ block::{hash_tree_root, Block, BlockBody, BlockHeader}, checkpoint::Checkpoint, @@ -69,11 +70,11 @@ pub fn sample_checkpoint() -> Checkpoint { } } -pub fn base_state(config: Config) -> State { +pub fn base_state(config: Config) -> Result { base_state_with_validators(config, TEST_VALIDATOR_COUNT) } -pub fn base_state_with_validators(config: Config, num_validators: usize) -> State { +pub fn base_state_with_validators(config: Config, num_validators: usize) -> Result { use containers::{ validator::Validator, HistoricalBlockHashes, JustificationRoots, JustificationsValidators, JustifiedSlots, Uint64, @@ -86,10 +87,10 @@ pub fn base_state_with_validators(config: Config, num_validators: usize) -> Stat pubkey: Default::default(), index: Uint64(i as u64), }; - validators.push(validator).expect("within limit"); + validators.push(validator).context("within limit")?; } - State { + Ok(State { config, slot: Slot(0), latest_block_header: sample_block_header(), @@ -100,7 +101,7 @@ pub fn base_state_with_validators(config: Config, num_validators: usize) -> Stat validators, justifications_roots: JustificationRoots::default(), justifications_validators: JustificationsValidators::default(), - } + }) } pub fn sample_config() -> Config { diff --git a/lean_client/containers/tests/unit_tests/state_justifications.rs b/lean_client/containers/tests/unit_tests/state_justifications.rs index afdd220..92416a6 100644 --- a/lean_client/containers/tests/unit_tests/state_justifications.rs +++ b/lean_client/containers/tests/unit_tests/state_justifications.rs @@ -15,7 +15,7 @@ fn config() -> Config { #[fixture] fn state(config: Config) -> State { - base_state(config) + base_state(config).unwrap() } #[test] @@ -104,7 +104,7 @@ fn test_get_justifications_multiple_roots() { #[test] fn test_with_justifications_empty() { let config = sample_config(); - let mut initial_state = base_state(config.clone()); + let mut initial_state = base_state(config.clone()).unwrap(); let mut roots_list = List::default(); roots_list @@ -120,7 +120,8 @@ fn test_with_justifications_empty() { let new_state = initial_state .clone() - .with_justifications(std::collections::BTreeMap::new()); + .with_justifications(std::collections::BTreeMap::new()) + .unwrap(); assert!(new_state.justifications_roots.get(0).is_err()); assert!(new_state.justifications_validators.get(0).is_none()); @@ -142,7 +143,7 @@ fn test_with_justifications_deterministic_order() { justifications.insert(root2, votes2.clone()); justifications.insert(root1, votes1.clone()); - let new_state = state.with_justifications(justifications); + let new_state = state.with_justifications(justifications).unwrap(); // Expected roots in sorted order (root1 < root2) assert_eq!(new_state.justifications_roots.get(0).ok(), Some(&root1)); @@ -204,7 +205,9 @@ fn test_justifications_roundtrip( ) { let state = state(sample_config()); - let new_state = state.with_justifications(justifications_map.clone()); + let new_state = state + .with_justifications(justifications_map.clone()) + .unwrap(); let reconstructed_map = new_state.get_justifications(); let expected_map = justifications_map; diff --git a/lean_client/containers/tests/unit_tests/state_process.rs b/lean_client/containers/tests/unit_tests/state_process.rs index 976a8cf..0dcbc74 100644 --- a/lean_client/containers/tests/unit_tests/state_process.rs +++ b/lean_client/containers/tests/unit_tests/state_process.rs @@ -141,7 +141,9 @@ fn test_process_attestations_justification_and_finalization() { let state_after_header1 = state_at_slot_1 .process_block_header(&block1.message.block) .unwrap(); - state = state_after_header1.process_attestations(&block1.message.block.body.attestations); + state = state_after_header1 + .process_attestations(&block1.message.block.body.attestations) + .unwrap(); // Process slot 4 and block let mut state_at_slot_4 = state.process_slots(Slot(4)).unwrap(); @@ -149,7 +151,9 @@ fn test_process_attestations_justification_and_finalization() { let state_after_header4 = state_at_slot_4 .process_block_header(&block4.message.block) .unwrap(); - state = state_after_header4.process_attestations(&block4.message.block.body.attestations); + state = state_after_header4 + .process_attestations(&block4.message.block.body.attestations) + .unwrap(); // Advance to slot 5 state = state.process_slots(Slot(5)).unwrap(); @@ -182,7 +186,7 @@ fn test_process_attestations_justification_and_finalization() { attestations_list.push(a).unwrap(); } - let new_state = state.process_attestations(&attestations_list); + let new_state = state.process_attestations(&attestations_list).unwrap(); assert_eq!(new_state.latest_justified, checkpoint4); let justified_slot_4 = new_state diff --git a/lean_client/containers/tests/unit_tests/state_transition.rs b/lean_client/containers/tests/unit_tests/state_transition.rs index fe64273..04eb096 100644 --- a/lean_client/containers/tests/unit_tests/state_transition.rs +++ b/lean_client/containers/tests/unit_tests/state_transition.rs @@ -29,7 +29,9 @@ fn test_state_transition_full() { // Use process_block_header + process_operations to avoid state root validation during setup let state_after_header = state_at_slot_1.process_block_header(&block).unwrap(); - let expected_state = state_after_header.process_attestations(&block.body.attestations); + let expected_state = state_after_header + .process_attestations(&block.body.attestations) + .unwrap(); let block_with_correct_root = Block { state_root: hash_tree_root(&expected_state), @@ -62,7 +64,9 @@ fn test_state_transition_invalid_signatures() { // Use process_block_header + process_operations to avoid state root validation during setup let state_after_header = state_at_slot_1.process_block_header(&block).unwrap(); - let expected_state = state_after_header.process_attestations(&block.body.attestations); + let expected_state = state_after_header + .process_attestations(&block.body.attestations) + .unwrap(); let block_with_correct_root = Block { state_root: hash_tree_root(&expected_state), From 8cf8ae9c2d25ec8ce2b3a52f5ad0235933a68fa6 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 18:53:11 +0200 Subject: [PATCH 3/7] remove redundant anyhow:: --- lean_client/networking/src/enr_ext.rs | 2 +- lean_client/validator/src/keys.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lean_client/networking/src/enr_ext.rs b/lean_client/networking/src/enr_ext.rs index 4932039..c3dfb17 100644 --- a/lean_client/networking/src/enr_ext.rs +++ b/lean_client/networking/src/enr_ext.rs @@ -315,7 +315,7 @@ impl CombinedKeyExt for CombinedKey { // helper function to convert a peer_id to a node_id. This is only possible for secp256k1/ed25519 libp2p // peer_ids -pub fn peer_id_to_node_id(peer_id: &PeerId) -> anyhow::Result { +pub fn peer_id_to_node_id(peer_id: &PeerId) -> Result { // A libp2p peer id byte representation should be 2 length bytes + 4 protobuf bytes + compressed pk bytes // if generated from a PublicKey with Identity multihash. let pk_bytes = &peer_id.to_bytes()[2..]; diff --git a/lean_client/validator/src/keys.rs b/lean_client/validator/src/keys.rs index 6d39a6a..424006d 100644 --- a/lean_client/validator/src/keys.rs +++ b/lean_client/validator/src/keys.rs @@ -1,4 +1,4 @@ -use anyhow::{ensure, Context, Result}; +use anyhow::{anyhow, ensure, Context, Result}; use containers::attestation::U3112; use containers::ssz::ByteVector; use containers::Signature; @@ -77,11 +77,11 @@ impl KeyManager { ::SecretKey; let secret_key = SecretKey::from_bytes(key_bytes) - .map_err(|e| anyhow::anyhow!("Failed to deserialize secret key: {:?}", e))?; + .map_err(|e| anyhow!("Failed to deserialize secret key: {:?}", e))?; let leansig_signature = SIGTopLevelTargetSumLifetime32Dim64Base8::sign(&secret_key, epoch, message) - .map_err(|e| anyhow::anyhow!("Failed to sign message: {:?}", e))?; + .map_err(|e| anyhow!("Failed to sign message: {:?}", e))?; let sig_bytes = leansig_signature.to_bytes(); From 53fa3e178520aeb313f34ad2d59f7600e8992756 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 19:35:36 +0200 Subject: [PATCH 4/7] handle errors --- lean_client/containers/src/block.rs | 9 ++------- lean_client/containers/src/serde_helpers.rs | 8 ++++---- lean_client/fork_choice/src/store.rs | 5 +---- .../tests/fork_choice_test_vectors.rs | 4 ++-- lean_client/src/main.rs | 18 ++++++++---------- 5 files changed, 17 insertions(+), 27 deletions(-) diff --git a/lean_client/containers/src/block.rs b/lean_client/containers/src/block.rs index 464783d..565f363 100644 --- a/lean_client/containers/src/block.rs +++ b/lean_client/containers/src/block.rs @@ -1,9 +1,10 @@ -use anyhow::{bail, ensure, Context, Result}; use crate::{ Attestation, Attestations, BlockSignatures, Bytes32, Signature, Slot, State, ValidatorIndex, }; +use anyhow::{bail, ensure, Context, Result}; use serde::{Deserialize, Serialize}; use ssz_derive::Ssz; + #[cfg(feature = "xmss-verify")] use leansig::signature::generalized_xmss::instantiations_poseidon::lifetime_2_to_the_20::target_sum::SIGTargetSumLifetime20W2NoOff; @@ -167,12 +168,6 @@ impl SignedBlockWithAttestation { // Verify each attestation signature for (attestation, signature) in all_attestations.iter().zip(signatures_vec.iter()) { - // Ensure validator exists in the active set - ensure!( - attestation.validator_id.0 < num_validators, - "Validator index out of range" - ); - let validator = validators .get(attestation.validator_id.0) .with_context(|| { diff --git a/lean_client/containers/src/serde_helpers.rs b/lean_client/containers/src/serde_helpers.rs index da2fb6d..9d2e75e 100644 --- a/lean_client/containers/src/serde_helpers.rs +++ b/lean_client/containers/src/serde_helpers.rs @@ -1,6 +1,7 @@ // Serde helpers for handling test vector JSON format // Test vectors wrap SSZ collections in {"data": [...]} objects -use anyhow::Result; + +use anyhow::{Context, Result}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Wrapper for deserializing {"data": T} format @@ -188,7 +189,6 @@ pub mod signature { pub mod block_signatures { use super::*; use crate::{BlockSignatures, Signature}; - use anyhow::Context; use serde_json::Value; use ssz::PersistentList; @@ -209,14 +209,14 @@ pub mod block_signatures { // Check if it's a hex string (normal format) if let Value::String(hex_str) = value { let hex_str = hex_str.trim_start_matches("0x"); - let bytes = hex::decode(hex_str).context("Invalid hex string: {}")?; + let bytes = hex::decode(hex_str).context("Invalid hex string")?; return Signature::try_from(bytes.as_slice()).context("Invalid signature length"); } // Otherwise, parse as structured XMSS signature let xmss_sig: XmssSignature = - serde_json::from_value(value.clone()).context("Failed to parse XMSS signature: {}")?; + serde_json::from_value(value.clone()).context("Failed to parse XMSS signature")?; // Serialize the XMSS signature to bytes // Format: siblings (variable length) + rho (28 bytes) + hashes (variable length) diff --git a/lean_client/fork_choice/src/store.rs b/lean_client/fork_choice/src/store.rs index 7aa871d..bb54574 100644 --- a/lean_client/fork_choice/src/store.rs +++ b/lean_client/fork_choice/src/store.rs @@ -77,10 +77,7 @@ pub fn get_fork_choice_head( .iter() .min_by_key(|(_, block)| block.message.block.slot) .map(|(r, _)| *r) - .unwrap_or_else(|| { - // Should never happen - genesis block should always be present - store.head - }); + .expect("Error: Empty block."); } let mut vote_weights: HashMap = HashMap::new(); let root_slot = store.blocks[&root].message.block.slot; diff --git a/lean_client/fork_choice/tests/fork_choice_test_vectors.rs b/lean_client/fork_choice/tests/fork_choice_test_vectors.rs index c713039..84df6fc 100644 --- a/lean_client/fork_choice/tests/fork_choice_test_vectors.rs +++ b/lean_client/fork_choice/tests/fork_choice_test_vectors.rs @@ -669,7 +669,7 @@ fn test_fork_choice_head_vectors() { .file_name() .and_then(|n| n.to_str()) .unwrap_or("unknown"); - println!("\nTest file: {}", file_name); + println!("\nTest file: {:?}", file_name); let path_str = path.to_str().expect("Path contains invalid UTF-8"); @@ -679,7 +679,7 @@ fn test_fork_choice_head_vectors() { pass_count += 1; } Err(e) => { - println!(" ✗ FAILED: {:#}", e); + println!(" ✗ FAILED: {}", e); fail_count += 1; } } diff --git a/lean_client/src/main.rs b/lean_client/src/main.rs index 7c1d57d..67f65ed 100644 --- a/lean_client/src/main.rs +++ b/lean_client/src/main.rs @@ -150,23 +150,21 @@ async fn main() -> Result<()> { let (genesis_time, validators) = if let Some(genesis_path) = &args.genesis { let genesis_config = containers::GenesisConfig::load_from_file(genesis_path) - .context("Failed to load genesis config")?; + .expect("Failed to load genesis config"); let validators: Vec = genesis_config .genesis_validators .iter() .enumerate() .map(|(i, v_str)| { - let pubkey = - containers::validator::BlsPublicKey::from_hex(v_str).with_context(|| { - format!("Invalid genesis validator pubkey at index {}: {}", i, v_str) - })?; - Ok(containers::validator::Validator { + let pubkey = containers::validator::BlsPublicKey::from_hex(v_str) + .expect("Invalid genesis validator pubkey"); + containers::validator::Validator { pubkey, index: Uint64(i as u64), - }) + } }) - .collect::>>()?; + .collect(); (genesis_config.genesis_time, validators) } else { @@ -310,7 +308,7 @@ async fn main() -> Result<()> { keypair, ) .await - .context("Failed to create network service with custom key")? + .expect("Failed to create network service with custom key") } Err(e) => { warn!("Failed to load node key: {}, using random key", e); @@ -321,7 +319,7 @@ async fn main() -> Result<()> { peer_count, ) .await - .context("Failed to create network service")? + .expect("Failed to create network service") } } } else { From 150cde13b2a10f662c6cb690badc516186fc3b34 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 19:39:53 +0200 Subject: [PATCH 5/7] handle errors --- lean_client/fork_choice/src/handlers.rs | 15 +++++---------- lean_client/src/main.rs | 4 ++-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/lean_client/fork_choice/src/handlers.rs b/lean_client/fork_choice/src/handlers.rs index 27d3b66..4878f58 100644 --- a/lean_client/fork_choice/src/handlers.rs +++ b/lean_client/fork_choice/src/handlers.rs @@ -1,5 +1,5 @@ use crate::store::*; -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{ensure, Context, Result}; use containers::{ attestation::SignedAttestation, block::SignedBlockWithAttestation, Bytes32, ValidatorIndex, }; @@ -8,11 +8,8 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum BlockTransitionError { - #[error("Block queued: parent not yet available (pending: {pending_count} blocks)")] - BlockQueued { pending_count: usize }, - - #[error("No parent state found for block")] - NoParentState, + #[error("Block queued: parent not yet available")] + BlockQueued, #[error(transparent)] StateTransition(#[from] anyhow::Error), @@ -116,9 +113,7 @@ pub fn on_block( .entry(parent_root) .or_insert_with(Vec::new) .push(signed_block); - return Err(BlockTransitionError::BlockQueued { - pending_count: store.blocks_queue.values().map(|v| v.len()).sum(), - }); + return Err(BlockTransitionError::BlockQueued); } process_block_internal(store, signed_block, block_root)?; @@ -138,7 +133,7 @@ fn process_block_internal( let state = store .states .get(&block.parent_root) - .ok_or(BlockTransitionError::NoParentState)?; + .context("No parent state found")?; // Execute state transition to get post-state let new_state = state diff --git a/lean_client/src/main.rs b/lean_client/src/main.rs index 67f65ed..dd700c7 100644 --- a/lean_client/src/main.rs +++ b/lean_client/src/main.rs @@ -330,7 +330,7 @@ async fn main() -> Result<()> { peer_count, ) .await - .context("Failed to create network service")? + .expect("Failed to create network service") }; let network_handle = task::spawn(async move { network_service.start().await }); @@ -498,7 +498,7 @@ async fn main() -> Result<()> { } } } - Err(BlockTransitionError::BlockQueued { pending_count: _ }) => { + Err(BlockTransitionError::BlockQueued) => { debug!("Block queued, requesting missing parent"); // Request missing parent block from peers From 825686b19b958def02b0fa3ebbb6c79f755c0124 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 20:01:31 +0200 Subject: [PATCH 6/7] more idiomatic --- lean_client/containers/src/block.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lean_client/containers/src/block.rs b/lean_client/containers/src/block.rs index 565f363..a0357f9 100644 --- a/lean_client/containers/src/block.rs +++ b/lean_client/containers/src/block.rs @@ -1,7 +1,7 @@ use crate::{ Attestation, Attestations, BlockSignatures, Bytes32, Signature, Slot, State, ValidatorIndex, }; -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; use serde::{Deserialize, Serialize}; use ssz_derive::Ssz; @@ -198,28 +198,26 @@ impl SignedBlockWithAttestation { // Deserialize the public key using Serializable trait type PubKey = ::PublicKey; - let pubkey = match PubKey::from_bytes(pubkey_bytes) { - Ok(pk) => pk, - Err(e) => bail!( + let pubkey = PubKey::from_bytes(pubkey_bytes).map_err(|e| { + anyhow!( "Failed to deserialize public key at slot {:?}: {:?}", attestation.data.slot, e - ), - }; + ) + })?; // Get signature bytes - use as_bytes() method let sig_bytes = signature.as_bytes(); // Deserialize the signature using Serializable trait type Sig = ::Signature; - let sig = match Sig::from_bytes(sig_bytes) { - Ok(s) => s, - Err(e) => bail!( + let sig = Sig::from_bytes(sig_bytes).map_err(|e| { + anyhow!( "Failed to deserialize signature at slot {:?}: {:?}", attestation.data.slot, e - ), - }; + ) + })?; // Verify the signature ensure!( From 5f5254109ec6985ff141ebd99a3e300d311c0fd1 Mon Sep 17 00:00:00 2001 From: Dariusspr <108625236+Dariusspr@users.noreply.github.com> Date: Thu, 22 Jan 2026 20:16:54 +0200 Subject: [PATCH 7/7] revert panics in tests --- lean_client/containers/tests/test_vectors/runner.rs | 12 ++++++------ lean_client/containers/tests/unit_tests/common.rs | 11 +++++------ .../tests/unit_tests/state_justifications.rs | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lean_client/containers/tests/test_vectors/runner.rs b/lean_client/containers/tests/test_vectors/runner.rs index 8fc52c1..6bb8d2a 100644 --- a/lean_client/containers/tests/test_vectors/runner.rs +++ b/lean_client/containers/tests/test_vectors/runner.rs @@ -18,7 +18,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -120,7 +120,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -215,7 +215,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("Running test: {}", test_name); println!("Description: {}", test_case.info.description); @@ -316,7 +316,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("\n{}: {}", test_name, test_case.info.description); @@ -427,7 +427,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("\n{}: {}", test_name, test_case.info.description); @@ -584,7 +584,7 @@ impl TestRunner { .tests .into_iter() .next() - .context("No test case found in JSON")?; + .expect("No test case found in JSON"); println!("\n{}: {}", test_name, test_case.info.description); diff --git a/lean_client/containers/tests/unit_tests/common.rs b/lean_client/containers/tests/unit_tests/common.rs index 88cf45a..1535732 100644 --- a/lean_client/containers/tests/unit_tests/common.rs +++ b/lean_client/containers/tests/unit_tests/common.rs @@ -1,4 +1,3 @@ -use anyhow::{Context, Result}; use containers::{ block::{hash_tree_root, Block, BlockBody, BlockHeader}, checkpoint::Checkpoint, @@ -70,11 +69,11 @@ pub fn sample_checkpoint() -> Checkpoint { } } -pub fn base_state(config: Config) -> Result { +pub fn base_state(config: Config) -> State { base_state_with_validators(config, TEST_VALIDATOR_COUNT) } -pub fn base_state_with_validators(config: Config, num_validators: usize) -> Result { +pub fn base_state_with_validators(config: Config, num_validators: usize) -> State { use containers::{ validator::Validator, HistoricalBlockHashes, JustificationRoots, JustificationsValidators, JustifiedSlots, Uint64, @@ -87,10 +86,10 @@ pub fn base_state_with_validators(config: Config, num_validators: usize) -> Resu pubkey: Default::default(), index: Uint64(i as u64), }; - validators.push(validator).context("within limit")?; + validators.push(validator).expect("within limit"); } - Ok(State { + State { config, slot: Slot(0), latest_block_header: sample_block_header(), @@ -101,7 +100,7 @@ pub fn base_state_with_validators(config: Config, num_validators: usize) -> Resu validators, justifications_roots: JustificationRoots::default(), justifications_validators: JustificationsValidators::default(), - }) + } } pub fn sample_config() -> Config { diff --git a/lean_client/containers/tests/unit_tests/state_justifications.rs b/lean_client/containers/tests/unit_tests/state_justifications.rs index 92416a6..a3e07ff 100644 --- a/lean_client/containers/tests/unit_tests/state_justifications.rs +++ b/lean_client/containers/tests/unit_tests/state_justifications.rs @@ -15,7 +15,7 @@ fn config() -> Config { #[fixture] fn state(config: Config) -> State { - base_state(config).unwrap() + base_state(config) } #[test] @@ -104,7 +104,7 @@ fn test_get_justifications_multiple_roots() { #[test] fn test_with_justifications_empty() { let config = sample_config(); - let mut initial_state = base_state(config.clone()).unwrap(); + let mut initial_state = base_state(config.clone()); let mut roots_list = List::default(); roots_list