Remove all occurences of Any* enums from light-clients #2332

hu55a1n1 · 2022-06-23T09:13:36Z

Partially addresses: #2335

Description

Implements phase-1 of #2335:

Attempts to make light-client traits object safe.
Removes all usages of Any* enums from light-client code.

Note: The target branch is a long-lived branch hu55a1n1/light-client-extraction. The plan is to merge PRs (addressing #2335) such as this into hu55a1n1/light-client-extraction and ultimately merge that into master.

PR author checklist:

Added changelog entry, using unclog.
Added tests: integration (for Hermes) or unit/mock tests (for modules).
Linked to GitHub issue.
Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

Reviewed Files changed in the GitHub PR explorer.
Manually tested (in case integration/unit/mock tests are absent).

modules/src/core/ics02_client/client_consensus.rs

modules/src/core/ics02_client/context.rs

modules/src/core/ics02_client/client_def.rs

modules/src/core/ics04_channel/context.rs

plafer · 2022-07-28T17:55:38Z

modules/src/core/ics04_channel/context.rs

+    fn block_delay(&self, delay_period_time: Duration) -> u64;
+}
+
+impl<T: ChannelReader> ChannelMetaReader for T {


Instead of duplicating the methods of ChannelMetaReader in ChannelReader, and providing a blanket implementation, why not have ChannelMetaReader a supertrait of ChannelReader, and remove the duplicated methods from ChannelReader?

It would also simplify everywhere that we have the trait bound ChannelReader + ChannelMetaReader down to just ChannelReader.

This is definitely something we should do once we remove the Any* enums. I didn't do it in this PR because some of the trait methods have default impls that depend on other trait methods that use the Any* enums. (e.g. host_timestamp() and pending_host_consensus_state())

modules/src/core/ics04_channel/context.rs

modules/src/core/ics02_client/context.rs

plafer · 2022-07-28T18:35:19Z

modules/src/core/ics02_client/client_consensus.rs

    /// Type of client associated with this consensus state (eg. Tendermint)
    fn client_type(&self) -> ClientType;

    /// Commitment root of the consensus state, which is used for key-value pair verification.
    fn root(&self) -> &CommitmentRoot;

-    /// Wrap into an `AnyConsensusState`
-    fn wrap_any(self) -> AnyConsensusState;
+    fn encode_vec(&self) -> Result<Vec<u8>, Error>;


I think we decided against including encode_vec() directly in the traits, and instead using a trait object safe version of Protobuf, right? We can address this in another PR, perhaps #2412; let's just not forget to do it!

modules/src/core/ics02_client/client_consensus.rs

plafer

Great job!

…LightClient

* Remove all occurences of Any* enums from light-clients (#2332) * Make ConsensusState object-safe * Make ClientState trait object safe * Extract ConsensusReader * Fix clippy errors * Remove all Any* usages from clients * Remove client specific errors from Ics02Error * Extract ChannelMetaReader * Fix Ci * Remove unnecessary Clone supertrait from Header & Misbehaviour * Fix bad std import * Remove redundant Clone supertrait from ClientDef * Rename ConsensusReader -> LightClientReader * Add docstring for LightClientReader * Add docstring for ChannelMetaReader * Remove unused LightClientReader trait bound * AnyTrait -> CoreAny * Remove ClientReader::maybe_consensus_state() * Rename LightClientReader -> ClientReaderLightClient and ChannelReaderLightClient Co-authored-by: Philippe Laferriere <[email protected]> * Object safe Protobuf (#2412) * Define protobuf error * Implement object safe Protobuf trait * Add test for object safety * Provide blanket impls for ToBoxed and TryFromIfSized * Add blanket impl test * Use more specific bounds * Seal Protobuf supertraits * Add mod erased * Rename erased traits * Doc comments for Protobuf * Update code to use ibc_proto::protobuf::Protobuf * Fix no_std check * Address review feedback * Rename Protobuf<T> to Protobuf<Raw> * Remove Sealed mod * Rename Into -> CloneInto and return T instead of Box<T> * Remove `AnyHeader` from the modules (#2524) * Define protobuf error * Implement object safe Protobuf trait * Add test for object safety * Provide blanket impls for ToBoxed and TryFromIfSized * Add blanket impl test * Use more specific bounds * Seal Protobuf supertraits * Add mod erased * Rename erased traits * Doc comments for Protobuf * Update code to use ibc_proto::protobuf::Protobuf * Fix no_std check * Address review feedback * Rename Protobuf<T> to Protobuf<Raw> * commit before dealing with Serialize * Implement erased_serde Serialize on Header * comment redacted * Made `Header` trait object safe * error * Impl Protobuf<Any> for TmHeader * Impl Protobuf<Any> for MockHeader * Change Header supertrait to `Protobuf<Any>`` instead of `Protobuf<RawHeader>` * ErasedProtobuf * Fix encode_to_string compile error * implement downcast_header * Move AsAny to dynamic_typing module * fix 1 more compile error: clone * Make UpdateClient `PartialEq` again by making `dyn Header` PartialEq * Add `Header::into_box` * Use `Any` in place of `MsgUpdateAnyClient.header` * Update relayer for MsgUpdateAnyClient `Any` header changes * Avoid using `Header` methods in update_client handler * Remove AnyHeader from ClientDef * Update tests for ClientDef changes * Remove redundant clone * Remove AnyHeader usage from mock * MsgUpdateClient -> RawMsgUpdateClient * MsgUpdateAnyClient -> MsgUpdateClient * Remove use of AnyHeader * Move MisbehaviourEvidence to the relayer * Remove use of AnyHeader in mock * Remove AnyHeader from modelator * Fix MockHeader test * Move AnyHeader into the relayer * use alloc String in protobuf * fix no-std issue * Cleanup SyntheticTmBlock impl * Rename EqualsHeader and move to sealed module * Remove unused Error=Error * Remove Deref impl for SyntheticTmBlock * Fix default `trusted_height` in `HostBlock::generate_tm_block()` * fix test * fix errors Co-authored-by: hu55a1n1 <[email protected]> * Remove `AnyMisbehaviour` from modules (#2527) * Implement PartialEq for dyn Misbehaviour * Add DynClone to Misbehaviour * MsgSubmitMisbehaviour now uses Any * Move AnyMisbehaviour to the relayer * Header: move sealed module at the end of file * Rename misbehavior -> misbehaviour * Misbehaviour: mark Mock stuff cfg(test) * Header: put Mock stuff behind cfg(test) * Remove `AnyConsensusState` from modules (#2528) * Update Cargo.lock * Fix header comment * Implement Erased traits for ConsensusState * Implement supertraits for concrete `ConsensusState`s * Remove AnyConsensusState from ClientDef * Remove check for matching ClientTypes in MsgCreateAnyClient domain type conv * Remove AnyConsensusState from client handlers * Fix tests compile * Fix typo in comment * Add ConsensusState::timestamp() * Remove `AnyConsensusState` from readers & keepers * Remove `AnyConsensusState` from mock context * Fix tests * Impl conversions for Host block to dyn ConsensusState * Treat Tm light client as first class citizen and allow Mock impl to depend on it's data structures * Fix no-std check * Replace AnyConsensusState in Messages with Box<dyn ConsensusState> * Modify `verify_upgrade_and_update_state()` to accept `Any` consensus state * Remove redundant conversion from MockHeader to AnyConsensusState * Remove any remnants of AnyConsensusState from tests * Move `AnyConsensusState` to the relayer crate * Move type url consts to the light client mods * Extract mock::consensus_state mod * Rename client_consensus.rs to consensus_state.rs * Move header type url consts to the light client mods * Minor refactoring and doc comments * Move misbehaviour type url consts to the light client mods * Rename Clientdef::validate_consensus_state -> initialise * Remove `AnyClientState` from the modules (#2559) * dyn UpgradeOptions * Object safe ClientState * Add PartialEq for ClientState * Remove AnyClientState * Update remaining handlers and relayer code * Fix tests * Remove AnyClientState from ICS03 * Remove AnyClientState from mock * Fix typos * Fix CI * Move AnyClientState to relayer crate * Move client state type URLs to light clients * Remove ClientReaderLightClient * Fix comment * Avoid decoding expected client state on counterparty (#2569) * Merge `Clientdef` into `Clientstate` (#2570) * add verification functions to ClientState * Implement tendermint clientstate * Implemented verification functions for MockClientState * Remove ClientDef * MsgCreateAnyClient -> MsgCreateClient * MsgUpgradeAnyClient -> MsgUpgradeClient * fix test * Cleanup fully qualified imports * Rename TmClientState * Remove ChannelReaderLightClient (#2572) Co-authored-by: hu55a1n1 <[email protected]> * Re-add Deserialize on Any enums in relayer crate where possible * Impl From<&dyn> for AnyClientState and AnyConsensusState * Remove redundant line Co-authored-by: Philippe Laferriere <[email protected]> Co-authored-by: Romain Ruetschi <[email protected]>

* Remove all occurences of Any* enums from light-clients (informalsystems#2332) * Make ConsensusState object-safe * Make ClientState trait object safe * Extract ConsensusReader * Fix clippy errors * Remove all Any* usages from clients * Remove client specific errors from Ics02Error * Extract ChannelMetaReader * Fix Ci * Remove unnecessary Clone supertrait from Header & Misbehaviour * Fix bad std import * Remove redundant Clone supertrait from ClientDef * Rename ConsensusReader -> LightClientReader * Add docstring for LightClientReader * Add docstring for ChannelMetaReader * Remove unused LightClientReader trait bound * AnyTrait -> CoreAny * Remove ClientReader::maybe_consensus_state() * Rename LightClientReader -> ClientReaderLightClient and ChannelReaderLightClient Co-authored-by: Philippe Laferriere <[email protected]> * Object safe Protobuf (informalsystems#2412) * Define protobuf error * Implement object safe Protobuf trait * Add test for object safety * Provide blanket impls for ToBoxed and TryFromIfSized * Add blanket impl test * Use more specific bounds * Seal Protobuf supertraits * Add mod erased * Rename erased traits * Doc comments for Protobuf * Update code to use ibc_proto::protobuf::Protobuf * Fix no_std check * Address review feedback * Rename Protobuf<T> to Protobuf<Raw> * Remove Sealed mod * Rename Into -> CloneInto and return T instead of Box<T> * Remove `AnyHeader` from the modules (informalsystems#2524) * Define protobuf error * Implement object safe Protobuf trait * Add test for object safety * Provide blanket impls for ToBoxed and TryFromIfSized * Add blanket impl test * Use more specific bounds * Seal Protobuf supertraits * Add mod erased * Rename erased traits * Doc comments for Protobuf * Update code to use ibc_proto::protobuf::Protobuf * Fix no_std check * Address review feedback * Rename Protobuf<T> to Protobuf<Raw> * commit before dealing with Serialize * Implement erased_serde Serialize on Header * comment redacted * Made `Header` trait object safe * error * Impl Protobuf<Any> for TmHeader * Impl Protobuf<Any> for MockHeader * Change Header supertrait to `Protobuf<Any>`` instead of `Protobuf<RawHeader>` * ErasedProtobuf * Fix encode_to_string compile error * implement downcast_header * Move AsAny to dynamic_typing module * fix 1 more compile error: clone * Make UpdateClient `PartialEq` again by making `dyn Header` PartialEq * Add `Header::into_box` * Use `Any` in place of `MsgUpdateAnyClient.header` * Update relayer for MsgUpdateAnyClient `Any` header changes * Avoid using `Header` methods in update_client handler * Remove AnyHeader from ClientDef * Update tests for ClientDef changes * Remove redundant clone * Remove AnyHeader usage from mock * MsgUpdateClient -> RawMsgUpdateClient * MsgUpdateAnyClient -> MsgUpdateClient * Remove use of AnyHeader * Move MisbehaviourEvidence to the relayer * Remove use of AnyHeader in mock * Remove AnyHeader from modelator * Fix MockHeader test * Move AnyHeader into the relayer * use alloc String in protobuf * fix no-std issue * Cleanup SyntheticTmBlock impl * Rename EqualsHeader and move to sealed module * Remove unused Error=Error * Remove Deref impl for SyntheticTmBlock * Fix default `trusted_height` in `HostBlock::generate_tm_block()` * fix test * fix errors Co-authored-by: hu55a1n1 <[email protected]> * Remove `AnyMisbehaviour` from modules (informalsystems#2527) * Implement PartialEq for dyn Misbehaviour * Add DynClone to Misbehaviour * MsgSubmitMisbehaviour now uses Any * Move AnyMisbehaviour to the relayer * Header: move sealed module at the end of file * Rename misbehavior -> misbehaviour * Misbehaviour: mark Mock stuff cfg(test) * Header: put Mock stuff behind cfg(test) * Remove `AnyConsensusState` from modules (informalsystems#2528) * Update Cargo.lock * Fix header comment * Implement Erased traits for ConsensusState * Implement supertraits for concrete `ConsensusState`s * Remove AnyConsensusState from ClientDef * Remove check for matching ClientTypes in MsgCreateAnyClient domain type conv * Remove AnyConsensusState from client handlers * Fix tests compile * Fix typo in comment * Add ConsensusState::timestamp() * Remove `AnyConsensusState` from readers & keepers * Remove `AnyConsensusState` from mock context * Fix tests * Impl conversions for Host block to dyn ConsensusState * Treat Tm light client as first class citizen and allow Mock impl to depend on it's data structures * Fix no-std check * Replace AnyConsensusState in Messages with Box<dyn ConsensusState> * Modify `verify_upgrade_and_update_state()` to accept `Any` consensus state * Remove redundant conversion from MockHeader to AnyConsensusState * Remove any remnants of AnyConsensusState from tests * Move `AnyConsensusState` to the relayer crate * Move type url consts to the light client mods * Extract mock::consensus_state mod * Rename client_consensus.rs to consensus_state.rs * Move header type url consts to the light client mods * Minor refactoring and doc comments * Move misbehaviour type url consts to the light client mods * Rename Clientdef::validate_consensus_state -> initialise * Remove `AnyClientState` from the modules (informalsystems#2559) * dyn UpgradeOptions * Object safe ClientState * Add PartialEq for ClientState * Remove AnyClientState * Update remaining handlers and relayer code * Fix tests * Remove AnyClientState from ICS03 * Remove AnyClientState from mock * Fix typos * Fix CI * Move AnyClientState to relayer crate * Move client state type URLs to light clients * Remove ClientReaderLightClient * Fix comment * Avoid decoding expected client state on counterparty (informalsystems#2569) * Merge `Clientdef` into `Clientstate` (informalsystems#2570) * add verification functions to ClientState * Implement tendermint clientstate * Implemented verification functions for MockClientState * Remove ClientDef * MsgCreateAnyClient -> MsgCreateClient * MsgUpgradeAnyClient -> MsgUpgradeClient * fix test * Cleanup fully qualified imports * Rename TmClientState * Remove ChannelReaderLightClient (informalsystems#2572) Co-authored-by: hu55a1n1 <[email protected]> * Re-add Deserialize on Any enums in relayer crate where possible * Impl From<&dyn> for AnyClientState and AnyConsensusState * Remove redundant line Co-authored-by: Philippe Laferriere <[email protected]> Co-authored-by: Romain Ruetschi <[email protected]>

seunlanlege reviewed Jun 23, 2022

View reviewed changes

modules/src/core/ics02_client/client_consensus.rs Show resolved Hide resolved

seunlanlege reviewed Jun 23, 2022

View reviewed changes

modules/src/core/ics02_client/client_consensus.rs Show resolved Hide resolved

seunlanlege reviewed Jun 23, 2022

View reviewed changes

modules/src/core/ics02_client/context.rs Outdated Show resolved Hide resolved

plafer reviewed Jun 23, 2022

View reviewed changes

modules/src/core/ics02_client/context.rs Outdated Show resolved Hide resolved

plafer reviewed Jun 23, 2022

View reviewed changes

modules/src/core/ics02_client/client_def.rs Show resolved Hide resolved

hu55a1n1 marked this pull request as ready for review June 24, 2022 10:05

hu55a1n1 requested review from adizere, romac, seanchen1991 and ancazamfir as code owners June 24, 2022 10:05

seanchen1991 reviewed Jun 27, 2022

View reviewed changes

modules/src/core/ics04_channel/context.rs Outdated Show resolved Hide resolved

hu55a1n1 force-pushed the hu55a1n1/remove-clients-any-enums-usages branch 2 times, most recently from ff09e88 to ea0876e Compare July 20, 2022 16:50

hu55a1n1 changed the base branch from master to hu55a1n1/light-client-extraction July 20, 2022 16:53

hu55a1n1 changed the title ~~Remove all occurences of Any* enums from light-clients~~ ADR011: Remove all occurences of Any* enums from light-clients Jul 20, 2022

hu55a1n1 changed the title ~~ADR011: Remove all occurences of Any* enums from light-clients~~ ADR011 impl: Remove all occurences of Any* enums from light-clients Jul 20, 2022

hu55a1n1 added the adr011-impl label Jul 26, 2022

hu55a1n1 changed the title ~~ADR011 impl: Remove all occurences of Any* enums from light-clients~~ Remove all occurences of Any* enums from light-clients Jul 26, 2022

hu55a1n1 added 12 commits July 28, 2022 18:27

Make ConsensusState object-safe

3697bc1

Make ClientState trait object safe

f8597b7

Extract ConsensusReader

c9f6f2a

Fix clippy errors

18b1ac4

Remove all Any* usages from clients

dea1054

Remove client specific errors from Ics02Error

9be4813

Extract ChannelMetaReader

4b07f6d

Fix Ci

f1c3a66

Remove unnecessary Clone supertrait from Header & Misbehaviour

56dbeda

Fix bad std import

875a52c

Remove redundant Clone supertrait from ClientDef

27da6b7

Rename ConsensusReader -> LightClientReader

70a4b85

hu55a1n1 added 2 commits July 28, 2022 18:27

Add docstring for LightClientReader

f53c60c

Add docstring for ChannelMetaReader

a27c1bb

hu55a1n1 force-pushed the hu55a1n1/remove-clients-any-enums-usages branch from ea0876e to a27c1bb Compare July 28, 2022 16:28

hu55a1n1 requested review from mzabaluev and soareschen as code owners July 28, 2022 16:28

hu55a1n1 changed the base branch from hu55a1n1/light-client-extraction to master July 28, 2022 16:28

hu55a1n1 changed the base branch from master to hu55a1n1/light-client-extraction July 28, 2022 16:29

Remove unused LightClientReader trait bound

6896ab5

plafer reviewed Jul 28, 2022

View reviewed changes

modules/src/core/ics02_client/context.rs Outdated Show resolved Hide resolved

plafer reviewed Jul 28, 2022

View reviewed changes

AnyTrait -> CoreAny

394b20c

plafer reviewed Jul 28, 2022

View reviewed changes

modules/src/core/ics02_client/client_consensus.rs Show resolved Hide resolved

plafer approved these changes Jul 28, 2022

View reviewed changes

hu55a1n1 added 2 commits July 29, 2022 16:41

Remove ClientReader::maybe_consensus_state()

65e4ed4

Rename LightClientReader -> ClientReaderLightClient and ChannelReader…

b9d0add

…LightClient

hu55a1n1 merged commit a384fa1 into hu55a1n1/light-client-extraction Jul 29, 2022

hu55a1n1 deleted the hu55a1n1/remove-clients-any-enums-usages branch July 29, 2022 15:14

hu55a1n1 mentioned this pull request Jul 29, 2022

Light client extraction #2483

Merged

20 tasks

hu55a1n1 mentioned this pull request Aug 17, 2022

Remove ChannelReaderLightClient #2572

Merged

7 tasks

This was referenced Oct 6, 2022

Rename leftover modules crate to ibc-relayer-types and trim it down #2710

Closed

Undo the changes introduced in #2332 #2412 and #2483 #2715

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove all occurences of Any* enums from light-clients #2332

Remove all occurences of Any* enums from light-clients #2332

hu55a1n1 commented Jun 23, 2022 •

edited

Loading

plafer Jul 28, 2022

hu55a1n1 Jul 29, 2022

plafer Jul 28, 2022

plafer left a comment

Remove all occurences of Any* enums from light-clients #2332

Remove all occurences of Any* enums from light-clients #2332

Conversation

hu55a1n1 commented Jun 23, 2022 • edited Loading

Description

PR author checklist:

Reviewer checklist:

plafer Jul 28, 2022

Choose a reason for hiding this comment

hu55a1n1 Jul 29, 2022

Choose a reason for hiding this comment

plafer Jul 28, 2022

Choose a reason for hiding this comment

plafer left a comment

Choose a reason for hiding this comment

hu55a1n1 commented Jun 23, 2022 •

edited

Loading