Tightly coupled market-commons with prediction markets.

[vivekvpandya]

Fixes #805

[maltekliemann]

Double-booking the implementation of MarketCommonsPalletApi into the implementation of the zrml_market_commons::Pallet::<T> seems strange. Is there a particular reason why you did that, instead of replacing every zrml_market_commons::Pallet::<T> with <zrml_market_commons::Pallet::<T> as MarketCommonsPalletApi>?

[vivekvpandya]

Double-booking the implementation of MarketCommonsPalletApi into the implementation of the zrml_market_commons::Pallet::<T> seems strange. Is there a particular reason why you did that, instead of replacing every zrml_market_commons::Pallet::<T> with <zrml_market_commons::Pallet::<T> as MarketCommonsPalletApi>?

Frankly I did not get this idea when editing, I though for loose coupling we use MarketCommonsPalletApi and for tight coupling we directly use functions from pallet and don't need to import MarketCommonsPalletApi.

[Chralt98]

Starting my review now..

[maltekliemann]

Double-booking the implementation of MarketCommonsPalletApi into the implementation of the zrml_market_commons::Pallet::<T> seems strange. Is there a particular reason why you did that, instead of replacing every zrml_market_commons::Pallet::<T> with <zrml_market_commons::Pallet::<T> as MarketCommonsPalletApi>?

Frankly I did not get this idea when editing, I though for loose coupling we use MarketCommonsPalletApi and for tight coupling we directly use functions from pallet and don't need to import MarketCommonsPalletApi.

It's a matter of dependency. With loose coupling, you can only access the config parameter MarketCommons, which implements MarketCommonsPalletApi. This allows you to use any struct which implements that particular trait. With tight coupling, that config parameter is gone and we have to use the particular zrml_market_commons pallet, which happens to implement MarketCommonsPalletApi. In particular, the user has no choice in the matter. Before this change, they could have used their own "market manager". Now they have to use ours.

But since the MDMs are still loosely coupled, I think we should move MarketCommonsPalletApi into the primitives. Is there any argument against this?

[Chralt98]

You like to have the tight coupling of zrml_swaps in another PR?

[vivekvpandya]

You like to have the tight coupling of zrml_swaps in another PR?

yes if it is really required, as per discussion in one of the meeting we feel that swaps should not be tightly coupled with market-commons.

[Chralt98]

I like the idea of the tightly couple.

[vivekvpandya]

But since the MDMs are still loosely coupled, I think we should move MarketCommonsPalletApi into the primitives. Is there any argument against this?

so what is the motivation here? Just that above trait is imported from primitives and not from market_commons? I mean only implementer of that trait is MarketCommons so I am fine with any approach.

[maltekliemann]

But since the MDMs are still loosely coupled, I think we should move MarketCommonsPalletApi into the primitives. Is there any argument against this?

so what is the motivation here? Just that above trait is imported from primitives and not from market_commons? I mean only implementer of that trait is MarketCommons so I am fine with any approach.

Because the MDMs and swaps are still loosely coupled, people could potentially use them with some other struct that implements MarketCommonsPalletApi. If they wanted to do that, they would have to import zrml_market_commons to get the trait, which is an unnecessary import. Instead, they should be able to import the trait from the primitives.

[Chralt98]

LGTM

[maltekliemann]

👍