How to avoid cycle trait dependency in `LiquidityManager`?

[yanganto]

It is nice when this commit comes in, the LiquidityManager now dependents on high-level traits and dependent on a lot of low-level things. However, we ran into a problem after this.

The LiquidityManager now will need APeerManager as a dependency in trait. And PeerManager already implemented APeerManager, but PeerManager also needs CustomMessageHandler. When using the LiquidityManager as the CustomMessageHandler. We will run into a cyclic trait dependency, and that is not allowed in Rust.

PeerManager <- LiquidityManager <- CustomMessageHandler <- PeerManager

Do you think there is a good suggestion to use this crate? Many thanks for considering our use case.

[tnull]

PeerManager <- LiquidityManager <- CustomMessageHandler <- PeerManager

Ah, yeah, note that this cycle isn't new though, it has been necessary for a while. While there are some ways to make it work, it's kind of annoying and we should find a better solution. Will look into it asap.

[johncantrell97]

The way I've avoided this issue at c= and in mutiny code-base is by introducing a struct like MyCustomMessageHandler that holds a reference to the liquidity manager. It can act as a composite message handler if your code base requires multiple message handlers as well.

With this in place you now have a concrete type you can use as your CustomMessageHandler without running into cyclic reference issues.

[yanganto]

Yeh, If a user just wants to use vanilla things, it should not be annoying like this.

We only used one handler and did not customize any behavior in it, and now we need to wrap it with a struct, and then implement at least two traits on it, CustomMessageHandler, CustomMessageReader, and nothing new just recite.

Just wonder, if it is possible to provide A CustomizeHandler struct in the lib. One or more managers can be passed into when new a CustomizedHandler. User can customize their manager and pass in, such that users do not need to wrap the bricks to another shape before building things.

It is just like Lego, every brick provided by Lego can match each other and does not need to be wrapped before using.

[tnull]

Yeh, If a user just wants to use vanilla things, it should not be annoying like this.

We only used one handler and did not customize any behavior in it, and now we need to wrap it with a struct, and then implement at least two traits on it, CustomMessageHandler, CustomMessageReader, and nothing new just recite.

Just wonder, if it is possible to provide A CustomizeHandler struct in the lib. One or more managers can be passed into when new a CustomizedHandler. User can customize their manager and pass in, such that users do not need to wrap the bricks to another shape before building things.

It is just like Lego, every brick provided by Lego can match each other and does not need to be wrapped before using.

Yes, providing a separate CustomMessageHandler impl for the user kinda would solve that issue, but not really: in the end the CustomMessageHandler is exactly the entity that needs to trigger PeerManager::process_events once it queued new messages. So even if we split it out of LiquidityManager, the cycle generally is still there. I now went another route in #73: we just rip out the APeerManager dependency and allow the user to set a generic callback that will be called after new messages are enqueued. While this still doesn't resolve the logic cycle (which we can't resolve), it resolves the cycle in the types.