From 5e8cbdcaa3ca858aade660a1260fd06b191dd5d0 Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Mon, 29 Jul 2024 12:31:18 +0200 Subject: [PATCH 1/4] fix: process pending bundle diffs --- bolt-sidecar/src/state/execution.rs | 37 +++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index d241201e..3d65d4e2 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -11,7 +11,7 @@ use thiserror::Error; use crate::{ builder::BlockTemplate, - common::{calculate_max_basefee, validate_transaction}, + common::{calculate_max_basefee, max_transaction_cost, validate_transaction}, config::Limits, primitives::{AccountState, CommitmentRequest, SignedConstraints, Slot}, }; @@ -273,7 +273,18 @@ impl ExecutionState { return Err(ValidationError::SlotTooLow(self.slot)); } - for tx in &req.txs { + // Validate each transaction in the request against the account state, + // keeping track of the nonce and balance diffs, including: + // - any existing state in the account trie + // - any previously committed transactions + // - any previous transaction in the same request + // + // NOTE: it's also possible for a request to contain multiple transactions + // from different senders, in this case each sender will have its own nonce + // and balance diffs that will be applied to the account state. + let mut bundle_nonce_diff_map = HashMap::new(); + let mut bundle_balance_diff_map = HashMap::new(); + for tx in req.txs.iter() { let sender = tx.sender().expect("Recovered sender"); // From previous preconfirmations requests retrieve @@ -326,12 +337,30 @@ impl ExecutionState { } }; + let sender_nonce_diff = bundle_nonce_diff_map.entry(sender).or_insert(0); + let sender_balance_diff = bundle_balance_diff_map.entry(sender).or_insert(U256::ZERO); + + // Apply the diffs to this account according to the info fetched from the templates + // and the current bundle diffs for this sender. let account_state_with_diffs = AccountState { - transaction_count: account_state.transaction_count.saturating_add(nonce_diff), - balance: account_state.balance.saturating_sub(balance_diff), + transaction_count: account_state + .transaction_count + .saturating_add(nonce_diff) + .saturating_add(*sender_nonce_diff), + + balance: account_state + .balance + .saturating_sub(balance_diff) + .saturating_sub(*sender_balance_diff), + + // TODO(nico): what if this changes in the middle of the request? has_code: account_state.has_code, }; + // Increase the bundle nonce and balance diffs for this sender for the next iteration + *sender_nonce_diff += 1; + *sender_balance_diff += max_transaction_cost(tx); + // Validate the transaction against the account state with existing diffs validate_transaction(&account_state_with_diffs, tx)?; From fe1b0674b4e332edf9176705c7bbf629467cbcc1 Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Mon, 29 Jul 2024 12:52:53 +0200 Subject: [PATCH 2/4] chore: minor fixes --- bolt-client/src/main.rs | 2 +- bolt-sidecar/src/primitives/constraint.rs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bolt-client/src/main.rs b/bolt-client/src/main.rs index bef27c29..0d9edb3d 100644 --- a/bolt-client/src/main.rs +++ b/bolt-client/src/main.rs @@ -121,7 +121,7 @@ async fn main() -> Result<()> { } else { // Send rpc requests singularly for each transaction send_rpc_request( - vec![tx_rlp.clone()], + vec![tx_rlp], vec![*tx_hash], target_slot, target_sidecar_url.clone(), diff --git a/bolt-sidecar/src/primitives/constraint.rs b/bolt-sidecar/src/primitives/constraint.rs index c8c06620..a6459f8c 100644 --- a/bolt-sidecar/src/primitives/constraint.rs +++ b/bolt-sidecar/src/primitives/constraint.rs @@ -55,10 +55,11 @@ pub struct ConstraintsMessage { impl ConstraintsMessage { /// Builds a constraints message from an inclusion request and metadata pub fn build(validator_index: u64, request: InclusionRequest) -> Self { - let mut constraints = Vec::with_capacity(request.txs.len()); - for tx in request.txs { - constraints.push(Constraint::from_transaction(tx, None)); - } + let constraints = request + .txs + .into_iter() + .map(|tx| Constraint::from_transaction(tx, None)) + .collect(); Self { validator_index, From a4dd266b79836dd321ef968da03be861390fbdea Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Mon, 29 Jul 2024 13:30:31 +0200 Subject: [PATCH 3/4] chore: added unit tests --- bolt-sidecar/src/api/commitments/server.rs | 8 +- bolt-sidecar/src/state/execution.rs | 121 ++++++++++++++++++--- bolt-sidecar/src/test_util.rs | 17 +-- 3 files changed, 122 insertions(+), 24 deletions(-) diff --git a/bolt-sidecar/src/api/commitments/server.rs b/bolt-sidecar/src/api/commitments/server.rs index 47bc1581..f73802da 100644 --- a/bolt-sidecar/src/api/commitments/server.rs +++ b/bolt-sidecar/src/api/commitments/server.rs @@ -283,7 +283,9 @@ mod test { let sk = SecretKey::random(&mut rand::thread_rng()); let signer = PrivateKeySigner::from(sk.clone()); let tx = default_test_transaction(signer.address(), None); - let req = create_signed_commitment_request(tx, &sk, 12).await.unwrap(); + let req = create_signed_commitment_request(&[tx], &sk, 12) + .await + .unwrap(); let payload = json!({ "jsonrpc": "2.0", @@ -325,7 +327,9 @@ mod test { let sk = SecretKey::random(&mut rand::thread_rng()); let signer = PrivateKeySigner::from(sk.clone()); let tx = default_test_transaction(signer.address(), None); - let req = create_signed_commitment_request(tx, &sk, 12).await.unwrap(); + let req = create_signed_commitment_request(&[tx], &sk, 12) + .await + .unwrap(); let sig = req.signature().unwrap().to_hex(); diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 3d65d4e2..3a67e450 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -37,7 +37,7 @@ pub enum ValidationError { #[error("Transaction nonce too low. Expected {0}, got {1}")] NonceTooLow(u64, u64), /// The transaction nonce is too high. - #[error("Transaction nonce too high")] + #[error("Transaction nonce too high. Expected {0}, got {1}")] NonceTooHigh(u64, u64), /// The sender account is a smart contract and has code. #[error("Account has code")] @@ -530,7 +530,7 @@ mod tests { let tx = default_test_transaction(*sender, None); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(state .validate_commitment_request(&mut request) @@ -559,7 +559,7 @@ mod tests { // Create a transaction with a nonce that is too high let tx = default_test_transaction(*sender, Some(1)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; // Insert a constraint diff for slot 11 let mut diffs = HashMap::new(); @@ -611,7 +611,7 @@ mod tests { // Create a transaction with a nonce that is too low let tx = default_test_transaction(*sender, Some(0)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -623,7 +623,7 @@ mod tests { // Create a transaction with a nonce that is too high let tx = default_test_transaction(*sender, Some(2)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -653,7 +653,7 @@ mod tests { let tx = default_test_transaction(*sender, None) .with_value(uint!(11_000_U256 * Uint::from(ETH_TO_WEI))); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -687,7 +687,7 @@ mod tests { // burn the balance let tx = default_test_transaction(*sender, Some(0)).with_value(uint!(balance_to_burn)); - let request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; let tx_bytes = request .as_inclusion_request() .unwrap() @@ -704,7 +704,7 @@ mod tests { // create a new transaction and request a preconfirmation for it let tx = default_test_transaction(*sender, Some(1)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(state .validate_commitment_request(&mut request) @@ -718,7 +718,7 @@ mod tests { // create a new transaction and request a preconfirmation for it let tx = default_test_transaction(*sender, Some(2)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; // this should fail because the balance is insufficient as we spent // all of it on the previous preconfirmation @@ -753,7 +753,7 @@ mod tests { .with_max_fee_per_gas(basefee - 1) .with_max_priority_fee_per_gas(basefee / 2); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -785,7 +785,7 @@ mod tests { let tx = default_test_transaction(*sender, None).with_gas_limit(6_000_000); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -820,7 +820,7 @@ mod tests { let signed = tx.clone().build(&signer).await?; let target_slot = 10; - let mut request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, target_slot).await?; let inclusion_request = request.as_inclusion_request().unwrap().clone(); assert!(state @@ -884,7 +884,7 @@ mod tests { let tx = default_test_transaction(*sender, None); let target_slot = 10; - let mut request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, target_slot).await?; let inclusion_request = request.as_inclusion_request().unwrap().clone(); assert!(state @@ -939,7 +939,7 @@ mod tests { let tx = default_test_transaction(*sender, None).with_gas_limit(4_999_999); let target_slot = 10; - let mut request = create_signed_commitment_request(tx, sender_pk, target_slot).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, target_slot).await?; let inclusion_request = request.as_inclusion_request().unwrap().clone(); assert!(state @@ -965,7 +965,7 @@ mod tests { // This tx will exceed the committed gas limit let tx = default_test_transaction(*sender, Some(1)); - let mut request = create_signed_commitment_request(tx, sender_pk, 10).await?; + let mut request = create_signed_commitment_request(&[tx], sender_pk, 10).await?; assert!(matches!( state.validate_commitment_request(&mut request).await, @@ -974,4 +974,95 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn test_valid_bundle_inclusion_request() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let mut state = ExecutionState::new(client.clone(), Limits::default()).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 tx1 = default_test_transaction(*sender, Some(0)); + let tx2 = default_test_transaction(*sender, Some(1)); + let tx3 = default_test_transaction(*sender, Some(2)); + + let mut request = create_signed_commitment_request(&[tx1, tx2, tx3], sender_pk, 10).await?; + + assert!(state + .validate_commitment_request(&mut request) + .await + .is_ok()); + + Ok(()) + } + + #[tokio::test] + async fn test_invalid_bundle_inclusion_request_nonce() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let mut state = ExecutionState::new(client.clone(), Limits::default()).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 tx1 = default_test_transaction(*sender, Some(0)); + let tx2 = default_test_transaction(*sender, Some(1)); + let tx3 = default_test_transaction(*sender, Some(3)); // wrong nonce, should be 2 + + let mut request = create_signed_commitment_request(&[tx1, tx2, tx3], sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&mut request).await, + Err(ValidationError::NonceTooHigh(2, 3)) + )); + + Ok(()) + } + + #[tokio::test] + async fn test_invalid_bundle_inclusion_request_balance() -> eyre::Result<()> { + let _ = tracing_subscriber::fmt::try_init(); + + let anvil = launch_anvil(); + let client = StateClient::new(anvil.endpoint_url()); + + let mut state = ExecutionState::new(client.clone(), Limits::default()).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 tx1 = default_test_transaction(*sender, Some(0)); + let tx2 = default_test_transaction(*sender, Some(1)); + let tx3 = default_test_transaction(*sender, Some(2)) + .with_value(uint!(11_000_U256 * Uint::from(ETH_TO_WEI))); + + let mut request = create_signed_commitment_request(&[tx1, tx2, tx3], sender_pk, 10).await?; + + assert!(matches!( + state.validate_commitment_request(&mut request).await, + Err(ValidationError::InsufficientBalance) + )); + + Ok(()) + } } diff --git a/bolt-sidecar/src/test_util.rs b/bolt-sidecar/src/test_util.rs index fdb64134..d65e7bc9 100644 --- a/bolt-sidecar/src/test_util.rs +++ b/bolt-sidecar/src/test_util.rs @@ -16,7 +16,7 @@ use secp256k1::Message; use crate::{ crypto::{ecdsa::SignableECDSA, SignableBLS}, - primitives::{CommitmentRequest, InclusionRequest}, + primitives::{CommitmentRequest, FullTransaction, InclusionRequest}, Config, }; @@ -144,7 +144,7 @@ impl SignableECDSA for TestSignableData { /// Create a valid signed commitment request for testing purposes /// from the given transaction, private key of the sender, and slot. pub(crate) async fn create_signed_commitment_request( - tx: TransactionRequest, + txs: &[TransactionRequest], sk: &K256SecretKey, slot: u64, ) -> eyre::Result { @@ -152,12 +152,15 @@ pub(crate) async fn create_signed_commitment_request( let signer = PrivateKeySigner::from_signing_key(sk.clone()); let wallet = EthereumWallet::from(signer.clone()); - let tx_signed = tx.build(&wallet).await?; - let raw_encoded = tx_signed.encoded_2718(); - let tx_pooled = PooledTransactionsElement::decode_enveloped(&mut raw_encoded.as_slice())?; - + let mut full_txs = Vec::with_capacity(txs.len()); + for tx in txs { + let tx_signed = tx.clone().build(&wallet).await?; + let raw_encoded = tx_signed.encoded_2718(); + let tx_pooled = PooledTransactionsElement::decode_enveloped(&mut raw_encoded.as_slice())?; + full_txs.push(FullTransaction::from(tx_pooled)); + } let mut request = InclusionRequest { - txs: vec![tx_pooled.into()], + txs: full_txs, slot, signature: None, signer: None, From 066ba20bd03c0490e5ff132c9235ea1fc245f264 Mon Sep 17 00:00:00 2001 From: nicolas <48695862+merklefruit@users.noreply.github.com> Date: Mon, 29 Jul 2024 13:38:04 +0200 Subject: [PATCH 4/4] chore: rm comment --- bolt-sidecar/src/state/execution.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 3a67e450..7d3299c6 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -353,7 +353,6 @@ impl ExecutionState { .saturating_sub(balance_diff) .saturating_sub(*sender_balance_diff), - // TODO(nico): what if this changes in the middle of the request? has_code: account_state.has_code, };