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

Upgrade ictest and add register-zone test #521

Merged

Conversation

anhductn2001
Copy link
Contributor

1. Summary

  • Upgrade InterchainTest to V7 for sdk47
  • Add Register-Zone test

3. Implementation details

  • Bump ictest to v7, use fork github.com/strangelove-ventures/interchaintest/v7 => github.com/notional-labs/interchaintest/v7
  • RegisterZone need have address in deposit_address

4. How to test/use

  • Run command to test make ictest-rz

@aljo242
Copy link

aljo242 commented Aug 7, 2023

@anhductn2001 why are we using the notional fork of interchaintest?

@anhductn2001
Copy link
Contributor Author

anhductn2001 commented Aug 7, 2023

@anhductn2001 why are we using the notional fork of interchaintest?

Because we cannot use flag --x-crisis-skip-assert-invariants when start chain

@aljo242
Copy link

aljo242 commented Aug 7, 2023

@anhductn2001 why are we using the notional fork of interchaintest?

Because we cannot use flag --x-crisis-skip-assert-invariants when start chain

Can you update your fork to the latest version of interchaintest? This will include the sidecar functionality?

Is there a PR opened against interchaintest for your fix for crisis? We want to be using the mainline version and not a fork

@anhductn2001
Copy link
Contributor Author

yes, @ThanhNhann had a discussion to remove crisis and we can not use this flag. Can you explain morẻ? @ThanhNhann

@ThanhNhann
Copy link
Contributor

In the sdk47 of unstable-v1.5 we dont use crisis so @anhductn2001 have to make a fork, I think you should make a pr to interchaintest for this update @anhductn2001

@anhductn2001
Copy link
Contributor Author

Hey @aljo242 , I just update fork to latest version.

@aljo242
Copy link

aljo242 commented Aug 8, 2023

lint failing. Please run make format

@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2023

Codecov Report

❗ No coverage uploaded for pull request base (unstable-v1.5@7348e85). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             unstable-v1.5     #521   +/-   ##
================================================
  Coverage                 ?   53.00%           
================================================
  Files                    ?      189           
  Lines                    ?    15343           
  Branches                 ?        0           
================================================
  Hits                     ?     8132           
  Misses                   ?     6543           
  Partials                 ?      668           
Flag Coverage Δ
unittests 53.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

ThanhNhann added a commit to notional-labs/quicksilver that referenced this pull request Aug 16, 2023
@aljo242
Copy link

aljo242 commented Aug 16, 2023

@anhductn2001 conflicts

aljo242 pushed a commit that referenced this pull request Aug 17, 2023
* sdk 47

* cleanup

* use wasmd 47

* cosmossdk.io/simapp

* use lsm sdk 47

* update pruning & distclient paths

* intergrate with sdk47 => using ibc v7.0.0

* tendermint/spm/ibckeeper -> ibc-go/v7/ibckeeper

* update packet forward moiddleware to v7

* tendermint -> comebft

* fix keepers: ConsensusParamsKeeper, add logger to InitKeepers, NewCommunityPoolSpendProposalHandler no longer available

* fix app.go

* upgrade handler from v46 => v47

* fix cometbft path

* fix app folder

* fix cmd & wasm

* need to cast type

* fix x/ modules

* fix iavl conflict version: use 0.20.0

* gogoproto => cosmos proto

* add consensusparams & crisiskeeper keys

* add tendermint appmodulebasic and set baseapp chain id in test setup

* move set baseapp chainID in if nest

* add govrouter to gov module legacy router and add checksum in test submit wasm prop

* add checksum

* change testchainID

* using icaControllerKeeper msg_server for registerInterchainAccount and SendTx

* clean up

* fix zone test

* restore app_test.go

* fix testcase

* move get context

* Update app/config.go

Co-authored-by: Alex Johnson <[email protected]>

* minor clean up

* revert

* resolve some reviews

* fix install err

* lint

* fix lint

