added a new RPC call trace_replayBlockTransactions #7366

tzapu · 2017-12-22T10:56:34Z

in relation to #7365

as i mentioned this is my first go at rust so I could do with some pointers about error checking or tests, or whatever else is needed to make this mergeable

thank you guys for everything

add execution proof

parity-cla-bot · 2017-12-22T10:56:36Z

It looks like @tzapu hasn'signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, plesae reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

tzapu · 2017-12-22T10:58:11Z

[clabot:check]

parity-cla-bot · 2017-12-22T10:58:12Z

It looks like @tzapu signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

tomusdrw

Looks good!

The RPC tests should go to rpc/src/v1/tests/mocked/traces.rs, this will at least check the public API.

As for client tests it will be more tricky, have a look at ethcore/src/tests/helpers.rs and generate_dummy_client_with_spec_accounts_and_data to create a client, and later pick the last block and replay it.

tomusdrw · 2017-12-22T11:12:30Z

ethcore/src/client/client.rs

+		const EXECUTE_PROOF: &'static str = "Transaction replayed; qed";
+		for t in txs {
+			let t = SignedTransaction::new(t).expect(PROOF);
+			let x = self.do_virtual_call(&env_info, &mut state, &t, analytics).expect(EXECUTE_PROOF);


You can just do self.do_virtual_call(..)? here. But you are right that the call should never error, so I'm fine with expect as well.

tomusdrw · 2017-12-22T11:12:50Z

ethcore/src/client/test_client.rs

@@ -429,6 +429,12 @@ impl BlockChainClient for TestBlockChainClient {
 		self.execution_result.read().clone().unwrap()
 	}

+	// TODO add test
+	fn replay_block_transactions(&self, _block: BlockId, _analytics: CallAnalytics) -> Result<Vec<Executed>, CallError> {
+		let res = Vec::with_capacity(0);


Leave this as unimplemented!()

tomusdrw · 2017-12-22T11:14:34Z

ethcore/src/client/client.rs

@@ -1318,6 +1318,25 @@ impl BlockChainClient for Client {
 		self.do_virtual_call(&env_info, &mut state, &t, analytics)
 	}

+	fn replay_block_transactions(&self, block: BlockId, analytics: CallAnalytics) -> Result<Vec<Executed>, CallError> {


To share the code with replay() you could return a Result<Box<Iterator<Item = Executed>>, CallError> here and then replay() just becomes:

let index = <get-tx-index>; Ok(self.replay_block(...)?.nth(index))

tzapu · 2017-12-22T15:37:54Z

@tomusdrw thank you for taking at look at this

i tried implementing the suggested changes, but my non existent rust knowledge got the best of me
i have no idea how i would ofrmulate this to work

	fn replay_block_transactions(&self, block_number: BlockNumber, flags: TraceOptions) -> Result<Box<Iterator<Item = TraceResults>>> {
		self.client.replay_block_transactions(block_number.into(), to_call_analytics(flags))
			.map(|results| results.into_iter().map(TraceResults::from).collect())
			.map_err(errors::call)
	}

with the new result, or if the rest of the changes are ok
thank you again, maybe you can give me some tips

tzapu · 2017-12-23T08:05:24Z

i think i may have sorted it
while testing, if i make the request for a not synced yet block i get This request is not supported because your node is running with state pruning. Run with --pruning=archive. which is not quite accurate.
How would i go changing that to something more appropriate?
Aaaannnndddd what else do you think I need to sort to get this merged? :D
Thank you very much

tomusdrw

if i make the request for a not synced yet block i get This request is not supported because your node is running with state pruning. Run with --pruning=archive.

I think we should deal with it in a separate issue, it's currently common behaviour in all RPC methods.

tomusdrw · 2017-12-27T12:19:08Z

ethcore/src/client/client.rs

@@ -1293,7 +1293,7 @@ impl BlockChainClient for Client {
 		trace!(target: "estimate_gas", "estimate_gas chopping {} .. {}", lower, upper);
 		binary_chop(lower, upper, cond)
 	}
-
+/*


Please remove commented out code.

tomusdrw · 2017-12-27T12:19:58Z

ethcore/src/client/client.rs

+		let address = self.transaction_address(id).ok_or(CallError::TransactionNotFound)?;
+		let block = BlockId::Hash(address.block_hash);
+
+		Ok(self.replay_block_transactions(block, analytics)?.nth(address.index).unwrap())


Instead of unwrap use expect(<proof>). Here the proof will be something like: "The transaction address contains a valid index within block; qed".

tomusdrw · 2017-12-27T12:22:33Z

ethcore/src/client/client.rs

+			replays.push(x);
+		}
+
+		Ok(Box::new(replays.into_iter()))


It's almost fine - right now you are executing all transactions and putting the results into a vector, but Iterator allows you to do it lazily (which will make replay work faster).
It should look like this (instead of a loop you should use map):

let txs = block.transactions(); Ok(Box::new(txs.into_iter() .map(|tx| { ... self.do_virtual_call(...).expect(EXECUTE_PROOF) })))

tomusdrw · 2017-12-27T12:22:46Z

ethcore/src/client/test_client.rs

@@ -429,6 +429,11 @@ impl BlockChainClient for TestBlockChainClient {
 		self.execution_result.read().clone().unwrap()
 	}

+	// TODO add test


Please remove.

tomusdrw · 2017-12-27T12:24:31Z

Could you also prepare a mocked RPC test?

can't workout lifetime issues yet

Fix lifetimes issue.

tzapu · 2017-12-30T23:19:26Z

again, i would like to thank you for all the hand holding
i have added a naive rpc test, it looks like the tests pass

andresilva

Nice 👍
Just some minor comments regarding docs.

andresilva · 2018-01-04T17:05:56Z

rpc/src/v1/traits/traces.rs

@@ -56,5 +56,9 @@ build_rpc_trait! {
 		/// Executes the transaction with the given hash and returns a number of possible traces for it.
 		#[rpc(name = "trace_replayTransaction")]
 		fn replay_transaction(&self, H256, TraceOptions) -> Result<TraceResults>;
+
+		/// Executes the transaction with the given hash and returns a number of possible traces for it.


Executes all the transactions at the given block and returns a number of possible traces for each transaction.

andresilva · 2018-01-04T17:07:48Z

ethcore/src/client/traits.rs

@@ -196,6 +196,9 @@ pub trait BlockChainClient : Sync + Send {
 	/// Replays a given transaction for inspection.
 	fn replay(&self, t: TransactionId, analytics: CallAnalytics) -> Result<Executed, CallError>;

+	/// Replays a given transaction for inspection.


Replays all the transactions in a given block for inspection.

tzapu · 2018-01-05T06:41:41Z

thanks guys, much appreciated, happy new year
updated

tzapu · 2018-01-10T10:36:53Z

thanks guys, for everything

ankitchiplunkar · 2018-04-03T08:19:32Z

Would it be possible to also add uncle rewards and miner rewards with this JSON RPC?

tzapu added 2 commits December 21, 2017 14:37

intial add trace_replayBlockTransactions

1a4dca0

cleanup timing calls

967461e

add execution proof

tomusdrw reviewed Dec 22, 2017

View reviewed changes

tomusdrw added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. M6-rpcapi 📣 RPC API. labels Dec 22, 2017

WIP implementing changes

912d1a8

fix for trace_replayBlockTransactions rpc call

1f58e04

tomusdrw suggested changes Dec 27, 2017

View reviewed changes

tomusdrw closed this Dec 27, 2017

tomusdrw reopened this Dec 27, 2017

tzapu and others added 3 commits December 28, 2017 00:25

cleanup comments

0005793

cleanup, proof,

d26cc42

can't workout lifetime issues yet

Fix lifetimes issue.

d569034

5chdn added this to the 1.10 milestone Dec 30, 2017

5chdn added the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Dec 30, 2017

tzapu added 2 commits December 30, 2017 10:03

Merge pull request #1 from tomusdrw/master

3c071e6

Fix lifetimes issue.

naive rpc test

16a98d1

5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 1, 2018

tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 3, 2018

tomusdrw approved these changes Jan 3, 2018

View reviewed changes

andresilva approved these changes Jan 4, 2018

View reviewed changes

tzapu added 2 commits January 5, 2018 08:01

updated docs

379225e

Merge branch 'master' into master

fcf1d15

5chdn removed the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Jan 9, 2018

5chdn merged commit df5d27d into openethereum:master Jan 10, 2018

Serozd mentioned this pull request Jan 12, 2018

rpc: trace_replayBlockTransactions respond with error to set pruning to archive even if it is set #7547

Closed

andresilva mentioned this pull request Feb 22, 2018

add a trace_replayBlockTransactions RPC call #7365

Closed

XertroV mentioned this pull request Jun 19, 2019

[WIP] Historical Eras: keep full state for certain blocks and prune the rest - alternative to full archive node in some circumstances #10760

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added a new RPC call trace_replayBlockTransactions #7366

added a new RPC call trace_replayBlockTransactions #7366

tzapu commented Dec 22, 2017 •

edited

Loading

parity-cla-bot commented Dec 22, 2017

tzapu commented Dec 22, 2017

parity-cla-bot commented Dec 22, 2017

tomusdrw left a comment

tomusdrw Dec 22, 2017 •

edited

Loading

tomusdrw Dec 22, 2017

tomusdrw Dec 22, 2017

tzapu commented Dec 22, 2017 •

edited

Loading

tzapu commented Dec 23, 2017 •

edited

Loading

tomusdrw left a comment

tomusdrw Dec 27, 2017

tomusdrw Dec 27, 2017

tomusdrw Dec 27, 2017

tomusdrw Dec 27, 2017

tomusdrw commented Dec 27, 2017

tzapu commented Dec 30, 2017

andresilva left a comment

andresilva Jan 4, 2018

andresilva Jan 4, 2018

tzapu commented Jan 5, 2018

tzapu commented Jan 10, 2018

ankitchiplunkar commented Apr 3, 2018

added a new RPC call trace_replayBlockTransactions #7366

added a new RPC call trace_replayBlockTransactions #7366

Conversation

tzapu commented Dec 22, 2017 • edited Loading

parity-cla-bot commented Dec 22, 2017

tzapu commented Dec 22, 2017

parity-cla-bot commented Dec 22, 2017

tomusdrw left a comment

Choose a reason for hiding this comment

tomusdrw Dec 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzapu commented Dec 22, 2017 • edited Loading

tzapu commented Dec 23, 2017 • edited Loading

tomusdrw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomusdrw commented Dec 27, 2017

tzapu commented Dec 30, 2017

andresilva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzapu commented Jan 5, 2018

tzapu commented Jan 10, 2018

ankitchiplunkar commented Apr 3, 2018

tzapu commented Dec 22, 2017 •

edited

Loading

tomusdrw Dec 22, 2017 •

edited

Loading

tzapu commented Dec 22, 2017 •

edited

Loading

tzapu commented Dec 23, 2017 •

edited

Loading