Problem: traceTransaction fails for succesful tx #720

yihuang · 2021-11-05T04:24:42Z

Description

Solution:

Change the context to the beginning of the block, rather than the end
of it
fix block context to the target one, while load state from the previous one
replay predecessor txs to get correct context for tracing.

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Solution: - Change the context to the begining of the block, rather than the end of it, while override block context to correct one pass predecessors pass current block information to grpc query

crypto-facs · 2021-11-05T13:22:49Z

This PR fixes important issues. LGTM! 🚀

x/evm/keeper/grpc_query.go

codecov · 2021-11-05T15:21:08Z

Codecov Report

Merging #720 (5cd28c3) into main (7c7f3f0) will increase coverage by 0.20%.
The diff coverage is 57.81%.

@@            Coverage Diff             @@
##             main     #720      +/-   ##
==========================================
+ Coverage   56.36%   56.56%   +0.20%     
==========================================
  Files          64       64              
  Lines        5715     5776      +61     
==========================================
+ Hits         3221     3267      +46     
- Misses       2300     2309       +9     
- Partials      194      200       +6

Impacted Files	Coverage Δ
x/evm/types/query.go	`0.00% <0.00%> (ø)`
x/evm/keeper/grpc_query.go	`66.09% <68.51%> (+3.14%)`	⬆️
x/evm/keeper/keeper.go	`76.00% <0.00%> (+0.57%)`	⬆️

khoslaventures · 2021-11-05T20:34:36Z

Do you have a way to test this works?

yihuang · 2021-11-06T00:37:20Z

Do you have a way to test this works?

We ported the fix to 0.7.2 here: https://github.com/crypto-org-chain/ethermint/releases/tag/v0.7.2-cronos20211105, and verified on our mainnet dry-run, at least exiting known cases are all fixed by this. But yes, it would be good to have some integrated testing.
There's still known issue with traceBlock which @crypto-facs is still working on.

fedekunze

Minor comments. Please also add a test case

rpc/ethereum/namespaces/debug/api.go

proto/ethermint/evm/v1/query.proto

rpc/ethereum/namespaces/debug/api.go

fedekunze · 2021-11-08T08:13:40Z

x/evm/keeper/grpc_query.go

+	for i, tx := range req.Predecessors {
+		ethTx := tx.AsTransaction()
+		msg, err := ethTx.AsMessage(signer, baseFee)
+		if err != nil {


this error should be logged

x/evm/keeper/grpc_query.go

…to fix-tracetx-context

proto/ethermint/evm/v1/tx.proto

proto/ethermint/evm/v1/query.proto

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

go.mod

rpc/ethereum/namespaces/debug/api.go

x/evm/keeper/grpc_query.go

fedekunze · 2021-11-09T13:02:36Z

x/evm/keeper/grpc_query.go

+	for i, tx := range req.Predecessors {
+		ethTx := tx.AsTransaction()
+		msg, err := ethTx.AsMessage(signer, baseFee)
+		if err != nil {


Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

…to fix-tracetx-context

fedekunze

ACK, minor logger comments

rpc/ethereum/namespaces/debug/api.go

Co-authored-by: Freddy Caceres <freddy.caceres@crypto.com> Co-authored-by: crypto-facs <84574577+crypto-facs@users.noreply.github.com> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

yihuang requested review from fedekunze, jolube and khoslaventures as code owners November 5, 2021 04:24

Problem: traceTransaction fails for succesful tx

7233abb

Solution: - Change the context to the begining of the block, rather than the end of it, while override block context to correct one pass predecessors pass current block information to grpc query

yihuang force-pushed the fix-tracetx-context branch from b67273d to 7233abb Compare November 5, 2021 12:51

changelog

1b27db1

crypto-facs reviewed Nov 5, 2021

View reviewed changes

x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved

fix build

f1c7d55

yihuang and others added 4 commits November 6, 2021 08:39

fix lint

f5e0a6c

refactor traceBlock

2f9197c

update protobuf

eeb7d22

fix Predecessors

346105b

fedekunze reviewed Nov 8, 2021

View reviewed changes

crypto-facs added 3 commits November 8, 2021 08:51

traceBlock refactor

e1ff8fd

Merge branch 'fix-tracetx-context' of github.com:yihuang/ethermint in…

84b6e27

…to fix-tracetx-context

refactor traceBlock response

8034d60

fedekunze reviewed Nov 8, 2021

View reviewed changes

crypto-facs and others added 9 commits November 8, 2021 11:04

Update proto/ethermint/evm/v1/tx.proto

fa47b90

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update proto/ethermint/evm/v1/query.proto

bb7c8cb

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update proto/ethermint/evm/v1/query.proto

042c4df

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update proto/ethermint/evm/v1/query.proto

4e99b42

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update proto/ethermint/evm/v1/query.proto

fb9cb2f

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update proto/ethermint/evm/v1/query.proto

28946cb

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

fix conflicts

5826910

check tx index is not out of bound

e55fa23

fix build

2468ec8

crypto-facs and others added 5 commits November 9, 2021 06:52

Update rpc/ethereum/namespaces/debug/api.go

0a3ee7d

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update rpc/ethereum/namespaces/debug/api.go

40e9675

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update rpc/ethereum/namespaces/debug/api.go

4f3f827

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Update rpc/ethereum/namespaces/debug/api.go

3d1ea65

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

remove prealloc

f493ccf

fedekunze reviewed Nov 9, 2021

View reviewed changes

crypto-facs and others added 6 commits November 9, 2021 11:15

add traceBlock test

0567e8f

Update x/evm/keeper/grpc_query.go

74b491a

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Merge branch 'fix-tracetx-context' of github.com:yihuang/ethermint in…

805d2e4

…to fix-tracetx-context

use bytes2Hex

2c9c9f2

fix error message

14203f8

add comment

1ea5970

fedekunze approved these changes Nov 9, 2021

View reviewed changes

rpc/ethereum/namespaces/debug/api.go Outdated Show resolved Hide resolved

rpc/ethereum/namespaces/debug/api.go Show resolved Hide resolved

Apply suggestions from code review

417624a

fedekunze enabled auto-merge (squash) November 9, 2021 18:33

fedekunze reviewed Nov 9, 2021

View reviewed changes

rpc/ethereum/namespaces/debug/api.go Outdated Show resolved Hide resolved

Update rpc/ethereum/namespaces/debug/api.go

5cd28c3

fedekunze merged commit 2a205e5 into evmos:main Nov 9, 2021

yihuang deleted the fix-tracetx-context branch November 9, 2021 23:43

thomas-nguy mentioned this pull request Nov 16, 2021

Commits needed for release v0.7.3 #752

Closed

coderabbitai bot mentioned this pull request Apr 19, 2024

Problem: panic on invalid elasticity_multiplier crypto-org-chain/cronos#1397

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem: traceTransaction fails for succesful tx #720

Problem: traceTransaction fails for succesful tx #720

yihuang commented Nov 5, 2021 •

edited

Loading

crypto-facs commented Nov 5, 2021

codecov bot commented Nov 5, 2021 •

edited

Loading

khoslaventures commented Nov 5, 2021

yihuang commented Nov 6, 2021 •

edited

Loading

fedekunze left a comment

fedekunze Nov 8, 2021

fedekunze Nov 9, 2021

fedekunze Nov 9, 2021

fedekunze left a comment

Problem: traceTransaction fails for succesful tx #720

Problem: traceTransaction fails for succesful tx #720

Conversation

yihuang commented Nov 5, 2021 • edited Loading

Description

crypto-facs commented Nov 5, 2021

codecov bot commented Nov 5, 2021 • edited Loading

Codecov Report

khoslaventures commented Nov 5, 2021

yihuang commented Nov 6, 2021 • edited Loading

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze Nov 8, 2021

Choose a reason for hiding this comment

fedekunze Nov 9, 2021

Choose a reason for hiding this comment

fedekunze Nov 9, 2021

Choose a reason for hiding this comment

fedekunze left a comment

Choose a reason for hiding this comment

yihuang commented Nov 5, 2021 •

edited

Loading

codecov bot commented Nov 5, 2021 •

edited

Loading

yihuang commented Nov 6, 2021 •

edited

Loading