evm: params #458

fedekunze · 2020-08-18T12:06:50Z

Closes: #443

Description

TODO:

param tests

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 · 2020-08-18T12:08:53Z

Codecov Report

Merging #458 into development will increase coverage by 1.29%.
The diff coverage is 94.41%.

@@               Coverage Diff               @@
##           development     #458      +/-   ##
===============================================
+ Coverage        70.15%   71.45%   +1.29%     
===============================================
  Files               38       40       +2     
  Lines             2520     2662     +142     
===============================================
+ Hits              1768     1902     +134     
- Misses             620      624       +4     
- Partials           132      136       +4

Impacted Files	Coverage Δ
x/evm/types/key.go	`50.00% <ø> (ø)`
x/evm/handler.go	`91.45% <50.00%> (-3.15%)`	⬇️
x/evm/keeper/querier.go	`71.22% <50.00%> (ø)`
x/evm/keeper/keeper.go	`73.43% <86.66%> (+3.43%)`	⬆️
app/ante/eth.go	`65.35% <89.47%> (+1.17%)`	⬆️
x/evm/types/chain_config.go	`97.59% <97.50%> (-2.41%)`	⬇️
app/ante/ante.go	`63.33% <100.00%> (ø)`
app/ethermint.go	`88.23% <100.00%> (+0.06%)`	⬆️
x/evm/genesis.go	`76.74% <100.00%> (+3.05%)`	⬆️
x/evm/keeper/params.go	`100.00% <100.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26816e2...1b8a25b. Read the comment docs.

* attempt to fix * cleanup * add idx check * update csdb.Copy

…-params

…params

fedekunze · 2020-09-02T07:13:36Z

x/evm/types/chain_config.go

-		ConstantinopleBlock: big.NewInt(0),
-		PetersburgBlock:     big.NewInt(0),
+		EIP150Block:         sdk.ZeroInt(),
+		EIP150Hash:          "0x2086799aeebeae135c246c65021c82b4e15a2c451340993aacfd2751886514f0",


I actually think this default hash is invalid should be populated on initialization

@noot @araskachoi thoughts?

fedekunze · 2020-09-02T09:55:28Z

the simulations are failing because the evm module is not supported yet on sims :

--- FAIL: TestAppSimulationAfterImport (101.00s)
panic: UnmarshalJSON cannot decode empty bytes [recovered]
	panic: UnmarshalJSON cannot decode empty bytes

goroutine 61 [running]:
testing.tRunner.func1.1(0x4d37e00, 0xc004197540)
	/usr/local/Cellar/go/1.15/libexec/src/testing/testing.go:1057 +0x30d
testing.tRunner.func1(0xc000602c00)
	/usr/local/Cellar/go/1.15/libexec/src/testing/testing.go:1060 +0x41a
panic(0x4d37e00, 0xc004197540)
	/usr/local/Cellar/go/1.15/libexec/src/runtime/panic.go:969 +0x175
github.com/cosmos/cosmos-sdk/x/params/subspace.Subspace.Get(0xc000266000, 0x51ae9e0, 0xc000023e50, 0x51aea20, 0xc000023ea0, 0xc000038190, 0x3, 0xb, 0xc0002038f0, 0x51bea60, ...)
	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.39.1/x/params/subspace/subspace.go:106 +0x2ca
github.com/cosmos/cosmos-sdk/x/params/subspace.Subspace.GetParamSet(0xc000266000, 0x51ae9e0, 0xc000023e50, 0x51aea20, 0xc000023ea0, 0xc000038190, 0x3, 0xb, 0xc0002038f0, 0x51bea60, ...)
	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.39.1/x/params/subspace/subspace.go:217 +0x1a8
github.com/cosmos/ethermint/x/evm/types.(*CommitStateDB).GetParams(...)
	/Users/federico/ethermint/x/evm/types/statedb.go:250
github.com/cosmos/ethermint/x/evm/keeper.Keeper.GetParams(0xc000266000, 0x51ae9e0, 0xc000023e80, 0xc00014f800, 0x0, 0xc0002ccd40, 0x51bea60, 0xc000126008, 0x51cc000, 0xc0021db280, ...)
	/Users/federico/ethermint/x/evm/keeper/params.go:11 +0x1b4
github.com/cosmos/ethermint/x/evm.ExportGenesis(0x51bea60, 0xc000126008, 0x51cc000, 0xc0021db280, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/federico/ethermint/x/evm/genesis.go:86 +0x279
github.com/cosmos/ethermint/x/evm.AppModule.ExportGenesis(0xc000266000, 0x51ae9e0, 0xc000023e80, 0xc00014f800, 0x0, 0xc0002ccd40, 0x51c20e0, 0xc000266a80, 0x51bea60, 0xc000126008, ...)
	/Users/federico/ethermint/x/evm/module.go:133 +0xa8
github.com/cosmos/cosmos-sdk/types/module.(*Manager).ExportGenesis(0xc000266b60, 0x51bea60, 0xc000126008, 0x51cc000, 0xc0021db280, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.39.1/types/module/module.go:285 +0xfe
github.com/cosmos/ethermint/app.(*EthermintApp).ExportAppStateAndValidators(0xc000180b00, 0xc000122001, 0xc003119c70, 0x0, 0x0, 0x0, 0x0, 0x15, 0x0, 0x0, ...)
	/Users/federico/ethermint/app/export.go:40 +0x125
github.com/cosmos/ethermint/app.TestAppSimulationAfterImport(0xc000602c00)
	/Users/federico/ethermint/app/simulation_test.go:212 +0x71a
testing.tRunner(0xc000602c00, 0x50574d0)
	/usr/local/Cellar/go/1.15/libexec/src/testing/testing.go:1108 +0xef
created by testing.(*T).Run
	/usr/local/Cellar/go/1.15/libexec/src/testing/testing.go:1159 +0x386
FAIL	github.com/cosmos/ethermint/app	101.773s

fedekunze · 2020-09-02T10:12:35Z

Not sure what's going on with the storage rpc test tho:

=== RUN   TestEth_ExportAccount_WithStorage
    rpc_test.go:837: 
        	Error Trace:	rpc_test.go:837
        	Error:      	Not equal: 
        	            	expected: hexutil.Bytes{0x60, 0x80, 0x60, 0x40, 0x52, 0x34, 0x80, 0x15, 0x60, 0xf, 0x57, 0x60, 0x0, 0x80, 0xfd, 0x5b, 0x50, 0x60, 0x4, 0x36, 0x10, 0x60, 0x28, 0x57, 0x60, 0x0, 0x35, 0x60, 0xe0, 0x1c, 0x80, 0x63, 0xeb, 0x8a, 0xc9, 0x21, 0x14, 0x60, 0x2d, 0x57, 0x5b, 0x60, 0x0, 0x80, 0xfd, 0x5b, 0x60, 0x60, 0x60, 0x4, 0x80, 0x36, 0x3, 0x60, 0x40, 0x81, 0x10, 0x15, 0x60, 0x41, 0x57, 0x60, 0x0, 0x80, 0xfd, 0x5b, 0x81, 0x1, 0x90, 0x80, 0x80, 0x35, 0x90, 0x60, 0x20, 0x1, 0x90, 0x92, 0x91, 0x90, 0x80, 0x35, 0x90, 0x60, 0x20, 0x1, 0x90, 0x92, 0x91, 0x90, 0x50, 0x50, 0x50, 0x60, 0x62, 0x56, 0x5b, 0x0, 0x5b, 0x81, 0x60, 0x0, 0x81, 0x90, 0x55, 0x50, 0x80, 0x82, 0x7f, 0xf3, 0xca, 0x12, 0x4a, 0x69, 0x7b, 0xa0, 0x7e, 0x8c, 0x5e, 0x80, 0xbe, 0xbc, 0xfc, 0xc4, 0x89, 0x91, 0xfc, 0x16, 0xa6, 0x31, 0x70, 0xe8, 0xa9, 0x20, 0x6e, 0x30, 0x50, 0x89, 0x60, 0xd0, 0x3, 0x60, 0x40, 0x51, 0x60, 0x40, 0x51, 0x80, 0x91, 0x3, 0x90, 0xa3, 0x50, 0x50, 0x56, 0xfe, 0xa2, 0x65, 0x62, 0x7a, 0x7a, 0x72, 0x31, 0x58, 0x20, 0x1d, 0x94, 0xd2, 0x18, 0x7a, 0xaf, 0x3a, 0x67, 0x90, 0x52, 0x7b, 0x61, 0x5f, 0xcc, 0x40, 0x97, 0xf, 0xeb, 0xf0, 0x38, 0x5f, 0xa6, 0xd7, 0x2a, 0x23, 0x44, 0x84, 0x8e, 0xbd, 0xd, 0xf3, 0xe9, 0x64, 0x73, 0x6f, 0x6c, 0x63, 0x43, 0x0, 0x5, 0x11, 0x0, 0x32}
        	            	actual  : hexutil.Bytes(nil)
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,16 +1,2 @@
        	            	-(hexutil.Bytes) (len=208) {
        	            	- 00000000  60 80 60 40 52 34 80 15  60 0f 57 60 00 80 fd 5b  |`.`@R4..`.W`...[|
        	            	- 00000010  50 60 04 36 10 60 28 57  60 00 35 60 e0 1c 80 63  |P`.6.`(W`.5`...c|
        	            	- 00000020  eb 8a c9 21 14 60 2d 57  5b 60 00 80 fd 5b 60 60  |...!.`-W[`...[``|
        	            	- 00000030  60 04 80 36 03 60 40 81  10 15 60 41 57 60 00 80  |`..6.`@...`AW`..|
        	            	- 00000040  fd 5b 81 01 90 80 80 35  90 60 20 01 90 92 91 90  |.[.....5.` .....|
        	            	- 00000050  80 35 90 60 20 01 90 92  91 90 50 50 50 60 62 56  |.5.` .....PPP`bV|

noot

the solidity tests fail on this branch with Error: while migrating Migrations: The contract code couldn't be stored, please check your gas limit.

same with deploying aragonOS. this is probably why the export account test is failing

I can't tell exactly why from looking at the changes, going to look into it more

fedekunze · 2020-09-02T16:38:39Z

the solidity tests fail on this branch with Error: while migrating Migrations: The contract code couldn't be stored, please check your gas limit.
same with deploying aragonOS. this is probably why the export account test is failing

The ChainConfig has to do with the gas cost updates. Prob looking into that is the first thing we should do

noot

contract deployments work for me now, looks good!

fedekunze · 2020-09-02T19:32:43Z

tests/rpc_test.go

 	err := json.Unmarshal(rpcRes.Result, &gas)
 	require.NoError(t, err, string(rpcRes.Result))

-	require.Equal(t, "0xffdf", gas.String())
+	require.Equal(t, "0x1051d", gas)


not sure if we should address this? technically it should remain the same? @noot

yeah it should likely be the same, do we have any tests for actual gas consumption vs estimation? if not we should add some

evm: params

46ae305

fedekunze added 3 commits August 18, 2020 14:36

setup

3bd141b

bump commit

72fc40e

fixes

7d041d7

fedekunze added the x/evm EVM module issues label Aug 18, 2020

fedekunze added 15 commits August 18, 2020 21:06

increase gas usage

7b4e423

tests

6fa8e29

conflicts

2338725

evm denom param

9ffd1e8

more config updates

452cc78

update genesis

8269746

update ante handler

39048af

csdb param test

2289f34

more tests and fixes

2a364e7

update statedb.Copy

fa83504

lint

ab18272

additional test

c7c89ab

fix importer tests

d1c1e0a

fix AnteHandler test

d267cc3

minor update

7fe95ba

fedekunze marked this pull request as ready for review September 1, 2020 16:34

fedekunze requested a review from noot as a code owner September 1, 2020 16:34

fedekunze added 3 commits September 1, 2020 18:35

revert

94adf0f

undo gas update

86dbdb5

stringer test

64957ac

fedekunze requested review from araskachoi and J-Thompson12 September 1, 2020 17:50

fedekunze added the Status: Needs Review label Sep 1, 2020

changelog

7c09c23

fix csdb index error (#493)

8fa07a8

* attempt to fix * cleanup * add idx check * update csdb.Copy

fedekunze added this to the v0.2.0 milestone Sep 1, 2020

fedekunze and others added 3 commits September 2, 2020 08:59

Merge branch 'development' into evm-params

5b9e443

Merge branch 'development' of github.com:ChainSafe/ethermint into evm…

c01ca51

…-params

Merge branch 'evm-params' of github.com:ChainSafe/ethermint into evm-…

ff3e6b4

…params

fedekunze commented Sep 2, 2020

View reviewed changes

update default hash

7958be1

fedekunze added 2 commits September 2, 2020 12:26

update querier

6fd7af7

update rpc tests

0ec092d

noot reviewed Sep 2, 2020

View reviewed changes

noot mentioned this pull request Sep 2, 2020

evm: debug non-determinism #496

Merged

11 tasks

updates from development

e5d1736

noot approved these changes Sep 2, 2020

View reviewed changes

fix estimate gas test

1b8a25b

fedekunze commented Sep 2, 2020

View reviewed changes

fedekunze merged commit 792c1ff into development Sep 2, 2020

fedekunze deleted the evm-params branch September 2, 2020 19:41

noot mentioned this pull request Sep 2, 2020

test gas estimation vs gas consumption #497

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

evm: params #458

evm: params #458

fedekunze commented Aug 18, 2020 •

edited

codecov bot commented Aug 18, 2020 •

edited

fedekunze Sep 2, 2020

fedekunze Sep 2, 2020

fedekunze commented Sep 2, 2020

fedekunze commented Sep 2, 2020

noot left a comment •

edited

fedekunze commented Sep 2, 2020

noot left a comment

fedekunze Sep 2, 2020 •

edited

noot Sep 2, 2020

evm: params #458

evm: params #458

Conversation

fedekunze commented Aug 18, 2020 • edited

Description

codecov bot commented Aug 18, 2020 • edited

Codecov Report

fedekunze Sep 2, 2020

Choose a reason for hiding this comment

fedekunze Sep 2, 2020

Choose a reason for hiding this comment

fedekunze commented Sep 2, 2020

fedekunze commented Sep 2, 2020

noot left a comment • edited

Choose a reason for hiding this comment

fedekunze commented Sep 2, 2020

noot left a comment

Choose a reason for hiding this comment

fedekunze Sep 2, 2020 • edited

Choose a reason for hiding this comment

noot Sep 2, 2020

Choose a reason for hiding this comment

fedekunze commented Aug 18, 2020 •

edited

codecov bot commented Aug 18, 2020 •

edited

noot left a comment •

edited

fedekunze Sep 2, 2020 •

edited