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

test: adding e2e for grandpa client recovery test #5027

Merged
merged 185 commits into from
Nov 20, 2023

Conversation

damiannolan
Copy link
Member

@damiannolan damiannolan commented Nov 6, 2023

Description

This PR adds an e2e test using ibc-go-simd with 08-wasm and a polkadot/substrate counterparty for testing client recovery - i.e. an active client expires and can use a substitute client of the same type to return it to an active state.

This leverages the new contract sudo entrypoint MigrateClientStore.

closes: #4970


How to run the test

  1. Create a e2e yml dedicated for 08-wasm tests. Drop it into a folder named dev-configs inside e2e
cd e2e
mkdir dev-configs
touch e2e-grandpa.yml
  1. Open the file e2e-grandpa.yml and drop the content inside.
# This file contains configuration for running e2e tests.
# Many of these fields can be overridden with environment variables.
# All fields that support this have the corresponding environment variable name in a comment beside the field.

---
chains:
  # the entry at index 0 corresponds to CHAIN_A
  # NOTE: The chain A spec is overridden directly within 08-wasm e2e tests to use substrate nodes
- chainId: chainA-1
  numValidators: 2
  numFullNodes: 1
  image: ghcr.io/cosmos/ibc-go-simd # override with CHAIN_IMAGE
  tag: release-v8.0.x  # override with CHAIN_A_TAG
  binary: simd # override with CHAIN_BINARY

  # the entry at index 1 corresponds to CHAIN_B
- chainId: chainB-1
  numValidators: 1
  numFullNodes: 0
  image: ghcr.io/cosmos/ibc-go-wasm-simd # override with CHAIN_IMAGE
  tag: pr-5000  # override with CHAIN_B_TAG
  binary: simd # override with CHAIN_BINARY

activeRelayer: hyperspace # override with RELAYER_ID
relayers:
  - id: hermes
    image: ghcr.io/informalsystems/hermes
    tag: "bef2f53"
  - id: rly
    image: ghcr.io/cosmos/relayer
    tag: "latest"
  - id: hyperspace
    image: "ghcr.io/misko9/hyperspace"
    tag: "local"


cometbft:
  logLevel: info

debug:
  # setting this value to true will force log collection even if the test passes.
  dumpLogs: false

# Required only for upgrade tests.
# Chain A will be upgraded the specified tag.
# The plan name must be registered as an upgrade handler in the given tag.
upgrade:
  planName: ""
  tag: ""
  1. Run make e2e-test and select the config e2e-grandpa.yml and search for TestRecoverClient_Succeeds_GrandpaContract - note the test is long running ~20 mins

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • 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 and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

misko9 and others added 30 commits July 6, 2023 08:34
Co-authored-by: Jack Zampolin <[email protected]>
Co-authored-by: antstalepresh <[email protected]>
Co-authored-by: Vladislav Markushin <[email protected]>
Co-authored-by: Blas Rodriguez Irizar <[email protected]>
Co-authored-by: Jacob Gadikian <[email protected]>
Co-authored-by: vuong <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
* new error

* changed all the errors

* feat: Changed some more erros from sdkerrors to errorsmod

* added ibcerrors

* fixes and linting

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
* imp: add separate go.mod for 08-wasm

* fix e2e build

* remove import

* lint

