Skip to content
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

Utilize BalanceChangeReason in tracing #2789

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ganeshvanahalli
Copy link
Contributor

@ganeshvanahalli ganeshvanahalli commented Nov 15, 2024

This PR enables usage of BalanceChangeReason type in tracing, and also enables tracing of balance changes involved in a transaction using jsTracer (sample trace has been printed out from a system_test), enabling clearer way to reason with the balance changes of almost all transaction types.

Pulls in geth PR- OffchainLabs/go-ethereum#375
Resolves NIT-2943

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Nov 15, 2024
eljobe
eljobe previously approved these changes Nov 18, 2024
Copy link
Member

@eljobe eljobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eljobe eljobe self-requested a review January 15, 2025 09:01
eljobe
eljobe previously approved these changes Jan 15, 2025
@eljobe
Copy link
Member

eljobe commented Jan 16, 2025

@ganeshvanahalli, can you resolve the merge conflicts so we can get this merged?

@ganeshvanahalli
Copy link
Contributor Author

@ganeshvanahalli, can you resolve the merge conflicts so we can get this merged?

@eljobe, I've resolved the conflicts but the corresponding geth PR needs a review before we merge this in.

Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 18.18182% with 36 lines in your changes missing coverage. Please review.

Project coverage is 22.61%. Comparing base (353207d) to head (84ef370).
Report is 510 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2789      +/-   ##
==========================================
+ Coverage   22.56%   22.61%   +0.04%     
==========================================
  Files         269      269              
  Lines       39923    39922       -1     
==========================================
+ Hits         9009     9028      +19     
+ Misses      29438    29420      -18     
+ Partials     1476     1474       -2     

@tsahee tsahee added the after-next-version This PR shouldn't be merged until after the next version is released label Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
after-next-version This PR shouldn't be merged until after the next version is released design-approved s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants