From f4cadee210514856f06db71e5c4b0d4e1c40d4af Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 16 Jul 2024 17:01:20 +0200 Subject: [PATCH 01/14] fix(sidecar): same nonce pre-confirmations on the same slot Co-authored-by: merklefruit --- bolt-sidecar/bin/sidecar.rs | 13 +++--- bolt-sidecar/src/json_rpc/api.rs | 10 +++-- bolt-sidecar/src/primitives/commitment.rs | 18 ++++---- bolt-sidecar/src/primitives/constraint.rs | 8 +++- bolt-sidecar/src/state/execution.rs | 52 +++++++++++++---------- bolt-sidecar/src/state/mod.rs | 4 +- bolt-sidecar/src/test_util.rs | 1 + 7 files changed, 58 insertions(+), 48 deletions(-) diff --git a/bolt-sidecar/bin/sidecar.rs b/bolt-sidecar/bin/sidecar.rs index 30decb895..fb9093eb3 100644 --- a/bolt-sidecar/bin/sidecar.rs +++ b/bolt-sidecar/bin/sidecar.rs @@ -79,13 +79,10 @@ async fn main() -> eyre::Result<()> { } }; - let sender = match execution_state.validate_commitment_request(&request).await { - Ok(sender) => sender, - Err(e) => { - tracing::error!(err = ?e, "Failed to commit request"); - let _ = response_tx.send(Err(ApiError::Custom(e.to_string()))); - continue; - } + if let Err(e) = execution_state.validate_commitment_request(&request).await { + tracing::error!(err = ?e, "Failed to commit request"); + let _ = response_tx.send(Err(ApiError::Custom(e.to_string()))); + continue; }; // TODO: match when we have more request types @@ -99,7 +96,7 @@ async fn main() -> eyre::Result<()> { // TODO: review all this `clone` usage // parse the request into constraints and sign them with the sidecar signer - let message = ConstraintsMessage::build(validator_index, request.clone(), sender); + let message = ConstraintsMessage::build(validator_index, request.clone()); let signature = signer.sign(&message.digest())?.to_string(); let signed_constraints = SignedConstraints { message, signature }; diff --git a/bolt-sidecar/src/json_rpc/api.rs b/bolt-sidecar/src/json_rpc/api.rs index ac7bbffaa..2b4f88aa6 100644 --- a/bolt-sidecar/src/json_rpc/api.rs +++ b/bolt-sidecar/src/json_rpc/api.rs @@ -104,7 +104,7 @@ impl CommitmentsRpc for JsonRpcApi { let request = serde_json::from_value::(params)?; #[allow(irrefutable_let_patterns)] // TODO: remove this when we have more request types - let CommitmentRequest::Inclusion(request) = request + let CommitmentRequest::Inclusion(mut request) = request else { return Err(ApiError::Custom( "request must be an inclusion request".to_string(), @@ -113,7 +113,9 @@ impl CommitmentsRpc for JsonRpcApi { info!(?request, "received inclusion commitment request"); - let tx_sender = request.tx.recover_signer().ok_or(ApiError::Custom( + // NOTE: request.sender is skipped from deserialization and initialized as Address::ZERO + // by the default Deserialization. It must be set here. + request.sender = request.tx.recover_signer().ok_or(ApiError::Custom( "failed to recover signer from transaction".to_string(), ))?; @@ -124,9 +126,9 @@ impl CommitmentsRpc for JsonRpcApi { // TODO: relax this check to allow for external signers to request commitments // about transactions that they did not sign themselves - if signer_address != tx_sender { + if signer_address != request.sender { return Err(ApiError::SignaturePubkeyMismatch { - expected: tx_sender.to_string(), + expected: request.sender.to_string(), got: signer_address.to_string(), }); } diff --git a/bolt-sidecar/src/primitives/commitment.rs b/bolt-sidecar/src/primitives/commitment.rs index 03a37da6a..9db058198 100644 --- a/bolt-sidecar/src/primitives/commitment.rs +++ b/bolt-sidecar/src/primitives/commitment.rs @@ -1,7 +1,7 @@ use serde::{de, Deserialize, Deserializer, Serialize}; use std::str::FromStr; -use alloy_primitives::{keccak256, Signature, B256}; +use alloy_primitives::{keccak256, Address, Signature, B256}; use reth_primitives::PooledTransactionsElement; use super::TransactionExt; @@ -37,11 +37,12 @@ pub struct InclusionRequest { /// The signature over the "slot" and "tx" fields by the user. /// A valid signature is the only proof that the user actually requested /// this specific commitment to be included at the given slot. - #[serde( - deserialize_with = "deserialize_from_str", - serialize_with = "signature_as_str" - )] + #[serde(deserialize_with = "deserialize_sig", serialize_with = "serialize_sig")] pub signature: Signature, + /// The ec-recovered address of the signature, for internal use. + /// If not explicitly set, this defaults to `Address::ZERO`. + #[serde(skip)] + pub sender: Address, } impl InclusionRequest { @@ -79,7 +80,7 @@ where serializer.serialize_str(&format!("0x{}", hex::encode(&data))) } -fn deserialize_from_str<'de, D, T>(deserializer: D) -> Result +fn deserialize_sig<'de, D, T>(deserializer: D) -> Result where D: Deserializer<'de>, T: FromStr, @@ -89,10 +90,7 @@ where T::from_str(s.trim_start_matches("0x")).map_err(de::Error::custom) } -fn signature_as_str( - sig: &Signature, - serializer: S, -) -> Result { +fn serialize_sig(sig: &Signature, serializer: S) -> Result { let parity = sig.v(); // As bytes encodes the parity as 27/28, need to change that. let mut bytes = sig.as_bytes(); diff --git a/bolt-sidecar/src/primitives/constraint.rs b/bolt-sidecar/src/primitives/constraint.rs index 9281ed102..f1073ce53 100644 --- a/bolt-sidecar/src/primitives/constraint.rs +++ b/bolt-sidecar/src/primitives/constraint.rs @@ -55,8 +55,12 @@ pub struct ConstraintsMessage { impl ConstraintsMessage { /// Builds a constraints message from an inclusion request and metadata - pub fn build(validator_index: u64, request: InclusionRequest, sender: Address) -> Self { - let constraints = vec![Constraint::from_transaction(request.tx, None, sender)]; + pub fn build(validator_index: u64, request: InclusionRequest) -> Self { + let constraints = vec![Constraint::from_transaction( + request.tx, + None, + request.sender, + )]; Self { validator_index, slot: request.slot, diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 265de6ed9..d85047eab 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -1,5 +1,5 @@ use alloy_eips::eip4844::MAX_BLOBS_PER_BLOCK; -use alloy_primitives::{Address, SignatureError}; +use alloy_primitives::{Address, SignatureError, U256}; use alloy_transport::TransportError; use reth_primitives::{ revm_primitives::EnvKzgSettings, BlobTransactionValidationError, PooledTransactionsElement, @@ -200,7 +200,8 @@ impl ExecutionState { } // Check if there is room for more commitments - if let Some(template) = self.get_block_template(req.slot) { + let template = self.get_block_template(req.slot); + if let Some(template) = template { if template.transactions_len() >= self.max_commitments_per_slot.get() { return Err(ValidationError::MaxCommitmentsReachedForSlot( self.slot, @@ -235,10 +236,7 @@ impl ExecutionState { return Err(ValidationError::MaxPriorityFeePerGasTooHigh); } - let sender = req - .tx - .recover_signer() - .ok_or(ValidationError::RecoverSigner)?; + let sender = req.sender; tracing::debug!(%sender, target_slot = req.slot, "Trying to commit inclusion request to block template"); @@ -254,30 +252,40 @@ impl ExecutionState { return Err(ValidationError::BaseFeeTooLow(max_basefee)); } - // If we have the account state, use it here - if let Some(account_state) = self.account_state(&sender) { - // Validate the transaction against the account state - tracing::debug!(address = %sender, "Known account state: {account_state:?}"); - validate_transaction(&account_state, &req.tx)?; - } else { - tracing::debug!(address = %sender, "Unknown account state"); - // If we don't have the account state, we need to fetch it - let account_state = - self.client + // Retrieve the nonce and balance diffs from previous preconfirmations for this slot. + // If the template does not exist, or this is the first request for this sender, + // its diffs will be zero. + let (nonce_diff, balance_diff) = self + .block_templates + .get(&req.slot) + .and_then(|template| template.state_diff().get_diff(&sender)) + // TODO: should balance diff be signed? + .unwrap_or((0, U256::ZERO)); + + let account_state = match self.account_state(&sender) { + Some(account) => account, + None => { + let account = self + .client .get_account_state(&sender, None) .await .map_err(|e| { ValidationError::Internal(format!("Failed to fetch account state: {:?}", e)) })?; - tracing::debug!(address = %sender, "Fetched account state: {account_state:?}"); + self.account_states.insert(sender, account); + account + } + }; - // Record the account state for later - self.account_states.insert(sender, account_state); + let account_state_with_diffs = AccountState { + transaction_count: account_state.transaction_count + nonce_diff, + balance: account_state.balance - balance_diff, + has_code: account_state.has_code, + }; - // Validate the transaction against the account state - validate_transaction(&account_state, &req.tx)?; - } + // Validate the transaction against the account state with existing diffs + validate_transaction(&account_state_with_diffs, &req.tx)?; // Check EIP-4844-specific limits if let Some(transaction) = req.tx.as_eip4844() { diff --git a/bolt-sidecar/src/state/mod.rs b/bolt-sidecar/src/state/mod.rs index 767b4582f..f1967cb47 100644 --- a/bolt-sidecar/src/state/mod.rs +++ b/bolt-sidecar/src/state/mod.rs @@ -253,7 +253,7 @@ mod tests { assert!(state.validate_commitment_request(&request).await.is_ok()); let bls_signer = Signer::random(); - let message = ConstraintsMessage::build(0, inclusion_request, *sender); + let message = ConstraintsMessage::build(0, inclusion_request); let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); let signed_constraints = SignedConstraints { message, signature }; @@ -317,7 +317,7 @@ mod tests { assert!(state.validate_commitment_request(&request).await.is_ok()); let bls_signer = Signer::random(); - let message = ConstraintsMessage::build(0, inclusion_request, *sender); + let message = ConstraintsMessage::build(0, inclusion_request); let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); let signed_constraints = SignedConstraints { message, signature }; diff --git a/bolt-sidecar/src/test_util.rs b/bolt-sidecar/src/test_util.rs index a17ab1ea3..86b01d747 100644 --- a/bolt-sidecar/src/test_util.rs +++ b/bolt-sidecar/src/test_util.rs @@ -169,5 +169,6 @@ pub(crate) async fn create_signed_commitment_request( tx: tx_pooled, slot, signature, + sender: signer.address(), })) } From a2a00c214fdeb597e2cddc76016cbb032fdc987b Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 16 Jul 2024 17:51:51 +0200 Subject: [PATCH 02/14] fix(sidecar): use separate get_block_template and remove_block_template --- bolt-sidecar/bin/sidecar.rs | 2 +- bolt-sidecar/src/state/execution.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bolt-sidecar/bin/sidecar.rs b/bolt-sidecar/bin/sidecar.rs index fb9093eb3..2044b02ea 100644 --- a/bolt-sidecar/bin/sidecar.rs +++ b/bolt-sidecar/bin/sidecar.rs @@ -120,7 +120,7 @@ async fn main() -> eyre::Result<()> { Some(slot) = consensus_state.commitment_deadline.wait() => { tracing::info!(slot, "Commitment deadline reached, starting to build local block"); - let Some(template) = execution_state.get_block_template(slot) else { + let Some(template) = execution_state.remove_block_template(slot) else { tracing::warn!("No block template found for slot {slot} when requested"); continue; }; diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index d85047eab..a277fce88 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -401,9 +401,13 @@ impl ExecutionState { } } + pub fn get_block_template(&mut self, slot: u64) -> Option<&BlockTemplate> { + self.block_templates.get(&slot) + } + /// Gets the block template for the given slot number and removes it from the cache. /// This should be called when we need to propose a block for the given slot. - pub fn get_block_template(&mut self, slot: u64) -> Option { + pub fn remove_block_template(&mut self, slot: u64) -> Option { self.block_templates.remove(&slot) } } From 6e6881481e305093d47fe7a407763944636c554a Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 16 Jul 2024 17:54:11 +0200 Subject: [PATCH 03/14] chore(sidecar): docs, fmt --- bolt-sidecar/src/state/execution.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index a277fce88..067f3cbc9 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -343,7 +343,7 @@ impl ExecutionState { self.apply_state_update(update); // Remove any block templates that are no longer valid - self.block_templates.remove(&slot); + self.remove_block_template(slot); Ok(()) } @@ -406,7 +406,8 @@ impl ExecutionState { } /// Gets the block template for the given slot number and removes it from the cache. - /// This should be called when we need to propose a block for the given slot. + /// This should be called when we need to propose a block for the given slot, + /// or when a new head comes in which makes an older block template useless. pub fn remove_block_template(&mut self, slot: u64) -> Option { self.block_templates.remove(&slot) } From dc63777bc06c0aba27791074acec51b4e0d54cb1 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Tue, 16 Jul 2024 18:02:42 +0200 Subject: [PATCH 04/14] fix(sidecar): cumulated diffs check wip --- bolt-sidecar/bin/sidecar.rs | 1 - bolt-sidecar/src/state/execution.rs | 38 ++++++++++++++++++----------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/bolt-sidecar/bin/sidecar.rs b/bolt-sidecar/bin/sidecar.rs index 2044b02ea..66d22d48d 100644 --- a/bolt-sidecar/bin/sidecar.rs +++ b/bolt-sidecar/bin/sidecar.rs @@ -68,7 +68,6 @@ async fn main() -> eyre::Result<()> { tokio::select! { Some(ApiEvent { request, response_tx }) = api_events_rx.recv() => { let start = std::time::Instant::now(); - tracing::info!("Received commitment request: {:?}", request); let validator_index = match consensus_state.validate_request(&request) { Ok(index) => index, diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 067f3cbc9..06fd407aa 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -253,25 +253,35 @@ impl ExecutionState { } // Retrieve the nonce and balance diffs from previous preconfirmations for this slot. - // If the template does not exist, or this is the first request for this sender, + // If the templates do not exist, or this is the first request for this sender, // its diffs will be zero. - let (nonce_diff, balance_diff) = self - .block_templates - .get(&req.slot) - .and_then(|template| template.state_diff().get_diff(&sender)) - // TODO: should balance diff be signed? - .unwrap_or((0, U256::ZERO)); + let (nonce_diff, balance_diff) = self.block_templates().values().fold( + (0, U256::ZERO), + |(nonce_diff_acc, balance_diff_acc), block_template| { + let (nonce_diff, balance_diff) = block_template + .state_diff() + .get_diff(&sender) + // TODO: should balance diff be signed? + .unwrap_or((0, U256::ZERO)); + + (nonce_diff_acc + nonce_diff, balance_diff_acc + balance_diff) + }, + ); + + tracing::debug!(%sender, nonce_diff, %balance_diff, "Applying diffs to account state"); let account_state = match self.account_state(&sender) { Some(account) => account, None => { - let account = self - .client - .get_account_state(&sender, None) - .await - .map_err(|e| { - ValidationError::Internal(format!("Failed to fetch account state: {:?}", e)) - })?; + let account = match self.client.get_account_state(&sender, None).await { + Ok(account) => account, + Err(err) => { + return Err(ValidationError::Internal(format!( + "Error fetching account state: {:?}", + err + ))) + } + }; self.account_states.insert(sender, account); account From 7bf5dd86f0db1f2ee6f269ffa58d8a21ed9315e9 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 10:10:42 +0200 Subject: [PATCH 05/14] fix(sidecar): check against cumulated nonce and balance diff + highest slot for which I request a preconf Co-authored-by: merklefruit --- bolt-sidecar/src/state/execution.rs | 31 ++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 06fd407aa..ac43ec29d 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -55,6 +55,11 @@ pub enum ValidationError { #[error("Too many EIP-4844 transactions in target block")] Eip4844Limit, /// The maximum commitments have been reached for the slot. + #[error( + "Already requested a preconfirmation for slot {0}. Slot must be greater than or equal {0}" + )] + SlotTooLow(u64), + /// The maximum commitments have been reached for the slot. #[error("Max commitments reached for slot {0}: {1}")] MaxCommitmentsReachedForSlot(u64, usize), /// The signature is invalid. @@ -252,22 +257,34 @@ impl ExecutionState { return Err(ValidationError::BaseFeeTooLow(max_basefee)); } - // Retrieve the nonce and balance diffs from previous preconfirmations for this slot. + // From previous preconfirmations requests retrieve + // - the nonce difference from the account state. + // - the balance difference from the account state. + // - the highest slot number for which the user has requested a preconfirmation. // If the templates do not exist, or this is the first request for this sender, // its diffs will be zero. - let (nonce_diff, balance_diff) = self.block_templates().values().fold( - (0, U256::ZERO), - |(nonce_diff_acc, balance_diff_acc), block_template| { - let (nonce_diff, balance_diff) = block_template + let (nonce_diff, balance_diff, highest_slot) = self.block_templates().iter().fold( + (0, U256::ZERO, 0), + |(nonce_diff_acc, balance_diff_acc, highest_slot), (slot, block_template)| { + let (nonce_diff, balance_diff, slot) = block_template .state_diff() .get_diff(&sender) + .map(|d| (d.0, d.1, *slot)) // TODO: should balance diff be signed? - .unwrap_or((0, U256::ZERO)); + .unwrap_or((0, U256::ZERO, 0)); - (nonce_diff_acc + nonce_diff, balance_diff_acc + balance_diff) + ( + nonce_diff_acc + nonce_diff, + balance_diff_acc + balance_diff, + u64::max(highest_slot, slot), + ) }, ); + if req.slot < highest_slot { + return Err(ValidationError::SlotTooLow(highest_slot)); + } + tracing::debug!(%sender, nonce_diff, %balance_diff, "Applying diffs to account state"); let account_state = match self.account_state(&sender) { From e24eebd1c57d23d73ae8c7d9d40f53c539c2a99c Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 10:16:39 +0200 Subject: [PATCH 06/14] chore(sidecar): expected and actual values in nonce validation errors --- bolt-sidecar/src/common.rs | 10 ++++++++-- bolt-sidecar/src/state/execution.rs | 6 +++--- bolt-sidecar/src/state/mod.rs | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/bolt-sidecar/src/common.rs b/bolt-sidecar/src/common.rs index 73bed1016..e73c8dbd4 100644 --- a/bolt-sidecar/src/common.rs +++ b/bolt-sidecar/src/common.rs @@ -47,11 +47,17 @@ pub fn validate_transaction( ) -> Result<(), ValidationError> { // Check if the nonce is correct (should be the same as the transaction count) if transaction.nonce() < account_state.transaction_count { - return Err(ValidationError::NonceTooLow); + return Err(ValidationError::NonceTooLow( + account_state.transaction_count, + transaction.nonce(), + )); } if transaction.nonce() > account_state.transaction_count { - return Err(ValidationError::NonceTooHigh); + return Err(ValidationError::NonceTooHigh( + account_state.transaction_count, + transaction.nonce(), + )); } // Check if the balance is enough diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index ac43ec29d..d96910e14 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -31,11 +31,11 @@ pub enum ValidationError { #[error("Invalid max basefee calculation: overflow")] MaxBaseFeeCalcOverflow, /// The transaction nonce is too low. - #[error("Transaction nonce too low")] - NonceTooLow, + #[error("Transaction nonce too low. Expected {0}, got {1}")] + NonceTooLow(u64, u64), /// The transaction nonce is too high. #[error("Transaction nonce too high")] - NonceTooHigh, + NonceTooHigh(u64, u64), /// The sender account is a smart contract and has code. #[error("Account has code")] AccountHasCode, diff --git a/bolt-sidecar/src/state/mod.rs b/bolt-sidecar/src/state/mod.rs index f1967cb47..fa0e7c48c 100644 --- a/bolt-sidecar/src/state/mod.rs +++ b/bolt-sidecar/src/state/mod.rs @@ -152,7 +152,7 @@ mod tests { assert!(matches!( state.validate_commitment_request(&request).await, - Err(ValidationError::NonceTooHigh) + Err(ValidationError::NonceTooHigh(_, _)) )); Ok(()) From 0e00315c98067b30aff6046822f25ea97f7906ca Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Wed, 17 Jul 2024 10:24:31 +0200 Subject: [PATCH 07/14] chore: minor fixes --- bolt-sidecar/src/builder/template.rs | 4 +-- bolt-sidecar/src/state/execution.rs | 40 ++++++++++++---------------- bolt-sidecar/src/state/mod.rs | 11 +++----- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/bolt-sidecar/src/builder/template.rs b/bolt-sidecar/src/builder/template.rs index 7fe4eb95a..0fcf8b730 100644 --- a/bolt-sidecar/src/builder/template.rs +++ b/bolt-sidecar/src/builder/template.rs @@ -37,8 +37,8 @@ pub struct BlockTemplate { impl BlockTemplate { /// Return the state diff of the block template. - pub fn state_diff(&self) -> &StateDiff { - &self.state_diff + pub fn get_diff(&self, address: &Address) -> Option<(u64, U256)> { + self.state_diff.get_diff(address) } /// Returns the cloned list of transactions from the constraints. diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index ac43ec29d..bed69408d 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -55,9 +55,7 @@ pub enum ValidationError { #[error("Too many EIP-4844 transactions in target block")] Eip4844Limit, /// The maximum commitments have been reached for the slot. - #[error( - "Already requested a preconfirmation for slot {0}. Slot must be greater than or equal {0}" - )] + #[error("Already requested a preconfirmation for slot {0}. Slot must be >= {0}")] SlotTooLow(u64), /// The maximum commitments have been reached for the slot. #[error("Max commitments reached for slot {0}: {1}")] @@ -177,11 +175,6 @@ impl ExecutionState { self.basefee } - /// Returns the current block templates mapped by slot number - pub fn block_templates(&self) -> &HashMap { - &self.block_templates - } - /// Validates the commitment request against state (historical + intermediate). /// /// NOTE: This function only simulates against execution state, it does not consider @@ -199,14 +192,16 @@ impl ExecutionState { ) -> Result { let CommitmentRequest::Inclusion(req) = request; + let sender = req.sender; + let target_slot = req.slot; + // Validate the chain ID if !req.validate_chain_id(self.chain_id) { return Err(ValidationError::ChainIdMismatch); } // Check if there is room for more commitments - let template = self.get_block_template(req.slot); - if let Some(template) = template { + if let Some(template) = self.get_block_template(target_slot) { if template.transactions_len() >= self.max_commitments_per_slot.get() { return Err(ValidationError::MaxCommitmentsReachedForSlot( self.slot, @@ -241,12 +236,10 @@ impl ExecutionState { return Err(ValidationError::MaxPriorityFeePerGasTooHigh); } - let sender = req.sender; - - tracing::debug!(%sender, target_slot = req.slot, "Trying to commit inclusion request to block template"); + tracing::debug!(%sender, target_slot, "Trying to commit inclusion request to block template"); // Check if the max_fee_per_gas would cover the maximum possible basefee. - let slot_diff = req.slot.saturating_sub(self.slot); + let slot_diff = target_slot.saturating_sub(self.slot); // Calculate the max possible basefee given the slot diff let max_basefee = calculate_max_basefee(self.basefee, slot_diff) @@ -261,16 +254,15 @@ impl ExecutionState { // - the nonce difference from the account state. // - the balance difference from the account state. // - the highest slot number for which the user has requested a preconfirmation. + // // If the templates do not exist, or this is the first request for this sender, // its diffs will be zero. - let (nonce_diff, balance_diff, highest_slot) = self.block_templates().iter().fold( + let (nonce_diff, balance_diff, highest_slot) = self.block_templates.iter().fold( (0, U256::ZERO, 0), |(nonce_diff_acc, balance_diff_acc, highest_slot), (slot, block_template)| { let (nonce_diff, balance_diff, slot) = block_template - .state_diff() .get_diff(&sender) - .map(|d| (d.0, d.1, *slot)) - // TODO: should balance diff be signed? + .map(|(nonce, balance)| (nonce, balance, *slot)) .unwrap_or((0, U256::ZERO, 0)); ( @@ -281,15 +273,16 @@ impl ExecutionState { }, ); - if req.slot < highest_slot { + if target_slot < highest_slot { return Err(ValidationError::SlotTooLow(highest_slot)); } - tracing::debug!(%sender, nonce_diff, %balance_diff, "Applying diffs to account state"); + tracing::trace!(%sender, nonce_diff, %balance_diff, "Applying diffs to account state"); let account_state = match self.account_state(&sender) { Some(account) => account, None => { + // Fetch the account state from the client if it does not exist let account = match self.client.get_account_state(&sender, None).await { Ok(account) => account, Err(err) => { @@ -316,7 +309,7 @@ impl ExecutionState { // Check EIP-4844-specific limits if let Some(transaction) = req.tx.as_eip4844() { - if let Some(template) = self.block_templates.get(&req.slot) { + if let Some(template) = self.block_templates.get(&target_slot) { if template.blob_count() >= MAX_BLOBS_PER_BLOCK { return Err(ValidationError::Eip4844Limit); } @@ -397,7 +390,7 @@ impl ExecutionState { template.retain(*address, *account_state); // Update the account state with the remaining state diff for the next iteration. - if let Some((nonce_diff, balance_diff)) = template.state_diff().get_diff(address) { + if let Some((nonce_diff, balance_diff)) = template.get_diff(address) { // Nonce will always be increased account_state.transaction_count += nonce_diff; // Balance will always be decreased @@ -414,7 +407,7 @@ impl ExecutionState { if let Some(mut account_state) = account_state { // Iterate over all block templates and apply the state diff for (_, template) in self.block_templates.iter() { - if let Some((nonce_diff, balance_diff)) = template.state_diff().get_diff(address) { + if let Some((nonce_diff, balance_diff)) = template.get_diff(address) { // Nonce will always be increased account_state.transaction_count += nonce_diff; // Balance will always be decreased @@ -428,6 +421,7 @@ impl ExecutionState { } } + /// Gets the block template for the given slot number. pub fn get_block_template(&mut self, slot: u64) -> Option<&BlockTemplate> { self.block_templates.get(&slot) } diff --git a/bolt-sidecar/src/state/mod.rs b/bolt-sidecar/src/state/mod.rs index f1967cb47..50d52f284 100644 --- a/bolt-sidecar/src/state/mod.rs +++ b/bolt-sidecar/src/state/mod.rs @@ -261,8 +261,7 @@ mod tests { assert!( state - .block_templates() - .get(&target_slot) + .get_block_template(target_slot) .unwrap() .transactions_len() == 1 @@ -281,8 +280,7 @@ mod tests { .await?; let transactions_len = state - .block_templates() - .get(&target_slot) + .get_block_template(target_slot) .unwrap() .transactions_len(); @@ -325,8 +323,7 @@ mod tests { assert!( state - .block_templates() - .get(&target_slot) + .get_block_template(target_slot) .unwrap() .transactions_len() == 1 @@ -336,7 +333,7 @@ mod tests { // because it's now stale state.update_head(None, target_slot).await?; - assert!(state.block_templates().get(&target_slot).is_none()); + assert!(state.get_block_template(target_slot).is_none()); Ok(()) } From 98e58f02326b1a3a2038dd6e66e1cc4af6ef82ed Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 10:37:14 +0200 Subject: [PATCH 08/14] chore(sidecar): move tests --- bolt-sidecar/src/state/execution.rs | 275 ++++++++++++++++++++++++++++ bolt-sidecar/src/state/mod.rs | 273 --------------------------- 2 files changed, 275 insertions(+), 273 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 84d8929b5..b29468487 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -441,3 +441,278 @@ pub struct StateUpdate { pub min_blob_basefee: u128, pub block_number: u64, } + +#[cfg(test)] +mod tests { + use crate::state::CommitmentDeadline; + use crate::state::Duration; + use std::num::NonZero; + + use alloy_consensus::constants::ETH_TO_WEI; + use alloy_eips::eip2718::Encodable2718; + use alloy_network::EthereumWallet; + use alloy_primitives::{uint, Uint}; + use alloy_provider::{network::TransactionBuilder, Provider, ProviderBuilder}; + use alloy_signer_local::PrivateKeySigner; + use fetcher::{StateClient, StateFetcher}; + + use crate::{ + crypto::{bls::Signer, SignableBLS, SignerBLS}, + primitives::{ConstraintsMessage, SignedConstraints}, + state::fetcher, + test_util::{create_signed_commitment_request, default_test_transaction, launch_anvil}, + }; + + use super::*; + + #[tokio::test] + async fn test_commitment_deadline() { + let time = std::time::Instant::now(); + let mut deadline = CommitmentDeadline::new(0, Duration::from_secs(1)); + + let slot = deadline.wait().await; + println!("Deadline reached. Passed {:?}", time.elapsed()); + assert_eq!(slot, Some(0)); + + let time = std::time::Instant::now(); + let slot = deadline.wait().await; + println!("Deadline reached. Passed {:?}", time.elapsed()); + assert_eq!(slot, None); + } + + #[tokio::test] + async fn test_valid_inclusion_request() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + let tx = default_test_transaction(*sender, None); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(state.validate_commitment_request(&request).await.is_ok()); + + Ok(()) + } + + #[tokio::test] + async fn test_invalid_inclusion_request_nonce() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // Create a transaction with a nonce that is too high + let tx = default_test_transaction(*sender, Some(1)); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::NonceTooHigh(0, 1)) + )); + + Ok(()) + } + + #[tokio::test] + async fn test_invalid_inclusion_request_balance() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // Create a transaction with a value that is too high + let tx = default_test_transaction(*sender, None) + .with_value(uint!(11_000_U256 * Uint::from(ETH_TO_WEI))); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::InsufficientBalance) + )); + + Ok(()) + } + + #[tokio::test] + async fn test_invalid_inclusion_request_basefee() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let basefee = state.basefee(); + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // Create a transaction with a basefee that is too low + let tx = default_test_transaction(*sender, None).with_max_fee_per_gas(basefee - 1); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::BaseFeeTooLow(_)) + )); + + Ok(()) + } + + #[tokio::test] + async fn test_invalidate_inclusion_request() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + let provider = ProviderBuilder::new().on_http(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + let tx = default_test_transaction(*sender, None); + + // build the signed transaction for submission later + let wallet: PrivateKeySigner = anvil.keys()[0].clone().into(); + let signer: EthereumWallet = wallet.into(); + let signed = tx.clone().build(&signer).await?; + + let target_slot = 10; + let request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; + let inclusion_request = request.as_inclusion_request().unwrap().clone(); + + assert!(state.validate_commitment_request(&request).await.is_ok()); + + let bls_signer = Signer::random(); + let message = ConstraintsMessage::build(0, inclusion_request); + let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); + let signed_constraints = SignedConstraints { message, signature }; + + state.add_constraint(target_slot, signed_constraints); + + assert!( + state + .get_block_template(target_slot) + .unwrap() + .transactions_len() + == 1 + ); + + let notif = provider + .send_raw_transaction(&signed.encoded_2718()) + .await?; + + // Wait for confirmation + let receipt = notif.get_receipt().await?; + + // Update the head, which should invalidate the transaction due to a nonce conflict + state + .update_head(receipt.block_number, receipt.block_number.unwrap()) + .await?; + + let transactions_len = state + .get_block_template(target_slot) + .unwrap() + .transactions_len(); + + assert!(transactions_len == 0); + + Ok(()) + } + + #[tokio::test] + async fn test_invalidate_stale_template() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + let tx = default_test_transaction(*sender, None); + + let target_slot = 10; + let request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; + let inclusion_request = request.as_inclusion_request().unwrap().clone(); + + assert!(state.validate_commitment_request(&request).await.is_ok()); + + let bls_signer = Signer::random(); + let message = ConstraintsMessage::build(0, inclusion_request); + let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); + let signed_constraints = SignedConstraints { message, signature }; + + state.add_constraint(target_slot, signed_constraints); + + assert!( + state + .get_block_template(target_slot) + .unwrap() + .transactions_len() + == 1 + ); + + // fast-forward the head to the target slot, which should invalidate the entire template + // because it's now stale + state.update_head(None, target_slot).await?; + + assert!(state.get_block_template(target_slot).is_none()); + + Ok(()) + } +} diff --git a/bolt-sidecar/src/state/mod.rs b/bolt-sidecar/src/state/mod.rs index 1c0eabcbf..84a349734 100644 --- a/bolt-sidecar/src/state/mod.rs +++ b/bolt-sidecar/src/state/mod.rs @@ -65,276 +65,3 @@ impl Future for CommitmentDeadline { } } } - -#[cfg(test)] -mod tests { - use std::num::NonZero; - - use alloy_consensus::constants::ETH_TO_WEI; - use alloy_eips::eip2718::Encodable2718; - use alloy_network::EthereumWallet; - use alloy_primitives::{uint, Uint}; - use alloy_provider::{network::TransactionBuilder, Provider, ProviderBuilder}; - use alloy_signer_local::PrivateKeySigner; - use execution::{ExecutionState, ValidationError}; - use fetcher::{StateClient, StateFetcher}; - - use crate::{ - crypto::{bls::Signer, SignableBLS, SignerBLS}, - primitives::{ConstraintsMessage, SignedConstraints}, - test_util::{create_signed_commitment_request, default_test_transaction, launch_anvil}, - }; - - use super::*; - - #[tokio::test] - async fn test_commitment_deadline() { - let time = std::time::Instant::now(); - let mut deadline = CommitmentDeadline::new(0, Duration::from_secs(1)); - - let slot = deadline.wait().await; - println!("Deadline reached. Passed {:?}", time.elapsed()); - assert_eq!(slot, Some(0)); - - let time = std::time::Instant::now(); - let slot = deadline.wait().await; - println!("Deadline reached. Passed {:?}", time.elapsed()); - assert_eq!(slot, None); - } - - #[tokio::test] - async fn test_valid_inclusion_request() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - let tx = default_test_transaction(*sender, None); - - let request = create_signed_commitment_request(tx, sender_pk, 10).await?; - - assert!(state.validate_commitment_request(&request).await.is_ok()); - - Ok(()) - } - - #[tokio::test] - async fn test_invalid_inclusion_request_nonce() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - // Create a transaction with a nonce that is too high - let tx = default_test_transaction(*sender, Some(1)); - - let request = create_signed_commitment_request(tx, sender_pk, 10).await?; - - assert!(matches!( - state.validate_commitment_request(&request).await, - Err(ValidationError::NonceTooHigh(_, _)) - )); - - Ok(()) - } - - #[tokio::test] - async fn test_invalid_inclusion_request_balance() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - // Create a transaction with a value that is too high - let tx = default_test_transaction(*sender, None) - .with_value(uint!(11_000_U256 * Uint::from(ETH_TO_WEI))); - - let request = create_signed_commitment_request(tx, sender_pk, 10).await?; - - assert!(matches!( - state.validate_commitment_request(&request).await, - Err(ValidationError::InsufficientBalance) - )); - - Ok(()) - } - - #[tokio::test] - async fn test_invalid_inclusion_request_basefee() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let basefee = state.basefee(); - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - // Create a transaction with a basefee that is too low - let tx = default_test_transaction(*sender, None).with_max_fee_per_gas(basefee - 1); - - let request = create_signed_commitment_request(tx, sender_pk, 10).await?; - - assert!(matches!( - state.validate_commitment_request(&request).await, - Err(ValidationError::BaseFeeTooLow(_)) - )); - - Ok(()) - } - - #[tokio::test] - async fn test_invalidate_inclusion_request() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - let provider = ProviderBuilder::new().on_http(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - let tx = default_test_transaction(*sender, None); - - // build the signed transaction for submission later - let wallet: PrivateKeySigner = anvil.keys()[0].clone().into(); - let signer: EthereumWallet = wallet.into(); - let signed = tx.clone().build(&signer).await?; - - let target_slot = 10; - let request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; - let inclusion_request = request.as_inclusion_request().unwrap().clone(); - - assert!(state.validate_commitment_request(&request).await.is_ok()); - - let bls_signer = Signer::random(); - let message = ConstraintsMessage::build(0, inclusion_request); - let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); - let signed_constraints = SignedConstraints { message, signature }; - - state.add_constraint(target_slot, signed_constraints); - - assert!( - state - .get_block_template(target_slot) - .unwrap() - .transactions_len() - == 1 - ); - - let notif = provider - .send_raw_transaction(&signed.encoded_2718()) - .await?; - - // Wait for confirmation - let receipt = notif.get_receipt().await?; - - // Update the head, which should invalidate the transaction due to a nonce conflict - state - .update_head(receipt.block_number, receipt.block_number.unwrap()) - .await?; - - let transactions_len = state - .get_block_template(target_slot) - .unwrap() - .transactions_len(); - - assert!(transactions_len == 0); - - Ok(()) - } - - #[tokio::test] - async fn test_invalidate_stale_template() -> eyre::Result<()> { - let _ = tracing_subscriber::fmt::try_init(); - - let anvil = launch_anvil(); - let client = StateClient::new(anvil.endpoint_url()); - - let max_comms = NonZero::new(10).unwrap(); - let mut state = ExecutionState::new(client.clone(), max_comms).await?; - - let sender = anvil.addresses().first().unwrap(); - let sender_pk = anvil.keys().first().unwrap(); - - // initialize the state by updating the head once - let slot = client.get_head().await?; - state.update_head(None, slot).await?; - - let tx = default_test_transaction(*sender, None); - - let target_slot = 10; - let request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; - let inclusion_request = request.as_inclusion_request().unwrap().clone(); - - assert!(state.validate_commitment_request(&request).await.is_ok()); - - let bls_signer = Signer::random(); - let message = ConstraintsMessage::build(0, inclusion_request); - let signature = bls_signer.sign(&message.digest()).unwrap().to_string(); - let signed_constraints = SignedConstraints { message, signature }; - - state.add_constraint(target_slot, signed_constraints); - - assert!( - state - .get_block_template(target_slot) - .unwrap() - .transactions_len() - == 1 - ); - - // fast-forward the head to the target slot, which should invalidate the entire template - // because it's now stale - state.update_head(None, target_slot).await?; - - assert!(state.get_block_template(target_slot).is_none()); - - Ok(()) - } -} From 9b9db34115800d2b70f15b58c9b1aa99cb6e2685 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 10:52:19 +0200 Subject: [PATCH 09/14] test(sidecar): invalid inclusion slot --- bolt-sidecar/src/builder/template.rs | 4 +-- bolt-sidecar/src/state/execution.rs | 42 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/bolt-sidecar/src/builder/template.rs b/bolt-sidecar/src/builder/template.rs index 0fcf8b730..f424c431d 100644 --- a/bolt-sidecar/src/builder/template.rs +++ b/bolt-sidecar/src/builder/template.rs @@ -30,7 +30,7 @@ use crate::{ #[derive(Debug, Default)] pub struct BlockTemplate { /// The state diffs per address given the list of commitments. - state_diff: StateDiff, + pub(crate) state_diff: StateDiff, /// The signed constraints associated to the block pub signed_constraints_list: Vec, } @@ -206,7 +206,7 @@ impl BlockTemplate { pub struct StateDiff { /// Map of diffs per address. Each diff is a tuple of the nonce and balance diff /// that should be applied to the current state. - diffs: HashMap, + pub(crate) diffs: HashMap, } impl StateDiff { diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index b29468487..5b100aee2 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -444,6 +444,7 @@ pub struct StateUpdate { #[cfg(test)] mod tests { + use crate::builder::template::StateDiff; use crate::state::CommitmentDeadline; use crate::state::Duration; use std::num::NonZero; @@ -506,6 +507,47 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_invalid_inclusion_slot() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // Create a transaction with a nonce that is too high + let tx = default_test_transaction(*sender, Some(1)); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + // Insert a constraint diff for slot 11 + let mut diffs = HashMap::new(); + diffs.insert(*sender, (1, U256::ZERO)); + state.block_templates.insert( + 11, + BlockTemplate { + state_diff: StateDiff { diffs }, + signed_constraints_list: vec![], + }, + ); + + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::SlotTooLow(11)) + )); + + Ok(()) + } + #[tokio::test] async fn test_invalid_inclusion_request_nonce() -> eyre::Result<()> { let _ = tracing_subscriber::fmt::try_init(); From da4e53764d0e0385db629080b0c1795c72e9d216 Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:10:31 +0200 Subject: [PATCH 10/14] chore: added commitment balance validation multiple test --- bolt-sidecar/src/client/rpc.rs | 5 ++ bolt-sidecar/src/state/execution.rs | 89 ++++++++++++++++++++++------- bolt-sidecar/src/state/fetcher.rs | 7 +++ bolt-sidecar/src/state/mod.rs | 21 +++++++ 4 files changed, 101 insertions(+), 21 deletions(-) diff --git a/bolt-sidecar/src/client/rpc.rs b/bolt-sidecar/src/client/rpc.rs index deac47047..6acf3246f 100644 --- a/bolt-sidecar/src/client/rpc.rs +++ b/bolt-sidecar/src/client/rpc.rs @@ -193,6 +193,11 @@ impl RpcClient { self.0.request("debug_traceCall", params).await } + + /// Send a raw transaction to the network. + pub async fn send_raw_transaction(&self, raw: Bytes) -> TransportResult { + self.0.request("eth_sendRawTransaction", [raw]).await + } } impl Deref for RpcClient { diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 5b100aee2..c71ae88a3 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -267,7 +267,7 @@ impl ExecutionState { ( nonce_diff_acc + nonce_diff, - balance_diff_acc + balance_diff, + balance_diff_acc.saturating_add(balance_diff), u64::max(highest_slot, slot), ) }, @@ -299,8 +299,8 @@ impl ExecutionState { }; let account_state_with_diffs = AccountState { - transaction_count: account_state.transaction_count + nonce_diff, - balance: account_state.balance - balance_diff, + transaction_count: account_state.transaction_count.saturating_add(nonce_diff), + balance: account_state.balance.saturating_sub(balance_diff), has_code: account_state.has_code, }; @@ -445,9 +445,8 @@ pub struct StateUpdate { #[cfg(test)] mod tests { use crate::builder::template::StateDiff; - use crate::state::CommitmentDeadline; - use crate::state::Duration; - use std::num::NonZero; + use std::str::FromStr; + use std::{num::NonZero, time::Duration}; use alloy_consensus::constants::ETH_TO_WEI; use alloy_eips::eip2718::Encodable2718; @@ -466,21 +465,6 @@ mod tests { use super::*; - #[tokio::test] - async fn test_commitment_deadline() { - let time = std::time::Instant::now(); - let mut deadline = CommitmentDeadline::new(0, Duration::from_secs(1)); - - let slot = deadline.wait().await; - println!("Deadline reached. Passed {:?}", time.elapsed()); - assert_eq!(slot, Some(0)); - - let time = std::time::Instant::now(); - let slot = deadline.wait().await; - println!("Deadline reached. Passed {:?}", time.elapsed()); - assert_eq!(slot, None); - } - #[tokio::test] async fn test_valid_inclusion_request() -> eyre::Result<()> { let _ = tracing_subscriber::fmt::try_init(); @@ -609,6 +593,69 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_invalid_inclusion_request_balance_multiple() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let max_comms = NonZero::new(10).unwrap(); + let mut state = ExecutionState::new(client.clone(), max_comms).await?; + + let sender = anvil.addresses().first().unwrap(); + let sender_pk = anvil.keys().first().unwrap(); + let signer = Signer::random(); + + // initialize the state by updating the head once + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // Set the sender balance to just enough to pay for 1 transaction + let balance = U256::from_str("500000000000000").unwrap(); // leave just 0.0005 ETH + let sender_account = client.get_account_state(sender, None).await.unwrap(); + let balance_to_burn = sender_account.balance - balance; + + // burn the balance + let tx = default_test_transaction(*sender, Some(0)).with_value(uint!(balance_to_burn)); + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let tx_bytes = request + .as_inclusion_request() + .unwrap() + .tx + .envelope_encoded(); + let _ = client.inner().send_raw_transaction(tx_bytes).await?; + + // wait for the transaction to be included to update the sender balance + tokio::time::sleep(Duration::from_secs(2)).await; + let slot = client.get_head().await?; + state.update_head(None, slot).await?; + + // create a new transaction and request a preconfirmation for it + let tx = default_test_transaction(*sender, Some(1)); + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(state.validate_commitment_request(&request).await.is_ok()); + + let message = ConstraintsMessage::build(0, request.as_inclusion_request().unwrap().clone()); + let signature = signer.sign(&message.digest())?.to_string(); + let signed_constraints = SignedConstraints { message, signature }; + state.add_constraint(10, signed_constraints); + + // create a new transaction and request a preconfirmation for it + let tx = default_test_transaction(*sender, Some(3)); + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + // this should fail because the balance is insufficient as we spent + // all of it on the previous preconfirmation + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::InsufficientBalance) + )); + + Ok(()) + } + #[tokio::test] async fn test_invalid_inclusion_request_basefee() -> eyre::Result<()> { let _ = tracing_subscriber::fmt::try_init(); diff --git a/bolt-sidecar/src/state/fetcher.rs b/bolt-sidecar/src/state/fetcher.rs index cebf5b021..2513c383c 100644 --- a/bolt-sidecar/src/state/fetcher.rs +++ b/bolt-sidecar/src/state/fetcher.rs @@ -217,6 +217,13 @@ impl StateFetcher for StateClient { } } +#[cfg(test)] +impl StateClient { + pub fn inner(&self) -> &RpcClient { + &self.client + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/bolt-sidecar/src/state/mod.rs b/bolt-sidecar/src/state/mod.rs index 84a349734..02dfa0237 100644 --- a/bolt-sidecar/src/state/mod.rs +++ b/bolt-sidecar/src/state/mod.rs @@ -65,3 +65,24 @@ impl Future for CommitmentDeadline { } } } + +#[cfg(test)] +mod tests { + + use super::*; + + #[tokio::test] + async fn test_commitment_deadline() { + let time = std::time::Instant::now(); + let mut deadline = CommitmentDeadline::new(0, Duration::from_secs(1)); + + let slot = deadline.wait().await; + println!("Deadline reached. Passed {:?}", time.elapsed()); + assert_eq!(slot, Some(0)); + + let time = std::time::Instant::now(); + let slot = deadline.wait().await; + println!("Deadline reached. Passed {:?}", time.elapsed()); + assert_eq!(slot, None); + } +} From e9a5c8ce514c08d499d5640fd0bbdcdbc5b1a0b5 Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:24:50 +0200 Subject: [PATCH 11/14] chore: rm duplicated logic of checking intermediate diffs --- bolt-sidecar/src/state/execution.rs | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index c71ae88a3..02587b84e 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -279,7 +279,7 @@ impl ExecutionState { tracing::trace!(%sender, nonce_diff, %balance_diff, "Applying diffs to account state"); - let account_state = match self.account_state(&sender) { + let account_state = match self.account_state(&sender).copied() { Some(account) => account, None => { // Fetch the account state from the client if it does not exist @@ -400,25 +400,9 @@ impl ExecutionState { } } - /// Returns the account state for the given address INCLUDING any intermediate block templates state. - fn account_state(&self, address: &Address) -> Option { - let account_state = self.account_states.get(address).copied(); - - if let Some(mut account_state) = account_state { - // Iterate over all block templates and apply the state diff - for (_, template) in self.block_templates.iter() { - if let Some((nonce_diff, balance_diff)) = template.get_diff(address) { - // Nonce will always be increased - account_state.transaction_count += nonce_diff; - // Balance will always be decreased - account_state.balance -= balance_diff; - } - } - - Some(account_state) - } else { - None - } + /// Returns the cached account state for the given address + fn account_state(&self, address: &Address) -> Option<&AccountState> { + self.account_states.get(address) } /// Gets the block template for the given slot number. @@ -643,7 +627,7 @@ mod tests { state.add_constraint(10, signed_constraints); // create a new transaction and request a preconfirmation for it - let tx = default_test_transaction(*sender, Some(3)); + let tx = default_test_transaction(*sender, Some(2)); let request = create_signed_commitment_request(tx, sender_pk, 10).await?; // this should fail because the balance is insufficient as we spent From c83061e19d9afde2e5680bf50373de9c1d559f6d Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 11:32:18 +0200 Subject: [PATCH 12/14] test(sidecar): nonce too high and too low with diffs --- bolt-sidecar/src/state/execution.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 02587b84e..0d53997d9 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -533,14 +533,37 @@ mod tests { let slot = client.get_head().await?; state.update_head(None, slot).await?; + // Insert a constraint diff for slot 9 to simulate nonce increment + let mut diffs = HashMap::new(); + diffs.insert(*sender, (1, U256::ZERO)); + state.block_templates.insert( + 9, + BlockTemplate { + state_diff: StateDiff { diffs }, + signed_constraints_list: vec![], + }, + ); + + // Create a transaction with a nonce that is too low + let tx = default_test_transaction(*sender, Some(0)); + + let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&request).await, + Err(ValidationError::NonceTooLow(1, 0)) + )); + + assert!(state.account_states.get(sender).unwrap().transaction_count == 0); + // Create a transaction with a nonce that is too high - let tx = default_test_transaction(*sender, Some(1)); + let tx = default_test_transaction(*sender, Some(2)); let request = create_signed_commitment_request(tx, sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&request).await, - Err(ValidationError::NonceTooHigh(0, 1)) + Err(ValidationError::NonceTooHigh(1, 2)) )); Ok(()) From b1cc672d21532f842828b05d80862001bcb55a0d Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 12:01:27 +0200 Subject: [PATCH 13/14] test(sidecar): fix test base fee too low --- bolt-sidecar/src/state/execution.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 0d53997d9..84db76e1c 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -683,7 +683,9 @@ mod tests { state.update_head(None, slot).await?; // Create a transaction with a basefee that is too low - let tx = default_test_transaction(*sender, None).with_max_fee_per_gas(basefee - 1); + let tx = default_test_transaction(*sender, None) + .with_max_fee_per_gas(basefee - 1) + .with_max_priority_fee_per_gas(basefee / 2); let request = create_signed_commitment_request(tx, sender_pk, 10).await?; From 4cb7be76949396a8bf6aedc08b49016b5946dd52 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Wed, 17 Jul 2024 12:28:05 +0200 Subject: [PATCH 14/14] test --- bolt-sidecar/src/state/execution.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 84db76e1c..bec286b5c 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -806,7 +806,7 @@ mod tests { ); // fast-forward the head to the target slot, which should invalidate the entire template - // because it's now stale + // because it's now stale. state.update_head(None, target_slot).await?; assert!(state.get_block_template(target_slot).is_none());