From ffc5e5ed36d8969a5d60da097ee99a91815a6620 Mon Sep 17 00:00:00 2001 From: gd-0 <90608901+gd-0@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:04:01 +0700 Subject: [PATCH 1/4] introduce max_block_value_to_verify in constraints config --- crates/api/src/builder/api.rs | 95 ++++++++++++++++++++------------ crates/api/src/proposer/api.rs | 25 +++++++-- crates/api/src/proposer/tests.rs | 2 +- crates/api/src/service.rs | 2 +- crates/api/src/test_utils.rs | 4 +- crates/common/src/config.rs | 4 ++ 6 files changed, 87 insertions(+), 45 deletions(-) diff --git a/crates/api/src/builder/api.rs b/crates/api/src/builder/api.rs index 65de884..0477cb5 100644 --- a/crates/api/src/builder/api.rs +++ b/crates/api/src/builder/api.rs @@ -679,42 +679,10 @@ where .await?; // Fetch constraints, and if available verify inclusion proofs and save them to cache - if let Some(constraints) = api.auctioneer.get_constraints(payload.slot()).await? { - let transactions_root: B256 = payload - .transactions() - .clone() - .hash_tree_root()? - .to_vec() - .as_slice() - .try_into() - .map_err(|e| { - error!(error = %e, "failed to convert root to hash32"); - BuilderApiError::InternalError - })?; - let proofs = payload.proofs().ok_or(BuilderApiError::InclusionProofsNotFound)?; - let constraints_proofs: Vec<_> = constraints.iter().map(|c| &c.proof_data).collect(); - - verify_multiproofs(constraints_proofs.as_slice(), proofs, transactions_root).map_err( - |e| { - error!(error = %e, "failed to verify inclusion proofs"); - BuilderApiError::InclusionProofVerificationFailed(e) - }, - )?; - - // Save inclusion proof to auctioneer. - api.save_inclusion_proof( - payload.slot(), - payload.proposer_public_key(), - payload.block_hash(), - proofs, - &request_id, - ) - .await?; - - info!(request_id = %request_id, head_slot, "inclusion proofs verified and saved to auctioneer"); - } else { - info!(request_id = %request_id, "no constraints found for slot, proof verification is not needed"); - }; + if let Err(err) = api.verify_and_save_inclusion_proofs(&payload, &request_id).await { + warn!(request_id = %request_id, error = %err, "failed to verify and save inclusion proofs"); + return Err(err) + } // If cancellations are enabled, then abort now if there is a later submission if is_cancellations_enabled { @@ -2197,6 +2165,61 @@ where ); } } + + /// Fetch constraints, and if available verify inclusion proofs and save them to cache. + async fn verify_and_save_inclusion_proofs( + api: &BuilderApi, + payload: &SignedBidSubmissionDeneb, + request_id: Uuid, + ) -> Result<(), BuilderApiError> { + if let Some(max_block_value_to_verify) = api.relay_config.constraints_api_config.max_block_value_to_verify { + if payload.value() > max_block_value_to_verify { + info!( + request_id = %request_id, + block_value = %payload.value(), + "block value is greater than max value to verify, skipping inclusion proof verification", + ); + return Ok(()); + } + } + + if let Some(constraints) = api.auctioneer.get_constraints(payload.slot()).await? { + let transactions_root: B256 = payload + .transactions() + .clone() + .hash_tree_root()? + .to_vec() + .as_slice() + .try_into() + .map_err(|e| { + error!(error = %e, "failed to convert root to hash32"); + BuilderApiError::InternalError + })?; + let proofs = payload.proofs().ok_or(BuilderApiError::InclusionProofsNotFound)?; + let constraints_proofs: Vec<_> = constraints.iter().map(|c| &c.proof_data).collect(); + + verify_multiproofs(constraints_proofs.as_slice(), proofs, transactions_root).map_err( + |e| { + error!(error = %e, "failed to verify inclusion proofs"); + BuilderApiError::InclusionProofVerificationFailed(e) + }, + )?; + + // Save inclusion proof to auctioneer. + api.save_inclusion_proof( + payload.slot(), + payload.proposer_public_key(), + payload.block_hash(), + proofs, + &request_id, + ) + .await?; + info!(request_id = %request_id, head_slot, "inclusion proofs verified and saved to auctioneer"); + } else { + info!(request_id = %request_id, "no constraints found for slot, proof verification is not needed"); + }; + Ok(()) + } } // STATE SYNC diff --git a/crates/api/src/proposer/api.rs b/crates/api/src/proposer/api.rs index f2ebb9c..a6194e5 100644 --- a/crates/api/src/proposer/api.rs +++ b/crates/api/src/proposer/api.rs @@ -46,7 +46,7 @@ use helix_common::{ try_execution_header_from_payload, versioned_payload::PayloadAndBlobs, BidRequest, Filtering, GetHeaderTrace, GetPayloadTrace, RegisterValidatorsTrace, - ValidatorPreferences, + ValidatorPreferences, RelayConfig, }; use helix_database::DatabaseService; use helix_datastore::{error::AuctioneerError, Auctioneer}; @@ -88,7 +88,7 @@ where chain_info: Arc, validator_preferences: Arc, - target_get_payload_propagation_duration_ms: u64, + relay_config: Arc, } impl ProposerApi @@ -107,8 +107,8 @@ where chain_info: Arc, slot_update_subscription: Sender>, validator_preferences: Arc, - target_get_payload_propagation_duration_ms: u64, gossip_receiver: Receiver, + relay_config: Arc, ) -> Self { let api = Self { auctioneer, @@ -119,7 +119,7 @@ where curr_slot_info: Arc::new(RwLock::new((0, None))), chain_info, validator_preferences, - target_get_payload_propagation_duration_ms, + relay_config, }; // Spin up gossip processing task @@ -571,7 +571,21 @@ where let constraints = proposer_api.auctioneer.get_constraints(slot).await?.unwrap_or_default(); - if !constraints.is_empty() { + let value_above_max_to_verify = proposer_api + .relay_config + .constraints_api_config + .max_block_value_to_verify + .map_or(false, |max| bid.value() > max); + if value_above_max_to_verify { + info!( + %request_id, + slot, + value = ?bid.value(), + "block value is greater than max value to verify, skipping inclusion proof check in get_header_with_proofs", + ); + } + + if !constraints.is_empty() && !value_above_max_to_verify { error!( request_id = %request_id, slot, @@ -937,6 +951,7 @@ where let elapsed_since_propagate_start_ms = (get_nanos_timestamp()?.saturating_sub(trace.beacon_client_broadcast)) / 1_000_000; let remaining_sleep_ms = self + .relay_config .target_get_payload_propagation_duration_ms .saturating_sub(elapsed_since_propagate_start_ms); if remaining_sleep_ms > 0 { diff --git a/crates/api/src/proposer/tests.rs b/crates/api/src/proposer/tests.rs index 15041ff..ef61bf3 100644 --- a/crates/api/src/proposer/tests.rs +++ b/crates/api/src/proposer/tests.rs @@ -997,8 +997,8 @@ mod proposer_api_tests { Arc::new(ChainInfo::for_holesky()), slot_update_sender.clone(), Arc::new(ValidatorPreferences::default()), - 0, gossip_receiver, + Arc::new(Default::default()), ); let mut x = gen_signed_vr(); diff --git a/crates/api/src/service.rs b/crates/api/src/service.rs index 085b249..39b84f9 100644 --- a/crates/api/src/service.rs +++ b/crates/api/src/service.rs @@ -182,8 +182,8 @@ impl ApiService { chain_info.clone(), slot_update_sender.clone(), validator_preferences.clone(), - config.target_get_payload_propagation_duration_ms, proposer_gossip_receiver, + config.clone(), )); let data_api = Arc::new(DataApiProd::new(validator_preferences.clone(), db.clone())); diff --git a/crates/api/src/test_utils.rs b/crates/api/src/test_utils.rs index 6012121..0e9107d 100644 --- a/crates/api/src/test_utils.rs +++ b/crates/api/src/test_utils.rs @@ -56,8 +56,8 @@ pub fn app() -> Router { Arc::new(ChainInfo::for_mainnet()), slot_update_sender, Arc::new(ValidatorPreferences::default()), - 0, gossip_receiver, + Arc::new(Default::default()), )); let data_api = Arc::new(DataApi::::new( @@ -218,8 +218,8 @@ pub fn proposer_api_app() -> ( Arc::new(ChainInfo::for_mainnet()), slot_update_sender.clone(), Arc::new(ValidatorPreferences::default()), - 0, gossip_receiver, + Arc::new(Default::default()), )); let router = Router::new() diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index a7a5434..27cfcc5 100644 --- a/crates/common/src/config.rs +++ b/crates/common/src/config.rs @@ -136,6 +136,10 @@ pub struct ConstraintsApiConfig { /// [`/constraints/v1/builder/constraints`](https://docs.boltprotocol.xyz/technical-docs/api/builder#constraints) /// endpoint will not be checked. pub check_constraints_signature: bool, + /// Only verify and save inclusion proofs if the block value is less than this threshold. + /// We do this to ensure that high value blocks are not rejected. + #[serde(default)] + pub max_block_value_to_verify: Option, } impl Default for ConstraintsApiConfig { From 61d611f0154e33175134f986e12c54af1d36d7aa Mon Sep 17 00:00:00 2001 From: gd-0 <90608901+gd-0@users.noreply.github.com> Date: Mon, 11 Nov 2024 19:54:59 +0700 Subject: [PATCH 2/4] address PR comments + other fixes - fix lints + other refactoring - max_block_value_to_verify_wei now U256 to handle big value cases - return earlier in get_header_with_proofs call --- crates/api/src/builder/api.rs | 52 ++++++++++++++++---------------- crates/api/src/proposer/api.rs | 55 ++++++++++++++++++---------------- crates/common/src/config.rs | 13 +++++--- 3 files changed, 65 insertions(+), 55 deletions(-) diff --git a/crates/api/src/builder/api.rs b/crates/api/src/builder/api.rs index 0477cb5..5a4255a 100644 --- a/crates/api/src/builder/api.rs +++ b/crates/api/src/builder/api.rs @@ -679,9 +679,22 @@ where .await?; // Fetch constraints, and if available verify inclusion proofs and save them to cache - if let Err(err) = api.verify_and_save_inclusion_proofs(&payload, &request_id).await { - warn!(request_id = %request_id, error = %err, "failed to verify and save inclusion proofs"); - return Err(err) + let skip_inclusion_proof_verify_and_save = api + .relay_config + .constraints_api_config + .max_block_value_to_verify_wei + .map_or(false, |max_block_value_to_verify| payload.value() > max_block_value_to_verify); + if !skip_inclusion_proof_verify_and_save { + if let Err(err) = api.verify_and_save_inclusion_proofs(&payload, &request_id).await { + warn!(request_id = %request_id, error = %err, "failed to verify and save inclusion proofs"); + return Err(err) + } + } else { + info!( + request_id = %request_id, + block_value = %payload.value(), + "block value is greater than max value to verify, inclusion proof verification and saving is skipped", + ); } // If cancellations are enabled, then abort now if there is a later submission @@ -2168,22 +2181,11 @@ where /// Fetch constraints, and if available verify inclusion proofs and save them to cache. async fn verify_and_save_inclusion_proofs( - api: &BuilderApi, - payload: &SignedBidSubmissionDeneb, - request_id: Uuid, + &self, + payload: &SignedBidSubmission, + request_id: &Uuid, ) -> Result<(), BuilderApiError> { - if let Some(max_block_value_to_verify) = api.relay_config.constraints_api_config.max_block_value_to_verify { - if payload.value() > max_block_value_to_verify { - info!( - request_id = %request_id, - block_value = %payload.value(), - "block value is greater than max value to verify, skipping inclusion proof verification", - ); - return Ok(()); - } - } - - if let Some(constraints) = api.auctioneer.get_constraints(payload.slot()).await? { + if let Some(constraints) = self.auctioneer.get_constraints(payload.slot()).await? { let transactions_root: B256 = payload .transactions() .clone() @@ -2191,8 +2193,8 @@ where .to_vec() .as_slice() .try_into() - .map_err(|e| { - error!(error = %e, "failed to convert root to hash32"); + .map_err(|error| { + error!(?error, "failed to convert root to hash32"); BuilderApiError::InternalError })?; let proofs = payload.proofs().ok_or(BuilderApiError::InclusionProofsNotFound)?; @@ -2206,20 +2208,20 @@ where )?; // Save inclusion proof to auctioneer. - api.save_inclusion_proof( + self.save_inclusion_proof( payload.slot(), payload.proposer_public_key(), payload.block_hash(), proofs, - &request_id, + request_id, ) .await?; - info!(request_id = %request_id, head_slot, "inclusion proofs verified and saved to auctioneer"); + info!(%request_id, "inclusion proofs verified and saved to auctioneer"); } else { - info!(request_id = %request_id, "no constraints found for slot, proof verification is not needed"); + info!(%request_id, "no constraints found for slot, proof verification is not needed"); }; Ok(()) - } + } } // STATE SYNC diff --git a/crates/api/src/proposer/api.rs b/crates/api/src/proposer/api.rs index a6194e5..2dc1fcf 100644 --- a/crates/api/src/proposer/api.rs +++ b/crates/api/src/proposer/api.rs @@ -45,8 +45,8 @@ use helix_common::{ signed_proposal::VersionedSignedProposal, try_execution_header_from_payload, versioned_payload::PayloadAndBlobs, - BidRequest, Filtering, GetHeaderTrace, GetPayloadTrace, RegisterValidatorsTrace, - ValidatorPreferences, RelayConfig, + BidRequest, Filtering, GetHeaderTrace, GetPayloadTrace, RegisterValidatorsTrace, RelayConfig, + ValidatorPreferences, }; use helix_database::DatabaseService; use helix_datastore::{error::AuctioneerError, Auctioneer}; @@ -88,7 +88,7 @@ where chain_info: Arc, validator_preferences: Arc, - relay_config: Arc, + relay_config: RelayConfig, } impl ProposerApi @@ -108,7 +108,7 @@ where slot_update_subscription: Sender>, validator_preferences: Arc, gossip_receiver: Receiver, - relay_config: Arc, + relay_config: RelayConfig, ) -> Self { let api = Self { auctioneer, @@ -535,18 +535,12 @@ where return Err(ProposerApiError::BidValueZero) } - // Get inclusion proofs - let proofs = proposer_api - .auctioneer - .get_inclusion_proof(slot, &bid_request.public_key, bid.block_hash()) - .await?; - // Save trace to DB proposer_api .save_get_header_call( slot, bid_request.parent_hash, - bid_request.public_key, + bid_request.public_key.clone(), bid.block_hash().clone(), trace, request_id, @@ -554,6 +548,29 @@ where ) .await; + // If the block value is greater than the max value to verify, return the bid + // without proofs. + let value_above_max_to_verify = proposer_api + .relay_config + .constraints_api_config + .max_block_value_to_verify_wei + .map_or(false, |max| bid.value() > max); + if value_above_max_to_verify { + info!( + %request_id, + slot, + value = ?bid.value(), + "block value is greater than max value to verify, returning bid without proofs", + ); + return Ok(axum::Json(bid)) + } + + // Get inclusion proofs + let proofs = proposer_api + .auctioneer + .get_inclusion_proof(slot, &bid_request.public_key, bid.block_hash()) + .await?; + // Attach the proofs to the bid before sending it back if let Some(proofs) = proofs { bid.set_inclusion_proofs(proofs); @@ -571,21 +588,7 @@ where let constraints = proposer_api.auctioneer.get_constraints(slot).await?.unwrap_or_default(); - let value_above_max_to_verify = proposer_api - .relay_config - .constraints_api_config - .max_block_value_to_verify - .map_or(false, |max| bid.value() > max); - if value_above_max_to_verify { - info!( - %request_id, - slot, - value = ?bid.value(), - "block value is greater than max value to verify, skipping inclusion proof check in get_header_with_proofs", - ); - } - - if !constraints.is_empty() && !value_above_max_to_verify { + if !constraints.is_empty() { error!( request_id = %request_id, slot, diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index 27cfcc5..b346863 100644 --- a/crates/common/src/config.rs +++ b/crates/common/src/config.rs @@ -1,6 +1,9 @@ use crate::{api::*, BuilderInfo, ValidatorPreferences}; use clap::Parser; -use ethereum_consensus::{primitives::BlsPublicKey, ssz::prelude::Node}; +use ethereum_consensus::{ + primitives::BlsPublicKey, + ssz::prelude::{Node, U256}, +}; use helix_utils::{ request_encoding::Encoding, serde::{default_bool, deserialize_url, serialize_url}, @@ -138,13 +141,15 @@ pub struct ConstraintsApiConfig { pub check_constraints_signature: bool, /// Only verify and save inclusion proofs if the block value is less than this threshold. /// We do this to ensure that high value blocks are not rejected. - #[serde(default)] - pub max_block_value_to_verify: Option, + pub max_block_value_to_verify_wei: Option, } impl Default for ConstraintsApiConfig { fn default() -> Self { - ConstraintsApiConfig { check_constraints_signature: true } + ConstraintsApiConfig { + check_constraints_signature: true, + max_block_value_to_verify_wei: None, + } } } From ea154748632a5ab3c872f690f01885d297ea4170 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 12 Nov 2024 10:13:39 +0100 Subject: [PATCH 3/4] fix(tests): make them compile --- crates/api/src/proposer/tests.rs | 2 +- crates/api/src/test_utils.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/api/src/proposer/tests.rs b/crates/api/src/proposer/tests.rs index ef61bf3..677d099 100644 --- a/crates/api/src/proposer/tests.rs +++ b/crates/api/src/proposer/tests.rs @@ -998,7 +998,7 @@ mod proposer_api_tests { slot_update_sender.clone(), Arc::new(ValidatorPreferences::default()), gossip_receiver, - Arc::new(Default::default()), + Default::default(), ); let mut x = gen_signed_vr(); diff --git a/crates/api/src/test_utils.rs b/crates/api/src/test_utils.rs index 0e9107d..fac23de 100644 --- a/crates/api/src/test_utils.rs +++ b/crates/api/src/test_utils.rs @@ -57,7 +57,7 @@ pub fn app() -> Router { slot_update_sender, Arc::new(ValidatorPreferences::default()), gossip_receiver, - Arc::new(Default::default()), + Default::default(), )); let data_api = Arc::new(DataApi::::new( @@ -219,7 +219,7 @@ pub fn proposer_api_app() -> ( slot_update_sender.clone(), Arc::new(ValidatorPreferences::default()), gossip_receiver, - Arc::new(Default::default()), + Default::default(), )); let router = Router::new() From ae5974982ada384433fb644b99e181585f1e9fef Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 12 Nov 2024 10:14:05 +0100 Subject: [PATCH 4/4] chore(api): should verify proofs logic --- crates/api/src/builder/api.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/api/src/builder/api.rs b/crates/api/src/builder/api.rs index 5a4255a..b3cf264 100644 --- a/crates/api/src/builder/api.rs +++ b/crates/api/src/builder/api.rs @@ -679,12 +679,12 @@ where .await?; // Fetch constraints, and if available verify inclusion proofs and save them to cache - let skip_inclusion_proof_verify_and_save = api + let should_verify_and_save_proofs = api .relay_config .constraints_api_config .max_block_value_to_verify_wei - .map_or(false, |max_block_value_to_verify| payload.value() > max_block_value_to_verify); - if !skip_inclusion_proof_verify_and_save { + .map_or(true, |max_block_value_to_verify| payload.value() <= max_block_value_to_verify); + if should_verify_and_save_proofs { if let Err(err) = api.verify_and_save_inclusion_proofs(&payload, &request_id).await { warn!(request_id = %request_id, error = %err, "failed to verify and save inclusion proofs"); return Err(err)