From d1e71875a429171144a9c8cc601087871b8bcb5f Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Mon, 28 Oct 2024 14:15:55 -0300 Subject: [PATCH 1/6] Merge branch 'fix-bootstrap-node-in-localnet' into tx-spammer-and-kurtosis-initial-params --- cmd/ethereum_rust/Cargo.toml | 1 + cmd/ethereum_rust/ethereum_rust.rs | 13 +++++++++++-- test_data/network_params.yaml | 6 +++--- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/ethereum_rust/Cargo.toml b/cmd/ethereum_rust/Cargo.toml index 0d0f2ec47..cd14bad00 100644 --- a/cmd/ethereum_rust/Cargo.toml +++ b/cmd/ethereum_rust/Cargo.toml @@ -26,6 +26,7 @@ tokio = { version = "1.38.0", features = ["full"] } anyhow = "1.0.86" rand = "0.8.5" k256 = { version = "0.13.3", features = ["ecdh"] } +local-ip-address = "0.6" tokio-util.workspace = true cfg-if = "1.0.0" diff --git a/cmd/ethereum_rust/ethereum_rust.rs b/cmd/ethereum_rust/ethereum_rust.rs index af94e955f..4f10fd1bc 100644 --- a/cmd/ethereum_rust/ethereum_rust.rs +++ b/cmd/ethereum_rust/ethereum_rust.rs @@ -8,6 +8,7 @@ use ethereum_rust_net::node_id_from_signing_key; use ethereum_rust_net::types::Node; use ethereum_rust_storage::{EngineType, Store}; use k256::ecdsa::SigningKey; +use local_ip_address::local_ip; use std::future::IntoFuture; use std::path::Path; use std::str::FromStr as _; @@ -15,7 +16,7 @@ use std::time::Duration; use std::{ fs::File, io, - net::{SocketAddr, ToSocketAddrs}, + net::{Ipv4Addr, SocketAddr, ToSocketAddrs}, }; use tokio_util::task::TaskTracker; use tracing::{info, warn}; @@ -147,8 +148,16 @@ async fn main() { let signer = SigningKey::from_slice(key_bytes.as_bytes()).unwrap(); let local_node_id = node_id_from_signing_key(&signer); + // TODO: If hhtp.addr is 0.0.0.0 we get the local ip as the one of the node, otherwise we use the provided one. + // This is fine for now, but we might need to support more options in the future. + let p2p_node_ip = if udp_socket_addr.ip() == Ipv4Addr::new(0, 0, 0, 0) { + local_ip().expect("Failed to get local ip") + } else { + udp_socket_addr.ip() + }; + let local_p2p_node = Node { - ip: udp_socket_addr.ip(), + ip: p2p_node_ip, udp_port: udp_socket_addr.port(), tcp_port: tcp_socket_addr.port(), node_id: local_node_id, diff --git a/test_data/network_params.yaml b/test_data/network_params.yaml index b20776e53..cbb4ac117 100644 --- a/test_data/network_params.yaml +++ b/test_data/network_params.yaml @@ -1,13 +1,13 @@ participants: + - el_type: ethereumrust + cl_type: lighthouse + validator_count: 32 - el_type: geth cl_type: lighthouse validator_count: 32 - el_type: geth cl_type: prysm validator_count: 32 - - el_type: ethereumrust - cl_type: lighthouse - validator_count: 32 additional_services: - assertoor From ee2458aa1b7c292ced001cd5dde442d7006454ef Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Mon, 28 Oct 2024 22:32:05 -0300 Subject: [PATCH 2/6] Solved the initial issue regarding blocks not found, popped a new one related to gas limit --- crates/networking/rpc/eth/account.rs | 5 ++--- crates/networking/rpc/eth/transaction.rs | 16 ++++++++-------- test_data/el-stability-check.yml | 2 +- test_data/network_params.yaml | 4 ++-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/networking/rpc/eth/account.rs b/crates/networking/rpc/eth/account.rs index 283d83b3f..953999daa 100644 --- a/crates/networking/rpc/eth/account.rs +++ b/crates/networking/rpc/eth/account.rs @@ -156,9 +156,8 @@ impl RpcHandler for GetTransactionCountRequest { ); let Some(block_number) = self.block.resolve_block_number(&storage)? else { - return Err(RpcErr::Internal( - "Could not resolve block number".to_owned(), - )); // Should we return Null here? + return serde_json::to_value("0x0") + .map_err(|error| RpcErr::Internal(error.to_string())); }; let nonce = storage diff --git a/crates/networking/rpc/eth/transaction.rs b/crates/networking/rpc/eth/transaction.rs index a8d8481a3..307b5dad1 100644 --- a/crates/networking/rpc/eth/transaction.rs +++ b/crates/networking/rpc/eth/transaction.rs @@ -459,14 +459,14 @@ impl RpcHandler for EstimateGasRequest { None => block_header.gas_limit, }; - if self.transaction.gas_price != 0 { - highest_gas_limit = recap_with_account_balances( - highest_gas_limit, - &self.transaction, - &storage, - block_header.number, - )?; - } + // if self.transaction.gas_price != 0 { + // highest_gas_limit = recap_with_account_balances( + // highest_gas_limit, + // &self.transaction, + // &storage, + // block_header.number, + // )?; + // } // Check whether the execution is possible let mut transaction = self.transaction.clone(); diff --git a/test_data/el-stability-check.yml b/test_data/el-stability-check.yml index 6b4a38a64..d28765a06 100644 --- a/test_data/el-stability-check.yml +++ b/test_data/el-stability-check.yml @@ -24,7 +24,7 @@ tasks: - name: run_task_matrix title: "Check block proposals from all client pairs" - timeout: 2m + timeout: 3m configVars: matrixValues: "validatorPairNames" config: diff --git a/test_data/network_params.yaml b/test_data/network_params.yaml index cbb4ac117..2efcdab08 100644 --- a/test_data/network_params.yaml +++ b/test_data/network_params.yaml @@ -12,8 +12,8 @@ participants: additional_services: - assertoor - dora - - el_forkmon - - tx_spammer + #- el_forkmon + #- tx_spammer assertoor_params: run_stability_check: false From 28183248135e2e26f89c437e1b870977048e6367 Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Tue, 29 Oct 2024 17:52:27 -0300 Subject: [PATCH 3/6] Resolving pending blocks default to latest if no pending and change an unwrap due to a filtering condition not covered --- crates/blockchain/payload.rs | 4 +++- crates/networking/rpc/types/block_identifier.rs | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/payload.rs b/crates/blockchain/payload.rs index 3ad9398c0..994ddc477 100644 --- a/crates/blockchain/payload.rs +++ b/crates/blockchain/payload.rs @@ -445,9 +445,11 @@ impl TransactionQueue { // Pull the first tx from each list and add it to the heads list // This should be a newly filtered tx list so we are guaranteed to have a first element let head_tx = txs.remove(0); + use tracing::info; + info!("head_tx: {:?}", head_tx); heads.push(HeadTransaction { // We already ran this method when filtering the transactions from the mempool so it shouldn't fail - tip: head_tx.effective_gas_tip(base_fee).unwrap(), + tip: head_tx.effective_gas_tip(base_fee).unwrap_or_else(|| 0), tx: head_tx, sender: *address, }); diff --git a/crates/networking/rpc/types/block_identifier.rs b/crates/networking/rpc/types/block_identifier.rs index a995b35d2..0cdff168c 100644 --- a/crates/networking/rpc/types/block_identifier.rs +++ b/crates/networking/rpc/types/block_identifier.rs @@ -39,7 +39,12 @@ impl BlockIdentifier { BlockTag::Finalized => storage.get_finalized_block_number(), BlockTag::Safe => storage.get_safe_block_number(), BlockTag::Latest => storage.get_latest_block_number(), - BlockTag::Pending => storage.get_pending_block_number(), + BlockTag::Pending => storage.get_pending_block_number().and_then(|option_block_number| { + match option_block_number { + Some(block_number) => Ok(Some(block_number)), + None => storage.get_latest_block_number(), + } + }), }, } } From 54a8a3fb0b74088316d060e6e077f786b89796bb Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Wed, 30 Oct 2024 17:19:21 -0300 Subject: [PATCH 4/6] Reactivate all additional services after the latest merges --- crates/blockchain/mempool.rs | 8 ++++++++ test_data/network_params.yaml | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/blockchain/mempool.rs b/crates/blockchain/mempool.rs index f32ed10bc..7c4a25636 100644 --- a/crates/blockchain/mempool.rs +++ b/crates/blockchain/mempool.rs @@ -79,6 +79,14 @@ pub fn filter_transactions( if filter.only_plain_txs && is_blob_tx || filter.only_blob_txs && !is_blob_tx { return false; } + + // This is a temporary fix to avoid invalid transactions to be included. + // This should be removed once https://github.com/lambdaclass/ethereum_rust/issues/680 + // is addressed. + if tx.effective_gas_tip(filter.base_fee).is_none() { + println!("Transaction with invalid tip: {:?}", tx.effective_gas_tip(filter.base_fee)); + return false; + } // Filter by tip & base_fee if let Some(min_tip) = filter.min_tip { if !tx diff --git a/test_data/network_params.yaml b/test_data/network_params.yaml index 2efcdab08..d2c4fc7ec 100644 --- a/test_data/network_params.yaml +++ b/test_data/network_params.yaml @@ -12,12 +12,14 @@ participants: additional_services: - assertoor - dora - #- el_forkmon - #- tx_spammer + - el_forkmon + - tx_spammer assertoor_params: run_stability_check: false run_block_proposal_check: false run_blob_transaction_test: true tests: - - 'https://raw.githubusercontent.com/lambdaclass/lambda_ethereum_rust/refs/heads/main/test_data/el-stability-check.yml' \ No newline at end of file + - 'https://raw.githubusercontent.com/lambdaclass/lambda_ethereum_rust/refs/heads/main/test_data/el-stability-check.yml' +tx_spammer_params: + tx_spammer_extra_args: ["--accounts=10", --txcount=10] From afb6c89aeaf39332576002ad07d02926e7d3a914 Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Wed, 30 Oct 2024 17:40:12 -0300 Subject: [PATCH 5/6] Removed unnecesary diffs --- crates/blockchain/mempool.rs | 1 - crates/blockchain/payload.rs | 4 +--- crates/networking/rpc/eth/transaction.rs | 16 ++++++++-------- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/blockchain/mempool.rs b/crates/blockchain/mempool.rs index 7c4a25636..de9884f28 100644 --- a/crates/blockchain/mempool.rs +++ b/crates/blockchain/mempool.rs @@ -84,7 +84,6 @@ pub fn filter_transactions( // This should be removed once https://github.com/lambdaclass/ethereum_rust/issues/680 // is addressed. if tx.effective_gas_tip(filter.base_fee).is_none() { - println!("Transaction with invalid tip: {:?}", tx.effective_gas_tip(filter.base_fee)); return false; } // Filter by tip & base_fee diff --git a/crates/blockchain/payload.rs b/crates/blockchain/payload.rs index 50a14a303..885cac812 100644 --- a/crates/blockchain/payload.rs +++ b/crates/blockchain/payload.rs @@ -470,11 +470,9 @@ impl TransactionQueue { // Pull the first tx from each list and add it to the heads list // This should be a newly filtered tx list so we are guaranteed to have a first element let head_tx = txs.remove(0); - use tracing::info; - info!("head_tx: {:?}", head_tx); heads.push(HeadTransaction { // We already ran this method when filtering the transactions from the mempool so it shouldn't fail - tip: head_tx.effective_gas_tip(base_fee).unwrap_or_else(|| 0), + tip: head_tx.effective_gas_tip(base_fee).unwrap(), tx: head_tx, sender: *address, }); diff --git a/crates/networking/rpc/eth/transaction.rs b/crates/networking/rpc/eth/transaction.rs index 9e7c4d4a5..d0cbcca82 100644 --- a/crates/networking/rpc/eth/transaction.rs +++ b/crates/networking/rpc/eth/transaction.rs @@ -460,14 +460,14 @@ impl RpcHandler for EstimateGasRequest { None => block_header.gas_limit, }; - // if self.transaction.gas_price != 0 { - // highest_gas_limit = recap_with_account_balances( - // highest_gas_limit, - // &self.transaction, - // &storage, - // block_header.number, - // )?; - // } + if self.transaction.gas_price != 0 { + highest_gas_limit = recap_with_account_balances( + highest_gas_limit, + &self.transaction, + &storage, + block_header.number, + )?; + } // Check whether the execution is possible let mut transaction = self.transaction.clone(); From 58639d58064cd7819b62ceca30c0e4f864c347cf Mon Sep 17 00:00:00 2001 From: Rodrigo Oliveri Date: Wed, 30 Oct 2024 18:09:39 -0300 Subject: [PATCH 6/6] Added a comment + format --- crates/networking/rpc/types/block_identifier.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/networking/rpc/types/block_identifier.rs b/crates/networking/rpc/types/block_identifier.rs index 0cdff168c..5b596ccad 100644 --- a/crates/networking/rpc/types/block_identifier.rs +++ b/crates/networking/rpc/types/block_identifier.rs @@ -39,12 +39,15 @@ impl BlockIdentifier { BlockTag::Finalized => storage.get_finalized_block_number(), BlockTag::Safe => storage.get_safe_block_number(), BlockTag::Latest => storage.get_latest_block_number(), - BlockTag::Pending => storage.get_pending_block_number().and_then(|option_block_number| { - match option_block_number { - Some(block_number) => Ok(Some(block_number)), - None => storage.get_latest_block_number(), - } - }), + BlockTag::Pending => { + storage + .get_pending_block_number() + // If there are no pending blocks, we return the latest block number + .and_then(|pending_block_number| match pending_block_number { + Some(block_number) => Ok(Some(block_number)), + None => storage.get_latest_block_number(), + }) + } }, } }