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

feat(sidecar): add max_committed_gas_per_slot to sidecar Limits #150

Merged
merged 12 commits into from
Jul 19, 2024
Merged

feat(sidecar): add max_committed_gas_per_slot to sidecar Limits #150

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1470eb8
feat(sidecar): max committed gas wip
namn-grg Jul 19, 2024
c408c2b
chore(sidecar): fix mutable borrow conflict
namn-grg Jul 19, 2024
28a17c5
chore(sidecar): add max gas to cli
namn-grg Jul 19, 2024
2c15cec
chore(sidecar): clean code and address review
namn-grg Jul 19, 2024
7dd705a
chore(sidecar): add tests and fix bug
namn-grg Jul 19, 2024
bdbe371
chore(sidecar): test
namn-grg Jul 19, 2024
75f4dfe
chore(sidecar): test
namn-grg Jul 19, 2024
0a2d652
misc: update comment
namn-grg Jul 19, 2024
c36b542
chore(sidecar): refactor excess gas check
namn-grg Jul 19, 2024
fbe8559
chore(sidecar): fix refactor
namn-grg Jul 19, 2024
6e3aae9
chore(sidecar): solve nits
namn-grg Jul 19, 2024
d2e6a8e
misc: default limit
namn-grg Jul 19, 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
3 changes: 1 addition & 2 deletions bolt-sidecar/bin/sidecar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ async fn main() -> eyre::Result<()> {
let signer = Signer::new(config.private_key.clone().unwrap());

let state_client = StateClient::new(config.execution_api_url.clone());
let mut execution_state =
ExecutionState::new(state_client, config.limits.max_commitments_per_slot).await?;
let mut execution_state = ExecutionState::new(state_client, config.limits.clone()).await?;
namn-grg marked this conversation as resolved.
Show resolved Hide resolved

let mevboost_client = MevBoostClient::new(config.mevboost_url.clone());
let beacon_client = BeaconClient::new(config.beacon_api_url.clone());
Expand Down
12 changes: 12 additions & 0 deletions bolt-sidecar/src/builder/template.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ impl BlockTemplate {
.fold(0, |acc, sc| acc + sc.message.constraints.len())
}

/// Returns the committed gas in the block template.
#[inline]
pub fn committed_gas(&self) -> u64 {
self.signed_constraints_list.iter().fold(0, |acc, sc| {
acc + sc
.message
.constraints
.iter()
.fold(0, |acc, c| acc + &c.transaction.gas_limit())
})
}

/// Returns the blob count of the block template.
#[inline]
pub fn blob_count(&self) -> usize {
Expand Down
9 changes: 9 additions & 0 deletions bolt-sidecar/src/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub struct Opts {
/// Max number of commitments to accept per block
#[clap(short = 'm', long)]
pub(super) max_commitments: Option<NonZero<usize>>,
/// Max committed gas per slot
#[clap(short = 'g', long)]
pub(super) max_committed_gas: Option<NonZero<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")
Expand Down Expand Up @@ -137,12 +140,14 @@ impl Default for Config {
pub struct Limits {
/// Maximum number of commitments to accept per block
pub max_commitments_per_slot: NonZero<usize>,
pub max_committed_gas_per_slot: NonZero<u64>,
}

impl Default for Limits {
fn default() -> Self {
Self {
max_commitments_per_slot: NonZero::new(6).expect("Valid non-zero"),
max_committed_gas_per_slot: NonZero::new(10_000_000).expect("Valid non-zero"),
}
}
}
Expand All @@ -169,6 +174,10 @@ impl TryFrom<Opts> for Config {
config.limits.max_commitments_per_slot = max_commitments;
}

if let Some(max_committed_gas) = opts.max_committed_gas {
config.limits.max_committed_gas_per_slot = max_committed_gas;
}

config.commit_boost_url = opts
.signing
.commit_boost_url
Expand Down
87 changes: 63 additions & 24 deletions bolt-sidecar/src/state/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use thiserror::Error;
use crate::{
builder::BlockTemplate,
common::{calculate_max_basefee, validate_transaction},
config::Limits,
primitives::{AccountState, CommitmentRequest, SignedConstraints, Slot, TransactionExt},
};

Expand Down Expand Up @@ -60,6 +61,9 @@ pub enum ValidationError {
/// The maximum commitments have been reached for the slot.
#[error("Max commitments reached for slot {0}: {1}")]
MaxCommitmentsReachedForSlot(u64, usize),
/// The maximum committed gas has been reached for the slot.
#[error("Max committed gas reached for slot {0}: {1}")]
MaxCommittedGasReachedForSlot(u64, u64),
/// The signature is invalid.
#[error("Signature error: {0:?}")]
Signature(#[from] SignatureError),
Expand Down Expand Up @@ -113,6 +117,8 @@ pub struct ExecutionState<C> {
chain_id: u64,
/// The maximum number of commitments per slot.
max_commitments_per_slot: NonZero<usize>,
/// The maximum committed gas per slot.
max_committed_gas_per_slot: NonZero<u64>,
/// The KZG settings for validating blobs.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it better to make them into a single field - limits?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes in my opinion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

kzg_settings: EnvKzgSettings,
/// The state fetcher client.
Expand Down Expand Up @@ -142,10 +148,7 @@ impl Default for ValidationParams {
impl<C: StateFetcher> ExecutionState<C> {
/// Creates a new state with the given client, initializing the
/// basefee and head block number.
pub async fn new(
client: C,
max_commitments_per_slot: NonZero<usize>,
) -> Result<Self, TransportError> {
pub async fn new(client: C, limits: Limits) -> Result<Self, TransportError> {
let (basefee, blob_basefee, block_number, chain_id) = tokio::try_join!(
client.get_basefee(None),
client.get_blob_basefee(None),
Expand All @@ -158,7 +161,8 @@ impl<C: StateFetcher> ExecutionState<C> {
blob_basefee,
block_number,
chain_id,
max_commitments_per_slot,
max_commitments_per_slot: limits.max_commitments_per_slot,
max_committed_gas_per_slot: limits.max_committed_gas_per_slot,
client,
slot: 0,
account_states: HashMap::new(),
Expand Down Expand Up @@ -200,12 +204,23 @@ impl<C: StateFetcher> ExecutionState<C> {
return Err(ValidationError::ChainIdMismatch);
}

// Check if there is room for more commitments
let max_commitments_per_slot = self.max_commitments_per_slot.get();
let max_committed_gas_per_slot = self.max_committed_gas_per_slot.get();

// Check if there is room for more commitments and gas in the block template
if let Some(template) = self.get_block_template(target_slot) {
if template.transactions_len() >= self.max_commitments_per_slot.get() {
if template.transactions_len() >= max_commitments_per_slot {
return Err(ValidationError::MaxCommitmentsReachedForSlot(
self.slot,
self.max_commitments_per_slot.get(),
max_commitments_per_slot,
));
}

// Check if the committed gas exceeds the maximum
if template.committed_gas() + req.tx.gas_limit() >= max_committed_gas_per_slot {
return Err(ValidationError::MaxCommittedGasReachedForSlot(
self.slot,
max_committed_gas_per_slot,
));
}
}
Expand Down Expand Up @@ -460,8 +475,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand All @@ -486,8 +504,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down Expand Up @@ -527,8 +548,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down Expand Up @@ -580,8 +604,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down Expand Up @@ -611,8 +638,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down Expand Up @@ -674,8 +704,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let basefee = state.basefee();

Expand Down Expand Up @@ -709,8 +742,11 @@ mod tests {
let client = StateClient::new(anvil.endpoint_url());
let provider = ProviderBuilder::new().on_http(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down Expand Up @@ -776,8 +812,11 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let max_comms = NonZero::new(10).unwrap();
let mut state = ExecutionState::new(client.clone(), max_comms).await?;
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(10_000_000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;
namn-grg marked this conversation as resolved.
Show resolved Hide resolved

let sender = anvil.addresses().first().unwrap();
let sender_pk = anvil.keys().first().unwrap();
Expand Down
Loading