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
2 changes: 1 addition & 1 deletion bolt-sidecar/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
/// The version of the Bolt sidecar binary.
pub const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");

/// Calculates the max_basefee `slot_diff` blocks in the future given a current basefee (in gwei).
/// Calculates the max_basefee `slot_diff` blocks in the future given a current basefee (in wei).
/// Returns None if an overflow would occur.
/// Cfr. https://github.com/flashbots/ethers-provider-flashbots-bundle/blob/7ddaf2c9d7662bef400151e0bfc89f5b13e72b4c/src/index.ts#L308
mempirate marked this conversation as resolved.
Show resolved Hide resolved
pub fn calculate_max_basefee(current: u128, block_diff: u64) -> Option<u128> {
Expand Down
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 = 1000000000; // 1 Gwei
mempirate marked this conversation as resolved.
Show resolved Hide resolved

/// 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 in wei
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
19 changes: 14 additions & 5 deletions bolt-sidecar/src/primitives/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,28 @@ impl InclusionRequest {
true
}

pub fn validate_priority_fee(&self) -> bool {
/// Validates the priority fee against the max fee per gas.
/// Returns true if the fee is less than or equal to the max fee per gas, false otherwise.
/// Ref: https://github.com/paradigmxyz/reth/blob/2d592125128c3742ff97b321884f93f9063abcb2/crates/transaction-pool/src/validate/eth.rs#L242
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())
{
if tx.max_priority_fee_per_gas() > Some(tx.max_fee_per_gas()) {
return false;
}
}

true
}

/// Validates the priority fee against a minimum priority fee.
/// Returns `true` if the "effective priority fee" is greater than or equal to the set minimum
/// priority fee, `false` otherwise.
pub fn validate_min_priority_fee(&self, max_base_fee: u128, min_priority_fee: u128) -> bool {
self.txs.iter().all(|tx| {
tx.effective_tip_per_gas(max_base_fee).map_or(false, |tip| tip >= min_priority_fee)
})
}

/// Returns the total gas limit of all transactions in this request.
pub fn gas_limit(&self) -> u64 {
self.txs.iter().map(|tx| tx.gas_limit()).sum()
Expand Down
31 changes: 27 additions & 4 deletions bolt-sidecar/src/primitives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,29 @@ impl FullTransaction {
pub fn sender(&self) -> Option<&Address> {
self.sender.as_ref()
}

/// Returns the effective miner gas tip cap (`gasTipCap`) for the given base fee:
/// `min(maxFeePerGas - baseFee, maxPriorityFeePerGas)`
///
/// Returns `None` if the basefee is higher than the [`Transaction::max_fee_per_gas`].
/// Ref: https://github.com/paradigmxyz/reth/blob/2d592125128c3742ff97b321884f93f9063abcb2/crates/primitives/src/transaction/mod.rs#L444
pub fn effective_tip_per_gas(&self, base_fee: u128) -> Option<u128> {
let max_fee_per_gas = self.max_fee_per_gas();

if max_fee_per_gas < base_fee {
return None;
}

// Calculate the difference between max_fee_per_gas and base_fee
let fee = max_fee_per_gas - base_fee;

// Compare the fee with max_priority_fee_per_gas (or gas price for non-EIP1559 transactions)
if let Some(priority_fee) = self.max_priority_fee_per_gas() {
Some(fee.min(priority_fee))
} else {
Some(fee)
}
}
Comment on lines +396 to +412
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

LGTM. Consider adding a comment for clarity.

The implementation looks correct. For better readability, consider adding a brief comment explaining the fee calculation.

 // Calculate the difference between max_fee_per_gas and base_fee
 let fee = max_fee_per_gas - base_fee;
+
+// Effective tip is the minimum of fee and max_priority_fee_per_gas (if available)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn effective_tip_per_gas(&self, base_fee: u128) -> Option<u128> {
let max_fee_per_gas = self.max_fee_per_gas();
if max_fee_per_gas < base_fee {
return None;
}
// Calculate the difference between max_fee_per_gas and base_fee
let fee = max_fee_per_gas - base_fee;
// Compare the fee with max_priority_fee_per_gas (or gas price for non-EIP1559 transactions)
if let Some(priority_fee) = self.max_priority_fee_per_gas() {
Some(fee.min(priority_fee))
} else {
Some(fee)
}
}
pub fn effective_tip_per_gas(&self, base_fee: u128) -> Option<u128> {
let max_fee_per_gas = self.max_fee_per_gas();
if max_fee_per_gas < base_fee {
return None;
}
// Calculate the difference between max_fee_per_gas and base_fee
let fee = max_fee_per_gas - base_fee;
// Effective tip is the minimum of fee and max_priority_fee_per_gas (if available)
// Compare the fee with max_priority_fee_per_gas (or gas price for non-EIP1559 transactions)
if let Some(priority_fee) = self.max_priority_fee_per_gas() {
Some(fee.min(priority_fee))
} else {
Some(fee)
}
}

}

