Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: build local payload with timestamp from beacon clock #194

Merged
merged 4 commits into from
Aug 9, 2024
Merged

fix: build local payload with timestamp from beacon clock #194

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9bad77c
fix: build local payload with timestamp from beacon clock
merklefruit Aug 9, 2024
7816452
chore: added sidecar version bump check
merklefruit Aug 9, 2024
c2d0441
chore: restore expected withdrawals at head
merklefruit Aug 9, 2024
a6561db
chore: addressed review comments
merklefruit Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions bolt-sidecar/src/builder/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloy::primitives::U256;
use beacon_api_client::mainnet::Client as BeaconClient;
use blst::min_pk::SecretKey;
use ethereum_consensus::{
crypto::{KzgCommitment, PublicKey},
Expand Down Expand Up @@ -37,7 +38,7 @@ pub mod payload_builder;
#[allow(missing_docs)]
pub enum BuilderError {
#[error("Failed to parse from integer: {0}")]
Parse(#[from] std::num::ParseIntError),
ParseInt(#[from] std::num::ParseIntError),
#[error("Failed to de/serialize JSON: {0}")]
Json(#[from] serde_json::Error),
#[error("Failed to decode hex: {0}")]
Expand Down Expand Up @@ -77,10 +78,10 @@ pub struct LocalBuilder {

impl LocalBuilder {
/// Create a new local builder with the given secret key.
pub fn new(config: &Config) -> Self {
pub fn new(config: &Config, beacon_api_client: BeaconClient, genesis_time: u64) -> Self {
Self {
payload_and_bid: None,
fallback_builder: FallbackPayloadBuilder::new(config),
fallback_builder: FallbackPayloadBuilder::new(config, beacon_api_client, genesis_time),
secret_key: config.builder_private_key.clone(),
chain: config.chain.clone(),
}
Expand All @@ -90,6 +91,7 @@ impl LocalBuilder {
/// cache the payload in the local builder instance, and make it available
pub async fn build_new_local_payload(
&mut self,
slot: u64,
template: &BlockTemplate,
) -> Result<(), BuilderError> {
let transactions = template.as_signed_transactions();
Expand All @@ -98,7 +100,7 @@ impl LocalBuilder {

// 1. build a fallback payload with the given transactions, on top of
// the current head of the chain
let sealed_block = self.fallback_builder.build_fallback_payload(&transactions).await?;
let block = self.fallback_builder.build_fallback_payload(slot, &transactions).await?;

// NOTE: we use a big value for the bid to ensure it gets chosen by mev-boost.
// the client has no way to actually verify this, and we don't need to trust
Expand All @@ -108,11 +110,11 @@ impl LocalBuilder {
// to ALWAYS prefer PBS blocks. This is a safety measure that doesn't hurt to keep.
let value = U256::from(100_000_000_000_000_000_000u128);

let eth_payload = compat::to_consensus_execution_payload(&sealed_block);
let eth_payload = compat::to_consensus_execution_payload(&block);
let payload_and_blobs = PayloadAndBlobs { execution_payload: eth_payload, blobs_bundle };

// 2. create a signed builder bid with the sealed block header we just created
let eth_header = compat::to_execution_payload_header(&sealed_block, transactions);
let eth_header = compat::to_execution_payload_header(&block, transactions);

// 3. sign the bid with the local builder's BLS key
let signed_bid = self.create_signed_builder_bid(value, eth_header, kzg_commitments)?;
Expand Down
109 changes: 69 additions & 40 deletions bolt-sidecar/src/builder/payload_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ pub struct FallbackPayloadBuilder {
beacon_api_client: BeaconClient,
execution_rpc_client: RpcClient,
engine_hinter: EngineHinter,
slot_time_in_seconds: u64,
slot_time: u64,
genesis_time: u64,
}

impl FallbackPayloadBuilder {
/// Create a new fallback payload builder
pub fn new(config: &Config) -> Self {
pub fn new(config: &Config, beacon_api_client: BeaconClient, genesis_time: u64) -> Self {
let engine_hinter = EngineHinter {
client: reqwest::Client::new(),
jwt_hex: config.jwt_hex.to_string(),
Expand All @@ -64,9 +65,10 @@ impl FallbackPayloadBuilder {
engine_hinter,
extra_data: DEFAULT_EXTRA_DATA.into(),
fee_recipient: config.fee_recipient,
beacon_api_client: BeaconClient::new(config.beacon_api_url.clone()),
execution_rpc_client: RpcClient::new(config.execution_api_url.clone()),
slot_time_in_seconds: config.chain.slot_time(),
slot_time: config.chain.slot_time(),
genesis_time,
beacon_api_client,
}
}
}
Expand All @@ -86,7 +88,7 @@ struct Context {
transactions_root: B256,
withdrawals_root: B256,
parent_beacon_block_root: B256,
slot_time_in_seconds: u64,
block_timestamp: u64,
}

#[derive(Debug, Default)]
Expand All @@ -103,43 +105,20 @@ impl FallbackPayloadBuilder {
/// to provide a valid payload that fulfills the commitments made by Bolt.
pub async fn build_fallback_payload(
&self,
target_slot: u64,
transactions: &[TransactionSigned],
) -> Result<SealedBlock, BuilderError> {
// TODO: what if the latest block ends up being reorged out?
// We fetch the latest block to get the necessary parent values for the new block.
// For the timestamp, we must use the one expected by the beacon chain instead, to
// prevent edge cases where the proposer before us has missed their slot.
let latest_block = self.execution_rpc_client.get_block(None, true).await?;
debug!(num = ?latest_block.header.number, "got latest block");

let withdrawals = self
.beacon_api_client
// Slot: Defaults to the slot after the parent state if not specified.
.get_expected_withdrawals(StateId::Head, None)
.await?
.into_iter()
.map(to_reth_withdrawal)
.collect::<Vec<_>>();

let withdrawals = self.get_withdrawals_for_slot(target_slot).await?;
debug!(amount = ?withdrawals.len(), "got withdrawals");

// let prev_randao = self
// .beacon_api_client
// .get_randao(StateId::Head, None)
// .await?;
// let prev_randao = B256::from_slice(&prev_randao);

// NOTE: for some reason, this call fails with an ApiResult deserialization error
// when using the beacon_api_client crate directly, so we use reqwest temporarily.
// this is to be refactored.
let prev_randao = reqwest::Client::new()
.get(self.beacon_api_client.endpoint.join("/eth/v1/beacon/states/head/randao").unwrap())
.send()
.await
.unwrap()
.json::<Value>()
.await
.unwrap();
let prev_randao = prev_randao.pointer("/data/randao").unwrap().as_str().unwrap();
let prev_randao = B256::from_hex(prev_randao).unwrap();
debug!("got prev_randao");
let prev_randao = self.get_prev_randao().await?;
debug!(randao = ?prev_randao, "got prev_randao");

let parent_beacon_block_root =
self.beacon_api_client.get_beacon_block_root(BlockId::Head).await?;
Expand Down Expand Up @@ -167,6 +146,11 @@ impl FallbackPayloadBuilder {
let blob_gas_used =
transactions.iter().fold(0, |acc, tx| acc + tx.blob_gas_used().unwrap_or_default());

// We must calculate the next block timestamp manually rather than rely on the
// previous execution block, to cover the edge case where any previous slots have
// been missed by the proposers immediately before us.
let block_timestamp = self.genesis_time + (target_slot * self.slot_time);
merklefruit marked this conversation as resolved.
Show resolved Hide resolved

let ctx = Context {
base_fee,
blob_gas_used,
Expand All @@ -177,7 +161,7 @@ impl FallbackPayloadBuilder {
fee_recipient: self.fee_recipient,
transactions_root: proofs::calculate_transaction_root(transactions),
withdrawals_root: proofs::calculate_withdrawals_root(&withdrawals),
slot_time_in_seconds: self.slot_time_in_seconds,
block_timestamp,
};

let body = BlockBody {
Expand Down Expand Up @@ -243,6 +227,43 @@ impl FallbackPayloadBuilder {
i += 1;
}
}

/// Fetch the previous RANDAO value from the beacon chain.
///
/// NOTE: for some reason, using the ApiResult from `beacon_api_client` doesn't work, so
/// we are making a direct request to the beacon client endpoint.
Comment on lines +231 to +234
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm just curious to know some more detail

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it has to do with the type that the beacon_api_client crate expects to deserialize from the result of that API call - maybe it has changed or it's not updated to what Lighthouse is responding, so it fails to deserialize.

This is probably a bug in the beacon_api_client crate - but I haven't investigated

async fn get_prev_randao(&self) -> Result<B256, BuilderError> {
let url = self
.beacon_api_client
.endpoint
.join("/eth/v1/beacon/states/head/randao")
.map_err(|e| BuilderError::Custom(format!("Failed to join URL: {e:?}")))?;

reqwest::Client::new()
.get(url)
.send()
.await?
.json::<Value>()
.await?
.pointer("/data/randao")
.and_then(|value| value.as_str())
.map(|value| B256::from_hex(value).map_err(BuilderError::Hex))
.ok_or_else(|| BuilderError::Custom("Failed to fetch prev_randao".to_string()))?
}

/// Fetch the expected withdrawals for the given slot from the beacon chain.
async fn get_withdrawals_for_slot(
&self,
slot: u64,
) -> Result<Vec<reth_primitives::Withdrawal>, BuilderError> {
merklefruit marked this conversation as resolved.
Show resolved Hide resolved
Ok(self
.beacon_api_client
.get_expected_withdrawals(StateId::Head, Some(slot))
merklefruit marked this conversation as resolved.
Show resolved Hide resolved
.await?
.into_iter()
.map(to_reth_withdrawal)
.collect::<Vec<_>>())
}
}

/// Engine API hint values that can be fetched from the engine API
Expand Down Expand Up @@ -370,7 +391,7 @@ fn build_header_with_hints_and_context(
number: latest_block.header.number.unwrap_or_default() + 1,
gas_limit: latest_block.header.gas_limit as u64,
gas_used,
timestamp: latest_block.header.timestamp + context.slot_time_in_seconds,
timestamp: context.block_timestamp,
mix_hash: context.prev_randao,
nonce: BEACON_NONCE,
base_fee_per_gas: Some(context.base_fee),
Expand All @@ -388,19 +409,21 @@ impl fmt::Debug for FallbackPayloadBuilder {
.field("extra_data", &self.extra_data)
.field("fee_recipient", &self.fee_recipient)
.field("engine_hinter", &self.engine_hinter)
.field("slot_time_in_seconds", &self.slot_time_in_seconds)
.finish()
}
}

#[cfg(test)]
mod tests {
use std::time::{SystemTime, UNIX_EPOCH};

use alloy::{
eips::eip2718::Encodable2718,
network::{EthereumWallet, TransactionBuilder},
primitives::{hex, Address},
signers::{k256::ecdsa::SigningKey, local::PrivateKeySigner},
};
use beacon_api_client::mainnet::Client as BeaconClient;
use reth_primitives::TransactionSigned;
use tracing::warn;

Expand All @@ -420,7 +443,9 @@ mod tests {

let raw_sk = std::env::var("PRIVATE_KEY")?;

let builder = FallbackPayloadBuilder::new(&cfg);
let beacon_client = BeaconClient::new(cfg.beacon_api_url.clone());
let genesis_time = beacon_client.get_genesis_details().await?.genesis_time;
let builder = FallbackPayloadBuilder::new(&cfg, beacon_client, genesis_time);

let sk = SigningKey::from_slice(hex::decode(raw_sk)?.as_slice())?;
let signer = PrivateKeySigner::from_signing_key(sk.clone());
Expand All @@ -432,7 +457,11 @@ mod tests {
let raw_encoded = tx_signed.encoded_2718();
let tx_signed_reth = TransactionSigned::decode_enveloped(&mut raw_encoded.as_slice())?;

let block = builder.build_fallback_payload(&[tx_signed_reth]).await?;
let slot = genesis_time +
(SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs() / cfg.chain.slot_time()) +
1;

let block = builder.build_fallback_payload(slot, &[tx_signed_reth]).await?;
assert_eq!(block.body.len(), 1);

Ok(())
Expand Down
45 changes: 18 additions & 27 deletions bolt-sidecar/src/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,44 +87,29 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,
fetcher: C,
) -> eyre::Result<Self> {
let mevboost_client = MevBoostClient::new(cfg.mevboost_url.clone());
let execution = ExecutionState::new(fetcher, cfg.limits).await?;
let local_builder = LocalBuilder::new(&cfg);

let beacon_client = BeaconClient::new(cfg.beacon_api_url.clone());
let execution = ExecutionState::new(fetcher, cfg.limits).await?;

// start the commitments api server
let api_addr = format!("0.0.0.0:{}", cfg.rpc_port);
let (api_events_tx, api_events_rx) = mpsc::channel(1024);
CommitmentsApiServer::new(api_addr).run(api_events_tx).await;

// TODO: this can be replaced with ethereum_consensus::clock::from_system_time()
// but using beacon node events is easier to work on a custom devnet for now
// (as we don't need to specify genesis time and slot duration)
let head_tracker = HeadTracker::start(beacon_client.clone());

// Get the genesis time.
let genesis_time = beacon_client.get_genesis_details().await?.genesis_time;
let slot_stream =
clock::from_system_time(genesis_time, cfg.chain.slot_time(), SLOTS_PER_EPOCH)
.into_stream();

// TODO: head tracker initializes the genesis timestamp with '0' value
// we should add an async fn to fetch the value for safety
// Initialize the consensus clock.
let consensus_clock =
clock::from_system_time(genesis_time, cfg.chain.slot_time(), SLOTS_PER_EPOCH);
let slot_stream = consensus_clock.into_stream();
let local_builder = LocalBuilder::new(&cfg, beacon_client.clone(), genesis_time);
let head_tracker = HeadTracker::start(beacon_client.clone());

let consensus = ConsensusState::new(
beacon_client,
cfg.validator_indexes.clone(),
cfg.chain.commitment_deadline(),
);

let (payload_requests_tx, payload_requests_rx) = mpsc::channel(16);
let builder_proxy_cfg = BuilderProxyConfig {
mevboost_url: cfg.mevboost_url.clone(),
server_port: cfg.mevboost_proxy_port,
};

let (payload_requests_tx, payload_requests_rx) = mpsc::channel(16);

// start the builder api proxy server
tokio::spawn(async move {
let payload_fetcher = LocalPayloadFetcher::new(payload_requests_tx);
Expand All @@ -133,6 +118,11 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,
}
});

// start the commitments api server
let api_addr = format!("0.0.0.0:{}", cfg.rpc_port);
let (api_events_tx, api_events_rx) = mpsc::channel(1024);
CommitmentsApiServer::new(api_addr).run(api_events_tx).await;

Ok(SidecarDriver {
head_tracker,
execution,
Expand Down Expand Up @@ -241,7 +231,8 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,
}
}

/// Handle a commitment deadline event, submitting constraints to the MEV-Boost service.
/// Handle a commitment deadline event, submitting constraints to the MEV-Boost service
/// and starting to build a local payload for the given target slot.
async fn handle_commitment_deadline(&mut self, slot: u64) {
debug!(slot, "Commitment deadline reached, building local block");

Expand All @@ -250,6 +241,10 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,
return;
};

if let Err(e) = self.local_builder.build_new_local_payload(slot, template).await {
error!(err = ?e, "Error while building local payload at deadline for slot {slot}");
};

// TODO: fix retry logic, and move this to separate task in the mevboost client itself
let constraints = template.signed_constraints_list.clone();
let mevboost = self.mevboost_client.clone();
Expand All @@ -266,10 +261,6 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,
}
}
});

if let Err(e) = self.local_builder.build_new_local_payload(template).await {
tracing::error!(err = ?e, "CRITICAL: Error while building local payload at slot deadline for {slot}");
};
}

/// Handle a fetch payload request, responding with the local payload if available.
Expand Down
Loading