Connectors POC

[NunoAlexandre]

closes #955

To Do

• Working version of AddPool on a testnet
• Unit-test remote call encoding
• Use xcm_transactor::transact_through_sovereign
  For our use case, we want the messages to be sent from the sovereign account but the account calling the outer connectors' extrinsic paying for the fees involved in the Transact execution. transact_through_sovereign seems to be the best fit here.
• Extend XcmDomain
  The drawback here is that it makes it impossible to unit-test the remote call encoding since it would depend on T::Currency of sorts
• Clean and simplify
• Impl add_tranche and transfer ToDo's
• Run complete test on testnet

[NunoAlexandre]

Self review

[NunoAlexandre]

@offerijns @mikiquantum cc @mustermeiszer we just have a few todos to complete here, I am opening this PR for review to start addressing your reviews. Keep in mind the scope of this PR and that we are keeping it on development for now. Thanks 🏄

[NunoAlexandre]

Leaving two notes on the 2 TODOs

[NunoAlexandre]

This one is tricky since it needs to directly reference CurrencyId::Tranche, therefore breaking the genericness of the CurrencyId type. So either we make the type param be very strict on the cfg_types::CurrencyId enum type or we just reference it directly.

[mustermeiszer]

I am working on the currencyId enum anyways this week. We could make the TrancheCurrency a struct which connectors directly depends on and it will implement Into<CurrencyId>.

Other solution, we extend the PoolInspect trait and let it return TrancheCurrency and PoolCurrency for a pool, if it exists. Then you could simply use the returned Currency.

[NunoAlexandre]

Ah yes that would be nice @mustermeiszer 😃

[mustermeiszer]

Which one do you prefer? I am still in the process of making up my mind around the changes to CurrencyId.

I have two options in my head:

First - structured currency types that are wrapped in enums

// Also implements `Into<CurrencyId`
struct TrancheCurrency<PoolId, TrancheId> {
   pool_id: PoolId, 
   tranche_id: TrancheId
}

impl<PoolId, TrancheId> TrancheCurrency<PoolId, TrancheId> {
   const DECIMALS: u128 = ???; // Need to get this from the registry
   const SYMBOL: &str =  ???; //Need to get this from the registry
}

// Also implements `Into<CurrencyId`
struct KSM;

impl KSM {
   const DECIMALS: u128 = primitives::currency_decimals::KSM;
   const SYMBOL: &str =  &"KSM";
}

// Also implements `Into<CurrencyId`
struct AUSD;

impl AUSD {
   const DECIMALS: u128 = primitives::currency_decimals::AUSD;
   const SYMBOL: &str =  &"AUSD";
}

// Currencies that can. beused in a pool as a base currency
pub enum PoolCurrency {
   KSM(KSM),
   AUSD(AUSD)
}

// The currencies one can invest into 
pub enum InvestmentId {
   Tranche(TrancheCurrency<PoolId, TrancheId>)
}

// The overarching CurrencyId enum that is only used by pallets
// that need knowledge of all currencies, e.g. orml_tokens
//
// MUST NOT be differentiate between runtimes, but could be
mod altair {
   enum CurrencyId {
     KSM(KSM),
     AUSD(AUSD),
     TrancheCurrency(TrancheCurrency)
   }
}

Second - Do not use structured currencies but rather have the enums convert it

// Currencies that can. beused in a pool as a base currency
pub enum PoolCurrency {
   KSM,
   AUSD
}

impl Into<CurrencyId> for PoolCurrency {
...
}

// The currencies one can invest into 
pub enum InvestmentId {
   Tranche(TrancheCurrency<PoolId, TrancheId>)
}

impl Into<CurrencyId> for InvestmentId {
...
}

[NunoAlexandre]

@mustermeiszer do you mind creating an issue to discuss this? 🙏

[hieronx]

FYI the first option was implemented in #995

[NunoAlexandre]

Cool, thanks! @mustermeiszer I might ping you on this one :)

[hieronx]

@NunoAlexandre do we still want to address this TODO here? I think it should be possible now that #995 was merged

[mustermeiszer]

I am not sure, at which state we are with this. One thing I noticed is that the actual transfer to the local domain address is commented out. Is this intended even for the dev environment?

The only thing I think is wrong is that we allow anybody who has the PoolRole::TrancheInvestor to add a Domain as a tranche investor to a pool. But maybe I am misunderstanding the logic or this is intended. Could you clarify that @NunoAlexandre

if this is good, I would approve. ^^

[hieronx]

Correct me if I'm wrong, but I think the get_tranche_token_price method would fail on line https://github.com/centrifuge/centrifuge-chain/blob/main/pallets/pools/src/impls.rs#L48 as well if the pool_id or tranche_id was invalid. So it might be more efficient to remove T::PoolInspect::tranche_exists, wdyt?

[NunoAlexandre]

Hm yeah, if we do that we should change the error MissingPrice to something more general, otherwise it will sound like only the price of an non-existing tranche is missing. wdyt?

[hieronx]

Yeah that makes sense, not entirely sure what the ideal naming is though. Any suggestions?

[NunoAlexandre]

So, we need to check whether the tranche token exists in add_tranche whilst not needing to fetch the tranche price. This means that we need to have an error variant for TrancheNotFound for this case.

So I just renamed MissingPrice to MissingTranchePrice to be more specific it's Tranche-related and left a richer doc comment on what this error can mean.

[hieronx]

FYI the first option was implemented in #995

[NunoAlexandre]

@offerijns thanks a lot for the review 🚀 I will go through it later today!

[NunoAlexandre]

@offerijns this is pretty much ready but now that we merged the 0.9.29 I need to update our forks of the Moonbeam repos and it's not looking pretty. On it 🏃

[hieronx]

Just wondering if we want to address fixing the transfers in this PR. Besides that, everything is looking great! 🔥

[hieronx]

@NunoAlexandre do we still want to address this TODO here? I think it should be possible now that #995 was merged