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.

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

chore(sidecar): add parsing validator indexes as ranges #119

Merged
merged 1 commit into from
Jul 5, 2024
Merged
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
2 changes: 1 addition & 1 deletion bolt-sidecar/bin/sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async fn main() -> eyre::Result<()> {
let shutdown_tx = start_rpc_server(&config, api_events).await?;
let mut consensus_state = ConsensusState::new(
beacon_client.clone(),
&config.validator_indexes,
config.validator_indexes.clone(),
config.chain.commitment_deadline(),
);

Expand Down
19 changes: 13 additions & 6 deletions bolt-sidecar/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fs::read_to_string, path::Path};
use std::{fs::read_to_string, path::Path, str::FromStr};

use alloy_primitives::Address;
use blst::min_pk::SecretKey;
Expand All @@ -7,6 +7,9 @@ use reqwest::Url;

use crate::crypto::bls::random_bls_secret;

pub mod validator_indexes;
pub use validator_indexes::ValidatorIndexes;

pub mod chain;
pub use chain::ChainConfig;

Expand Down Expand Up @@ -43,9 +46,13 @@ pub struct Opts {
/// Max number of commitments to accept per block
#[clap(short = 'm', long)]
pub(super) max_commitments: Option<usize>,
/// Validator indexes
#[clap(short = 'v', long, value_parser, num_args = 1.., value_delimiter = ',')]
pub(super) validator_indexes: Vec<u64>,
/// Validator indexes of connected validators that the sidecar
/// should accept commitments on behalf of. Accepted values:
/// - a comma-separated list of indexes (e.g. "1,2,3,4")
/// - a contiguous range of indexes (e.g. "1..4")
/// - a mix of the above (e.g. "1,2..4,6..8")
#[clap(short = 'v', long, value_parser = ValidatorIndexes::from_str)]
pub(super) validator_indexes: ValidatorIndexes,
/// The JWT secret token to authenticate calls to the engine API.
///
/// It can either be a hex-encoded string or a file path to a file
Expand Down Expand Up @@ -95,7 +102,7 @@ pub struct Config {
pub limits: Limits,
/// Validator indexes of connected validators that the
/// sidecar should accept commitments on behalf of
pub validator_indexes: Vec<u64>,
pub validator_indexes: ValidatorIndexes,
/// Local bulider private key for signing fallback payloads.
/// If not provided, a random key will be used.
pub builder_private_key: SecretKey,
Expand All @@ -118,7 +125,7 @@ impl Default for Config {
fee_recipient: Address::ZERO,
builder_private_key: random_bls_secret(),
limits: Limits::default(),
validator_indexes: Vec::new(),
validator_indexes: ValidatorIndexes::default(),
chain: ChainConfig::default(),
}
}
Expand Down
72 changes: 72 additions & 0 deletions bolt-sidecar/src/config/validator_indexes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use std::str::FromStr;

#[derive(Debug, Clone, Default)]
pub struct ValidatorIndexes(Vec<u64>);

impl ValidatorIndexes {
pub fn contains(&self, index: u64) -> bool {
self.0.contains(&index)
}
}

impl FromStr for ValidatorIndexes {
type Err = eyre::Report;

/// Parse an array of validator indexes. Accepted values:
/// - a comma-separated list of indexes (e.g. "1,2,3,4")
/// - a contiguous range of indexes (e.g. "1..4")
/// - a mix of the above (e.g. "1,2..4,6..8")
///
/// TODO: add parsing from a directory path, using the format of
/// validator definitions
fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.trim();
let mut vec = Vec::new();

for comma_separated_part in s.split(',') {
if comma_separated_part.contains("..") {
let mut parts = comma_separated_part.split("..");

let start = parts.next().ok_or_else(|| eyre::eyre!("Invalid range"))?;
let start = start.parse::<u64>()?;

let end = parts.next().ok_or_else(|| eyre::eyre!("Invalid range"))?;
let end = end.parse::<u64>()?;

vec.extend(start..=end);
} else {
let index = comma_separated_part.parse::<u64>()?;
vec.push(index);
}
}

Ok(Self(vec))
}
Comment on lines +12 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM! The from_str method is correctly implemented.

The method correctly handles different input formats and uses appropriate error handling.

Reminder: Add support for parsing from a directory path.

The TODO comment indicates a missing feature.

Do you want me to implement the parsing from a directory path or open a GitHub issue to track this task?

}

impl From<Vec<u64>> for ValidatorIndexes {
fn from(vec: Vec<u64>) -> Self {
Self(vec)
}
}

#[cfg(test)]
mod tests {
#[test]
fn test_parse_validator_indexes() {
use super::ValidatorIndexes;
use std::str::FromStr;

let indexes = ValidatorIndexes::from_str("1,2,3,4").unwrap();
assert_eq!(indexes.0, vec![1, 2, 3, 4]);

let indexes = ValidatorIndexes::from_str("1..4").unwrap();
assert_eq!(indexes.0, vec![1, 2, 3, 4]);

let indexes = ValidatorIndexes::from_str("1..4,6..8").unwrap();
assert_eq!(indexes.0, vec![1, 2, 3, 4, 6, 7, 8]);

let indexes = ValidatorIndexes::from_str("1,2..4,6..8").unwrap();
assert_eq!(indexes.0, vec![1, 2, 3, 4, 6, 7, 8]);
}
}
14 changes: 6 additions & 8 deletions bolt-sidecar/src/state/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ethereum_consensus::{deneb::BeaconBlockHeader, phase0::mainnet::SLOTS_PER_EP

use super::CommitmentDeadline;
use crate::{
config::ValidatorIndexes,
primitives::{CommitmentRequest, Slot},
BeaconClient,
};
Expand Down Expand Up @@ -39,7 +40,7 @@ pub struct ConsensusState {
beacon_api_client: Client,
header: BeaconBlockHeader,
epoch: Epoch,
validator_indexes: Vec<u64>,
validator_indexes: ValidatorIndexes,
// Timestamp of when the latest slot was received
latest_slot_timestamp: Instant,
// The latest slot received
Expand All @@ -59,15 +60,15 @@ impl ConsensusState {
/// Create a new `ConsensusState` with the given configuration.
pub fn new(
beacon_api_client: BeaconClient,
validator_indexes: &[u64],
validator_indexes: ValidatorIndexes,
commitment_deadline_duration: Duration,
) -> Self {
ConsensusState {
beacon_api_client,
validator_indexes,
header: BeaconBlockHeader::default(),
epoch: Epoch::default(),
latest_slot: Default::default(),
validator_indexes: validator_indexes.to_vec(),
latest_slot_timestamp: Instant::now(),
commitment_deadline: CommitmentDeadline::new(0, commitment_deadline_duration),
commitment_deadline_duration,
Expand Down Expand Up @@ -147,10 +148,7 @@ impl ConsensusState {
.proposer_duties
.iter()
.find(|&duty| {
duty.slot == slot
&& self
.validator_indexes
.contains(&(duty.validator_index as u64))
duty.slot == slot && self.validator_indexes.contains(duty.validator_index as u64)
})
.map(|duty| duty.validator_index as u64)
.ok_or(ConsensusError::ValidatorNotFound)
Expand Down Expand Up @@ -185,7 +183,7 @@ mod tests {
];

// Validator indexes that we are interested in
let validator_indexes = vec![100, 102];
let validator_indexes = ValidatorIndexes::from(vec![100, 102]);

// Create a ConsensusState with the sample proposer duties and validator indexes
let state = ConsensusState {
Expand Down