Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Emit eth tx hash in ante handler to support query failed transactions #1062

Merged
merged 14 commits into from
May 31, 2022

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Apr 21, 2022

Description

WIP: #1045
Solution:

  • emit eth tx hash in ante handler
  • modify rpc apis to use it

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)

CHANGELOG.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #1062 (c99ba46) into main (afc09f9) will increase coverage by 1.11%.
The diff coverage is 89.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1062      +/-   ##
==========================================
+ Coverage   59.52%   60.64%   +1.11%     
==========================================
  Files          85       86       +1     
  Lines        7015     7085      +70     
==========================================
+ Hits         4176     4297     +121     
+ Misses       2622     2568      -54     
- Partials      217      220       +3     
Impacted Files Coverage Δ
rpc/types/utils.go 0.00% <ø> (ø)
app/ante/eth.go 82.67% <82.35%> (-0.02%) ⬇️
rpc/types/events.go 90.59% <90.59%> (ø)
app/ante/handler_options.go 78.26% <100.00%> (+0.31%) ⬆️

@yihuang
Copy link
Contributor Author

yihuang commented May 4, 2022

should be replaced by a fix for #1075.

@yihuang yihuang closed this May 4, 2022
@yihuang yihuang reopened this May 5, 2022
@yihuang yihuang force-pushed the emit-tx-hash branch 6 times, most recently from 6e03d89 to 9bdac6b Compare May 16, 2022 12:01
@yihuang yihuang force-pushed the emit-tx-hash branch 3 times, most recently from b8710f7 to 00c3bf7 Compare May 23, 2022 09:46
go.mod Outdated Show resolved Hide resolved
@yihuang yihuang force-pushed the emit-tx-hash branch 2 times, most recently from 8191230 to b6e92ed Compare May 23, 2022 16:40
@yihuang yihuang marked this pull request as ready for review May 23, 2022 16:41
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

the approach LGTM. Left some suggestions and comments

go.mod Show resolved Hide resolved
rpc/types/utils.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
rpc/backend/utils.go Outdated Show resolved Hide resolved
rpc/namespaces/ethereum/eth/api.go Outdated Show resolved Hide resolved
rpc/namespaces/ethereum/eth/api.go Outdated Show resolved Hide resolved
rpc/namespaces/ethereum/eth/api.go Outdated Show resolved Hide resolved
server/util.go Show resolved Hide resolved
WIP: evmos#1045
Solution:
- emit eth tx hash in ante handler
- modify rpc to use it

fix ante handler

support failed tx in receipt

add unit tests

need to patch cosmos-sdk to work

update cosmos-sdk to v0.45.x release branch

fix failed status

fix unit tests

add unit test cases

cleanup dead code

Apply suggestions from code review

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

fix lint

fix review suggestions

fix build

fix gas used of failed tx

add back the redundant events
@yihuang
Copy link
Contributor Author

yihuang commented May 25, 2022

I've done all the tests on my side:

  • cronos integration tests passed.
  • I upgraded the binary on a production node and checked some json-rpc apis on old blocks, they work too.

@yihuang yihuang requested a review from fedekunze May 25, 2022 06:49
rpc/types/events_test.go Outdated Show resolved Hide resolved
rpc/types/events.go Show resolved Hide resolved
app/ante/eth.go Show resolved Hide resolved
@yihuang yihuang requested a review from fedekunze May 31, 2022 02:41
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM, minor suggestions 👍

rpc/types/events.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
yihuang and others added 4 commits May 31, 2022 13:51
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

much better 👍 thanks for the descriptions @yihuang

rpc/types/events.go Outdated Show resolved Hide resolved
rpc/types/events.go Outdated Show resolved Hide resolved
@fedekunze fedekunze enabled auto-merge (squash) May 31, 2022 10:22
@fedekunze fedekunze merged commit 5533bee into evmos:main May 31, 2022
@yihuang yihuang deleted the emit-tx-hash branch May 31, 2022 10:31
yihuang added a commit to yihuang/ethermint that referenced this pull request Jun 6, 2022
…ry failed transactions (evmos#1062)

* Emit eth tx hash in ante handler to support query failed transactions

WIP: evmos#1045
Solution:
- emit eth tx hash in ante handler
- modify rpc to use it

fix ante handler

support failed tx in receipt

add unit tests

need to patch cosmos-sdk to work

update cosmos-sdk to v0.45.x release branch

fix failed status

fix unit tests

add unit test cases

cleanup dead code

Apply suggestions from code review

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

fix lint

fix review suggestions

fix build

fix gas used of failed tx

add back the redundant events

* fix get tx by index

* add unit tests for events

* Update rpc/types/events.go

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

* update comments

* refactoring

* Update rpc/namespaces/ethereum/eth/api.go

* fix lint

* Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants