Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

R4R: CheckTx AnteHandler for Ethereum Tx Messages #505

Merged
merged 33 commits into from
Dec 18, 2018
Merged

R4R: CheckTx AnteHandler for Ethereum Tx Messages #505

merged 33 commits into from
Dec 18, 2018

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Nov 29, 2018 •
edited
Loading

  • Add ethereum tx ante handler checks (comparable to geth's pool ValidateTx) w/ unit tests
  • Add secp256k1 key pair wrappers
  • Updated the Ethereum tx message type to also implement the sdk.Tx interface so we don't waste/have unnecessary fields.

closes: #504

/cc @ValarDragon @cwgoes can you guys take a look at the ante handler?

Note: The SDK version was upgraded to a specific revision to include the necessary but still open SDK PR: cosmos/cosmos-sdk#2998

  • 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.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]"

@alexanderbez alexanderbez added pre-release wip The PR is considered a "work in progress" but may still warrant active review. core Components and implementations related to any core components of Ethermint. labels Nov 29, 2018
Aleksandr Bezobchuk added 21 commits November 29, 2018 16:15
Update SDK and TM deps
8b884f7
Update sequence and account number types
ef52d37
Add CodeInvalidChainID
e64d448
Update TestMsgEthereumTxSig
7a70d3c
Rename Ethereum tx message
aef5bb2
Use new tx decoder in the ethermint app
801f28d
Update app tests APIs
f313c7c
Update ante handler to prevent spam/dos
480d4bb
Update ethereum msg signing/verification logic
f4c49c3
Update SDK to revision with auth params
2b34e2b
Update app test utils
f56010f
Update account keeper creation in app init
92716ed
Implement secp256k1 key types
b37671f
Register crypto codec in the app
d728333
Update the evm types and utils
9234d51
Update app test utils
db4cb00
Wrap up ante handler and unit tests
f25653e
Fix importer
5b151eb
Fix linting
a65e270
Remove debug require statement
1cebcf2
Remove pointer from To method
025233b
@alexanderbez alexanderbez changed the title WIP: CheckTx AnteHandler for Ethereum Tx Messages R4R: CheckTx AnteHandler for Ethereum Tx Messages Dec 7, 2018
@alexanderbez alexanderbez added ready for review The PR is ready for review. and removed wip The PR is considered a "work in progress" but may still warrant active review. labels Dec 7, 2018
@cwgoes cwgoes self-assigned this Dec 12, 2018
cwgoes
cwgoes suggested changes Dec 12, 2018
View reviewed changes
Copy link

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Structure looks reasonable to me, several comments / requests for clarification.

Gopkg.toml Outdated Show resolved Hide resolved
Gopkg.toml Outdated Show resolved Hide resolved
app/ante.go Outdated Show resolved Hide resolved
app/ante.go Show resolved Hide resolved
app/ante.go Show resolved Hide resolved
app/ante.go Outdated Show resolved Hide resolved
app/ante.go Outdated Show resolved Hide resolved
app/ante.go Outdated Show resolved Hide resolved
crypto/secp256k1.go Show resolved Hide resolved
x/evm/types/msg.go Show resolved Hide resolved
@alexanderbez
Copy link
Contributor Author

@cwgoes updated 👍

@alexanderbez
Copy link
Contributor Author

@cwgoes addressed your comments.

@alexanderbez alexanderbez merged commit b821a85 into master Dec 18, 2018
@alexanderbez alexanderbez deleted the bez/504-ethtx-ante-checktx branch December 18, 2018 16:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cwgoes cwgoes cwgoes approved these changes

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal

@ValarDragon ValarDragon Awaiting requested review from ValarDragon

Assignees

@cwgoes cwgoes

Labels
core Components and implementations related to any core components of Ethermint. pre-release ready for review The PR is ready for review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Ethereum Tx Ante Handler Checks for CheckTx
2 participants
@alexanderbez @cwgoes