Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem: dependencies are not up to date #429

Merged
merged 1 commit into from
May 18, 2022

Conversation

thomas-nguy
Copy link
Collaborator

@thomas-nguy thomas-nguy commented Apr 18, 2022

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

Update ethermint to latest main (v0.6.1-0.20220416173130-bc4b018b60b1)
Use ibc-go v3.0.0
Update cosmos sdk to v0.45.3
Upgrate gravity

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

@thomas-nguy thomas-nguy requested a review from a team as a code owner April 18, 2022 02:33
@thomas-nguy thomas-nguy requested review from JayT106 and adu-crypto and removed request for a team April 18, 2022 02:33
go.mod Outdated
@@ -153,18 +153,15 @@ require (
replace (
// TODO: fix keyring upstream
github.com/99designs/keyring => github.com/crypto-org-chain/keyring v1.1.6-fixes
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.45.2-0.20220218085826-7f949c0aa1d6
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure about if this replace is necessary with 0.42.3? @yihuang

Copy link
Collaborator

Choose a reason for hiding this comment

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

if there are multiple cosmos-sdk dependencies end up in go.sum, then better keep the replace I think.

@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #429 (a7ea2ed) into main (3ea70c5) will increase coverage by 17.88%.
The diff coverage is n/a.

❗ Current head a7ea2ed differs from pull request most recent head bf7a191. Consider uploading reports for the commit bf7a191 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #429       +/-   ##
===========================================
+ Coverage   21.51%   39.40%   +17.88%     
===========================================
  Files          27       31        +4     
  Lines        1729     1571      -158     
===========================================
+ Hits          372      619      +247     
+ Misses       1324      906      -418     
- Partials       33       46       +13     
Impacted Files Coverage Δ
x/cronos/genesis.go 61.53% <ø> (-38.47%) ⬇️
x/cronos/handler.go 100.00% <ø> (ø)
x/cronos/keeper/evm.go 58.00% <ø> (+6.27%) ⬆️
x/cronos/keeper/evm_hooks.go 80.00% <ø> (ø)
x/cronos/keeper/evm_log_handlers.go 83.50% <ø> (ø)
x/cronos/keeper/gravity_hooks.go 0.00% <ø> (ø)
x/cronos/keeper/grpc_query.go 0.00% <ø> (ø)
x/cronos/keeper/ibc.go 83.20% <ø> (+5.01%) ⬆️
x/cronos/keeper/ibc_hooks.go 50.00% <ø> (-8.83%) ⬇️
x/cronos/keeper/keeper.go 65.26% <ø> (-29.08%) ⬇️
... and 29 more

@@ -399,7 +399,7 @@ func New(
// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why 2 ChannelKeeper?

Copy link
Collaborator Author

@thomas-nguy thomas-nguy Apr 20, 2022

Choose a reason for hiding this comment

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

Its because we moved out the ICS4 (middleware) interface from the channel
https://github.com/cosmos/ibc-go/blob/v3.0.0/modules/apps/transfer/keeper/keeper.go#L35

We don't have any ibc middleware yet so we are just using the channel base implementation but probably need to be replaced for this issue
#297

adu-crypto
adu-crypto previously approved these changes May 4, 2022
@yihuang
Copy link
Collaborator

yihuang commented May 4, 2022

there are some conflicts with main branch @adu-crypto

@yihuang yihuang self-requested a review May 4, 2022 11:16
@adu-crypto
Copy link
Contributor

there are some conflicts with main branch @adu-crypto

I see. I will check and resolve confilicts.

@adu-crypto adu-crypto dismissed their stale review May 4, 2022 11:43

all checks have failed

@yihuang yihuang marked this pull request as draft May 5, 2022 09:35
@thomas-nguy
Copy link
Collaborator Author

Thanks for taking care of the PR @adu-crypto

@adu-crypto adu-crypto force-pushed the thomas/update-main branch from 9222a58 to 2ec4574 Compare May 6, 2022 09:29
@adu-crypto adu-crypto marked this pull request as ready for review May 6, 2022 13:35
gomod2nix.toml Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@adu-crypto adu-crypto force-pushed the thomas/update-main branch from 7982f09 to c1e3af0 Compare May 11, 2022 07:42
go.mod Outdated Show resolved Hide resolved
@thomas-nguy thomas-nguy changed the title [WIP] Problem: dependencies are not up to date Problem: dependencies are not up to date May 13, 2022
@thomas-nguy
Copy link
Collaborator Author

@yihuang @adu-crypto

It seems integration tests finally passing but it takes 72min to run (maybe because we updated rust)

is it safe to merge to main or we need to keep it clean for a potential v0.7.1 ?

@yihuang
Copy link
Collaborator

yihuang commented May 18, 2022

@yihuang @adu-crypto

It seems integration tests finally passing but it takes 72min to run (maybe because we updated rust)

is it safe to merge to main or we need to keep it clean for a potential v0.7.1 ?

Congratulations!
the long time is probably due to rebuilding dependencies.
it's ok to merge to main, v0.7.x branch is already deviated from main.

@yihuang yihuang self-requested a review May 18, 2022 09:13
go.mod Outdated

// Note: gorocksdb bindings for OptimisticTransactionDB are not merged upstream, so we use a fork
// See https://github.com/tecbot/gorocksdb/pull/216
github.com/tecbot/gorocksdb => github.com/cosmos/gorocksdb v1.1.1
Copy link
Collaborator

@yihuang yihuang May 18, 2022

Choose a reason for hiding this comment

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

it seems cosmos-sdk don't need this replace anymore, I think it's ok to remove this.

Copy link
Collaborator

@yihuang yihuang left a comment

Choose a reason for hiding this comment

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

the gorocksdb replace looks redundant now, looks good other than that

@yihuang
Copy link
Collaborator

yihuang commented May 18, 2022

adcd373#diff-6a5567abee992c210e5cb96abc84014e04fc10cc42eb0e9b27c342cee58ec002L667

you need to add this part manually due to a gomod2nix issue I think 😂

Copy link
Collaborator

@yihuang yihuang left a comment

Choose a reason for hiding this comment

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

LGTM

fix lint and add changelog

fix unit test

update gomod2nix

upgrade gravity

fix gomod2nix

fix integration tests

fix gomod2nix

update ethermint to latest commit to support evm simulation

update ethermint

cosmos sdk to 0.45.4

update gomod2nix from go.mod

Update gomod2nix.toml

Update CHANGELOG.md

Co-authored-by: yihuang <huang@crypto.com>

remove v0.7.0 upgradehandler

use base_fee in feemarket params to replace initial_base_fee

remove v0.7.0 related upgrade handler

update changelog

add chaincfg module manually

skip test_cosmovisor_upgrade

fix lint error

use string for feemarket base_fee value

update gravity bridge to commit a016e2b04866

use relPath

update gravity-bridge for integration tests

fix test gravity

remove gorockdb replace

Update CHANGELOG.md

manually add chaincfg config to avoid build failure
@thomas-nguy thomas-nguy force-pushed the thomas/update-main branch from a7ea2ed to bf7a191 Compare May 18, 2022 10:36
@thomas-nguy thomas-nguy merged commit b1dc428 into crypto-org-chain:main May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants