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
Feature/v2 v3 migration #1352
Feature/v2 v3 migration #1352
Conversation
(cherry picked from commit 21362ac)
chore: merge updated permit flow
a new circle of testing brings new bugs @satanworker BUG1: steps for reproduction:
should be:
BUG2:
BUG3:
markets before migration
where:
should be:
p.s. also this bug blocked some cases for testing |
Good to see previous bugs with HF are fixed, 2 first bugs looks minor, but the last one is weird, looking into it |
@satanworker there is one more issue in case of switching market on another tab migration page upload data from another market steps:
result:
|
closed due to #1442 |
General changes
Dedicated page with logic to handle migration from AAVE v2 to v3.
In nutshell, migrating from v2 to v3 positions requires, to set current market to v2, while simultaneously fetching data for v3.
Selecting positions from v2 should dynamically change health factor and display both before and after migration HF for both v2 and v3.
When user selects multiple assets to migrate, supply asset amount should be slightly increased to allow migrating the whole position, or the balance would grow during approving and transaction execution and method would fail as HF will fell below 1, with no supply and a little bit of debt.
In order to properly do that, changes in the whole transaction flow on the aave interface required. More specifically, changes to how
useTransactionHandler
is working.On any modal opens in the app after small delay, transactions are filtered into approval and actions by transaction type.
If approval state is not empty, that means we need to approve action first with permit if available after with on chain approval.
Currently, AAVE interface assumes that each
Action
in the app will have single approve or single permit and single following transaction. Although it worked great for most parts, specifically for migration it didn't work, since migrating multiple positions would require approving/signing permit for each supply asset.Also, it's the first pr with new changes done in zustand completely, which also changed a bit the flow working with Actions component, as mentioned in this discussion, we want to migrate from idea of dedicated actions component anyway. It also showcases how easy it is to nest multiple non-destructive data manipulation in selectors. i.e. increasing amount, filtering for non-empty states, etc. Which is the bulk of the logic in the app.
Isolation mode
When user has 0 collateral on v3, the order of supply assets will define the isolation mode, if isolated supply is supplied on v2.
Supply USDT as collateral to empty balance on v3, will automatically enable it as collateral and activate isolation mode, therefore supplying any other collateral will be ignored.
To make it easier for user to understand enabled as collateral toggles are presented.
The logic is following, if only isolated supplies are selected, the last one will be selected as collateral and therefore migrated first.
if any non-isolated supply is available, non-isolated supplies will be selected as collateral. Non-isolated supplies will be migrated first. Since it’s way harder to migrate from isolated supply to non-isolated one.
It’s still possible though to enable isolated supply manually and user choice will be respected.
The situation gets more complicated with non-empty balance on v3, which is not enabled as collateral.
In that case supplied assets on v3 can not be toggled as collateral, but the rest of the assets will still follow the same rules of the first migrated supply will trigger isolation mode. For example
In this example, user has DAI disabled as collateral on v3 and that’s why it’s disabled for usage as collateral on migration page.
But MATIC or USDT can be triggered as collateral and therefore define the isolation mode
Balancer will be enabled/disabled for migration based on isolation mode dynamically
Developer notes
Since v2-v3 migrator is not deployed, use this RPC for testing
Be careful fork chain is equal to real chain ID on purpose, in order to test permits, both fork and base chain id required to be the same
contract helpers version is set to deployed of this PR
aave/aave-utilities#459 once we merge it to main will change to bumped version
Demo
(top UI was changed a bit)
CleanShot.2022-12-06.at.18.54.07.1.mp4
To discuss
[ ] Discuss how to dynamically change market names based on chain ID after migration deployment