EVM tracing support #802

akru · 2022-12-09T06:55:06Z

Pull Request Summary

This PR based on Moonbeam great job with EVM tracing support. As base used EVM tracing module from PureStake repositories, adopted and improved to suits Astar requirements and code base.

Tracing modules PR: AstarNetwork/astar-frame#120

Check list

added or updated unit tests
updated Astar official documentation
added OnRuntimeUpgrade hook for precompile revert code registration
updated spec version
updated semver

Dinonard · 2023-01-03T10:22:39Z

bin/collator/src/cli.rs

+    /// Maximum number of logs in a query.
+    #[clap(long, default_value = "10000")]
+    pub max_past_logs: u32,
+


Good docs page will be needed for this.

Agree, parameters should be explained in details.

Dinonard · 2023-01-03T10:25:27Z

bin/collator/src/cli.rs

+    }
+}
+
+pub struct TracingConfig {


How about EvmTracingConfig? It's more precise.

Dinonard · 2023-01-03T10:28:31Z

bin/collator/src/rpc.rs

+pub mod tracing;
+
+#[derive(Clone)]
+pub struct TracingConfig {


EvmTracingConfig perhaps?

Definitely, let me fix it.

Dinonard · 2023-01-03T10:31:59Z

bin/collator/src/rpc.rs

+    if let Some(trace_filter_requester) = tracing_config.tracing_requesters.trace {
+        io.merge(
+            Trace::new(
+                client,
+                trace_filter_requester,
+                tracing_config.trace_filter_max_count,
+            )
+            .into_rpc(),
+        )?;
+    }
+
+    if let Some(debug_requester) = tracing_config.tracing_requesters.debug {
+        io.merge(Debug::new(debug_requester).into_rpc())?;
+    }


Haven't checked the code in details, but can't we just reuse the existing function and add the highlighted part only to this method?

Something like:

fn create_full_tracing(...) -> Result<...> { let io = create_full(...)?; // the code you added }

Good to have.

I planed to remove one of this function when all runtimes will implement Debug runtime API.

Dinonard · 2023-01-03T10:37:14Z

runtime/local/src/lib.rs

@@ -1467,4 +1467,70 @@ impl_runtime_apis! {
            Executive::try_execute_block(block, state_root_check, select).expect("execute-block failed")
        }
    }
+
+    #[cfg(feature = "evm-tracing")]


Could you please explain why is this feature flag used?

Not sure I understand given it's runtime code.

Technically we can build runtime without this feature, because it’s still experimental. But in near future it could be moved to default or removed.

Removed this feature

shunsukew

~~Ethereum doesn't allow Debug RPCs by default. Node operators need to configure to intentionally allow Debug APIs being exposed.~~
https://geth.ethereum.org/docs/interacting-with-geth/rpc

The default whitelist allows access to the eth, net and web3 namespaces. To enable access to other APIs like debugging (debug), they must be configured using the --http.api flag. Enabling these APIs over HTTP is not recommended because access to these methods increases the attack surface.

~~I think we should do the same at least by having a new flag, otherwise all public node operators need some measurements to block these RPCs in front of node processes.~~

I just noticed it's already implemented so.

Dinonard

Checked the delta & LGTM but perhaps @shunsukew or @0x7CFE who invested more time into EVM tracing can provide better feedback.

akru · 2023-01-13T01:18:43Z

@shunsukew @0x7CFE please take a look

bin/collator/src/local/service.rs

bin/collator/src/parachain/service.rs

Co-authored-by: Shunsuke Watanabe <[email protected]>

0x7CFE

Was that intentional that spec version was updated only for local runtime?

0x7CFE · 2023-01-13T07:44:57Z

runtime/local/src/lib.rs

@@ -1448,6 +1448,70 @@ impl_runtime_apis! {
        }
    }

+    impl moonbeam_rpc_primitives_debug::DebugRuntimeApi<Block> for Runtime {


Looks like this impl is mostly copypaste? Maybe we should extract it into a single module, used by all runtimes?

That’s right, usually multi-runtimes projects create something like common module with shared implementations. Problem here it’s under macros and move it outside makes code more and more sophisticated.

akru · 2023-01-13T08:27:24Z

@0x7CFE please point me to spec version you mean.

0x7CFE · 2023-01-17T08:09:56Z

@0x7CFE please point me to spec version you mean.

@akru please ignore that, I misread the code.

* EVM tracing support (#802) * Added EVM tracing into local runtime * Added tracing CLI * Compilation fixes * Added EVM tracing host functions * Added EVM tracing into parachain runtimes * TracingConfig -> EvmTracingConfig * Compilation fixes * Fix clippy errors * Fix formatting * Updated dependencies in Cargo.lock * Updated Cargo.lock * Update bin/collator/src/local/service.rs Co-authored-by: Shunsuke Watanabe <[email protected]> * Update bin/collator/src/parachain/service.rs Co-authored-by: Shunsuke Watanabe <[email protected]> * Bump astar/shiden runtime specs Co-authored-by: Shunsuke Watanabe <[email protected]> * Fix srtool build * [CI] temporary disable aarch64 target * [CI] fix binaries upload for release pipeline * Fix CI errors * Update rust toolchain version to 2023-01-11 * Update Cargo.lock * Added txpool rpc support (#855) * Added txpool rpc support * Fix licensing * Bump dependencies & runtime versions * Remove outdated migrations from runtimes * Sort review comments --------- Co-authored-by: Shunsuke Watanabe <[email protected]>

akru added 2 commits December 23, 2022 10:29

Added EVM tracing into local runtime

0c31b1f

Added tracing CLI

0fa613c

akru force-pushed the feature/evm-tracing branch from 00843ea to 0fa613c Compare December 23, 2022 07:42

Compilation fixes

d87e3cc

akru marked this pull request as ready for review December 23, 2022 08:45

akru requested review from Dinonard, 0x7CFE and shunsukew December 23, 2022 08:45

Added EVM tracing host functions

db80d20

Dinonard reviewed Jan 3, 2023

View reviewed changes

shunsukew reviewed Jan 3, 2023

View reviewed changes

shunsukew self-requested a review January 3, 2023 15:29

akru added 10 commits January 9, 2023 07:28

Added EVM tracing into parachain runtimes

0f17ad0

TracingConfig -> EvmTracingConfig

3e3fa2a

Compilation fixes

133cdd4

Fix clippy errors

1f1a161

Fix formatting

b8b9c13

Merge branch 'master' into feature/evm-tracing

4ef92ad

Merge remote-tracking branch 'origin/master'

aba8f28

Updated dependencies in Cargo.lock

2d88c91

Merge remote-tracking branch 'origin/master'

ad0e463

Updated Cargo.lock

b9a23d6

Dinonard approved these changes Jan 12, 2023

View reviewed changes

shunsukew reviewed Jan 13, 2023

View reviewed changes

bin/collator/src/local/service.rs Outdated Show resolved Hide resolved

bin/collator/src/parachain/service.rs Outdated Show resolved Hide resolved

akru and others added 2 commits January 13, 2023 14:30

Update bin/collator/src/local/service.rs

f85c1f8

Co-authored-by: Shunsuke Watanabe <[email protected]>

Update bin/collator/src/parachain/service.rs

08e12e7

Co-authored-by: Shunsuke Watanabe <[email protected]>

shunsukew self-requested a review January 13, 2023 05:58

shunsukew approved these changes Jan 13, 2023

View reviewed changes

0x7CFE approved these changes Jan 13, 2023

View reviewed changes

akru changed the base branch from master to 5.x.x January 16, 2023 04:56

akru added 2 commits January 16, 2023 09:58

Merge branch '5.x.x' into feature/evm-tracing

1641403

Bump astar/shiden runtime specs

1a02108

akru mentioned this pull request Jan 16, 2023

Added EVM debug page AstarNetwork/astar-docs#227

Merged

akru merged commit fe58b85 into 5.x.x Jan 16, 2023

akru deleted the feature/evm-tracing branch January 16, 2023 08:38

0x7CFE approved these changes Jan 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EVM tracing support #802

EVM tracing support #802

akru commented Dec 9, 2022 •

edited

Loading

Dinonard Jan 3, 2023

akru Jan 4, 2023

Dinonard Jan 3, 2023

akru Jan 9, 2023

Dinonard Jan 3, 2023

akru Jan 4, 2023

akru Jan 9, 2023

Dinonard Jan 3, 2023

akru Jan 4, 2023

akru Jan 9, 2023

Dinonard Jan 3, 2023

akru Jan 4, 2023

akru Jan 9, 2023

shunsukew left a comment •

edited

Loading

Dinonard left a comment

akru commented Jan 13, 2023

0x7CFE left a comment

0x7CFE Jan 13, 2023

akru Jan 13, 2023

akru commented Jan 13, 2023

0x7CFE commented Jan 17, 2023

EVM tracing support #802

EVM tracing support #802

Conversation

akru commented Dec 9, 2022 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shunsukew left a comment • edited Loading

Choose a reason for hiding this comment

Dinonard left a comment

Choose a reason for hiding this comment

akru commented Jan 13, 2023

0x7CFE left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akru commented Jan 13, 2023

0x7CFE commented Jan 17, 2023

akru commented Dec 9, 2022 •

edited

Loading

shunsukew left a comment •

edited

Loading