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

Replace the all-in-one traits with a more flexible components pattern #9

Closed
5 tasks
soareschen opened this issue Aug 1, 2023 · 0 comments · Fixed by #62 or #63
Closed
5 tasks

Replace the all-in-one traits with a more flexible components pattern #9

soareschen opened this issue Aug 1, 2023 · 0 comments · Fixed by #62 or #63

Comments

@soareschen
Copy link
Collaborator

soareschen commented Aug 1, 2023

Overview

The all-in-one traits in relayer-next was initially designed to reduce the fear factor of context-generic programming, and allow programmers to easily onboard to the code base using their existing knowledge on monolithic traits.

However, as the code base evolve, the all-in-one traits such as OfaChain are becoming too big to manage in just few traits. This bring us back to the original problem of the ChainHandle trait, which is also very bloated and difficult to manage. Furthermore, the all-in-one traits make significant trade off in terms of customizability for supposingly ease of development. This makes it difficult to solve issue like #2924, which require direct modification of inner components using context-generic programming.

Ultimately, it appears that despite the popularity of programmers wanting to define only one trait to handle everything, the all-in-one traits pattern may be proven to be an anti-pattern, even with the aid of context-generic programming. As a result, we should consider moving away from it, and find other ways to wire up the components defined using context-generic programming.

Paradox of Choice

The key advantage of the all-in-one traits is that it provides a default set of components to be used by the user, without them having to understand which component to pick. As an example, the all-in-one traits select the following set of components to implement the packet relayer:

pub type PacketRelayer = LockPacketRelayer<LoggerRelayer<FilterRelayer<RetryRelayer<FullCycleRelayer>>>>;

The use of context-generic programming allows for modular implementation of components such as FullCycleRelayer and FilterRelayer. However that level of modularity introduces the paradox of choice, of which the user is given too many choices, and don't know how to combine the components to get what they want. The all-in-one traits remove this paradox of choice by removing all choices and offer the user only one choice to pick from. This is good if the provided component is what the user really wants. But if the user wants to customize the component choice, such as removing FilterRelayer, then it becomes a challenge as they would have to abandon the all-in-one traits and implement the full component wiring from scratch.

Alternative Pattern

As an alternative, we will introduce a component graph pattern that focus on auto-deriving the consumer traits from the provider traits. For example, the CanRelayPacket trait would have an auto trait implementation as follows:

pub trait HasComponents {
  type Components;
}

impl<Relay> CanRelayPacket for Relay
where
  Relay: HasComponents,
  Relay::Components: PacketRelayer<Relay>,
{
    async fn relay_packet(&self, packet: &Self::Packet) -> Result<(), Self::Error> {
        Relay::Components::relay_packet(self, packet).await
    }
}

At a high level, every concrete context will just have to implement the HasComponents trait, which provides an associated Component type which implements a chosen set of provider traits. With that, the concrete implementation do not need to worry about wiring up the component graph, unless they want to customize the components.

The relayer-next library can then provide a default Components type that choose the same set of components as the current all-in-one trait. For example:

pub struct DefaultComponents<BaseComponents>(pub PhantomData<BaseComponents>);

type DefaultPacketRelayer = LockPacketRelayer<LoggerRelayer<FilterRelayer<RetryRelayer<FullCycleRelayer>>>>;

impl<Relay, BaseComponents> PacketRelayer<Relay> for DefaultComponents<BaseComponents>
where
    DefaultPacketRelayer: PacketRelayer<Relay>,
{
    async fn relay_packet(relay: &Relay, packet: &Relay::Packet) -> Result<(), Relay::Error> {
        DefaultPacketRelayer::relay_packet(relay, packet).await
    }
}

The DefaultComponents type would implement all provider traits like PacketRelayer, where the implementation is then forward to the chosen components such as DefaultPacketRelayer, which provide the actual implementation.

The BaseComponents generic argument is provided for the user to implement the base components needed for the DefaultComponents to work. For example:

type DefaultChainStatusQuerier<BaseQuerier> = CacheChainStatus<LogChainStatusTelemetry<BaseQuerier>>>

impl<Chain, BaseComponents> ChainStatusQuerier<Chain> for DefaultComponents<BaseComponents>
where
    DefaultChainStatusQuerier<BaseQuerier>: ChainStatusQuerier<Chain>,
{
    async fn query_chain_status(chain: &Chain) -> Result<Chain::ChainStatus, Chain::Error> {
        <DefaultChainStatusQuerier<BaseQuerier>>::query_chain_status(chain).await
    }
}

In the case above, the concrete implementation would have to provide the method of how to actually query the chain status from the chain. But it still allows the DefaultComponents to wrap around the concrete implementation and provide functionality such as caching an telemetry. With that, a Cosmos chain implementation would look something like:

pub struct CosmosComponents;

impl HasComponents for CosmosChain {
    type Components = DefaultComponents<CosmosComponents>;
}

impl ChainStatusQuerier<CosmosChain> for CosmosComponents {
    async fn query_chain_status(chain: &CosmosChain) -> Result<ChainStatus, Error> {
        ...
    }
}

With the above pattern, the type CosmosChain would automatically implement CanQueryChainStatus, since it implements HasComponents with DefaultComponents<CosmosComponents>, and CosmosComponents implement ChainStatusQuerier<CosmosChain>.

Similarly, all other one-for-all methods that we currently have will be converted into provider trait implementations for CosmosComponents. Although this would mean that more traits are involved, it might not be that big of a matter, since we already try to split out the concrete CosmosChain implementation into multiple files in the methods module.

Macros Derivation

The second step of this design pattern is to define derive macros that will automatically generate the impl boilerplate, so that new component graphs can be easily defined. For example, we could derive the PacketRelayer implementation for DefaultComponents as follows:

derive_chain_status_querier!(DefaultComponents<BaseComponents>,
    LockPacketRelayer<LoggerRelayer<FilterRelayer<RetryRelayer<FullCycleRelayer>>>>);

This would significantly reduce the boilerplate required for custom component graph implementation. For example, a user who wants to remove the FilterRelayer from the component graph would copy the code from DefaultComponents, and modify it such as follows:

derive_chain_status_querier!(MyCustomComponents<BaseComponents>,
    LockPacketRelayer<LoggerRelayer<RetryRelayer<FullCycleRelayer>>>);

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
1 participant