-
Notifications
You must be signed in to change notification settings - Fork 2
Added fetch_add_nonce method for NodeInteraction trait. Added extra logging. #25
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,11 +83,29 @@ | |
| Ok(output) => { | ||
| task.json_output = Some(output.output.clone()); | ||
| task.error = output.error; | ||
| self.contracts.insert(output.input, output.output); | ||
| self.contracts.insert(output.input, output.output.clone()); | ||
|
|
||
| if let Some(last_output) = self.contracts.values().last() { | ||
| if let Some(contracts) = &last_output.contracts { | ||
| for (file, contracts_map) in contracts { | ||
| for (contract_name, _) in contracts_map { | ||
| log::debug!( | ||
| "Compiled contract: {} from file: {}", | ||
| contract_name, | ||
| file | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| log::warn!("Compiled contracts field is None"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the only point of this heavily nested code here is to log a warning if there were no contracts compiled? This makes sense but could be simplified a lot.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually it is useful to log that the contract was succesfully compiled from a file and also when we don't have any contract to compile. |
||
| } | ||
| } | ||
|
|
||
| Report::compilation(span, T::config_id(), task); | ||
| Ok(()) | ||
| } | ||
| Err(error) => { | ||
| log::error!("Failed to compile contract: {:?}", error.to_string()); | ||
| task.error = Some(error.to_string()); | ||
| Err(error) | ||
| } | ||
|
|
@@ -99,13 +117,40 @@ | |
| input: &Input, | ||
| node: &T::Blockchain, | ||
| ) -> anyhow::Result<(GethTrace, DiffMode)> { | ||
| let receipt = node.execute_transaction(input.legacy_transaction( | ||
| self.config.network_id, | ||
| 0, | ||
| &self.deployed_contracts, | ||
| )?)?; | ||
| log::trace!("Calling execute_input for input: {:?}", input); | ||
|
|
||
| let nonce = node.fetch_add_nonce(input.caller)?; | ||
|
|
||
| log::debug!( | ||
| "Nonce calculated on the execute contract, calculated nonce {}, for contract {}, having address {} on node: {}", | ||
| &nonce, | ||
| &input.instance, | ||
| &input.caller, | ||
| std::any::type_name::<T>() | ||
| ); | ||
|
|
||
| let tx = | ||
| match input.legacy_transaction(self.config.network_id, nonce, &self.deployed_contracts) | ||
| { | ||
| Ok(tx) => tx, | ||
| Err(err) => { | ||
| log::error!("Failed to construct legacy transaction: {:?}", err); | ||
| return Err(err.into()); | ||
| } | ||
| }; | ||
|
|
||
| log::trace!("Executing transaction for input: {:?}", input); | ||
|
|
||
| let receipt = match node.execute_transaction(tx) { | ||
| Ok(receipt) => receipt, | ||
| Err(err) => { | ||
| log::error!("Failed to execute transaction: {:?}", err); | ||
| return Err(err.into()); | ||
| } | ||
| }; | ||
|
|
||
| log::trace!("Transaction receipt: {:?}", receipt); | ||
|
|
||
| let trace = node.trace_transaction(receipt.clone())?; | ||
| log::trace!("Trace result: {:?}", trace); | ||
|
|
||
|
|
@@ -115,14 +160,28 @@ | |
| } | ||
|
|
||
| pub fn deploy_contracts(&mut self, input: &Input, node: &T::Blockchain) -> anyhow::Result<()> { | ||
| log::debug!( | ||
| "Deploying contracts {}, having address {} on node: {}", | ||
| &input.instance, | ||
| &input.caller, | ||
| std::any::type_name::<T>() | ||
| ); | ||
| for output in self.contracts.values() { | ||
| let Some(contract_map) = &output.contracts else { | ||
| log::debug!("No contracts in output — skipping deployment for this input."); | ||
| log::debug!( | ||
| "No contracts in output — skipping deployment for this input {}", | ||
| &input.instance | ||
| ); | ||
| continue; | ||
| }; | ||
|
|
||
| for contracts in contract_map.values() { | ||
| for (contract_name, contract) in contracts { | ||
| log::debug!( | ||
| "Contract name is: {:?} and the input name is: {:?}", | ||
| &contract_name, | ||
| &input.instance | ||
| ); | ||
| if contract_name != &input.instance { | ||
| continue; | ||
| } | ||
|
|
@@ -134,24 +193,47 @@ | |
| .map(|b| b.object.clone()); | ||
|
|
||
| let Some(code) = bytecode else { | ||
| anyhow::bail!("no bytecode for contract `{}`", contract_name); | ||
| log::error!("no bytecode for contract {}", contract_name); | ||
| continue; | ||
| }; | ||
|
|
||
| let nonce = node.fetch_add_nonce(input.caller)?; | ||
|
|
||
| log::debug!( | ||
| "Calculated nonce {}, for contract {}, having address {} on node: {}", | ||
| &nonce, | ||
| &input.instance, | ||
| &input.caller, | ||
| std::any::type_name::<T>() | ||
| ); | ||
|
|
||
| let tx = TransactionRequest::default() | ||
| .with_from(input.caller) | ||
| .with_to(Address::ZERO) | ||
| .with_input(Bytes::from(code.clone())) | ||
| .with_gas_price(20_000_000_000) | ||
| .with_gas_limit(20_000_000_000) | ||
| .with_gas_price(5_000_000) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you need to change this? IIRC it was arbitrary but if it has to be a specific value it would be good to have the reason as comment.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have changed it because I have received this message:
And this is because: Total Fee = gas_price * gas_limit And the new value will be: 0.000025 ether |
||
| .with_gas_limit(5_000_000) | ||
| .with_chain_id(self.config.network_id) | ||
| .with_nonce(0); | ||
| .with_nonce(nonce); | ||
|
|
||
| let receipt = match node.execute_transaction(tx) { | ||
| Ok(receipt) => receipt, | ||
| Err(err) => { | ||
| log::error!( | ||
| "Failed to execute transaction when deploying the contract: {:?}, {:?}", | ||
| &contract_name, | ||
| err | ||
| ); | ||
| return Err(err.into()); | ||
| } | ||
| }; | ||
|
|
||
| let receipt = node.execute_transaction(tx)?; | ||
| let Some(address) = receipt.contract_address else { | ||
| anyhow::bail!( | ||
| "contract `{}` deployment did not return an address", | ||
| log::error!( | ||
| "contract {} deployment did not return an address", | ||
| contract_name | ||
| ); | ||
| continue; | ||
| }; | ||
|
|
||
| self.deployed_contracts | ||
|
|
@@ -161,6 +243,8 @@ | |
| } | ||
| } | ||
|
|
||
| log::debug!("Available contracts: {:?}", self.deployed_contracts.keys()); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
|
|
@@ -227,9 +311,11 @@ | |
|
|
||
| for case in &self.metadata.cases { | ||
| for input in &case.inputs { | ||
| log::debug!("Starting deploying contract {}", &input.instance); | ||
| leader_state.deploy_contracts(input, self.leader_node)?; | ||
| follower_state.deploy_contracts(input, self.follower_node)?; | ||
|
|
||
| log::debug!("Starting executing contract {}", &input.instance); | ||
| let (_, leader_diff) = leader_state.execute_input(input, self.leader_node)?; | ||
| let (_, follower_diff) = | ||
| follower_state.execute_input(input, self.follower_node)?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| //! This crate implements all node interactions. | ||
|
|
||
| use alloy::primitives::Address; | ||
| use alloy::rpc::types::trace::geth::{DiffMode, GethTrace}; | ||
| use alloy::rpc::types::{TransactionReceipt, TransactionRequest}; | ||
| use tokio_runtime::TO_TOKIO; | ||
|
|
||
| pub mod nonce; | ||
| mod tokio_runtime; | ||
| pub mod trace; | ||
| pub mod transaction; | ||
|
|
@@ -21,4 +23,6 @@ pub trait EthereumNode { | |
|
|
||
| /// Returns the state diff of the transaction hash in the [TransactionReceipt]. | ||
| fn state_diff(&self, transaction: TransactionReceipt) -> anyhow::Result<DiffMode>; | ||
|
|
||
| fn fetch_add_nonce(&self, address: Address) -> anyhow::Result<u64>; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a doc comment explaining what the trait method is supposed to do.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in the new revision |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| use std::pin::Pin; | ||
|
|
||
| use alloy::{ | ||
| primitives::Address, | ||
| providers::{Provider, ProviderBuilder}, | ||
| }; | ||
| use tokio::sync::oneshot; | ||
|
|
||
| use crate::{TO_TOKIO, tokio_runtime::AsyncNodeInteraction}; | ||
|
|
||
| pub type Task = Pin<Box<dyn Future<Output = anyhow::Result<u64>> + Send>>; | ||
|
|
||
| pub(crate) struct Nonce { | ||
| sender: oneshot::Sender<anyhow::Result<u64>>, | ||
| task: Task, | ||
| } | ||
|
|
||
| impl AsyncNodeInteraction for Nonce { | ||
| type Output = anyhow::Result<u64>; | ||
|
|
||
| fn split( | ||
| self, | ||
| ) -> ( | ||
| std::pin::Pin<Box<dyn Future<Output = Self::Output> + Send>>, | ||
| oneshot::Sender<Self::Output>, | ||
| ) { | ||
| (self.task, self.sender) | ||
| } | ||
| } | ||
|
|
||
| /// This is like `trace_transaction`, just for nonces. | ||
| pub fn fetch_onchain_nonce( | ||
| connection: String, | ||
| wallet: alloy::network::EthereumWallet, | ||
| address: Address, | ||
| ) -> anyhow::Result<u64> { | ||
| let sender = TO_TOKIO.lock().unwrap().nonce_sender.clone(); | ||
|
|
||
| let (tx, rx) = oneshot::channel(); | ||
| let task: Task = Box::pin(async move { | ||
| let provider = ProviderBuilder::new() | ||
| .wallet(wallet) | ||
| .connect(&connection) | ||
| .await?; | ||
| let onchain = provider.get_transaction_count(address).await?; | ||
| Ok(onchain) | ||
| }); | ||
|
|
||
| sender | ||
| .blocking_send(Nonce { task, sender: tx }) | ||
| .expect("not in async context"); | ||
|
|
||
| rx.blocking_recv() | ||
| .unwrap_or_else(|err| anyhow::bail!("nonce fetch failed: {err}")) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the added
.clone()necessary, I don't see the output being accessed further down?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it's not necessary.