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.

Sign up for GitHub

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

Jump to bottom

centrifuge: Anemoy pool currency migration #1566

Merged
merged 11 commits into from
Sep 25, 2023
Merged

centrifuge: Anemoy pool currency migration #1566

merged 11 commits into from
Sep 25, 2023

Conversation

NunoAlexandre
Copy link
Contributor

@NunoAlexandre NunoAlexandre commented Sep 22, 2023 •
edited
Loading

Description

We want to migrate the currency of the Anemoy pool from LpEthUSDC to (Polkadot-native) USDC on Centrifuge.

  • Anemoy has pool id 4,139,607,887 (the only pool currently registered on Centrifuge)
  • We will migrate the currency of that pool from ForeignAsset(100,001) (LpEthUSDC) to ForeignAsset(6) (Circle’s USDC native on Polkadot)
  • Both currencies have 6 decimals so no extra considerations in that regard

To Do

  • You can just add a sanity check to the migration that the pool value is indeed 0 and there are no locked orders
    • For all tranches within the Anemoy pool, verify that are no entries on pallet_investments' InvestOrders , RedeemOrders , ActiveInvestOrders and ActiveRedeemOrders.
    • Check that pool value is 0

@NunoAlexandre NunoAlexandre self-assigned this Sep 22, 2023
NunoAlexandre
NunoAlexandre commented Sep 22, 2023
View reviewed changes
libs/types/src/tokens.rs
@@ -187,8 +187,8 @@ where
)]
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
pub struct TrancheCurrency {
pub(crate) pool_id: PoolId,
pub(crate) tranche_id: TrancheId,
pub pool_id: PoolId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

any objections to open these fields visibility? by having them pub(crate) I would have to import he TrancheCurrencyT trait to call generate to build an instance of this type which is quite an overkill imo. let me know if I am not overseeing any requirement 👍

@NunoAlexandre
Copy link
Contributor Author

Implemented the sanity checks for orders under the Anemoy pool and tested again with try-runtime. Looking good:

2023-09-23 14:15:57.471  INFO main centrifuge_runtime::migrations::anemoy_pool: anemoy_pool::Migration: currency set to USDC ✓
2023-09-23 14:15:57.471  INFO main centrifuge_runtime::migrations::anemoy_pool: anemoy_pool::Migration: post_upgrade succeeded ✓

Still have to implement the check for the pool value being 0.

@wischli wischli added the D8-migration Pull request touches storage and needs migration code. label Sep 25, 2023
mustermeiszer
mustermeiszer previously approved these changes Sep 25, 2023
View reviewed changes
Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

Looks good!

runtime/centrifuge/src/migrations.rs Outdated Show resolved Hide resolved
@NunoAlexandre
Copy link
Contributor Author

NunoAlexandre commented Sep 25, 2023 •
edited
Loading

This PR is now feature-complete. Just testing it one last time 👍

Edit

Looking good:

2023-09-25 10:18:30.138  INFO main centrifuge_runtime::migrations::anemoy_pool: anemoy_pool::Migration: currency set to USDC ✓
2023-09-25 10:18:30.138  INFO main centrifuge_runtime::migrations::anemoy_pool: anemoy_pool::Migration: post_upgrade succeeded ✓

@NunoAlexandre NunoAlexandre marked this pull request as ready for review September 25, 2023 08:21
wischli
wischli reviewed Sep 25, 2023
View reviewed changes
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.

The migration LGTM! I would like to raise my concerns about making the pallet-investment storages public.

pallets/investments/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/migrations.rs Outdated Show resolved Hide resolved
wischli
wischli approved these changes Sep 25, 2023
View reviewed changes
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.

LGTM! Thanks for adding the post-merge issue!

@NunoAlexandre NunoAlexandre enabled auto-merge (squash) September 25, 2023 09:27
@NunoAlexandre NunoAlexandre merged commit ebfd236 into main Sep 25, 2023
10 checks passed
@NunoAlexandre NunoAlexandre deleted the centrifuge/anemoy-pool-currency branch September 25, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wischli wischli wischli approved these changes

@mustermeiszer mustermeiszer Awaiting requested review from mustermeiszer

@hieronx hieronx Awaiting requested review from hieronx

Assignees

@NunoAlexandre NunoAlexandre

Labels
D8-migration Pull request touches storage and needs migration code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NunoAlexandre @wischli @mustermeiszer