fn serialize_txs<S: serde::Serializer>(
Expand Down Expand Up @@ -438,8 +461,8 @@ pub struct DelegationMessage {
impl SignableBLS for DelegationMessage {
fn digest(&self) -> [u8; 32] {
let mut hasher = Sha256::new();
hasher.update(&self.validator_pubkey.to_vec());
hasher.update(&self.delegatee_pubkey.to_vec());
hasher.update(self.validator_pubkey.to_vec());
hasher.update(self.delegatee_pubkey.to_vec());

hasher.finalize().into()
}
Expand All @@ -460,8 +483,8 @@ pub struct RevocationMessage {
impl SignableBLS for RevocationMessage {
fn digest(&self) -> [u8; 32] {
let mut hasher = Sha256::new();
hasher.update(&self.validator_pubkey.to_vec());
hasher.update(&self.delegatee_pubkey.to_vec());
hasher.update(self.validator_pubkey.to_vec());
hasher.update(self.delegatee_pubkey.to_vec());

hasher.finalize().into()
}
Expand Down
119 changes: 113 additions & 6 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,7 +280,7 @@ 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);
}

Expand All @@ -295,6 +298,11 @@ impl<C: StateFetcher> ExecutionState<C> {
return Err(ValidationError::BaseFeeTooLow(max_basefee));
}

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

if target_slot < self.slot {
debug!(%target_slot, %self.slot, "Target slot lower than current slot");
return Err(ValidationError::SlotTooLow(self.slot));
Expand Down Expand Up @@ -523,14 +531,16 @@ 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 reth_primitives::constants::GWEI_TO_WEI;
use tracing::info;

use crate::{
crypto::{bls::Signer, SignableBLS, SignerBLS},
Expand Down Expand Up @@ -752,7 +762,13 @@ mod tests {
let anvil = launch_anvil();
let client = StateClient::new(anvil.endpoint_url());

let mut state = ExecutionState::new(client.clone(), Limits::default()).await?;
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(200000000).unwrap(), // 0.2 gwei
};

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

let basefee = state.basefee();

Expand Down Expand Up @@ -785,9 +801,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(2000000000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

Expand All @@ -810,6 +827,95 @@ mod tests {
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 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(2 * GWEI_TO_WEI as u128).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(GWEI_TO_WEI as u128);

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

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

// Create a transaction with a max priority fee that is correct
let tx = default_test_transaction(*sender, None)
.with_max_priority_fee_per_gas(3 * GWEI_TO_WEI as u128);

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

assert!(state.validate_request(&mut request).await.is_ok());

Ok(())
}

#[tokio::test]
async fn test_invalid_inclusion_request_min_priority_fee_legacy() -> eyre::Result<()> {
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(2 * GWEI_TO_WEI as u128).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?;

let base_fee = state.basefee();
let Some(max_base_fee) = calculate_max_basefee(base_fee, 10 - slot) else {
return Err(eyre::eyre!("Failed to calculate max base fee"));
};

// Create a transaction with a gas price that is too low
let tx = default_test_transaction(*sender, None)
.with_gas_price(max_base_fee + GWEI_TO_WEI as u128);

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

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

// Create a transaction with a gas price that is correct
let tx = default_test_transaction(*sender, None)
.with_gas_price(max_base_fee + 3 * GWEI_TO_WEI as u128);

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

assert!(state.validate_request(&mut request).await.is_ok());

Ok(())
}

#[tokio::test]
async fn test_invalidate_inclusion_request() -> eyre::Result<()> {
let _ = tracing_subscriber::fmt::try_init();
Expand Down Expand Up @@ -916,6 +1022,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(1000000000).unwrap(),
};
let mut state = ExecutionState::new(client.clone(), limits).await?;

Expand Down
4 changes: 2 additions & 2 deletions bolt-sidecar/src/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub(crate) fn default_test_transaction(sender: Address, nonce: Option<u64>) -> T
.with_nonce(nonce.unwrap_or(0))
.with_value(U256::from(100))
.with_gas_limit(21_000)
.with_max_priority_fee_per_gas(1_000_000_000)
.with_max_priority_fee_per_gas(1_000_000_000) // 1 gwei
.with_max_fee_per_gas(20_000_000_000)
}

Expand Down Expand Up @@ -189,7 +189,7 @@ fn random_constraints(count: usize) -> Vec<FullTransaction> {

let req: InclusionRequest = serde_json::from_str(json_req).unwrap();

req.txs.iter().cloned().take(count).collect()
req.txs.iter().take(count).cloned().collect()
}

#[tokio::test]
Expand Down
Loading