Fix concurrency issues (#142)

* Fix the OS FD error

* Cache the compiler versions

* Allow for auto display impl in declare wrapper type macro

* Better logging and fix concurrency issues

* Fix tests

* Format

* Make the code even more concurrent
This commit is contained in:
Omar
2025-08-19 09:47:36 +03:00
committed by GitHub
parent c58551803d
commit 76d6a154c1
33 changed files with 773 additions and 720 deletions
+84 -79
View File
@@ -33,9 +33,12 @@ use alloy::{
};
use anyhow::Context;
use revive_common::EVMVersion;
use tracing::{Instrument, Level};
use tracing::{Instrument, instrument};
use revive_dt_common::{fs::clear_directory, futures::poll};
use revive_dt_common::{
fs::clear_directory,
futures::{PollingWaitBehavior, poll},
};
use revive_dt_config::Arguments;
use revive_dt_format::traits::ResolverApi;
use revive_dt_node_interaction::EthereumNode;
@@ -52,6 +55,7 @@ static NODE_COUNT: AtomicU32 = AtomicU32::new(0);
///
/// Prunes the child process and the base directory on drop.
#[derive(Debug)]
#[allow(clippy::type_complexity)]
pub struct GethNode {
connection_string: String,
base_directory: PathBuf,
@@ -61,8 +65,9 @@ pub struct GethNode {
id: u32,
handle: Option<Child>,
start_timeout: u64,
wallet: EthereumWallet,
wallet: Arc<EthereumWallet>,
nonce_manager: CachedNonceManager,
chain_id_filler: ChainIdFiller,
/// This vector stores [`File`] objects that we use for logging which we want to flush when the
/// node object is dropped. We do not store them in a structured fashion at the moment (in
/// separate fields) as the logic that we need to apply to them is all the same regardless of
@@ -91,7 +96,7 @@ impl GethNode {
const TRACE_POLLING_DURATION: Duration = Duration::from_secs(60);
/// Create the node directory and call `geth init` to configure the genesis.
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn init(&mut self, genesis: String) -> anyhow::Result<&mut Self> {
let _ = clear_directory(&self.base_directory);
let _ = clear_directory(&self.logs_directory);
@@ -141,7 +146,7 @@ impl GethNode {
/// Spawn the go-ethereum node child process.
///
/// [Instance::init] must be called prior.
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn spawn_process(&mut self) -> anyhow::Result<&mut Self> {
// This is the `OpenOptions` that we wish to use for all of the log files that we will be
// opening in this method. We need to construct it in this way to:
@@ -197,7 +202,7 @@ impl GethNode {
/// Wait for the g-ethereum node child process getting ready.
///
/// [Instance::spawn_process] must be called priorly.
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn wait_ready(&mut self) -> anyhow::Result<&mut Self> {
let start_time = Instant::now();
@@ -231,80 +236,75 @@ impl GethNode {
}
}
#[tracing::instrument(skip_all, fields(geth_node_id = self.id), level = Level::TRACE)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn geth_stdout_log_file_path(&self) -> PathBuf {
self.logs_directory.join(Self::GETH_STDOUT_LOG_FILE_NAME)
}
#[tracing::instrument(skip_all, fields(geth_node_id = self.id), level = Level::TRACE)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn geth_stderr_log_file_path(&self) -> PathBuf {
self.logs_directory.join(Self::GETH_STDERR_LOG_FILE_NAME)
}
fn provider(
async fn provider(
&self,
) -> impl Future<
Output = anyhow::Result<
FillProvider<impl TxFiller<Ethereum>, impl Provider<Ethereum>, Ethereum>,
>,
> + 'static {
let connection_string = self.connection_string();
let wallet = self.wallet.clone();
// Note: We would like all providers to make use of the same nonce manager so that we have
// monotonically increasing nonces that are cached. The cached nonce manager uses Arc's in
// its implementation and therefore it means that when we clone it then it still references
// the same state.
let nonce_manager = self.nonce_manager.clone();
Box::pin(async move {
ProviderBuilder::new()
.disable_recommended_fillers()
.filler(FallbackGasFiller::new(
25_000_000,
1_000_000_000,
1_000_000_000,
))
.filler(ChainIdFiller::default())
.filler(NonceFiller::new(nonce_manager))
.wallet(wallet)
.connect(&connection_string)
.await
.map_err(Into::into)
})
) -> anyhow::Result<FillProvider<impl TxFiller<Ethereum>, impl Provider<Ethereum>, Ethereum>>
{
ProviderBuilder::new()
.disable_recommended_fillers()
.filler(FallbackGasFiller::new(
25_000_000,
1_000_000_000,
1_000_000_000,
))
.filler(self.chain_id_filler.clone())
.filler(NonceFiller::new(self.nonce_manager.clone()))
.wallet(self.wallet.clone())
.connect(&self.connection_string)
.await
.map_err(Into::into)
}
}
impl EthereumNode for GethNode {
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(
level = "info",
skip_all,
fields(geth_node_id = self.id, connection_string = self.connection_string),
err,
)]
async fn execute_transaction(
&self,
transaction: TransactionRequest,
) -> anyhow::Result<alloy::rpc::types::TransactionReceipt> {
let provider = Arc::new(self.provider().await?);
let transaction_hash = *provider.send_transaction(transaction).await?.tx_hash();
let provider = self.provider().await?;
// The following is a fix for the "transaction indexing is in progress" error that we
// used to get. You can find more information on this in the following GH issue in geth
let pending_transaction = provider.send_transaction(transaction).await.inspect_err(
|err| tracing::error!(%err, "Encountered an error when submitting the transaction"),
)?;
let transaction_hash = *pending_transaction.tx_hash();
// The following is a fix for the "transaction indexing is in progress" error that we used
// to get. You can find more information on this in the following GH issue in geth
// https://github.com/ethereum/go-ethereum/issues/28877. To summarize what's going on,
// before we can get the receipt of the transaction it needs to have been indexed by the
// node's indexer. Just because the transaction has been confirmed it doesn't mean that
// it has been indexed. When we call alloy's `get_receipt` it checks if the transaction
// was confirmed. If it has been, then it will call `eth_getTransactionReceipt` method
// which _might_ return the above error if the tx has not yet been indexed yet. So, we
// need to implement a retry mechanism for the receipt to keep retrying to get it until
// it eventually works, but we only do that if the error we get back is the "transaction
// node's indexer. Just because the transaction has been confirmed it doesn't mean that it
// has been indexed. When we call alloy's `get_receipt` it checks if the transaction was
// confirmed. If it has been, then it will call `eth_getTransactionReceipt` method which
// _might_ return the above error if the tx has not yet been indexed yet. So, we need to
// implement a retry mechanism for the receipt to keep retrying to get it until it
// eventually works, but we only do that if the error we get back is the "transaction
// indexing is in progress" error or if the receipt is None.
//
// Getting the transaction indexed and taking a receipt can take a long time especially
// when a lot of transactions are being submitted to the node. Thus, while initially we
// only allowed for 60 seconds of waiting with a 1 second delay in polling, we need to
// allow for a larger wait time. Therefore, in here we allow for 5 minutes of waiting
// with exponential backoff each time we attempt to get the receipt and find that it's
// not available.
// Getting the transaction indexed and taking a receipt can take a long time especially when
// a lot of transactions are being submitted to the node. Thus, while initially we only
// allowed for 60 seconds of waiting with a 1 second delay in polling, we need to allow for
// a larger wait time. Therefore, in here we allow for 5 minutes of waiting with exponential
// backoff each time we attempt to get the receipt and find that it's not available.
let provider = Arc::new(provider);
poll(
Self::RECEIPT_POLLING_DURATION,
Default::default(),
PollingWaitBehavior::Constant(Duration::from_millis(200)),
move || {
let provider = provider.clone();
async move {
@@ -329,7 +329,7 @@ impl EthereumNode for GethNode {
.await
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn trace_transaction(
&self,
transaction: &TransactionReceipt,
@@ -338,7 +338,7 @@ impl EthereumNode for GethNode {
let provider = Arc::new(self.provider().await?);
poll(
Self::TRACE_POLLING_DURATION,
Default::default(),
PollingWaitBehavior::Constant(Duration::from_millis(200)),
move || {
let provider = provider.clone();
let trace_options = trace_options.clone();
@@ -362,7 +362,7 @@ impl EthereumNode for GethNode {
.await
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn state_diff(&self, transaction: &TransactionReceipt) -> anyhow::Result<DiffMode> {
let trace_options = GethDebugTracingOptions::prestate_tracer(PreStateConfig {
diff_mode: Some(true),
@@ -379,7 +379,7 @@ impl EthereumNode for GethNode {
}
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn balance_of(&self, address: Address) -> anyhow::Result<U256> {
self.provider()
.await?
@@ -388,7 +388,7 @@ impl EthereumNode for GethNode {
.map_err(Into::into)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn latest_state_proof(
&self,
address: Address,
@@ -404,7 +404,7 @@ impl EthereumNode for GethNode {
}
impl ResolverApi for GethNode {
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn chain_id(&self) -> anyhow::Result<alloy::primitives::ChainId> {
self.provider()
.await?
@@ -413,7 +413,7 @@ impl ResolverApi for GethNode {
.map_err(Into::into)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn transaction_gas_price(&self, tx_hash: &TxHash) -> anyhow::Result<u128> {
self.provider()
.await?
@@ -423,7 +423,7 @@ impl ResolverApi for GethNode {
.map(|receipt| receipt.effective_gas_price)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_gas_limit(&self, number: BlockNumberOrTag) -> anyhow::Result<u128> {
self.provider()
.await?
@@ -433,7 +433,7 @@ impl ResolverApi for GethNode {
.map(|block| block.header.gas_limit as _)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_coinbase(&self, number: BlockNumberOrTag) -> anyhow::Result<Address> {
self.provider()
.await?
@@ -443,7 +443,7 @@ impl ResolverApi for GethNode {
.map(|block| block.header.beneficiary)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_difficulty(&self, number: BlockNumberOrTag) -> anyhow::Result<U256> {
self.provider()
.await?
@@ -453,7 +453,7 @@ impl ResolverApi for GethNode {
.map(|block| U256::from_be_bytes(block.header.mix_hash.0))
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_base_fee(&self, number: BlockNumberOrTag) -> anyhow::Result<u64> {
self.provider()
.await?
@@ -468,7 +468,7 @@ impl ResolverApi for GethNode {
})
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_hash(&self, number: BlockNumberOrTag) -> anyhow::Result<BlockHash> {
self.provider()
.await?
@@ -478,7 +478,7 @@ impl ResolverApi for GethNode {
.map(|block| block.header.hash)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn block_timestamp(&self, number: BlockNumberOrTag) -> anyhow::Result<BlockTimestamp> {
self.provider()
.await?
@@ -488,7 +488,7 @@ impl ResolverApi for GethNode {
.map(|block| block.header.timestamp)
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
async fn last_block_number(&self) -> anyhow::Result<BlockNumber> {
self.provider()
.await?
@@ -522,20 +522,26 @@ impl Node for GethNode {
id,
handle: None,
start_timeout: config.geth_start_timeout,
wallet,
wallet: Arc::new(wallet),
chain_id_filler: Default::default(),
nonce_manager: Default::default(),
// We know that we only need to be storing 2 files so we can specify that when creating
// the vector. It's the stdout and stderr of the geth node.
logs_file_to_flush: Vec::with_capacity(2),
nonce_manager: Default::default(),
}
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn id(&self) -> usize {
self.id as _
}
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn connection_string(&self) -> String {
self.connection_string.clone()
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn shutdown(&mut self) -> anyhow::Result<()> {
// Terminate the processes in a graceful manner to allow for the output to be flushed.
if let Some(mut child) = self.handle.take() {
@@ -557,13 +563,13 @@ impl Node for GethNode {
Ok(())
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn spawn(&mut self, genesis: String) -> anyhow::Result<()> {
self.init(genesis)?.spawn_process()?;
Ok(())
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id), err)]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn version(&self) -> anyhow::Result<String> {
let output = Command::new(&self.geth)
.arg("--version")
@@ -576,8 +582,7 @@ impl Node for GethNode {
Ok(String::from_utf8_lossy(&output).into())
}
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn matches_target(&self, targets: Option<&[String]>) -> bool {
fn matches_target(targets: Option<&[String]>) -> bool {
match targets {
None => true,
Some(targets) => targets.iter().any(|str| str.as_str() == "evm"),
@@ -590,7 +595,7 @@ impl Node for GethNode {
}
impl Drop for GethNode {
#[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
#[instrument(level = "info", skip_all, fields(geth_node_id = self.id))]
fn drop(&mut self) {
self.shutdown().expect("Failed to shutdown")
}