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

evm: implement vm.GetHashFn #620

Merged
merged 20 commits into from
Dec 7, 2020
Merged

evm: implement vm.GetHashFn #620

merged 20 commits into from
Dec 7, 2020

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Nov 25, 2020 •
edited
Loading

closes #618

TODO:

  • tests
  • store hash on end block

@fedekunze fedekunze changed the title fedekunze/618 getHash evm: implement vm.GetHashFn Nov 25, 2020
@fedekunze fedekunze marked this pull request as ready for review November 25, 2020 13:17
@fedekunze fedekunze requested a review from noot as a code owner November 25, 2020 13:17
@fedekunze
Copy link
Contributor Author

actually, we will need to store the (height, hash) pair on the EVM state

@fedekunze fedekunze marked this pull request as draft November 25, 2020 14:14
@lgtm-com
Copy link

lgtm-com bot commented Nov 26, 2020

This pull request introduces 1 alert when merging 5041f8d into 4eccac8 - view on LGTM.com

new alerts:

  • 1 for Unreachable statement

@lgtm-com
Copy link

lgtm-com bot commented Nov 26, 2020

This pull request introduces 1 alert when merging 2674e5b into 34a6011 - view on LGTM.com

new alerts:

  • 1 for Unreachable statement

@lgtm-com
Copy link

lgtm-com bot commented Nov 27, 2020

This pull request introduces 1 alert when merging 4a2e4c8 into 34a6011 - view on LGTM.com

new alerts:

  • 1 for Unreachable statement

@fedekunze fedekunze mentioned this pull request Nov 30, 2020
Closed
@lgtm-com
Copy link

lgtm-com bot commented Nov 30, 2020

This pull request introduces 1 alert when merging 80e5de5 into 048a8bd - view on LGTM.com

new alerts:

  • 1 for Unreachable statement

@fedekunze fedekunze marked this pull request as ready for review December 3, 2020 23:06
@fedekunze fedekunze self-assigned this Dec 3, 2020
summerpro
summerpro reviewed Dec 4, 2020
View reviewed changes
x/evm/keeper/abci.go Outdated Show resolved Hide resolved
noot
noot approved these changes Dec 4, 2020
View reviewed changes
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

looks good, also wondering about the epoch, is there a case where the different epochs/chainIDs would be using the same store?

@fedekunze
Copy link
Contributor Author

looks good, also wondering about the epoch, is there a case where the different epochs/chainIDs would be using the same store?

Yeah so after an upgrade the chain height could be reset and thus a new height from the latest epoch can override the previous one. After giving it a second thought, I think that could be ok as we care for the latest height only.

@fedekunze fedekunze marked this pull request as draft December 7, 2020 11:32
@fedekunze fedekunze marked this pull request as ready for review December 7, 2020 16:11
@fedekunze fedekunze requested a review from noot December 7, 2020 16:11
@fedekunze
Copy link
Contributor Author

updated @noot

@fedekunze fedekunze merged commit c4a3c0a into development Dec 7, 2020
@fedekunze fedekunze deleted the fedekunze/618-getHash branch December 7, 2020 20:09
araskachoi
araskachoi reviewed Dec 8, 2020
View reviewed changes
Copy link
Contributor

@araskachoi araskachoi left a comment

Choose a reason for hiding this comment

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

Question: why don't we use csdb.WithContext(ctx).GetHeightHash(height) here as well?

