Skip to content

Commit

Permalink
Move lock inside contract decoder
Browse files Browse the repository at this point in the history
  • Loading branch information
agostbiro committed Dec 20, 2024
1 parent c3a8cd3 commit 218290f
Show file tree
Hide file tree
Showing 23 changed files with 64 additions and 85 deletions.
8 changes: 3 additions & 5 deletions crates/edr_napi/src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use napi::{
Env, JsFunction, Status,
};
use napi_derive::napi;
use parking_lot::RwLock;

use crate::cast::TryCast;

Expand Down Expand Up @@ -117,7 +116,7 @@ impl Logger {
pub fn new(
env: &Env,
config: LoggerConfig,
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
) -> napi::Result<Self> {
Ok(Self {
collector: LogCollector::new(env, config, contract_decoder)?,
Expand Down Expand Up @@ -254,7 +253,7 @@ pub struct CollapsedMethod {

#[derive(Clone)]
struct LogCollector {
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
decode_console_log_inputs_fn: ThreadsafeFunction<Vec<Bytes>, ErrorStrategy::Fatal>,
indentation: usize,
is_enabled: bool,
Expand All @@ -268,7 +267,7 @@ impl LogCollector {
pub fn new(
env: &Env,
config: LoggerConfig,
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
) -> napi::Result<Self> {
let mut decode_console_log_inputs_fn = config
.decode_console_log_inputs_callback
Expand Down Expand Up @@ -549,7 +548,6 @@ impl LogCollector {
function_name,
} = self
.contract_decoder
.write()
.get_contract_and_function_names_for_call(&code, calldata.as_ref());
(contract_name, function_name)
}
Expand Down
16 changes: 4 additions & 12 deletions crates/edr_napi/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use edr_rpc_eth::jsonrpc;
use edr_solidity::contract_decoder::ContractDecoder;
use napi::{tokio::runtime, Either, Env, JsFunction, JsObject, Status};
use napi_derive::napi;
use parking_lot::RwLock;

use self::config::ProviderConfig;
use crate::{
Expand All @@ -23,7 +22,7 @@ use crate::{
pub struct Provider {
provider: Arc<edr_provider::Provider<LoggerError>>,
runtime: runtime::Handle,
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
#[cfg(feature = "scenarios")]
scenario_file: Option<napi::tokio::sync::Mutex<napi::tokio::fs::File>>,
}
Expand All @@ -45,15 +44,11 @@ impl Provider {

let config = edr_provider::ProviderConfig::try_from(config)?;

// TODO parsing the build info config and creating the contract decoder
// shouldn't happen here as it's blocking the JS event loop,
// but it's not straightforward to do it in a non-blocking way, because `Env` is
// not `Send`.
let build_info_config: edr_solidity::contract_decoder::BuildInfoConfig =
serde_json::from_value(tracing_config)?;
let contract_decoder = ContractDecoder::new(&build_info_config)
.map_err(|error| napi::Error::from_reason(error.to_string()))?;
let contract_decoder = Arc::new(RwLock::new(contract_decoder));
let contract_decoder = Arc::new(contract_decoder);

let logger = Box::new(Logger::new(
&env,
Expand Down Expand Up @@ -252,7 +247,7 @@ impl Provider {
#[derive(Debug)]
struct SolidityTraceData {
trace: Arc<edr_evm::trace::Trace>,
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
}

#[napi]
Expand Down Expand Up @@ -301,10 +296,7 @@ impl Response {
);

if let Some(vm_trace) = hierarchical_trace.result {
let decoded_trace = {
let mut decoder = contract_decoder.write();
decoder.try_to_decode_message_trace(vm_trace)
};
let decoded_trace = contract_decoder.try_to_decode_message_trace(vm_trace);
let stack_trace = edr_solidity::solidity_tracer::get_stack_trace(decoded_trace)
.map_err(|err| {
napi::Error::from_reason(format!(
Expand Down
13 changes: 5 additions & 8 deletions crates/edr_provider/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ use gas::gas_used_ratio;
use indexmap::IndexMap;
use itertools::izip;
use lru::LruCache;
use parking_lot::RwLock;
use revm_precompile::secp256r1;
use rpds::HashTrieMapSync;
use tokio::runtime;
Expand Down Expand Up @@ -213,9 +212,7 @@ pub struct ProviderData<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch = Cu
block_state_cache: LruCache<StateId, Arc<Box<dyn SyncState<StateError>>>>,
current_state_id: StateId,
block_number_to_state_id: HashTrieMapSync<u64, StateId>,
// DEAD LOCK WARNING: This should only be written to from a `hardhat_addCompilationResult` or
// `hardhat_reset` handler. Otherwise there is a risk of deadlocks.
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
}

impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErrorT, TimerT> {
Expand All @@ -225,7 +222,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
subscriber_callback: Box<dyn SyncSubscriberCallback>,
call_override: Option<Arc<dyn SyncCallOverride>>,
config: ProviderConfig,
contract_metadata: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
timer: TimerT,
) -> Result<Self, CreationError> {
let InitialAccounts {
Expand Down Expand Up @@ -322,7 +319,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
block_state_cache,
current_state_id,
block_number_to_state_id,
contract_decoder: contract_metadata,
contract_decoder,
})
}

Expand Down Expand Up @@ -395,7 +392,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
}

/// Get the locked contract decoder.
pub fn contract_decoder(&self) -> &RwLock<ContractDecoder> {
pub fn contract_decoder(&self) -> &ContractDecoder {
&self.contract_decoder
}

Expand Down Expand Up @@ -2899,7 +2896,7 @@ pub(crate) mod test_utils {
subscription_callback_noop,
None,
config.clone(),
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
4 changes: 2 additions & 2 deletions crates/edr_provider/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use edr_solidity::contract_decoder::ContractDecoder;
use lazy_static::lazy_static;
use logger::SyncLogger;
use mock::SyncCallOverride;
use parking_lot::{Mutex, RwLock};
use parking_lot::Mutex;
use requests::{eth::handle_set_interval_mining, hardhat::rpc_types::ResetProviderConfig};
use time::{CurrentTime, TimeSinceEpoch};
use tokio::{runtime, sync::Mutex as AsyncMutex, task};
Expand Down Expand Up @@ -109,7 +109,7 @@ impl<LoggerErrorT: Debug + Send + Sync + 'static, TimerT: Clone + TimeSinceEpoch
logger: Box<dyn SyncLogger<BlockchainError = BlockchainError, LoggerError = LoggerErrorT>>,
subscriber_callback: Box<dyn SyncSubscriberCallback>,
config: ProviderConfig,
contract_decoder: Arc<RwLock<ContractDecoder>>,
contract_decoder: Arc<ContractDecoder>,
timer: TimerT,
) -> Result<Self, CreationError> {
let data = ProviderData::new(
Expand Down
2 changes: 1 addition & 1 deletion crates/edr_provider/src/requests/hardhat/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn add_compilation_result_inner<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEp
create_models_and_decode_bytecodes(solc_version, &compiler_input, &compiler_output)
.map_err(|err| ProviderError::SolcDecoding(err.to_string()))?;

let mut contract_decoder = data.contract_decoder().write();
let contract_decoder = data.contract_decoder();
for contract in contracts {
contract_decoder.add_contract_metadata(contract);
}
Expand Down
15 changes: 7 additions & 8 deletions crates/edr_provider/tests/eip4844.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use edr_provider::{
};
use edr_rpc_eth::CallRequest;
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use tokio::runtime;

/// Helper struct to modify the pooled transaction from the value in
Expand Down Expand Up @@ -223,7 +222,7 @@ async fn call_unsupported() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -255,7 +254,7 @@ async fn estimate_gas_unsupported() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -287,7 +286,7 @@ async fn send_transaction_unsupported() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -331,7 +330,7 @@ async fn send_raw_transaction() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -371,7 +370,7 @@ async fn get_transaction() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -419,7 +418,7 @@ async fn block_header() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down Expand Up @@ -606,7 +605,7 @@ async fn blob_hash_opcode() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/eth_max_priority_fee_per_gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use edr_provider::{
ProviderRequest,
};
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -20,7 +19,7 @@ async fn eth_max_priority_fee_per_gas() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_324.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use edr_provider::{
use edr_rpc_eth::CallRequest;
use edr_solidity::contract_decoder::ContractDecoder;
use edr_test_utils::env::get_alchemy_url;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand Down Expand Up @@ -37,7 +36,7 @@ async fn issue_324() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_325.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use edr_provider::{
MethodInvocation, MiningConfig, NoopLogger, Provider, ProviderRequest,
};
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand Down Expand Up @@ -41,7 +40,7 @@ async fn issue_325() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_326.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use edr_provider::{
};
use edr_rpc_eth::CallRequest;
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand Down Expand Up @@ -41,7 +40,7 @@ async fn issue_326() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_346.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::sync::Arc;

use edr_provider::{test_utils::create_test_config, time::CurrentTime, NoopLogger, Provider};
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use serde_json::json;
use tokio::runtime;

Expand All @@ -17,7 +16,7 @@ async fn issue_346() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_356.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use edr_provider::{
use edr_rpc_eth::CallRequest;
use edr_solidity::contract_decoder::ContractDecoder;
use edr_test_utils::env::get_alchemy_url;
use parking_lot::RwLock;
use sha3::{Digest, Keccak256};
use tokio::runtime;

Expand Down Expand Up @@ -39,7 +38,7 @@ async fn issue_356() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_361.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use edr_provider::{
MethodInvocation, NoopLogger, Provider, ProviderRequest,
};
use edr_solidity::contract_decoder::ContractDecoder;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -38,7 +37,7 @@ async fn issue_361() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/edr_provider/tests/issues/issue_384.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use edr_provider::{
};
use edr_solidity::contract_decoder::ContractDecoder;
use edr_test_utils::env::get_infura_url;
use parking_lot::RwLock;
use tokio::runtime;

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -27,7 +26,7 @@ async fn avalanche_chain_mine_local_block() -> anyhow::Result<()> {
logger,
subscriber,
config,
Arc::<RwLock<ContractDecoder>>::default(),
Arc::<ContractDecoder>::default(),
CurrentTime,
)?;

Expand Down
Loading

0 comments on commit 218290f

Please sign in to comment.