Skip to content

Commit

Permalink
Merge pull request #137 from chainbound/fix/sidecar/same-nonce
Browse files Browse the repository at this point in the history
fix(sidecar): validate state with existing diffs
  • Loading branch information
thedevbirb authored Jul 17, 2024
2 parents aa021ab + 4cb7be7 commit 0002ce4
Show file tree
Hide file tree
Showing 11 changed files with 521 additions and 350 deletions.
16 changes: 6 additions & 10 deletions bolt-sidecar/bin/sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -79,13 +78,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 +95,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 All @@ -123,7 +119,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;
};
Expand Down
8 changes: 4 additions & 4 deletions bolt-sidecar/src/builder/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ 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<SignedConstraints>,
}

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.
Expand Down Expand Up @@ -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<Address, (u64, U256)>,
pub(crate) diffs: HashMap<Address, (u64, U256)>,
}

impl StateDiff {
Expand Down
5 changes: 5 additions & 0 deletions bolt-sidecar/src/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<B256> {
self.0.request("eth_sendRawTransaction", [raw]).await
}
}

impl Deref for RpcClient {
Expand Down
10 changes: 8 additions & 2 deletions bolt-sidecar/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
Loading

0 comments on commit 0002ce4

Please sign in to comment.