fix!: Fix eth tx hashes in json-rpc responses #1176

yihuang · 2022-07-12T03:52:40Z

Description

Fix eth tx hashes in json-rpc responses

Closes: #1175

Remove Size_ field
Validate From/Hash fields in ante handler
Recompute tx hashes in json-rpc apis to cope with old blocks

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)

codecov · 2022-07-12T03:57:54Z

Codecov Report

Merging #1176 (479a812) into main (ea81e15) will increase coverage by 0.03%.
The diff coverage is 73.00%.

❗ Current head 479a812 differs from pull request most recent head 2acc2fb. Consider uploading reports for the commit 2acc2fb to get more accurate results

@@            Coverage Diff             @@
##             main    #1176      +/-   ##
==========================================
+ Coverage   62.27%   62.31%   +0.03%     
==========================================
  Files          91       91              
  Lines        7369     7411      +42     
==========================================
+ Hits         4589     4618      +29     
- Misses       2553     2563      +10     
- Partials      227      230       +3

Impacted Files	Coverage Δ
rpc/types/utils.go	`0.00% <0.00%> (ø)`
x/evm/types/tx_args.go	`0.00% <0.00%> (ø)`
rpc/types/events.go	`88.09% <60.00%> (-2.51%)`	⬇️
app/ante/eth.go	`81.54% <65.45%> (-1.42%)`	⬇️
x/evm/keeper/params.go	`100.00% <100.00%> (ø)`
x/evm/types/msg.go	`85.04% <100.00%> (+1.12%)`	⬆️
x/evm/types/utils.go	`62.50% <100.00%> (+1.38%)`	⬆️
x/feemarket/keeper/params.go	`82.35% <100.00%> (+5.42%)`	⬆️

CHANGELOG.md

fedekunze

Thanks @yihuang, please add tests so that this doesn't break again

CHANGELOG.md

yihuang · 2022-07-13T02:02:15Z

Thanks @yihuang, please add tests so that this doesn't break again

unit tests added.
And I updated the PR to fix the root cause:

~~Removed Size_ field from MsgEthereumTx~~, reverted, maybe better to keep it to avoid breaking the current tx format.
Validate Hash/From field in MsgEthereumTx

app/ante/eth.go

Closes: evmos#1175 - Remove Size_ field - Validate From/Hash fields in ante handler - Recompute tx hashes in json-rpc apis to cope with old blocks Update CHANGELOG.md remove Size_, validate Hash/From, add unit tests update spec Update CHANGELOG.md Update app/ante/eth.go populate From in SendRawTransaction Apply suggestions from code review keep Size_ field to avoid breaking tx format

yihuang · 2022-07-15T06:38:17Z

Thanks @yihuang, please add tests so that this doesn't break again

tests added.

app/ante/eth.go

x/evm/types/msg.go

x/evm/spec/04_transactions.md

proto/ethermint/evm/v1/tx.proto

app/ante/eth.go

yihuang · 2022-07-19T02:40:46Z

Added a commit to workaround the issue in event parsing, this is needed to handle the legacy blocks even if we have the root cause fix, without require user to resync from scatch.

fedekunze

ACK. Let's create an issue to deal with the breaking change

fedekunze · 2022-07-19T04:49:33Z

@yihuang some checks are failing

yihuang · 2022-07-19T05:22:50Z

ACK. Let's create an issue to deal with the breaking change

Do you mean removing the Size_ field?

yihuang · 2022-07-19T05:40:57Z

@yihuang some checks are failing

done, unit tests fixed.

* Fix eth tx hashes in json-rpc responses Closes: evmos#1175 - Remove Size_ field - Validate From/Hash fields in ante handler - Recompute tx hashes in json-rpc apis to cope with old blocks Update CHANGELOG.md remove Size_, validate Hash/From, add unit tests update spec Update CHANGELOG.md Update app/ante/eth.go populate From in SendRawTransaction Apply suggestions from code review keep Size_ field to avoid breaking tx format * move some validation to ValidateBasic * move validation to ValidateBasic * make ToTransaction returns a valid msg * restructure the protoTxProvider check * add comment * workaround tx hash issue in event parsing * fix integration test * fix unit test Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* Fix eth tx hashes in json-rpc responses Closes: #1175 - Remove Size_ field - Validate From/Hash fields in ante handler - Recompute tx hashes in json-rpc apis to cope with old blocks Update CHANGELOG.md remove Size_, validate Hash/From, add unit tests update spec Update CHANGELOG.md Update app/ante/eth.go populate From in SendRawTransaction Apply suggestions from code review keep Size_ field to avoid breaking tx format * move some validation to ValidateBasic * move validation to ValidateBasic * make ToTransaction returns a valid msg * restructure the protoTxProvider check * add comment * workaround tx hash issue in event parsing * fix integration test * fix unit test Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* Fix eth tx hashes in json-rpc responses Closes: evmos#1175 - Remove Size_ field - Validate From/Hash fields in ante handler - Recompute tx hashes in json-rpc apis to cope with old blocks Update CHANGELOG.md remove Size_, validate Hash/From, add unit tests update spec Update CHANGELOG.md Update app/ante/eth.go populate From in SendRawTransaction Apply suggestions from code review keep Size_ field to avoid breaking tx format * move some validation to ValidateBasic * move validation to ValidateBasic * make ToTransaction returns a valid msg * restructure the protoTxProvider check * add comment * workaround tx hash issue in event parsing * fix integration test * fix unit test Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* Fix eth tx hashes in json-rpc responses Closes: evmos#1175 - Validate From/Hash fields in ante handler - Recompute tx hashes in json-rpc apis to cope with old blocks Update CHANGELOG.md validate Hash/From, add unit tests update spec Update CHANGELOG.md Update app/ante/eth.go populate From in SendRawTransaction Apply suggestions from code review keep Size_ field to avoid breaking tx format * move some validation to ValidateBasic * move validation to ValidateBasic * make ToTransaction returns a valid msg * restructure the protoTxProvider check * add comment * workaround tx hash issue in event parsing * fix integration test * fix unit test Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> (cherry pick from commit 5cae04e)

