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

feat(sidecar): min priority fee #246

Merged
merged 12 commits into from
Oct 2, 2024
22 changes: 20 additions & 2 deletions bolt-sidecar/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ pub const DEFAULT_RPC_PORT: u16 = 8000;
/// Default port for the Constraints proxy server.
pub const DEFAULT_CONSTRAINTS_PROXY_PORT: u16 = 18551;

// Default limit values
pub const DEFAULT_MAX_COMMITMENTS: usize = 128;
pub const DEFAULT_MAX_COMMITTED_GAS: u64 = 10_000_000;
pub const DEFAULT_MIN_PRIORITY_FEE: u128 = 2;

/// Command-line options for the Bolt sidecar
#[derive(Parser, Debug)]
pub struct Opts {
Expand Down Expand Up @@ -55,6 +60,9 @@ pub struct Opts {
/// Max committed gas per slot
#[clap(long, env = "BOLT_SIDECAR_MAX_COMMITTED_GAS")]
pub(super) max_committed_gas: Option<NonZero<u64>>,
/// Min priority fee to accept for a commitment
mempirate marked this conversation as resolved.
Show resolved Hide resolved
#[clap(long, env = "BOLT_SIDECAR_MIN_PRIORITY_FEE")]
pub(super) min_priority_fee: Option<NonZero<u128>>,
/// 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 @@ -158,13 +166,19 @@ 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>,

/// Minimum priority fee to accept for a commitment
pub min_priority_fee: NonZero<u128>,
}

impl Default for Limits {
fn default() -> Self {
Self {
max_commitments_per_slot: NonZero::new(128).expect("Valid non-zero"),
max_committed_gas_per_slot: NonZero::new(10_000_000).expect("Valid non-zero"),
max_commitments_per_slot: NonZero::new(DEFAULT_MAX_COMMITMENTS)
.expect("Valid non-zero"),
max_committed_gas_per_slot: NonZero::new(DEFAULT_MAX_COMMITTED_GAS)
.expect("Valid non-zero"),
min_priority_fee: NonZero::new(DEFAULT_MIN_PRIORITY_FEE).expect("Valid non-zero"),
}
}
}
Expand Down Expand Up @@ -195,6 +209,10 @@ impl TryFrom<Opts> for Config {
config.limits.max_committed_gas_per_slot = max_committed_gas;
}

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

if let Some(commit_boost_url) = &opts.signing.commit_boost_url {
if let Ok(url) = Url::parse(commit_boost_url) {
if let Ok(socket_addrs) = url.socket_addrs(|| None) {
Expand Down
21 changes: 16 additions & 5 deletions bolt-sidecar/src/primitives/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,23 @@ impl InclusionRequest {
true
}

pub fn validate_priority_fee(&self) -> bool {
pub fn validate_max_priority_fee(&self) -> bool {
for tx in &self.txs {
if tx
.max_priority_fee_per_gas()
.is_some_and(|max_priority_fee| max_priority_fee > tx.max_fee_per_gas())
{
let gas_price = tx.as_legacy().map(|tx| tx.gas_price).unwrap_or(0);
let max_priority_fee = tx.max_priority_fee_per_gas().unwrap_or(0);
if max_priority_fee + gas_price > tx.max_fee_per_gas() {
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw this was already there though

Copy link
Collaborator

Choose a reason for hiding this comment

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

oops I removed it, yeah I see it was already present but I still am not sure about the usage of gas_price here

Copy link
Collaborator

Choose a reason for hiding this comment

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

The above code (that was already there) doesn't make sense to me, trying to understand it more

}

true
}

pub fn validate_min_priority_fee(&self, min_priority_fee: u128) -> bool {
for tx in &self.txs {
let gas_price = tx.as_legacy().map(|tx| tx.gas_price).unwrap_or(0);
let max_priority_fee = tx.max_priority_fee_per_gas().unwrap_or(0);
if max_priority_fee + gas_price < min_priority_fee {
return false;
}
}
Expand Down
96 changes: 91 additions & 5 deletions bolt-sidecar/src/state/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ use super::fetcher::StateFetcher;

/// Possible commitment validation errors.
///
/// NOTE: unfortuntately it cannot implement `Clone` due to `BlobTransactionValidationError`
/// not implementing it
/// NOTE: `Clone` not implementable due to `BlobTransactionValidationError`
#[derive(Debug, Error)]
pub enum ValidationError {
/// The transaction fee is too low to cover the maximum base fee.
Expand Down Expand Up @@ -55,6 +54,9 @@ pub enum ValidationError {
/// Max priority fee per gas is greater than max fee per gas.
#[error("Max priority fee per gas is greater than max fee per gas")]
MaxPriorityFeePerGasTooHigh,
/// Max priority fee per gas is less than min priority fee.
#[error("Max priority fee per gas is less than min priority fee")]
MaxPriorityFeePerGasTooLow,
/// The sender does not have enough balance to pay for the transaction.
#[error("Not enough balance to pay for value + maximum fee")]
InsufficientBalance,
Expand Down Expand Up @@ -103,6 +105,7 @@ impl ValidationError {
ValidationError::GasLimitTooHigh => "gas_limit_too_high",
ValidationError::TransactionSizeTooHigh => "transaction_size_too_high",
ValidationError::MaxPriorityFeePerGasTooHigh => "max_priority_fee_per_gas_too_high",
ValidationError::MaxPriorityFeePerGasTooLow => "max_priority_fee_per_gas_too_low",
ValidationError::InsufficientBalance => "insufficient_balance",
ValidationError::Eip4844Limit => "eip4844_limit",
ValidationError::SlotTooLow(_) => "slot_too_low",
Expand Down Expand Up @@ -277,10 +280,15 @@ impl<C: StateFetcher> ExecutionState<C> {
}

// Ensure max_priority_fee_per_gas is less than max_fee_per_gas
if !req.validate_priority_fee() {
if !req.validate_max_priority_fee() {
return Err(ValidationError::MaxPriorityFeePerGasTooHigh);
}

// Ensure max_priority_fee_per_gas is greater than or equal to min_priority_fee
if !req.validate_min_priority_fee(self.limits.min_priority_fee.get()) {
return Err(ValidationError::MaxPriorityFeePerGasTooLow);
}

// Check if the max_fee_per_gas would cover the maximum possible basefee.
let slot_diff = target_slot.saturating_sub(self.slot);

Expand Down Expand Up @@ -523,14 +531,15 @@ mod tests {
use std::{num::NonZero, str::FromStr, time::Duration};

use alloy::{
consensus::constants::ETH_TO_WEI,
consensus::constants::{ETH_TO_WEI, LEGACY_TX_TYPE_ID},
eips::eip2718::Encodable2718,
network::EthereumWallet,
primitives::{uint, Uint},
providers::{network::TransactionBuilder, Provider, ProviderBuilder},
signers::local::PrivateKeySigner,
};
use fetcher::{StateClient, StateFetcher};
use tracing::info;

use crate::{
crypto::{bls::Signer, SignableBLS, SignerBLS},
Expand Down Expand Up @@ -785,9 +794,10 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

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

Expand All @@ -810,6 +820,81 @@ mod tests {
Ok(())
}

#[tokio::test]
async fn test_valid_inclusion_request_legacy_tx() -> eyre::Result<()> {
let _ = tracing_subscriber::fmt::try_init();

let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(5_000_000).unwrap(),
min_priority_fee: NonZero::new(15).unwrap(),
};

let mut state = ExecutionState::new(client.clone(), limits).await?;

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

// initialize the state by updating the head once
let slot = client.get_head().await?;
state.update_head(None, slot).await?;

// Create a legacy transaction with a max priority fee that is too low
let tx = default_test_transaction(*sender, None)
.transaction_type(LEGACY_TX_TYPE_ID)
.with_gas_price(10)
.with_max_priority_fee_per_gas(5);

info!(?tx, "Transaction");

let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?;

assert!(matches!(
state.validate_request(&mut request).await,
Err(ValidationError::MaxPriorityFeePerGasTooLow)
));

Ok(())
}

#[tokio::test]
async fn test_invalid_inclusion_request_min_priority_fee() -> eyre::Result<()> {
mempirate marked this conversation as resolved.
Show resolved Hide resolved
let _ = tracing_subscriber::fmt::try_init();

let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(5_000_000).unwrap(),
min_priority_fee: NonZero::new(10).unwrap(),
};

let mut state = ExecutionState::new(client.clone(), limits).await?;

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

// initialize the state by updating the head once
let slot = client.get_head().await?;
state.update_head(None, slot).await?;

// Create a transaction with a max priority fee that is too low
let tx = default_test_transaction(*sender, None).with_max_priority_fee_per_gas(5);

let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?;

assert!(matches!(
state.validate_request(&mut request).await,
Err(ValidationError::MaxPriorityFeePerGasTooLow)
));

Ok(())
}

#[tokio::test]
async fn test_invalidate_inclusion_request() -> eyre::Result<()> {
let _ = tracing_subscriber::fmt::try_init();
Expand Down Expand Up @@ -916,6 +1001,7 @@ mod tests {
let limits: Limits = Limits {
max_commitments_per_slot: NonZero::new(10).unwrap(),
max_committed_gas_per_slot: NonZero::new(5_000_000).unwrap(),
min_priority_fee: NonZero::new(2).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

Expand Down
Loading