* Amend CI testing to not build on arm, correctly cross compile on arm64. (#4065)

* Use alpine for building simd in docker. Link statically. (#4066)

---------

Co-authored-by: Charly <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
* remove code ID key from genesis type

* address review comment
* bump wasmvm to v1.2.4

* go mod tidy e2e

* Update Dockerfile

* update libwasm sha

* more go mod tidy

---------

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
* build and test wasm-client on pull request

* add arm64 for wasm build only
* merge functions

* address review comment
* pin code to the wasm VM cache

* follow ibc-go error wrapping standards

* Update modules/light-clients/08-wasm/keeper/keeper.go

Co-authored-by: Reece Williams <[email protected]>

---------

Co-authored-by: Damian Nolan <[email protected]>
* renaming to align with x/wasm naming

* fix typo

* Update docs/architecture/adr-027-ibc-wasm.md

Co-authored-by: Damian Nolan <[email protected]>

* address review comments

* review comment

---------

Co-authored-by: Damian Nolan <[email protected]>
* disallow submessage execution

* fix error code

* alignment

* fix
# Conflicts:
#	Dockerfile
#	testing/chain.go
* Add linting for 08-wasm.

* Go mod tidy.

* Please the linter.
* add 08-wasm global store key

* Update client_state.go

* Update modules/light-clients/08-wasm/types/vm.go

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

---------

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
…n `initialize` to contract (#4033)

* remove calls to set client state and consensus state from initialize

* linting

* Update contracts

* use errorsmod

---------

Co-authored-by: Vladislav Markushin <[email protected]>
…etrieve it (#4034)

* imp: remove timestamp from consensus state

* more linting

* make types unexported

* make proto-all

* code hash for code id

* Use hasConsensusState instead of GetConsensusState (#4171)

* Use hasConsensusState instead of GetConsensusState.

* Use wasmQuery for the rest of the query functions. (#4176)

* Use wasmQuery for the rest of the query functions.

---------

Co-authored-by: Carlos Rodriguez <[email protected]>

* review comments

* Remove check for consensus state.

* Add linting for 08-wasm module (#4206)

* Add linting for 08-wasm.

* Go mod tidy.

* Please the linter.

* add 08-wasm global store key (#4185)

* add 08-wasm global store key

* Update client_state.go

* Update modules/light-clients/08-wasm/types/vm.go

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

---------

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

* solve conflicts

* review comments/fix linter warnings

* address comments

* add missing check

* remove check that was wrong

---------

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
…our` (#4049)

* imp: merge header and misbehaviour types

* remove unused code
…calls (#4133)

* defined query and sudo message types to encapsulate all variants for calls

* pass client message

* add ClientMessage from #4049

* address review comments

* linter

* gofumpt

* review comment

* more gofumpt
…ce (#4109)

* add constructor that accepts pointer to Wasm VM instance

* typo

* add functions that returns wasm config with default values

* review comment

* Apply suggestions from code review

Co-authored-by: Cian Hatton <[email protected]>

* review comments

* rename constructor function

* address review comments

* set contract debug mode to false

---------

Co-authored-by: Cian Hatton <[email protected]>
* Rename from call to wasmCall.

* Move wasmQuery, wasmCall into vm.

* Add wasmInit, mirroring wasmCall and wasmQuery.

* Directly return after wasmInit.
# Conflicts:
#	testing/config.go
@github-actions github-actions bot removed the 08-wasm label Nov 14, 2023
Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

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

This is just so damn good! Well done, guys!

DimitrisJim and others added 12 commits November 14, 2023 15:12
* imp: unpin contract when removing code hash

* test: add cov for failing unpin.

* add test case for failed contract pin

* Update modules/light-clients/08-wasm/keeper/msg_server_test.go

Co-authored-by: Charly <[email protected]>

---------

Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Charly <[email protected]>
* imp: client is unauthorized if code hash not stored

* change comment
* add 08-wasm code owners

* fix
)

* imp: check response for wasm instantiate and migrate calls

* Apply suggestions from code review

Co-authored-by: DimitrisJim <[email protected]>

---------

Co-authored-by: DimitrisJim <[email protected]>
* Add an error-ing implementation for GoAPI.

* Move var to top level, add assertions in tests.

* Clean up responses in sucess cases.
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Excellent work as always 🙏 Will need a rebase before merge

@damiannolan damiannolan changed the base branch from feat/wasm-clients to main November 20, 2023 11:17
@github-actions github-actions bot added the protobuf Proto file definitions and protobuf code generation label Nov 20, 2023
@@ -13,7 +13,7 @@ service Query {
option (google.api.http).get = "/ibc/lightclients/wasm/v1/checksums";
}

// Get Wasm code for given code hash
// Get Wasm code for given checksum
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like there's an inconsistency on main here. I re-ran make proto-all when resolving conflicts which produced a change in the .pb.go godocs with checksum -> code hash. I believe it should be "checksum", right?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yea, seems I missed this when opening original PR, added it in the docs PR a few minutes ago too fc64b79

Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

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

LGTM glad we have this passing ❤️

@damiannolan damiannolan merged commit 336bf2c into main Nov 20, 2023
73 checks passed
@damiannolan damiannolan deleted the damian/4970-recover-client-grandpa-e2e branch November 20, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08-wasm e2e protobuf Proto file definitions and protobuf code generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write e2e test for client recovery with grandpa light client