* add port to unstable rule (#450) (#454)

(cherry picked from commit 200abd5)

Co-authored-by: Alex Johnson <[email protected]>

* fix last test

* fix port owner

* lint

* Pr tvl updates (#445)

* front-port v1.2.13

* fix delegation flush test

* add tests for HandleDepositTransaction

* update tests for new error handling

* remove superflous field in proto

* final v1.2.13 front-port and test fixes

* s/s/suite/

* remove fmt.Printlns used for debugging

* update participation rewards and protocoldatas, to add validation, automatic key generation, basechain/basedenom in osmosisparams and updated tvl calculation

* front-port v1.2.13

* remove balancerPoolPretty json marshal/unmarshal, as we deal with raw KV store objects; switch to native bech32 decode/encode as we dont expect prefix of pools to match sdk.GetConfig().Bech32Prefix

* fix: allow support for multiple denoms from same zone in tvl calc

* lint

* fix: refactor utils/address.go to avoid import cycle when using in osmosis-types

* add test for randomutils

* fix: remove potential non-det by iterating over maps using sorted keys

* resolve outstanding nits

* lint

* fix nits

* bump go to 1.20.5

* Update x/interchainstaking/keeper/receipt.go

* Apply suggestions from code review

* lint

* bump hermes to v1.5.0

* update icq and relayer in test scripts

* format and update

* fix makefile

* remove legacy

* remove legacy param sim

* fix hard code

* remove gogo

* update ibc-go

* remove

* format

* remove deprecated

* remove

* fix

* zones helpers

* zones helpers

* remove hard code for owner field

* use proper replaces

* make note for nolint

* make note for nolint

* use proper broadcast mode in test

* make note for nolint

* make note for nolint

* add error check

* clean

* fix proto-gen

* update protobuf make file

* proto-gen-swagger

* fix make build-docker

* fix warning proto

* fix gen

* fix set chainID

* lint

* ensure grpc port is opened for connections from other docker containers

* add register node grpc

* banktypes

* correct goleveldb ver

* add SetOrderExportGenesis

* proposal need title and summary

* update registerzone bash test

* update govkeeper

* Bump Interchain-test v7 for sdk47

* del duplicate sethook gov

* add a check for claimed capability in SubmitTX

* add register-zone ictest

* fix interchaintest

* use submit-proposal

* update amount deposit

* quicksilverEncoding

* setup params

* rm go.work.sum

* add claim chancap in createChannelICA test setup

* add err check

* proposal struct

* lint

* fix lint

* add register_zone_test

* go mod tidy

* set ICS module

* remove scope

* chore: clean duplicate and unused

* add consensus module

* fix can not register ICA

* seperate interchaintest to pr #521

* clean code commented

* revert SendTokenIBC func

* fix error when run make test-docker-regen

* fix check lint error

---------

Co-authored-by: Hieu Vu <[email protected]>
Co-authored-by: sontrinh16 <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: ThanhNhann <[email protected]>
Co-authored-by: Joe Bowman <[email protected]>
Co-authored-by: ducnt131 <[email protected]>
Co-authored-by: catShaark <[email protected]>
@joe-bowman
Copy link
Contributor

@aljo242 is this required now after rightway merge?

@anhductn2001 anhductn2001 requested review from aljo242 and ThanhNhann and removed request for cybertronprime and aljo242 September 13, 2023 10:09
Copy link
Contributor

@ThanhNhann ThanhNhann left a comment

Choose a reason for hiding this comment

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

LGTM! Need to fix the warning below then we can merge

Screenshot 2023-09-14 at 05 19 29

@anhductn2001
Copy link
Contributor Author

LGTM! Need to fix the warning below then we can merge

Screenshot 2023-09-14 at 05 19 29

Can you try to pull and run the test again? because I see no warning related to Makefile. We seem to have fixed the warnings related to proto in PR upgrade to 47. The important thing now is that the migration of repo to new organization name in unstable branch has not been done. Maybe I'll do it today. @ThanhNhann

@joe-bowman joe-bowman merged commit ae2bdc1 into quicksilver-zone:unstable-v1.5 Sep 19, 2023
12 of 14 checks passed
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.

5 participants