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

Fix: Loans demo migration #1765

Merged
merged 7 commits into from
Mar 11, 2024
Merged

Fix: Loans demo migration #1765

merged 7 commits into from
Mar 11, 2024

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Mar 8, 2024

Description

Loans for demo env have an on-chain version of 0, skipping the v2->v3 migration. This restores the correct on-chain version so that the migration can be applied.

This changes have no effect in altair or centrifuge

Logs

2024-03-08 15:07:36.834  INFO main runtime_common::migrations::increase_storage_version: BumpStorageVersion: Increasing storage version of "Loans" from StorageVersion(0) to 2
2024-03-08 15:07:36.834  INFO main runtime_common::migrations::increase_storage_version: BumpStorageVersion: chain_version StorageVersion(2)
...
2024-03-08 15:07:36.835  INFO main runtime::frame-support: ⚠️ Loans declares internal migrations (which *might* execute). On-chain `StorageVersion(2)` vs current storage version `StorageVersion(3)`
2024-03-08 15:07:36.835  INFO main pallet_loans::migrations: Loans: Starting migration v2 -> v3
2024-03-08 15:07:36.835  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 99
2024-03-08 15:07:36.835  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 2666134719
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 2869156029
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 2659193339
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 1
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 75741337
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 2
2024-03-08 15:07:36.836  INFO main pallet_loans::migrations: Loans: Migrated 7 pools

@lemunozm lemunozm added I2-bug The code fails to follow expected behaviour. P9-drop-everything Everyone should address this issue now. D8-migration Pull request touches storage and needs migration code. labels Mar 8, 2024
@lemunozm lemunozm self-assigned this Mar 8, 2024
@lemunozm
Copy link
Contributor Author

lemunozm commented Mar 8, 2024

I'm not sure if we want to bump up from 1042 to 1043, or if we do not want to because this is just a migration fix. If not, the PR is ready to be merged.

@lemunozm lemunozm force-pushed the fix/loans-demo-migration branch from bb57531 to 803858c Compare March 8, 2024 14:15
@wischli
Copy link
Contributor

wischli commented Mar 8, 2024

I'm not sure if we want to bump up from 1042 to 1043, or if we do not want to because this is just a migration fix. If not, the PR is ready to be merged.

We need to bump because else we cannot perform a RU.

wischli
wischli previously approved these changes Mar 8, 2024
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.

Thanks for fixing this so quickly! As mentioned, we need to bump spec version of Development in order to apply this migration.

@lemunozm
Copy link
Contributor Author

lemunozm commented Mar 8, 2024

Done! please double-check I'm not leaving any place to change 1042 to 1043

@mustermeiszer
Copy link
Collaborator

@lemunozm what are the logs for try-runtime on altiar and centrifuge? Could you post them here too,. THat we know all migrations are running?

@lemunozm
Copy link
Contributor Author

lemunozm commented Mar 9, 2024

This migration is not applied for altair which will reset the storage for loans. Later it tries to apply, but unsuccessful due to mismatching versions as expected:

2024-03-09 21:36:43.660  INFO main runtime::frame-support: ⚠️ Loans declares internal migrations (which *might* execute). On-chain `StorageVersion(3)` vs current storage version `StorageVersion(3)`
2024-03-09 21:36:43.660  WARN main pallet_loans::migrations: Loans: Migration did not execute. This probably should be removed

For centrifuge it shows:

2024-03-09 21:34:58.600  INFO main runtime::frame-support: ⚠️ Loans declares internal migrations (which *might* execute). On-chain `StorageVersion(2)` vs current storage version `StorageVersion(3)`
2024-03-09 21:34:58.600  INFO main pallet_loans::migrations: Loans: Starting migration v2 -> v3
2024-03-09 21:34:58.600  INFO main pallet_loans::migrations: Loans: updated portfolio for pool_id: 4139607887
2024-03-09 21:34:58.600  INFO main pallet_loans::migrations: Loans: Migrated 1 pools

But I also obtain the following unrelated error:

2024-03-09 20:34:53.362 ERROR main runtime: panicked at /Users/luis/Repos/centrifuge-chain3/runtime/common/src/migrations/local_currency.rs:312:13:
assertion failed: !has_pending_invest_orders::<T>(TargetPoolId::get())

Do we already know about it? @wischli

@mustermeiszer
Copy link
Collaborator

@lemunozm that is an outstanding investment. We can not migrate with that being present. I am asking Jay to bring that into the pool to let us proceed.

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!

@wischli wischli merged commit 878db0c into main Mar 11, 2024
9 checks passed
@wischli wischli deleted the fix/loans-demo-migration branch March 11, 2024 08:16
@wischli wischli mentioned this pull request Mar 12, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D8-migration Pull request touches storage and needs migration code. I2-bug The code fails to follow expected behaviour. P9-drop-everything Everyone should address this issue now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants