diff --git a/bolt-sidecar/bin/sidecar.rs b/bolt-sidecar/bin/sidecar.rs index 30decb89..fb9093eb 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 ac7bbffa..2b4f88aa 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 03a37da6..9db05819 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 9281ed10..f1073ce5 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 265de6ed..d85047ea 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 767b4582..f1967cb4 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 a17ab1ea..86b01d74 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(), })) }