Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

eth_getFilterLogs, eth_getLogs implementation #248

Merged
merged 88 commits into from
Apr 13, 2020
Merged

Conversation

noot
Copy link
Contributor

@noot noot commented Apr 13, 2020

Closes: #54 closes #55

Description

  • implement eth_getFilterLogs and eth_getLogs
  • also implements eth_getFilterChanges for log filters
  • fix transaction hash mismatch in handler/ state transition, was causing various issues like not being able to lookup tx logs

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

x/evm/types/state_transition.go Outdated Show resolved Hide resolved
rpc/filter_api.go Outdated Show resolved Hide resolved
rpc/tester/tester_test.go Outdated Show resolved Hide resolved
rpc/tester/tester_test.go Outdated Show resolved Hide resolved
rpc/filters.go Outdated Show resolved Hide resolved
rpc/filters.go Show resolved Hide resolved
@noot noot marked this pull request as ready for review April 13, 2020 16:47
rpc/backend.go Outdated Show resolved Hide resolved
rpc/backend.go Outdated Show resolved Hide resolved
rpc/filter_api.go Outdated Show resolved Hide resolved
rpc/filters.go Outdated Show resolved Hide resolved
rpc/filters.go Outdated Show resolved Hide resolved
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.

tested the changes and it seems that the contract can't be deployed (the transaction is processed but doesn't return anything to the web3 interface). Other than that the changes LGTM

yarn start             
yarn run v1.15.2
$ node ./web3/deploy_contract.js
Compiling contract code...
Unlocked account address: 	 0x0D08F68360524d01c0be5e2c4128836b653BD469
Deploying contract...
``

Simulate: ctx.IsCheckTx(),
}
// Prepare db for logs
k.CommitStateDB.Prepare(ethHash, common.Hash{}, k.TxCount)
// TODO: block hash
Copy link
Contributor

Choose a reason for hiding this comment

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

is this to add the block hash to the Prepare fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, right now it uses txHash for both fields

@noot
Copy link
Contributor Author

noot commented Apr 13, 2020

are you deploying the same contract in the script? will take a look

@fedekunze
Copy link
Contributor

are you deploying the same contract in the script? will take a look

yeah I'm using master on ethermint-deploy

@noot
Copy link
Contributor Author

noot commented Apr 13, 2020

@fedekunze should be fixed now!

however, the tx hash returned by eth_sendTransaction doesn't match the hash of the ethereum transaction message, do you know how the tx hash in eth_sendTransaction is being calculated? https://github.com/ChainSafe/ethermint/blob/af8133464b95db447090a7c6d618763672e510f2/rpc/eth_api.go#L294

edit: nvm I see what's happening, will update

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.

tested ACK. Thanks @noot. Can you add a new changelog entry under features?

@noot
Copy link
Contributor Author

noot commented Apr 13, 2020

@fedekunze will add now!

@noot noot merged commit 199484f into development Apr 13, 2020
@noot noot deleted the noot/log-filter branch April 13, 2020 19:18
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.

Implement eth_getLogs Implement eth_getFilterLogs
4 participants