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

Feat: Liquidity pools inter domain tranche token transfer #1860

Merged
merged 6 commits into from
Jun 6, 2024

Conversation

mustermeiszer
Copy link
Collaborator

Description

The PR allows to send tranche tokens from Domain A to Domain B by relaying the message over Centrifuge. The Solidity side of the LP logic already is capable of doing so, this enables this feature on the chain itself.

NOTE:
Pushing this change before the RU as the DYF pool will most likely need that feature.

Changes and Descriptions

  • Adds a new DomainAccountToDomainAddress converter to ease conversion from message to type
  • Add different handling logic for tranche token transfers
    • Always transfer from DomainAccount to T::DomainAddressToAccountId::convert(receiver)
      → for local receiver this will just be the normal account they choose to send to
      → for remote receiver this will be a derived account that temporarily holds the tokens
    • If centrifuge is not the final receiving domain → use normal local transfer logic of LP to forward to actual domain

Checklist:

  • I have added Rust doc comments to structs, enums, traits and functions
  • I have made corresponding changes to the documentation
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works

@mustermeiszer mustermeiszer requested review from wischli and lemunozm June 5, 2024 14:25
@mustermeiszer mustermeiszer marked this pull request as draft June 5, 2024 14:25
@mustermeiszer
Copy link
Collaborator Author

Should we start adding test to the pallet itself now? ^^ Integration test will be hard until the LP integration test is not done.

@@ -1,99 +0,0 @@
// Copyright 2023 Centrifuge Foundation (centrifuge.io).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these pallets removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused config, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's right! I forgot to remove those files after migrating them 🤦🏻‍♂️. Thanks

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM! We could add a simple integration test which ensures the 3 hop of tranche token amounts via events.

tranche_id,
receiver,
amount,
)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the simplicity of this feature!

lemunozm
lemunozm previously approved these changes Jun 6, 2024
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mustermeiszer mustermeiszer marked this pull request as ready for review June 6, 2024 13:46
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with adding tests later and verifying this change on Demo. Code LGTM!

@mustermeiszer mustermeiszer enabled auto-merge (squash) June 6, 2024 14:22
@mustermeiszer mustermeiszer merged commit 3769a0b into main Jun 6, 2024
10 checks passed
@mustermeiszer mustermeiszer mentioned this pull request Jun 8, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants