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

Connectors v2: Add ExecutedCollect* messages #1356

Closed
NunoAlexandre opened this issue May 22, 2023 · 5 comments · Fixed by #1363
Closed

Connectors v2: Add ExecutedCollect* messages #1356

NunoAlexandre opened this issue May 22, 2023 · 5 comments · Fixed by #1363
Assignees
Labels
I8-enhancement An additional feature.

Comments

@NunoAlexandre
Copy link
Contributor

NunoAlexandre commented May 22, 2023

Description

During our multi chain sync, we realised that the this flow doesn't work because we have no way to link the Transfer message to a specific pool, which is necessary for the centrifuge/liquidity-pools#66 work taking place on the Solidity side.

The current flow:

  1. Message CollectInvest | CollectRedeem is sent from EVM -> Centrifuge Chain
  2. The message is received and processed on the Centrifuge chain
  3. On epoch execution, Transfer would be sent from Centrifuge Chain to the EVM

Changes

A. Introduce two new message types which will essentially be a specialised version of Transfer:

  1. ExecutedCollectInvest
  2. ExecutedCollectRedeem

Both of these messages will contain:

  • pool id
  • tranche id
  • currency id
  • executed amount (how much was invested or redeemed, respectively)
  • investor's address

B. Send the appropriate message back to the EVM on epoch execution instead of Transfer

@NunoAlexandre NunoAlexandre added the I8-enhancement An additional feature. label May 22, 2023
@NunoAlexandre NunoAlexandre self-assigned this May 22, 2023
@wischli
Copy link
Contributor

wischli commented May 22, 2023

Right now, incoming Collect{Invest,Redeem} messages move the collectable funds to the provided Substrate AccountId32 investor address. When sending back to EVM, how do we ensure the correct AccountId20 recipient? E.g., can we default to the 20-byte-truncated investor address?

We might want to make the messages a bit more generic. AFAICT, we need similar ones for handling the refunded amounts of existing investments/redemptions triggered by Decrease{Investment,Redeem}Order. From the spec:

This message, if succesful, triggers a message being sent back to the sending domain. The message will take care of re-funding the investor with the given amount the order was reduced with. The investor address is used as the receiver of that tokens.

@NunoAlexandre
Copy link
Contributor Author

Right now, incoming Collect{Invest,Redeem} messages move the collectable funds to the provided Substrate AccountId32 investor address. When sending back to EVM, how do we ensure the correct AccountId20 recipient? E.g., can we default to the 20-byte-truncated investor address?

As far as I've thought of it, the investor address included in those invest/redeem messages will be an EVM, 20-byte long address; then when sending the amount back to the original EVM domain, we use that same EVM address. The type on the message itself is 32-bytes long but that's meant for future-proofness.

I might be wrong, let's wait for more input on this.

We might want to make the messages a bit more generic. AFAICT, we need similar ones for handling the refunded amounts of existing investments/redemptions triggered by Decrease{Investment,Redeem}Order. From the spec:

This message, if succesful, triggers a message being sent back to the sending domain. The message will take care of re-funding the investor with the given amount the order was reduced with. The investor address is used as the receiver of that tokens.

Frankly, most of those invest/collect messages are fairly similar as well and we could have also thought of just having one message and include a u8 byte encoding the actual operation (increase invest = 0, decrease invest = 1, etc) but shall we come to the point where we may need a specific field for one of the use cases, early abstracting will bites us in the ankles.

For the purpose of refunding amounts, I don't know how we need to match the "refunds" to a specific original trigger (I.e, do we need to match a refund to an invest or to a redeem, or is it all the same as long as we know the pool and tranche ids?)

cc @offerijns @ilinzweilin appreciate having your review and input on the above 🏄🏻‍♂️

@mustermeiszer
Copy link
Collaborator

mustermeiszer commented May 23, 2023

As far as I've thought of it, the investor address included in those invest/redeem messages will be an EVM, 20-byte long address; then when sending the amount back to the original EVM domain, we use that same EVM address. The type on the message itself is 32-bytes long but that's meant for future-proofness.

For the purpose of refunding amounts, I don't know how we need to match the "refunds" to a specific original trigger (I.e, do we need to match a refund to an invest or to a redeem, or is it all the same as long as we know the pool and tranche ids?)

We need derived addresses to be revertable to 20-bytes then. cc @branan regarding the message derivation.

@mustermeiszer
Copy link
Collaborator

mustermeiszer commented May 23, 2023

Given my discussion with Alina I understand now that we need the messages. But we need it with more fields.

ExecutedCollectInvest {
        pub investor: [u8; 32],

        pub pool_id: u64,

        pub tranche_id: [u8; 16],
        
        pub currency_id: u128,

        /// This is the payout in the denomination currency
	/// of an investment. I.e. the tranche token.
	/// -> investment in payment currency
	/// -> payout in denomination currency
	pub payout_investment_invest: u128,

	/// This is the remaining investment in the payment currency
	/// of an investment
	/// -> investment in payment currency
	/// -> payout in denomination currency
	pub remaining_investment_invest: u128,
}

and

ExecutedCollectRedeem {
        pub investor: [u8; 32],

        pub pool_id: u64,

        pub tranche_id: [u8; 16],
        
        pub currency_id: u128,

        /// This is the payout in the payment currency
	/// of an investment
	/// -> redemption in denomination currency
	/// -> payout in payment currency
	pub payout_investment_redeem: u128,

	/// This is the remaining redemption in the denomination currency
	/// of an investment
	/// -> redemption in denomination currency
	/// -> payout in payment currency
	pub remaining_investment_redeem: u128,
}

Futhermore, we need the Solidity connectors logic to be adapted. If we receive one of the above message then:

  • DO NOT mint directly into the investors account
    • BUT mint into our escrow account
  • Bookeeps how much a user can actually collect

Change the interface of Solidity connectors too:

  • Call fn collectfn trigger_collect that
  • Expose a new collect_invest & collect_redeem that
    • actually transfers the right amount to the user accounts

For 4626 the contract that is compatible will be allowed to call collect_invest and collect_redeem on behalf of another user.

@NunoAlexandre
Copy link
Contributor Author

Thanks for the follow up, @mustermeiszer.

I assume you meant ExecutedCollectInvest and ExecutedCollectRedeem.

Futhermore, we need the Solidity connectors logic to be adapted. If we receive one of the above message then:

DO NOT mint directly into the investors account
BUT mint into our escrow account
Bookeeps how much a user can actually collect
Change the interface of Solidity connectors too:

Call fn collect → fn trigger_collect that
Expose a new collect_invest & collect_redeem that
actually transfers the right amount to the user accounts
For 4626 the contract that is compatible will be allowed to call collect_invest and collect_redeem on behalf of another >
user.

Cool, that makes sense 👍 Leaving the Solidity side of things to @ilinzweilin as agreed yesterday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I8-enhancement An additional feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants