Fix CI to run solidity tests #278

yijiasu-crypto · 2021-07-13T09:10:12Z

Closes: #XXX

Description

This PR updates some stale code in Makefile and .github/workflows/deploy-contract.yml to make the CI works with performing solidity test on CI builds.

Some improvements:

use make test-solidity in Github Actions instead of make test-contract
Remove test-contract from Makefile. Contract & Solidity tests now can be run by make test-solidity.
Remove installing solcjs from Makefile. This is no longer needed. Truffle inside solidity tests will handle installation of solcjs
Add verbose logs parameter on yarn test to output ethermintd activities during CI test for better debugging

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)

yijiasu-crypto · 2021-07-13T09:13:51Z

This CI test is still failed since some ethermint issues haven't been fixed for the eth_getLogs RPC API.

However, the CI script itself is working well.
Reviews could see this: https://github.com/yijiasu-crypto/ethermint/pull/2/checks?check_run_id=3055077899 for the run result of CI. This run executed node test-helper.js --network ethermint --verbose-log initializable so it only tests the initializable test instead of performing all (basic test is currently failed)

leejw51crypto

lgtm

yijiasu-crypto · 2021-07-13T10:43:11Z

Wait for #248 to be closed to fix the broken CI test

yijiasu-crypto · 2021-07-15T08:51:03Z

Wait for issue #297 fixed. The test case basic failed so resulted in CI blockage

yijiasu-crypto · 2021-07-15T17:35:16Z

Last round of CI failed at initializable-buidler (basic and initializable are passed!). The reason is still the inaccurate estimated gas. (HardHat will call eth_estimateGas to calculate the gas cost before sending the transaction, while truffle will use values in ABI definition, thus result in PASS with truffle and fail with HardHat)

Wait for #272 to be merged. It should fix this error

yihuang · 2021-07-20T06:13:31Z

Last round of CI failed at initializable-buidler (basic and initializable are passed!). The reason is still the inaccurate estimated gas. (HardHat will call eth_estimateGas to calculate the gas cost before sending the transaction, while truffle will use values in ABI definition, thus result in PASS with truffle and fail with HardHat)

Wait for #272 to be merged. It should fix this error

it's merged now.

yijiasu-crypto · 2021-07-20T06:15:25Z

Now it breaks at the incorrectly formatted error message 😂 #314

fedekunze · 2021-07-29T12:07:59Z

looks like this is still failing

yijiasu-crypto · 2021-07-29T13:29:26Z

I am checking the EVM REVERT error logs on geth environments. Will update to #350.

Once #350 is done I think this will be passed.

fedekunze · 2021-10-01T09:47:19Z

@yijiasu-crypto seems that you are querying a height > current height? I will fix the panic

fedekunze · 2021-10-01T13:16:38Z

Contract: Counter
should subtract:
Error: Returned error: height must be greater than 0, but got 0

yijiasu-crypto · 2021-10-01T13:18:42Z

Checking on the latest issue

Contract: Counter
should subtract:
Error: Returned error: height must be greater than 0, but got 0

yijiasu-crypto · 2021-10-01T13:19:15Z

Now it's no longer crashing during access getBlockNumber

yijiasu-crypto · 2021-10-01T13:33:54Z

"height must be greater than 0, but got 0" -> This error is not issued on the truffle side or test side. The test suite passed on ganache.

It looks like the error comes from ethermint or tendermint side. Could you check? @fedekunze

yijiasu-crypto · 2021-10-01T14:15:12Z

all tests are now passed (ran locally) except the basic test !!

Getting this error: height must be greater than 0, but got 0

fedekunze · 2021-10-01T14:45:04Z

tests/solidity/suites/proxy/test/depositable_delegate_proxy.js

+      // it('can receive ETH from contract [@skip-on-coverage]', async () => {
+      //   const receipt = await ethSender.sendEth(proxy.address, { value })

