Skip to content

Commit

Permalink
Merge pull request #381 from chainbound/lore/feat/unsafe-disable-cons…
Browse files Browse the repository at this point in the history
…ensus-checks

feat(sidecar): add flag to disable consensus checks for testing purposes
  • Loading branch information
merklefruit authored Nov 13, 2024
2 parents e240d03 + 9656b68 commit 55b66b3
Show file tree
Hide file tree
Showing 10 changed files with 449 additions and 332 deletions.
560 changes: 330 additions & 230 deletions bolt-sidecar/Cargo.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions bolt-sidecar/src/api/commitments/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ mod test {

use crate::{
primitives::commitment::ECDSASignatureExt,
test_util::{create_signed_commitment_request, default_test_transaction},
test_util::{create_signed_inclusion_request, default_test_transaction},
};

use super::*;
Expand All @@ -194,7 +194,7 @@ mod test {
let sk = SecretKey::random(&mut rand::thread_rng());
let signer = PrivateKeySigner::from(sk.clone());
let tx = default_test_transaction(signer.address(), None);
let req = create_signed_commitment_request(&[tx], &sk, 12).await.unwrap();
let req = create_signed_inclusion_request(&[tx], &sk, 12).await.unwrap();

let payload = json!({
"jsonrpc": "2.0",
Expand Down Expand Up @@ -236,9 +236,9 @@ mod test {
let sk = SecretKey::random(&mut rand::thread_rng());
let signer = PrivateKeySigner::from(sk.clone());
let tx = default_test_transaction(signer.address(), None);
let req = create_signed_commitment_request(&[tx], &sk, 12).await.unwrap();
let req = create_signed_inclusion_request(&[tx], &sk, 12).await.unwrap();

let sig = req.signature().unwrap().to_hex();
let sig = req.signature.unwrap().to_hex();

let payload = json!({
"jsonrpc": "2.0",
Expand Down
7 changes: 6 additions & 1 deletion bolt-sidecar/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,14 @@ pub struct Opts {
/// then used when registering the operator in the `BoltManager` contract.
#[clap(long, env = "BOLT_SIDECAR_COMMITMENT_PRIVATE_KEY")]
pub commitment_private_key: EcdsaSecretKeyWrapper,
/// Unsafely disables consensus checks when validating commitments.
///
/// If enabled, the sidecar will sign every commitment request with the first private key
/// available without checking if connected validators are scheduled to propose a block.
#[clap(long, env = "BOLT_SIDECAR_UNSAFE_DISABLE_CONSENSUS_CHECKS", default_value_t = false)]
pub unsafe_disable_consensus_checks: bool,
/// Operating limits for the sidecar
#[clap(flatten)]
#[serde(default)]
pub limits: LimitsOpts,
/// Chain config for the chain on which the sidecar is running
#[clap(flatten)]
Expand Down
4 changes: 2 additions & 2 deletions bolt-sidecar/src/crypto/bls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ pub trait SignableBLS {
}

/// Convert a BLS public key from Consensus Types to a byte array.
pub fn cl_public_key_to_arr(pubkey: BlsPublicKey) -> [u8; BLS_PUBLIC_KEY_BYTES_LEN] {
pubkey.as_ref().try_into().expect("BLS keys are 48 bytes")
pub fn cl_public_key_to_arr(pubkey: impl AsRef<BlsPublicKey>) -> [u8; BLS_PUBLIC_KEY_BYTES_LEN] {
pubkey.as_ref().as_ref().try_into().expect("BLS keys are 48 bytes")
}
87 changes: 54 additions & 33 deletions bolt-sidecar/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use alloy::{rpc::types::beacon::events::HeadEvent, signers::local::PrivateKeySig
use beacon_api_client::mainnet::Client as BeaconClient;
use ethereum_consensus::{
clock::{self, SlotStream, SystemTimeProvider},
phase0::mainnet::SLOTS_PER_EPOCH,
phase0::mainnet::{BlsPublicKey, SLOTS_PER_EPOCH},
};
use eyre::Context;
use futures::StreamExt;
Expand All @@ -27,8 +27,8 @@ use crate::{
config::Opts,
crypto::{SignableBLS, SignerECDSA},
primitives::{
read_signed_delegations_from_file, CommitmentRequest, ConstraintsMessage,
FetchPayloadRequest, SignedConstraints, TransactionExt,
commitment::SignedCommitment, read_signed_delegations_from_file, CommitmentRequest,
ConstraintsMessage, FetchPayloadRequest, SignedConstraints, TransactionExt,
},
signer::{keystore::KeystoreSigner, local::LocalSigner, CommitBoostSigner, SignerBLS},
state::{fetcher::StateFetcher, ConsensusState, ExecutionState, HeadTracker, StateClient},
Expand Down Expand Up @@ -66,6 +66,8 @@ pub struct SidecarDriver<C, ECDSA> {
payload_requests_rx: mpsc::Receiver<FetchPayloadRequest>,
/// Stream of slots made from the consensus clock
slot_stream: SlotStream<SystemTimeProvider>,
/// Whether to skip consensus checks (should only be used for testing)
unsafe_skip_consensus_checks: bool,
}

impl SidecarDriver<StateClient, PrivateKeySigner> {
Expand Down Expand Up @@ -224,7 +226,10 @@ impl<C: StateFetcher, ECDSA: SignerECDSA> SidecarDriver<C, ECDSA> {
let (api_events_tx, api_events_rx) = mpsc::channel(1024);
CommitmentsApiServer::new(api_addr).run(api_events_tx).await;

let unsafe_skip_consensus_checks = opts.unsafe_disable_consensus_checks;

Ok(SidecarDriver {
unsafe_skip_consensus_checks,
head_tracker,
execution,
consensus,
Expand Down Expand Up @@ -268,67 +273,83 @@ impl<C: StateFetcher, ECDSA: SignerECDSA> SidecarDriver<C, ECDSA> {

/// Handle an incoming API event, validating the request and responding with a commitment.
async fn handle_incoming_api_event(&mut self, event: CommitmentEvent) {
let CommitmentEvent { mut request, response } = event;
let CommitmentEvent { request, response } = event;

info!("Received new commitment request: {:?}", request);
ApiMetrics::increment_inclusion_commitments_received();

let start = Instant::now();

let validator_pubkey = match self.consensus.validate_request(&request) {
Ok(pubkey) => pubkey,
Err(err) => {
warn!(?err, "Consensus: failed to validate request");
let _ = response.send(Err(CommitmentError::Consensus(err)));
// When we'll add more commitment types, we'll need to match on the request type here.
// For now, we only support inclusion requests so the flow is straightforward.
let CommitmentRequest::Inclusion(mut inclusion_request) = request;
let target_slot = inclusion_request.slot;

let available_pubkeys = self.constraint_signer.available_pubkeys();

// Determine the constraint signing public key for this request. Rationale:
// - If we're skipping consensus checks, we can use any available pubkey in the keystore.
// - On regular operation, we need to validate the request against the consensus state to
// determine if the sidecar is the proposer for the given slot. If so, we use the
// validator pubkey or any of its active delegatees to sign constraints.
let signing_pubkey = if self.unsafe_skip_consensus_checks {
// PERF: this is inefficient, but it's only used for testing purposes.
let mut ap = available_pubkeys.iter().collect::<Vec<_>>();
ap.sort();
ap.first().cloned().cloned().expect("at least one available pubkey")
} else {
let validator_pubkey = match self.consensus.validate_request(&inclusion_request) {
Ok(pubkey) => pubkey,
Err(err) => {
warn!(?err, "Consensus: failed to validate request");
let _ = response.send(Err(CommitmentError::Consensus(err)));
return;
}
};

// Find a public key to sign new constraints with for this slot.
// This can either be the validator pubkey or a delegatee (if one is available).
let Some(signing_key) =
self.constraints_client.find_signing_key(validator_pubkey, available_pubkeys)
else {
error!(%target_slot, "No available public key to sign constraints with");
let _ = response.send(Err(CommitmentError::Internal));
return;
}
};

signing_key
};

if let Err(err) = self.execution.validate_request(&mut request).await {
if let Err(err) = self.execution.validate_request(&mut inclusion_request).await {
warn!(?err, "Execution: failed to validate request");
ApiMetrics::increment_validation_errors(err.to_tag_str().to_owned());
let _ = response.send(Err(CommitmentError::Validation(err)));
return;
}

// When we'll add more commitment types, we'll need to match on the request type here.
// For now, we only support inclusion requests so the flow is straightforward.
let CommitmentRequest::Inclusion(inclusion_request) = request.clone();
let target_slot = inclusion_request.slot;

info!(
target_slot,
elapsed = ?start.elapsed(),
"Validation against execution state passed"
);

let available_pubkeys = self.constraint_signer.available_pubkeys();

// Find a public key to sign new constraints with for this slot.
// This can either be the validator pubkey or a delegatee (if one is available).
let Some(signing_pubkey) =
self.constraints_client.find_signing_key(validator_pubkey, available_pubkeys)
else {
error!(%target_slot, "No available public key to sign constraints with");
let _ = response.send(Err(CommitmentError::Internal));
return;
};

// NOTE: we iterate over the transactions in the request and generate a signed constraint
// for each one. This is because the transactions in the commitment request are not supposed
// to be treated as a relative-ordering bundle, but a batch with no ordering guarantees.
//
// For more information, check out the constraints API docs:
// https://docs.boltprotocol.xyz/technical-docs/api/builder#constraints
for tx in inclusion_request.txs {
for tx in inclusion_request.txs.iter() {
let tx_type = tx.tx_type();
let message = ConstraintsMessage::from_tx(signing_pubkey.clone(), target_slot, tx);
let message =
ConstraintsMessage::from_tx(signing_pubkey.clone(), target_slot, tx.clone());
let digest = message.digest();

let signature_result = match &self.constraint_signer {
SignerBLS::Local(signer) => signer.sign_commit_boost_root(digest),
SignerBLS::CommitBoost(signer) => signer.sign_commit_boost_root(digest).await,
SignerBLS::Keystore(signer) => {
signer.sign_commit_boost_root(digest, signing_pubkey.clone())
signer.sign_commit_boost_root(digest, &signing_pubkey)
}
};

Expand All @@ -346,10 +367,10 @@ impl<C: StateFetcher, ECDSA: SignerECDSA> SidecarDriver<C, ECDSA> {
}

// Create a commitment by signing the request
match request.commit_and_sign(&self.commitment_signer).await {
match inclusion_request.commit_and_sign(&self.commitment_signer).await {
Ok(commitment) => {
debug!(target_slot, elapsed = ?start.elapsed(), "Commitment signed and sent");
response.send(Ok(commitment)).ok()
response.send(Ok(SignedCommitment::Inclusion(commitment))).ok()
}
Err(err) => {
error!(?err, "Failed to sign commitment");
Expand Down
14 changes: 11 additions & 3 deletions bolt-sidecar/src/primitives/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ impl CommitmentRequest {
) -> eyre::Result<SignedCommitment> {
match self {
CommitmentRequest::Inclusion(req) => {
let digest = req.digest();
let signature = signer.sign_hash(&digest).await?;
Ok(SignedCommitment::Inclusion(InclusionCommitment { request: req, signature }))
req.commit_and_sign(signer).await.map(SignedCommitment::Inclusion)
}
}
}
Expand Down Expand Up @@ -97,6 +95,16 @@ pub struct InclusionRequest {
}

impl InclusionRequest {
/// Commits and signs the request with the provided signer. Returns an [InclusionCommitment].
pub async fn commit_and_sign<S: SignerECDSA>(
self,
signer: &S,
) -> eyre::Result<InclusionCommitment> {
let digest = self.digest();
let signature = signer.sign_hash(&digest).await?;
Ok(InclusionCommitment { request: self, signature })
}

/// Validates the transaction fees against a minimum basefee.
/// Returns true if the fee is greater than or equal to the min, false otherwise.
pub fn validate_basefee(&self, min: u128) -> bool {
Expand Down
16 changes: 5 additions & 11 deletions bolt-sidecar/src/signer/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ use lighthouse_bls::Keypair;
use lighthouse_eth2_keystore::Keystore;
use ssz::Encode;

use crate::{
builder::signature::compute_signing_root,
config::ChainConfig,
crypto::bls::{cl_public_key_to_arr, BLSSig},
};
use crate::{builder::signature::compute_signing_root, config::ChainConfig, crypto::bls::BLSSig};

use super::SignerResult;

Expand Down Expand Up @@ -116,7 +112,7 @@ impl KeystoreSigner {
pub fn sign_commit_boost_root(
&self,
root: [u8; 32],
public_key: BlsPublicKey,
public_key: &BlsPublicKey,
) -> SignerResult<BLSSig> {
self.sign_root(root, public_key, self.chain.commit_boost_domain())
}
Expand All @@ -125,17 +121,15 @@ impl KeystoreSigner {
fn sign_root(
&self,
root: [u8; 32],
public_key: BlsPublicKey,
public_key: &BlsPublicKey,
domain: [u8; 32],
) -> SignerResult<BLSSig> {
let sk = self
.keypairs
.iter()
// `as_ssz_bytes` returns the raw bytes we need
.find(|kp| kp.pk.as_ssz_bytes() == public_key.as_ref())
.ok_or(KeystoreError::UnknownPublicKey(hex::encode(cl_public_key_to_arr(
public_key,
))))?;
.ok_or(KeystoreError::UnknownPublicKey(public_key.to_string()))?;

let signing_root = compute_signing_root(root, domain);

Expand Down Expand Up @@ -377,7 +371,7 @@ mod tests {
let sig_keystore = keystore_signer_from_password
.sign_commit_boost_root(
[0; 32],
BlsPublicKey::try_from(public_key_bytes.as_ref()).unwrap(),
&BlsPublicKey::try_from(public_key_bytes.as_ref()).unwrap(),
)
.expect("to sign message");
assert_eq!(sig_local, sig_keystore);
Expand Down
9 changes: 2 additions & 7 deletions bolt-sidecar/src/state/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use tracing::debug;
use super::CommitmentDeadline;
use crate::{
client::BeaconClient,
primitives::{CommitmentRequest, Slot},
primitives::{InclusionRequest, Slot},
telemetry::ApiMetrics,
};

Expand Down Expand Up @@ -112,12 +112,7 @@ impl ConsensusState {
/// 2. The request hasn't passed the slot deadline.
///
/// If the request is valid, return the validator public key for the target slot.
pub fn validate_request(
&self,
request: &CommitmentRequest,
) -> Result<BlsPublicKey, ConsensusError> {
let CommitmentRequest::Inclusion(req) = request;

pub fn validate_request(&self, req: &InclusionRequest) -> Result<BlsPublicKey, ConsensusError> {
// Check if the slot is in the current epoch or next epoch (if unsafe lookahead is enabled)
if req.slot < self.epoch.start_slot || req.slot >= self.furthest_slot() {
return Err(ConsensusError::InvalidSlot(req.slot));
Expand Down
Loading

0 comments on commit 55b66b3

Please sign in to comment.