yihuang requested review from fedekunze, khoslaventures and jolube as code owners July 12, 2022 03:52

yihuang commented Jul 12, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fedekunze suggested changes Jul 12, 2022

View reviewed changes

yihuang force-pushed the fix-tx-hash branch from 653e281 to 4eb5f99 Compare July 13, 2022 01:58

yihuang commented Jul 13, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

yihuang requested a review from fedekunze July 13, 2022 02:01

yihuang commented Jul 13, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

yihuang commented Jul 13, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

yihuang commented Jul 13, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

yihuang force-pushed the fix-tx-hash branch from 44461de to 511ec29 Compare July 13, 2022 04:08

yihuang mentioned this pull request Jul 13, 2022

Problem: does the problem of TX missing been solved after upgrade to v0.7.1-rc0 crypto-org-chain/cronos#579

Closed

yihuang changed the title ~~Fix eth tx hashes in json-rpc responses~~ fix!: Fix eth tx hashes in json-rpc responses Jul 13, 2022

yihuang force-pushed the fix-tx-hash branch from 511ec29 to dbacd1a Compare July 14, 2022 07:18

yihuang mentioned this pull request Jul 14, 2022

Problem: eth tx hash not validated and some json-rpc return incorrect one crypto-org-chain/cronos#584

Merged

13 tasks

yihuang force-pushed the fix-tx-hash branch from dbacd1a to c186a3f Compare July 14, 2022 08:20

yihuang force-pushed the fix-tx-hash branch from c186a3f to 922a609 Compare July 14, 2022 08:47

crypto-facs approved these changes Jul 14, 2022

View reviewed changes

fedekunze reviewed Jul 15, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

app/ante/eth.go Outdated Show resolved Hide resolved

move some validation to ValidateBasic

c8deef5

yihuang force-pushed the fix-tx-hash branch from 126b453 to c8deef5 Compare July 18, 2022 04:58

fedekunze reviewed Jul 18, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

fedekunze reviewed Jul 18, 2022

View reviewed changes

app/ante/eth.go Outdated Show resolved Hide resolved

x/evm/types/msg.go Show resolved Hide resolved

x/evm/spec/04_transactions.md Show resolved Hide resolved

proto/ethermint/evm/v1/tx.proto Show resolved Hide resolved

app/ante/eth.go Outdated Show resolved Hide resolved

yihuang added 2 commits July 19, 2022 08:51

move validation to ValidateBasic

b533467

make ToTransaction returns a valid msg

7220aa7

yihuang added 4 commits July 19, 2022 09:01

restructure the protoTxProvider check

72158ac

add comment

fe67ed1

Merge remote-tracking branch 'origin/main' into fix-tx-hash

5ff83ed

workaround tx hash issue in event parsing

c6bf219

fedekunze approved these changes Jul 19, 2022

View reviewed changes

fix integration test

7d6a59e

fix unit test

b056db9

Merge branch 'main' into fix-tx-hash

479a812

fedekunze enabled auto-merge (squash) July 19, 2022 15:03

Merge branch 'main' into fix-tx-hash

2acc2fb

fedekunze merged commit ffe78da into evmos:main Jul 19, 2022

yihuang deleted the fix-tx-hash branch July 19, 2022 15:18

fedekunze added a commit that referenced this pull request Aug 3, 2022

revert PR #1176

b0aebf5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: Fix eth tx hashes in json-rpc responses #1176

fix!: Fix eth tx hashes in json-rpc responses #1176

yihuang commented Jul 12, 2022 •

edited

Loading

codecov bot commented Jul 12, 2022 •

edited

Loading

fedekunze left a comment

yihuang commented Jul 13, 2022 •

edited

Loading

yihuang commented Jul 15, 2022

yihuang commented Jul 19, 2022

fedekunze left a comment

fedekunze commented Jul 19, 2022

yihuang commented Jul 19, 2022

yihuang commented Jul 19, 2022

fix!: Fix eth tx hashes in json-rpc responses #1176

fix!: Fix eth tx hashes in json-rpc responses #1176

Conversation

yihuang commented Jul 12, 2022 • edited Loading

Description

codecov bot commented Jul 12, 2022 • edited Loading

Codecov Report

fedekunze left a comment

Choose a reason for hiding this comment

yihuang commented Jul 13, 2022 • edited Loading

yihuang commented Jul 15, 2022

yihuang commented Jul 19, 2022

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze commented Jul 19, 2022

yihuang commented Jul 19, 2022

yihuang commented Jul 19, 2022

yihuang commented Jul 12, 2022 •

edited

Loading

codecov bot commented Jul 12, 2022 •

edited

Loading

yihuang commented Jul 13, 2022 •

edited

Loading