[WIP][IBC] Implement ICS-02 Client Semantics

[h5law]

Note: This PR is still a WIP and will be rebased in the future to clean up the commits. Remaining for this PR: Tests?, ???

Description

Summary generated by Reviewpad on 24 Jul 23 09:11 UTC

This pull request introduces several changes across multiple files. Here is a summary of the changes:

1. The file "header.go" adds new types and functionality related to the Wasm client consensus algorithm. It introduces a new file that declares a package named "types" and imports packages from "github.com/pokt-network/pocket/shared". It defines a struct named "Header" and implements the "ClientMessage" interface. It also defines methods for returning the client type and performing basic validation.

2. The file "update.go" updates methods related to client message verification and state update in the IBC module. It adds functions for verifying client messages, updating state, and updating state on misbehavior. It includes some commented out code that requires further development.

3. The file "queries.go" adds functions for retrieving the ConsensusState and ClientState for a stored client in the IBC module. These functions make use of imported packages from "pokt-network/pocket" module.

4. The file "event_manager.go" includes changes such as renaming an imported package, modifying function signatures, and adding a new variable. These changes improve code consistency and correctness.

5. The file "pocket.proto" adds protocol buffer definitions for a Wasm light client in the Pocket network. It defines messages for the client state, consensus state, header, misbehavior, and height.

6. The file "ics-02.md" adds a new section titled "ICS-02 Client Semantics" to the IBC documentation. It provides an overview of ICS-02 and mentions creation, updates, upgrades of clients, and verifying proofs with the counterparty client's state. It also highlights the use of generic implementations and opaque serialized data for improved client upgradeability in Pocket.

7. The file "ics-02-client-semantics.md" introduces the implementation details of the ICS-02 client semantics. It includes definitions, an overview, and implementation details of the client manager. It also covers client queries and utilization of provable stores.

8. The file "ibc_events.proto" modifies the protocol buffer definitions for IBC events. It removes a field, renumbers another field, and adds a new message for attributes.

9. The file "bulk_store_cache.go" updates function names, error handling, and variable assignments in the "AddStore" function. These changes improve code consistency and correctness.

10. The file "validate_test.go" adds test functions for validating various types of client states, consensus states, headers, and misbehaviors in the IBC module.

11. The file "misbehaviour.go" adds a new type and methods related to misbehavior in the IBC client types package.

12. The file "persistence_module.go" adds and modifies methods related to persistence in the IBC module.

13. The file "consensus_state.go" adds a new type and methods related to the consensus state for a Wasm client. It includes functions for creating a new consensus state, getting the client type, and performing basic validation.

14. The file "wasm.proto" adds protocol buffer definitions for a Wasm light client in the ibc/client/types/proto directory.

15. The file "client/events.go" adds a new package and functions for emitting different types of events related to IBC clients.

16. The file "persistence_module.go" adds a new method to the "PostgresContext" struct and modifies two existing methods.

17. The file "height.go" adds a new package and types related to the height of an IBC client.

18. The file "bus.go" adds a new method to the "bus" struct for retrieving the client manager from the module registry.

19. The file "ibc_events.proto" adds a new message for attributes in IBC events.

These changes introduce new types, implement functionality related to the Wasm client consensus algorithm, update and add methods for client message verification and state update, enhance functionality related to IBC events, provide support for the Pocket Network blockchain project, and make various code improvements.

Let me know if you would like more information or specific details about any of the changes.

Issue

Fixes #894

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation
• Other

List of changes

• Adds ClientManager submodule to manage the client implementations
• Adds the interface definitions and WASM implementations (in part minus the WASM bit) implementaions
• Adds the Pocket ConsensusState and ClientState protobufs for use in Pocket WASM clients

Testing

• make develop_test; if any code changes were made
• make test_e2e on k8s LocalNet; if any code changes were made
• e2e-devnet-test passes tests on DevNet; if any code was changed
• Docker Compose LocalNet; if any major functionality was changed or introduced
• k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
• I have tested my changes using the available tooling
• I have updated the corresponding CHANGELOG

If Applicable Checklist

• I have updated the corresponding README(s); local and/or global
• I have added tests that prove my fix is effective or that my feature works
• I have added, or updated, mermaid.js diagrams in the corresponding README(s)
• I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

[Olshansk]

@h5law This is a 1,494 −225 draft PR. Before I review:

1. Are you sure there is no way to break it down to make it easier for the reviewer? Focus on:

• Independent functionality that can be merged in pieces
• Putting yourself in the shoes of the reviewer who has to build context and understand/learn everything

2. Do you need a review on the whole thing right now or just a specific part? If it's the latter, please let us know which it is.

[h5law]

1. Are you sure there is no way to break it down to make it easier for the reviewer? Focus on:

Without using the interface registry the WASM protobufs must be implemented in part (commented with a TODO where this will be fully implemented) which adds the majority of the code added. This cannot be separated without moving the ICS-02 implementation back in time to a non-working state, and everything else follows the same logic. If it helps we can ignore most of the ibc/client/*.go files related to the ibc/client/types/proto/wasm.proto file.

2. Do you need a review on the whole thing right now or just a specific part? If it's the latter, please let us know which it is.

This was put up in response to your comment here:

[Olshansk]

Without using the interface registry the WASM protobufs must be implemented in part (commented with a TODO where this will be fully implemented) which adds the majority of the code added. This cannot be separated without moving the ICS-02 implementation back in time to a non-working state, and everything else follows the same logic. If it helps we can ignore most of the ibc/client/*.go files related to the ibc/client/types/proto/wasm.proto file.

One potential option I see is having a PR that only contains the:

1. Interface
2. Protobufs

That way we can discuss what the interface is (names, etc...) and then the implementation follows. This is a typical design pattern in larger projects but could be used here solely due to the size of the PR.

I'm not saying this is "the right way" to do it. It's the way that makes it easier for the reviewer. If you think it causes you too much overhead to maintain multiple PRs and you'd just prefer a longer cycle on this one. Happy to do that too!

[Olshansk]

This was put up in response to your comment here:

I've validated the following commit with make protogen_clean && make protogen_local:

61977ec (#916)

The issue was a matter of paths (i.e. go_out), relative imports (i.e. import "client/types/proto/wasm.proto";) and what was included (i.e. -I flag).

---

~~I've removed us as reviewers until you give the next 👍 ~~

Nvm, seems like its ready for review

[h5law]

I'm not saying this is "the right way" to do it. It's the way that makes it easier for the reviewer. If you think it causes you too much overhead to maintain multiple PRs and you'd just prefer a longer cycle on this one. Happy to do that too!

@Olshansk I don't mind a longer cycle here, not in a rush to get this one in as I need to write the WASM code and get familiar with that side of thing + other tickets.

Personally I feel like once the review process begins it will become clearer why I dont think it should be split. It would require a lot of gutting the interface functions as they need the implementations to satisfy the interfaces. The logic for these implementations is largely missing bar some basic validation etc and using the already existing ICS-24 stores. Theres a lot of todo comments to actually carry out the implementation logic in another ticket already so this is mostly boilerplate (re the interface implementations)