Skip to content
Merged

Todo #115

Show file tree
Hide file tree
Changes from all commits
Commits
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
64 changes: 0 additions & 64 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion application/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,4 @@ prometheus-client = "0.22.3"

[features]
prom = ["metrics"]
base-bench = []
bench = []
6 changes: 2 additions & 4 deletions application/src/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ impl<
} else {
let parent_round = if parent.0.get() == 0 {
// Parent view is 0, which means that this is the first block of the epoch
// TODO(matthias): verify that the parent view of the first block is always 0 (nullify)
None
} else {
Some(Round::new(round.epoch(), parent.0))
Expand Down Expand Up @@ -332,7 +331,6 @@ impl<
} else {
let parent_round = if parent.0.get() == 0 {
// Parent view is 0, which means that this is the first block of the epoch
// TODO(matthias): verify that the parent view of the first block is always 0 (nullify)
None
} else {
Some(Round::new(round.epoch(), parent.0))
Expand Down Expand Up @@ -447,7 +445,7 @@ impl<
// Add pending withdrawals to the block
let withdrawals = pending_withdrawals.into_iter().map(|w| w.inner).collect();
let payload_id = {
#[cfg(any(feature = "bench", feature = "base-bench"))]
#[cfg(feature = "bench")]
{
self.engine_client
.start_building_block(
Expand All @@ -458,7 +456,7 @@ impl<
)
.await
}
#[cfg(not(any(feature = "bench", feature = "base-bench")))]
#[cfg(not(feature = "bench"))]
{
self.engine_client
.start_building_block(aux_data.forkchoice, current, withdrawals)
Expand Down
15 changes: 6 additions & 9 deletions finalizer/src/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use summit_types::{
};
use summit_types::{EngineClient, consensus_state::ConsensusState};
use tokio_util::sync::CancellationToken;
use tracing::{debug, info, trace, warn};
use tracing::{debug, error, info, trace, warn};

const WRITE_BUFFER: NonZero<usize> = NZUsize!(1024 * 1024);

Expand Down Expand Up @@ -111,7 +111,7 @@ impl<
ConsensusState,
FinalizerMailbox<bls12381_multisig::Scheme<PublicKey, V>, Block>,
) {
let (tx, rx) = mpsc::channel(cfg.mailbox_size); // todo(dalton) pull mailbox size from config
let (tx, rx) = mpsc::channel(cfg.mailbox_size);
let state_cfg = StateConfig {
log_partition: format!("{}-finalizer_state-log", cfg.db_prefix),
log_write_buffer: WRITE_BUFFER,
Expand Down Expand Up @@ -300,13 +300,14 @@ impl<
self.handle_aux_data_mailbox(height, parent_digest, response).await;
},
FinalizerMessage::GetEpochGenesisHash { epoch, response } => {
// TODO(matthias): verify that this can never happen
// The finalizer sends a message to the orchestrator to start the new epoch.
// The orchestrator will start the new Simplex instance, which will then request
// the epoch genesis hash from the finalizer.
// Since the finalizer increments `self.canonical_state.epoch` before sending the message to the
// orchestrator, the finalizer should never receive a GetEpochGenesisHash request for the wrong epoch.
assert_eq!(epoch, self.canonical_state.epoch);
if epoch != self.canonical_state.epoch {
error!("Finalizer received epoch genesis hash request from a diffent epoch. This should not happen and is a bug. Our epoch: {}, requested epoch {}", self.canonical_state.epoch, epoch);
}
let _ = response.send(self.canonical_state.epoch_genesis_hash);
},
FinalizerMessage::QueryState { request, response } => {
Expand Down Expand Up @@ -439,7 +440,6 @@ impl<
// last block of the epoch arrived out of order.
// This is not critical and will likely never happen on all validators
// at the same time.
// TODO(matthias): figure out a good solution for making checkpoints available
debug_assert!(block.header.digest == finalization.proposal.payload);

// Get participant count from the certificate signers
Expand Down Expand Up @@ -486,7 +486,6 @@ impl<
// The only case where the pending checkpoint doesn't exist here is if the node checkpointed.
// The checkpoint is created at the penultimate block of the epoch, and finalized at the last
// block. So if a node checkpoints, it will start at the height of the penultimate block.
// TODO(matthias): verify this
if let Some(checkpoint) = &self.canonical_state.pending_checkpoint.take() {
debug!(
epoch = self.canonical_state.epoch,
Expand Down Expand Up @@ -796,7 +795,6 @@ impl<
return;
};
let checkpoint_hash = checkpoint.digest;
// TODO(matthias): should we verify the ckpt height against the `height` variable?

// This is not the header from the last block, but the header from
// the block that contains the last checkpoint
Expand Down Expand Up @@ -946,7 +944,6 @@ impl<
warn!(next_epoch, "this node is being removed from validator set");
}

// TODO(matthias): I think this is not necessary. Inactive accounts will be removed after withdrawing.
let key_bytes: [u8; 32] = key.as_ref().try_into().unwrap();
if let Some(account) = self.canonical_state.validator_accounts.get_mut(&key_bytes) {
account.status = ValidatorStatus::Inactive;
Expand Down Expand Up @@ -1575,7 +1572,7 @@ async fn process_execution_requests<
for withdrawal in &block.payload.payload_inner.withdrawals {
let current_epoch = state.epoch;
let pending_withdrawal = state.pop_withdrawal(current_epoch);
// TODO(matthias): these checks should never fail. we have to make sure that these withdrawals are
// these checks should never fail. we have to make sure that these withdrawals are
// verified when the block is verified. it is too late when the block is committed.
let pending_withdrawal = pending_withdrawal.expect("pending withdrawal must be in state");
assert_eq!(pending_withdrawal.inner, *withdrawal);
Expand Down
1 change: 0 additions & 1 deletion node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,5 @@ prom = [
]
tokio-console = ["console-subscriber"]
jemalloc = ["dep:tikv-jemalloc-ctl"]
base-bench = ["summit-application/base-bench", "summit-types/base-bench"]
bench = ["summit-application/bench", "summit-types/bench"]
e2e = ["summit-types/e2e"]
40 changes: 5 additions & 35 deletions node/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ use std::{
path::Path,
str::FromStr as _,
};
#[cfg(feature = "base-bench")]
use summit_types::engine_client::base_benchmarking::HistoricalEngineClient;

#[cfg(feature = "bench")]
use summit_types::engine_client::benchmarking::EthereumHistoricalEngineClient;

use crate::config::MAILBOX_SIZE;
#[cfg(not(any(feature = "bench", feature = "base-bench")))]
#[cfg(not(feature = "bench"))]
use summit_types::RethEngineClient;
use summit_types::bootstrap::Bootstrappers;
use summit_types::keystore::KeyStore;
Expand Down Expand Up @@ -87,7 +85,7 @@ pub struct RunFlags {
#[arg(long, default_value_t = DEFAULT_ENGINE_IPC_PATH.into())]
pub engine_ipc_path: String,
/// Path to the directory containing historical blocks for benchmarking
#[cfg(any(feature = "base-bench", feature = "bench"))]
#[cfg(feature = "bench")]
#[arg(long)]
pub bench_block_dir: Option<String>,
/// Port Consensus runs on
Expand Down Expand Up @@ -116,7 +114,7 @@ pub struct RunFlags {
pub log_level: String,
#[arg(
long,
default_value_t = String::from("quartz")
default_value_t = String::from("summit")
)]
pub db_prefix: String,
/// Path to the genesis file
Expand Down Expand Up @@ -265,21 +263,6 @@ impl Command {
let engine_ipc_path = get_expanded_path(&flags.engine_ipc_path)
.expect("failed to expand engine ipc path");

#[allow(unused)]
#[cfg(feature = "base-bench")]
let engine_client = {
let block_dir = flags
.bench_block_dir
.as_ref()
.map(|p| get_expanded_path(p).expect("Invalid block directory path"))
.expect("bench_block_dir is required when using bench feature");
HistoricalEngineClient::new(
engine_ipc_path.to_string_lossy().to_string(),
block_dir,
)
.await
};

#[allow(unused)]
#[cfg(feature = "bench")]
let engine_client = {
Expand All @@ -295,7 +278,7 @@ impl Command {
.await
};

#[cfg(not(any(feature = "bench", feature = "base-bench")))]
#[cfg(not(feature = "bench"))]
let engine_client =
RethEngineClient::new(engine_ipc_path.to_string_lossy().to_string()).await;

Expand All @@ -321,7 +304,6 @@ impl Command {
context.with_label("telemetry"),
tokio::telemetry::Logging {
level: log_level,
// todo: dont know what this does
json: false,
},
Some(SocketAddr::new(
Expand Down Expand Up @@ -503,18 +485,6 @@ pub fn run_node_local(
let engine_ipc_path =
get_expanded_path(&flags.engine_ipc_path).expect("failed to expand engine ipc path");

#[allow(unused)]
#[cfg(feature = "base-bench")]
let engine_client = {
let block_dir = flags
.bench_block_dir
.as_ref()
.map(|p| get_expanded_path(p).expect("Invalid block directory path"))
.expect("bench_block_dir is required when using bench feature");
HistoricalEngineClient::new(engine_ipc_path.to_string_lossy().to_string(), block_dir)
.await
};

#[allow(unused)]
#[cfg(feature = "bench")]
let engine_client = {
Expand All @@ -530,7 +500,7 @@ pub fn run_node_local(
.await
};

#[cfg(not(any(feature = "bench", feature = "base-bench")))]
#[cfg(not(feature = "bench"))]
let engine_client =
RethEngineClient::new(engine_ipc_path.to_string_lossy().to_string()).await;

Expand Down
11 changes: 2 additions & 9 deletions node/src/bin/execute_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ use clap::{Arg, Command};
use commonware_utils::from_hex_formatted;
use std::path::PathBuf;
use summit_types::engine_client::EngineClient;
#[cfg(feature = "base-bench")]
use summit_types::engine_client::base_benchmarking::HistoricalEngineClient;
#[cfg(feature = "bench")]
use summit_types::engine_client::benchmarking::EthereumHistoricalEngineClient;
use summit_types::{Block, Digest};

#[cfg(all(feature = "base-bench", not(feature = "bench")))]
const GENESIS_HASH: &str = "0xf712aa9241cc24369b143cf6dce85f0902a9731e70d66818a3a5845b296c73dd";
#[cfg(feature = "bench")]
const GENESIS_HASH: &str = "0x655cc1ecc77fe1eab4b1e62a1f461b7fddc9b06109b5ab3e9dc68c144b30c773";

Expand Down Expand Up @@ -68,9 +64,6 @@ async fn main() -> Result<()> {
let start_block: u64 = matches.get_one::<String>("start-block").unwrap().parse()?;
let num_blocks: u64 = matches.get_one::<String>("num-blocks").unwrap().parse()?;

#[allow(unused)]
#[cfg(feature = "base-bench")]
let client = HistoricalEngineClient::new(engine_ipc_path.clone(), block_dir.clone()).await;
#[allow(unused)]
#[cfg(feature = "bench")]
let mut client = EthereumHistoricalEngineClient::new(engine_ipc_path, block_dir).await;
Expand All @@ -89,11 +82,11 @@ async fn main() -> Result<()> {
let mut block_number = start_block;
for _ in 0..num_blocks {
println!("Block number: {}", block_number);
#[cfg(any(feature = "bench", feature = "base-bench"))]
#[cfg(feature = "bench")]
let result = client
.start_building_block(forkchoice, 0, vec![], block_number)
.await;
#[cfg(not(any(feature = "bench", feature = "base-bench")))]
#[cfg(not(feature = "bench"))]
let result = client.start_building_block(forkchoice, 0, vec![]).await;
match result {
Some(payload_id) => {
Expand Down
10 changes: 2 additions & 8 deletions node/src/bin/protocol_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ struct NodeRuntime {
#[derive(Parser, Debug)]
struct Args {
/// Path to the directory containing historical blocks for benchmarking
#[cfg(any(feature = "base-bench", feature = "bench"))]
#[cfg(feature = "bench")]
#[arg(long)]
pub bench_block_dir: Option<String>,
/// Path to the log directory
Expand Down Expand Up @@ -90,7 +90,6 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
context.with_label("metrics"),
cw_tokio::telemetry::Logging {
level: log_level,
// todo: dont know what this does
json: false,
},
Some(SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), 6969)),
Expand Down Expand Up @@ -163,11 +162,6 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
#[allow(unused_mut)]
let mut flags = get_node_flags(x.into());

#[cfg(any(feature = "base-bench", feature = "bench"))]
{
flags.bench_block_dir = args.bench_block_dir.clone();
}

// Start our consensus engine in its own runtime/thread
let (stop_tx, mut stop_rx) = mpsc::unbounded_channel();
let data_dir_clone = args.data_dir.clone();
Expand Down Expand Up @@ -386,7 +380,7 @@ fn get_node_flags(node: usize) -> RunFlags {
db_prefix: format!("{node}-quarts"),
genesis_path: "./example_genesis.toml".into(),
engine_ipc_path: format!("/tmp/reth_engine_api{node}.ipc"),
#[cfg(any(feature = "base-bench", feature = "bench"))]
#[cfg(feature = "bench")]
bench_block_dir: None,
checkpoint_path: None,
checkpoint_or_default: false,
Expand Down
Loading