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

fix: update types & drop bundle support #256

Merged
merged 5 commits into from
Oct 3, 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
4 changes: 2 additions & 2 deletions bolt-boost/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ async fn delegate(
State(state): State<PbsState<BuilderState>>,
Json(delegation): Json<SignedDelegation>,
) -> Result<impl IntoResponse, PbsClientError> {
info!(pubkey = %delegation.message.pubkey, validator_index = delegation.message.validator_index, "Delegating signing rights");
info!(delegatee = %delegation.message.delegatee_pubkey, validator = %delegation.message.validator_pubkey, "Delegating signing rights");
post_request(state, DELEGATE_PATH, &delegation).await?;
Ok(StatusCode::OK)
}
Expand All @@ -158,7 +158,7 @@ async fn revoke(
State(state): State<PbsState<BuilderState>>,
Json(revocation): Json<SignedRevocation>,
) -> Result<impl IntoResponse, PbsClientError> {
info!(pubkey = %revocation.message.pubkey, validator_index = revocation.message.validator_index, "Revoking signing rights");
info!(delegatee = %revocation.message.delegatee_pubkey, validator = %revocation.message.validator_pubkey, "Revoking signing rights");
post_request(state, REVOKE_PATH, &revocation).await?;
Ok(StatusCode::OK)
}
Expand Down
8 changes: 4 additions & 4 deletions bolt-boost/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ pub struct SignedDelegation {

#[derive(Debug, Clone, Serialize, Deserialize, Encode, Decode)]
pub struct DelegationMessage {
pub validator_index: u64,
pub pubkey: BlsPublicKey,
pub validator_pubkey: BlsPublicKey,
pub delegatee_pubkey: BlsPublicKey,
}

#[derive(Debug, Clone, Serialize, Deserialize, Encode, Decode)]
Expand All @@ -126,8 +126,8 @@ pub struct SignedRevocation {

#[derive(Debug, Clone, Serialize, Deserialize, Encode, Decode)]
pub struct RevocationMessage {
pub validator_index: u64,
pub pubkey: BlsPublicKey,
pub validator_pubkey: BlsPublicKey,
pub delegatee_pubkey: BlsPublicKey,
}

pub type GetHeaderWithProofsResponse = VersionedResponse<SignedExecutionPayloadHeaderWithProofs>;
Expand Down
33 changes: 19 additions & 14 deletions bolt-sidecar/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,21 +220,26 @@ impl<C: StateFetcher, BLS: SignerBLS, ECDSA: SignerECDSA> SidecarDriver<C, BLS,

// parse the request into constraints and sign them
let slot = inclusion_request.slot;
let message = ConstraintsMessage::build(validator_pubkey, inclusion_request);
let signed_constraints = match self.constraint_signer.sign(&message.digest()).await {
Ok(signature) => SignedConstraints { message, signature },
Err(err) => {
error!(?err, "Failed to sign constraints");
let _ = response.send(Err(CommitmentError::Internal));
return;
}
};

// Track the number of transactions preconfirmed considering their type
signed_constraints.message.transactions.iter().for_each(|full_tx| {
ApiMetrics::increment_transactions_preconfirmed(full_tx.tx_type());
});
self.execution.add_constraint(slot, signed_constraints);
// NOTE: we iterate over the transactions in the request and generate a signed constraint for each one. This is because
// the transactions in the commitment request are not supposed to be treated as a relative-ordering bundle, but a batch
// with no ordering guarantees.
for tx in inclusion_request.txs {
let tx_type = tx.tx_type();
let message = ConstraintsMessage::from_transaction(validator_pubkey.clone(), slot, tx);

let signed_constraints = match self.constraint_signer.sign(&message.digest()).await {
Ok(signature) => SignedConstraints { message, signature },
Err(err) => {
error!(?err, "Failed to sign constraints");
let _ = response.send(Err(CommitmentError::Internal));
return;
}
};

ApiMetrics::increment_transactions_preconfirmed(tx_type);
self.execution.add_constraint(slot, signed_constraints);
thedevbirb marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +231 to +241
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work if a transaction appears twice, first time in a commitment request with just one tx and the second time in a commitment with a batch of two transactions?
Because there is a risk otherwise we increment the nonce of balance of the user two times

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a quick test with a duplicate tx in a batch, which should and does fail with nonce too low. It's not exactly what you described but should cover it.

}

// Create a commitment by signing the request
match request.commit_and_sign(&self.commitment_signer).await {
Expand Down
5 changes: 5 additions & 0 deletions bolt-sidecar/src/primitives/constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ impl ConstraintsMessage {

Self { pubkey, slot: request.slot, top: false, transactions }
}

/// Builds a constraints message from a single transaction.
pub fn from_transaction(pubkey: BlsPublicKey, slot: u64, transaction: FullTransaction) -> Self {
Self { pubkey, slot, top: false, transactions: vec![transaction] }
}
}

impl SignableBLS for ConstraintsMessage {
Expand Down
43 changes: 41 additions & 2 deletions bolt-sidecar/src/state/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ mod tests {
use std::{num::NonZero, str::FromStr, time::Duration};

use alloy::{
consensus::constants::{ETH_TO_WEI, LEGACY_TX_TYPE_ID},
consensus::constants::ETH_TO_WEI,
eips::eip2718::Encodable2718,
network::EthereumWallet,
primitives::{uint, Uint},
Expand All @@ -540,7 +540,6 @@ mod tests {
};
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 @@ -916,6 +915,46 @@ mod tests {
Ok(())
}

#[tokio::test]
async fn test_invalid_inclusion_request_duplicate_batch() -> 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 + 3 * GWEI_TO_WEI as u128);

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

let response = state.validate_request(&mut request).await;
println!("{response:?}");

assert!(matches!(response, Err(ValidationError::NonceTooLow(_, _))));

Ok(())
}

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