Skip to content

Commit

Permalink
fix(sidecar): same nonce pre-confirmations on the same slot
Browse files Browse the repository at this point in the history
Co-authored-by: merklefruit <[email protected]>
  • Loading branch information
thedevbirb and merklefruit committed Jul 16, 2024
1 parent aa021ab commit f4cadee
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 48 deletions.
13 changes: 5 additions & 8 deletions bolt-sidecar/bin/sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 };

Expand Down
10 changes: 6 additions & 4 deletions bolt-sidecar/src/json_rpc/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl CommitmentsRpc for JsonRpcApi {

let request = serde_json::from_value::<CommitmentRequest>(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(),
Expand All @@ -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(),
))?;

Expand All @@ -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(),
});
}
Expand Down
18 changes: 8 additions & 10 deletions bolt-sidecar/src/primitives/commitment.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -79,7 +80,7 @@ where
serializer.serialize_str(&format!("0x{}", hex::encode(&data)))
}

fn deserialize_from_str<'de, D, T>(deserializer: D) -> Result<T, D::Error>
fn deserialize_sig<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: FromStr,
Expand All @@ -89,10 +90,7 @@ where
T::from_str(s.trim_start_matches("0x")).map_err(de::Error::custom)
}

fn signature_as_str<S: serde::Serializer>(
sig: &Signature,
serializer: S,
) -> Result<S::Ok, S::Error> {
fn serialize_sig<S: serde::Serializer>(sig: &Signature, serializer: S) -> Result<S::Ok, S::Error> {
let parity = sig.v();
// As bytes encodes the parity as 27/28, need to change that.
let mut bytes = sig.as_bytes();
Expand Down
8 changes: 6 additions & 2 deletions bolt-sidecar/src/primitives/constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
52 changes: 30 additions & 22 deletions bolt-sidecar/src/state/execution.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -200,7 +200,8 @@ impl<C: StateFetcher> ExecutionState<C> {
}

// 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,
Expand Down Expand Up @@ -235,10 +236,7 @@ impl<C: StateFetcher> ExecutionState<C> {
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");

Expand All @@ -254,30 +252,40 @@ impl<C: StateFetcher> ExecutionState<C> {
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() {
Expand Down
4 changes: 2 additions & 2 deletions bolt-sidecar/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down Expand Up @@ -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 };

Expand Down
1 change: 1 addition & 0 deletions bolt-sidecar/src/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,6 @@ pub(crate) async fn create_signed_commitment_request(
tx: tx_pooled,
slot,
signature,
sender: signer.address(),
}))
}

0 comments on commit f4cadee

Please sign in to comment.