Skip to content

Add state sync transaction to debug_trace functions.#547

Merged
0xsharma merged 9 commits into
developfrom
shivam/POS-824
Oct 25, 2022
Merged

Add state sync transaction to debug_trace functions.#547
0xsharma merged 9 commits into
developfrom
shivam/POS-824

Conversation

@0xsharma

@0xsharma 0xsharma commented Oct 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Because state sync transaction is added by bor consensus, it will not be included in debug_traceByBlockNumber and debug_tranceByBlockHash. The problem seems to be that block.Transactions() doesn’t contain state sync transaction.

In this PR, tracing can be possible on state-sync transactions as well. All the debug_trace functions will contain the traces of state-sync transactions, if present.

Changes

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have added at least 2 reviewer or the whole pos-v1 team
  • I have added sufficient documentation in code
  • I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply

Cross repository changes

  • This PR requires changes to heimdall
    • In case link the PR here:
  • This PR requires changes to matic-cli
    • In case link the PR here:

Testing

  • I have added unit tests
  • I have added tests to CI
  • I have tested this code manually on local environment
  • I have tested this code manually on remote devnet using express-cli
  • I have tested this code manually on mainnet
  • I have created new e2e tests into express-cli

@0xsharma 0xsharma requested review from a team, cffls and temaniarpit27 October 12, 2022 14:45
@0xsharma 0xsharma changed the title add : Add state sync transaction to debug_traceBlock Add state sync transaction to debug_trace functions. Oct 12, 2022
@codecov-commenter

codecov-commenter commented Oct 14, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 56.81% // Head: 56.80% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (2542403) compared to base (54f97b8).
Patch coverage: 22.29% of modified lines in pull request are covered.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #547      +/-   ##
===========================================
- Coverage    56.81%   56.80%   -0.01%     
===========================================
  Files          606      606              
  Lines        70128    70272     +144     
===========================================
+ Hits         39840    39917      +77     
- Misses       26872    26925      +53     
- Partials      3416     3430      +14
Impacted Files Coverage Δ
eth/tracers/api_bor.go 0.00% <0.00%> (ø)
eth/tracers/api.go 22.24% <24.62%> (-0.57%) ⬇️
les/pruner.go 60.71% <0.00%> (-3.58%) ⬇️
p2p/simulations/mocker.go 30.00% <0.00%> (-1.12%) ⬇️
eth/downloader/fetchers_concurrent.go 53.74% <0.00%> (-0.69%) ⬇️
internal/cli/server/server.go 30.21% <0.00%> (-0.68%) ⬇️
p2p/simulations/http.go 66.02% <0.00%> (-0.65%) ⬇️
core/tx_pool.go 70.41% <0.00%> (-0.54%) ⬇️
core/blockchain.go 59.46% <0.00%> (-0.09%) ⬇️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@0xsharma 0xsharma marked this pull request as ready for review October 14, 2022 09:04
cffls
cffls reviewed Oct 14, 2022
View reviewed changes
Comment thread core/vm/interface.go
Comment thread eth/tracers/api.go
Comment thread eth/tracers/api_bor.go
@0xsharma

0xsharma commented Oct 21, 2022

Copy link
Copy Markdown
Member Author

debug.traceBlockByNumber("0x20FE900",{"borTraceEnabled": true}).length
203
debug.traceBlockByNumber("0x20FE900",{"borTraceEnabled": false}).length
202
debug.traceBlockByNumber("0x20FE900").length
202

We can now specify inside the rpc call, if we want the trace of stateSyncTx as well. By default, it is false. This works for all debug trace methods.

cffls
cffls reviewed Oct 24, 2022
View reviewed changes
Comment thread eth/tracers/api.go
@0xsharma 0xsharma merged commit be3f297 into develop Oct 25, 2022
@0xsharma 0xsharma deleted the shivam/POS-824 branch October 25, 2022 06:52
@cffls cffls mentioned this pull request Nov 2, 2022
Merged
15 tasks
pratikspatil024 added a commit that referenced this pull request Mar 16, 2023
@pratikspatil024
Revert "Merge pull request #547 from maticnetwork/shivam/POS-824"
97532b7 
This reverts commit be3f297, reversing
changes made to 3fdb45e.
pratikspatil024 added a commit that referenced this pull request Mar 16, 2023
@pratikspatil024
Revert "Revert "Merge pull request #547 from maticnetwork/shivam/POS-…
22aade6 
…824""

This reverts commit 97532b7.
@MRabenda

MRabenda commented May 26, 2023

Copy link
Copy Markdown

@0xsharma @pratikspatil024 should state-sync traces be also available in trace_block?

@0xsharma

0xsharma commented May 26, 2023

Copy link
Copy Markdown
Member Author

@MRabenda , yes they will be available but as empty traces as state-syncs are system transactions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pratikspatil024 pratikspatil024 pratikspatil024 approved these changes

@cffls cffls cffls approved these changes

@temaniarpit27 temaniarpit27 Awaiting requested review from temaniarpit27

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@0xsharma @codecov-commenter @MRabenda @pratikspatil024 @cffls