-        assertAmountOfEvents(receipt, 'ProxyDeposit', { decodeForAbi: proxy.abi })
-        assertEvent(receipt, 'ProxyDeposit', { decodeForAbi: proxy.abi, expectedArgs: { sender: ethSender.address, value } })
-      })
+      //   assertAmountOfEvents(receipt, 'ProxyDeposit', { decodeForAbi: proxy.abi })
+      //   assertEvent(receipt, 'ProxyDeposit', { decodeForAbi: proxy.abi, expectedArgs: { sender: ethSender.address, value } })
+      // })


fedekunze · 2021-10-01T14:47:06Z

@yijiasu-crypto can you point me to the test?

yijiasu-crypto · 2021-10-01T14:50:49Z

Crashed here: https://github.com/yijiasu-crypto/ethermint/blob/5d919d3cf56ad6fc4eba6bbf8ef1a97ab523d9dc/tests/solidity/suites/basic/test/counter.js#L79

the estimateGas test is now removed so it does not matter if it fails.

yijiasu-crypto · 2021-10-06T13:57:35Z

It looks like ethermint returns an error when call eth_getLogs with fromBlock=0

This RPC call is successful under a ganache environment. Please take a look at detailed RPC logs below.

RPC traffic from ethermint:

   > {
   >   "jsonrpc": "2.0",
   >   "id": 689,
   >   "method": "eth_getLogs",
   >   "params": [
   >     {
   >       "fromBlock": "0x0",
   >       "toBlock": "latest",
   >       "address": "0x5740b613cc08a8ed765a898e82aa62e49137b5f8",
   >       "topics": []
   >     }
   >   ]
   > }
 <   {
 <     "jsonrpc": "2.0",
 <     "id": 689,
 <     "error": {
 <       "code": -32000,
 <       "message": "height must be greater than 0, but got 0"
 <     }

RPC traffic from ganache:

   > {
   >   "jsonrpc": "2.0",
   >   "id": 84,
   >   "method": "eth_getLogs",
   >   "params": [
   >     {
   >       "fromBlock": "0x0",
   >       "toBlock": "latest",
   >       "address": "0xdc64a140aa3e981100a9beca4e685f962f0cf6c9",
   >       "topics": []
   >     }
   >   ]
   > }
 <   {
 <     "jsonrpc": "2.0",
 <     "id": 84,
 <     "result": [
 <       {
 <         "removed": false,
 <         "logIndex": "0x0",
 <         "transactionIndex": "0x0",
 <         "transactionHash": "0xd992d48954bccdc7ca4ef8e6bbaf2f80530cccd1f80780dad92243d7bceeabf4",
 <         "blockHash": "0xc57e3f3dc622dc4e61987b134b48d2f93de216a4be653ff04d4982832f53c227",
 <         "blockNumber": "0x8",
 <         "address": "0xdc64a140aa3e981100a9beca4e685f962f0cf6c9",
 <         "data": "0x0000000000000000000000000000000000000000000000000000000000000001",
 <         "topics": [
 <           "0x64a55044d1f2eddebe1b90e8e2853e8e96931cefadbfa0b2ceb34bee36061941"
 <         ]
 <       },
 <       {
 <         "removed": false,
 <         "logIndex": "0x1",
 <         "transactionIndex": "0x0",
 <         "transactionHash": "0xd992d48954bccdc7ca4ef8e6bbaf2f80530cccd1f80780dad92243d7bceeabf4",
 <         "blockHash": "0xc57e3f3dc622dc4e61987b134b48d2f93de216a4be653ff04d4982832f53c227",
 <         "blockNumber": "0x8",
 <         "address": "0xdc64a140aa3e981100a9beca4e685f962f0cf6c9",
 <         "data": "0x0000000000000000000000000000000000000000000000000000000000000001",
 <         "topics": [
 <           "0x938d2ee5be9cfb0f7270ee2eff90507e94b37625d9d2b3a61c97d30a4560b829"
 <         ]
 <       },
 <       {
 <         "removed": false,
 <         "logIndex": "0x0",
 <         "transactionIndex": "0x0",
 <         "transactionHash": "0xdea3ed00ab21f57f8866721084411936c22d9c16992d5fa98585124e977dcdbb",
 <         "blockHash": "0xf54a1b408c565a316b303785ad36078bea791aab79e1085f12e4976e8a9517d5",
 <         "blockNumber": "0x9",
 <         "address": "0xdc64a140aa3e981100a9beca4e685f962f0cf6c9",
 <         "data": "0x0000000000000000000000000000000000000000000000000000000000000000",
 <         "topics": [
 <           "0x938d2ee5be9cfb0f7270ee2eff90507e94b37625d9d2b3a61c97d30a4560b829"
 <         ]
 <       }
 <     ]
 <   }

yijiasu-crypto · 2021-10-06T13:58:14Z

@fedekunze please take a look at the latest diagnosis I posted above

…su/fix-contract-test-ci

codecov · 2021-10-06T15:37:10Z

Codecov Report

Merging #278 (2d853bd) into main (bc8c87c) will decrease coverage by 5.30%.
The diff coverage is n/a.

❗ Current head 2d853bd differs from pull request most recent head 3ca1367. Consider uploading reports for the commit 3ca1367 to get more accurate results

@@            Coverage Diff             @@
##             main     #278      +/-   ##
==========================================
- Coverage   55.88%   50.58%   -5.31%     
==========================================
  Files          63       50      -13     
  Lines        5518     4928     -590     
==========================================
- Hits         3084     2493     -591     
- Misses       2271     2328      +57     
+ Partials      163      107      -56

Impacted Files	Coverage Δ
x/evm/types/dynamic_fee_tx.go	`0.00% <0.00%> (-89.68%)`	⬇️
x/evm/types/access_list.go	`23.33% <0.00%> (-76.67%)`	⬇️
types/validation.go	`33.33% <0.00%> (-66.67%)`	⬇️
x/evm/types/tracer.go	`0.00% <0.00%> (-46.43%)`	⬇️
x/evm/types/params.go	`64.78% <0.00%> (-35.22%)`	⬇️
x/evm/types/legacy_tx.go	`73.21% <0.00%> (-26.79%)`	⬇️
x/evm/types/chain_config.go	`76.61% <0.00%> (-23.39%)`	⬇️
x/evm/types/tx_data.go	`74.35% <0.00%> (-9.74%)`	⬇️
x/evm/types/genesis.go	`91.17% <0.00%> (-8.83%)`	⬇️
x/evm/keeper/state_transition.go	`58.47% <0.00%> (-8.72%)`	⬇️
... and 42 more

yijiasu-crypto · 2021-10-08T08:55:52Z

PR #650 could fix the stuck issue. Wait for that to be merged and test again on this PR

* Fix CI * Remove verbose-log to reduce size * update timeout * rm deploy contract action * Update test-helper.js * Update workflow * Update workflow * fix gas estimate amount * Update test.yml * fix error assert issue * ignore bad test case * remove estimate gas test * Change fromBlock to 1 (TEMP, Reverse Required) * bump timeout Co-authored-by: Federico Kunze Küllmer <[email protected]> Co-authored-by: yihuang <[email protected]> Co-authored-by: Federico Kunze <[email protected]>

* ci: add bencher config (#652) Add bencher config with global +-10% threshold for improvements and regressions Co-authored-by: Federico Kunze Küllmer <[email protected]> * fix conflicts * evm: fix panic when transaction is reverted (#650) * fix panic when transaction is reverted * update changelog * Update x/evm/keeper/context_stack.go Co-authored-by: Federico Kunze Küllmer <[email protected]> * rpc: test fix (#608) * fix rpc tests with net namespace * skip personal test * skip rpc pending test * fix endpoint * fix rpc pending test * fix missing gas param in some rpc tests * fix eth_getproof when the block number is equal to pending or latest * fix rpc tests filter subscribe failed * lint * remove unused linter * fix PendingTransactionFilter and TestEth_GetFilterChanges_BlockFilter * fix eth_estimateGas * fix TestEth_EstimateGas_ContractDeployment * skip TestEth_ExportAccount_WithStorage * remove sleep in rpc test * Update changelog * add test-rpc in github action * bump golangci-lint version to v1.42.1 * release: v0.7.1 cherry-picks * changelog * ci: fix solidity tests (#278) * Fix CI * Remove verbose-log to reduce size * update timeout * rm deploy contract action * Update test-helper.js * Update workflow * Update workflow * fix gas estimate amount * Update test.yml * fix error assert issue * ignore bad test case * remove estimate gas test * Change fromBlock to 1 (TEMP, Reverse Required) * bump timeout Co-authored-by: Federico Kunze Küllmer <[email protected]> Co-authored-by: yihuang <[email protected]> Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Daniel Burckhardt <[email protected]> Co-authored-by: Thomas Nguy <[email protected]> Co-authored-by: JayT106 <[email protected]> Co-authored-by: Yijia Su <[email protected]> Co-authored-by: yihuang <[email protected]>

* integrate with sdk V47 & ibc-go V7 * update proto-gen and proto files * fix e2e tests * tmp * working tmp * fix rebase conflict * fix merge conflict * fix test * recover makefile changes * fix lint * fix protoc-gen-tool script * update tendermint dependency * update btcsuite/btcutil path * update gomod2nix * fix Makefile,regen proto files and remove third_party proto * update gomod2nix * fix lint * update nix * replace cometbft tm-db update to sdk v0.47.2 newer btcutil * algo -> key-type * set mempool * set chainId * update protobuf * fix import * fix test * debug test * update nix * fix btcsuite * make proto-gen follow by UPGRADING.md * fix sync issue with latest blk for more info, see https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#replaces * disable depguard * gogo/protobuf -> cosmos/gogoproto * no pass chain_id after setup ethermintd config chain-id * fix lint * fix err msg due to empty version check fix for more info, see https://github.com/cosmos/cosmos-sdk/pull/13355/files * add the crisis store in store upgrades * fix lint * add consensus upgrade * migrate consensus parameters * fix lint * allow flag overwrite chainId * fix ConsensusParams fix evm * fix lint * add paramKeyTable * add change log * err code change due to nested msg check * fix lint * cleanup mod * add nil check for consParams.Block * Revert "fix ConsensusParams" This reverts commit b739244. * sdk to v0.47.3 and ibc-go to v7.1.0 * point to logger fix * get consParams from keeper if not in ctx * point back cometbft * fix broadcast mode * fix lint * update default params for test * cleanup deps * fix ibc client route in gov router * fix broadcast mode * fix ibctm for more info, see https://github.com/cosmos/ibc-go/blob/v7.2.0/docs/migrations/v6-to-v7.md#chains * point to sdk cp fix * Update rpc/backend/utils.go Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> * Update rpc/types/utils.go Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> * fix build * revert get params by consensusparamkeeper * keep validator err msg * Revert "keep validator err msg" This reverts commit 20253ce. * test with more retries * add expect_cb * fix hooks * rm RandomizedParams ProposalContents * prune expire state --------- Signed-off-by: mmsqe <[email protected]> Co-authored-by: jay tseng <[email protected]> Co-authored-by: yihuang <[email protected]>

Fix CI

dae5c3d

yijiasu-crypto requested review from fedekunze and khoslaventures as code owners July 13, 2021 09:10

leejw51crypto approved these changes Jul 13, 2021

View reviewed changes

fedekunze approved these changes Jul 13, 2021

View reviewed changes

fedekunze enabled auto-merge (squash) July 13, 2021 10:12

yijiasu-crypto and others added 3 commits July 14, 2021 17:46

Merge branch 'main' into ysu/fix-contract-test-ci

e7b9c3d

Merge branch 'main' into ysu/fix-contract-test-ci

61c922f

Merge branch 'main' into ysu/fix-contract-test-ci

c57dd1d

Merge branch 'main' into ysu/fix-contract-test-ci

7e1b45b

Remove verbose-log to reduce size

0d120e6

auto-merge was automatically disabled July 16, 2021 02:26
Head branch was pushed to by a user without write access

Merge branch 'main' into ysu/fix-contract-test-ci

f0f4871

fedekunze added 2 commits July 20, 2021 08:55

Merge branch 'main' into ysu/fix-contract-test-ci

a93aada

Merge branch 'main' into ysu/fix-contract-test-ci

cc93929

yijiasu-crypto mentioned this pull request Jul 26, 2021

add exception revert test case #345

Closed

11 tasks

Merge branch 'main' into ysu/fix-contract-test-ci

8d4185a

fedekunze enabled auto-merge (squash) July 28, 2021 20:51

fedekunze added automerge Automatically merge PR once all prerequisites pass. Type: CI continuous integration labels Jul 28, 2021

fedekunze added 2 commits July 28, 2021 23:08

update timeout

1cac367

rm deploy contract action

53b700b

Merge branch 'main' into ysu/fix-contract-test-ci

1442f10

remove estimate gas test

5d919d3

fedekunze reviewed Oct 1, 2021

View reviewed changes

fedekunze and others added 2 commits October 5, 2021 12:19

Merge branch 'main' into ysu/fix-contract-test-ci

d00c771

Merge branch 'main' into ysu/fix-contract-test-ci

be93a0c

yijiasu-crypto and others added 3 commits October 6, 2021 22:03

Change fromBlock to 1 (TEMP, Reverse Required)

7a8dd37

Merge remote-tracking branch 'origin/ysu/fix-contract-test-ci' into y…

6ec2e2c

…su/fix-contract-test-ci

Merge branch 'main' into ysu/fix-contract-test-ci

08c49f8

fedekunze removed the automerge Automatically merge PR once all prerequisites pass. label Oct 6, 2021

Merge branch 'main' into ysu/fix-contract-test-ci

3c5705d

Merge branch 'main' into ysu/fix-contract-test-ci

b6af0a5

fedekunze and others added 2 commits October 8, 2021 14:08

Merge branch 'main' into ysu/fix-contract-test-ci

5556cfe

bump timeout

3ca1367

fedekunze enabled auto-merge (squash) October 8, 2021 13:36

fedekunze merged commit e9ab624 into evmos:main Oct 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CI to run solidity tests #278

Fix CI to run solidity tests #278

yijiasu-crypto commented Jul 13, 2021

yijiasu-crypto commented Jul 13, 2021

leejw51crypto left a comment

yijiasu-crypto commented Jul 13, 2021

yijiasu-crypto commented Jul 15, 2021 •

edited

Loading

yijiasu-crypto commented Jul 15, 2021

yihuang commented Jul 20, 2021

yijiasu-crypto commented Jul 20, 2021

fedekunze commented Jul 29, 2021

yijiasu-crypto commented Jul 29, 2021

fedekunze commented Oct 1, 2021 •

edited

Loading

fedekunze commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

fedekunze Oct 1, 2021

fedekunze commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 6, 2021 •

edited

Loading

yijiasu-crypto commented Oct 6, 2021

codecov bot commented Oct 6, 2021 •

edited

Loading

yijiasu-crypto commented Oct 8, 2021

Fix CI to run solidity tests #278

Fix CI to run solidity tests #278

Conversation

yijiasu-crypto commented Jul 13, 2021

Description

yijiasu-crypto commented Jul 13, 2021

leejw51crypto left a comment

Choose a reason for hiding this comment

yijiasu-crypto commented Jul 13, 2021

yijiasu-crypto commented Jul 15, 2021 • edited Loading

yijiasu-crypto commented Jul 15, 2021

yihuang commented Jul 20, 2021

yijiasu-crypto commented Jul 20, 2021

fedekunze commented Jul 29, 2021

yijiasu-crypto commented Jul 29, 2021

fedekunze commented Oct 1, 2021 • edited Loading

fedekunze commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

fedekunze Oct 1, 2021

Choose a reason for hiding this comment

fedekunze commented Oct 1, 2021

yijiasu-crypto commented Oct 1, 2021

yijiasu-crypto commented Oct 6, 2021 • edited Loading

yijiasu-crypto commented Oct 6, 2021

codecov bot commented Oct 6, 2021 • edited Loading

Codecov Report

yijiasu-crypto commented Oct 8, 2021

yijiasu-crypto commented Jul 15, 2021 •

edited

Loading

fedekunze commented Oct 1, 2021 •

edited

Loading

yijiasu-crypto commented Oct 6, 2021 •

edited

Loading

codecov bot commented Oct 6, 2021 •

edited

Loading