araskachoi
araskachoi reviewed Dec 8, 2020
View reviewed changes
x/evm/types/state_transition.go
case ctx.BlockHeight() == int64(height):
// Case 1: The requested height matches the one from the context so we can retrieve the header
// hash directly from the context.
return HashFromContext(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why don't we use csdb.WithContext(ctx).GetHeightHash(height) here as well?

fedekunze added a commit that referenced this pull request Dec 14, 2020
@fedekunze @araskachoi
@dependabot @summerpro @okwme
stargate: fix conflicts (#650)
ede2dbc 
* deps: update to cosmos-sdk v0.39.2-rc3 (#608)

* update to cosmos-sdk v0.39.2

* fix HD path test

* changelog (#609)

* build: require go 1.15+ (#610)

* evm: state transitions enabled params (#603)

* changelog

* evm: state transitions enabled params

* changelog entry

* changelog

* deps: bump cosmos-sdk version to v0.39.2 (#615)

* deps: bump cosmos-sdk version to v0.39.2

* changelog

* rpc: fix deadlock (#614)

* update tools configs (#611)

* update tools configs

* minor updates

* add jq (#616)

* build(deps): bump gopkg.in/yaml.v2 from 2.3.0 to 2.4.0 (#622)

Bumps [gopkg.in/yaml.v2](https://github.com/go-yaml/yaml) from 2.3.0 to 2.4.0.
- [Release notes](https://github.com/go-yaml/yaml/releases)
- [Commits](go-yaml/yaml@v2.3.0...v2.4.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* app: fix export genesis (#619)

* v0.3.1 changes

* fix export genesis

* changelog

* evm: fix genesis format mismatch (#623)

* evm: fix genesis format mismatch

* genesis tests

* fix test

* changelog

* nolint

* force build-ethermint and cleanup stdout (#628)

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

* join network doc (#606)

* join network doc

* include faucet instructions

* add rpc instructions

* edit persistent peers

* update to latest testnet

* evm: implement vm.GetHashFn (#620)

* evm: implement vm.GetHashFn

* check nil case

* test

* handle 3 cases

* use switch statement

* stateDB tests

* abci changes

* fix LGTM issue

* final tests

* changelog

* remove epoch

* update test

* clean test

* rm epoch

* open Istanbul and MuirGlacier from ChainConfig (#645)

* open Istanbul and MuirGlacier

* fix ut

* fix ut

* evm: extra eips param (#643)

* evm: extra eips param

* changelog

* rpc: protocol version (#575)

* evm: protocol version

* changelog

* version

* fix

* support latest version only

* evm: module specification (#538)

* evm: module specification

* params and events

* readme and messages

* minor updates

* concepts

* genesis state concept

* begin and end block

* update parameters and genesis

* state objects

* state table

* use permalink

* init and export genesis

* update abci

* extra eips param

* review comments

* precision

* link to photon doc

* build(deps): bump gaurav-nelson/github-action-markdown-link-check (#649)

Bumps [gaurav-nelson/github-action-markdown-link-check](https://github.com/gaurav-nelson/github-action-markdown-link-check) from 1.0.8 to 1.0.9.
- [Release notes](https://github.com/gaurav-nelson/github-action-markdown-link-check/releases)
- [Commits](gaurav-nelson/github-action-markdown-link-check@1.0.8...7481451)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* stargate: fix conflicts

* more fixes

* proto changes

* more tendermint changes

* test fixes

* atlas: evm module (#651)

* atlas: evm module

* typos

* update toml

* update description

* Update x/evm/atlas/atlas-v0.3.1.md

* Update x/evm/atlas/atlas-v0.3.1.md

Co-authored-by: billy rennekamp <billy.rennekamp@gmail.com>

* build(deps): bump actions/setup-node from v2.1.2 to v2.1.3 (#653)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from v2.1.2 to v2.1.3.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.2...27082ce)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump github.com/ethereum/go-ethereum from 1.9.24 to 1.9.25 (#654)

* build(deps): bump github.com/ethereum/go-ethereum from 1.9.24 to 1.9.25

Bumps [github.com/ethereum/go-ethereum](https://github.com/ethereum/go-ethereum) from 1.9.24 to 1.9.25.
- [Release notes](https://github.com/ethereum/go-ethereum/releases)
- [Commits](ethereum/go-ethereum@v1.9.24...v1.9.25)

Signed-off-by: dependabot[bot] <support@github.com>

* fixes

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Federico Kunze <federico.kunze94@gmail.com>

* build(deps): bump gaurav-nelson/github-action-markdown-link-check (#652)

Bumps [gaurav-nelson/github-action-markdown-link-check](https://github.com/gaurav-nelson/github-action-markdown-link-check) from 1.0.9 to 1.0.10.
- [Release notes](https://github.com/gaurav-nelson/github-action-markdown-link-check/releases)
- [Commits](gaurav-nelson/github-action-markdown-link-check@1.0.9...72d871b)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* update peers (#656)

* update peers

* update chainid in cmd

* build(deps): bump gaurav-nelson/github-action-markdown-link-check (#659)

Bumps [gaurav-nelson/github-action-markdown-link-check](https://github.com/gaurav-nelson/github-action-markdown-link-check) from 1.0.10 to 1.0.11.
- [Release notes](https://github.com/gaurav-nelson/github-action-markdown-link-check/releases)
- [Commits](gaurav-nelson/github-action-markdown-link-check@1.0.10...2a60e0f)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump ini from 1.3.5 to 1.3.8 in /docs (#658)

Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8.
- [Release notes](https://github.com/isaacs/ini/releases)
- [Commits](npm/ini@v1.3.5...v1.3.8)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Daniel Choi <choidanielw@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Pierre <974741468@qq.com>
Co-authored-by: billy rennekamp <billy.rennekamp@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@araskachoi araskachoi araskachoi left review comments

@summerpro summerpro summerpro left review comments

@noot noot Awaiting requested review from noot

Assignees

@fedekunze fedekunze

Labels
Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

In the "newEVM" function, the member variable "GetHash" is not initialized when "vm.Context" is initialized
4 participants
@fedekunze @araskachoi @summerpro @noot