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(ui-ux): allow dusd loops in vaults #4114

Merged
merged 13 commits into from
Nov 1, 2023
Merged

Conversation

lykalabrada
Copy link
Contributor

@lykalabrada lykalabrada commented Oct 26, 2023

What this PR does / why we need it:

In line with ain changes (DeFiCh/ain#1971), DUSD loops in vaults should now be allowed

Which issue(s) does this PR fixes?:

Fixes #4034, DFC-394

Additional comments?:

When testing this PR, please check the following scenarios:

  • Users should be able to take a DUSD loan when their vault has the following collaterals:

    • Vault that has 100% DFI collateral
    • Vault that 100% DUSD collateral (new)
    • 50% DFI + other tokens
  • Users should NOT be able to take a DUSD loan when their vault has the following collaterals:

    • 0% DFI + other tokens
    • <50% DFI + other tokens
    • DUSD + other tokens
  • Users should NOT be able to take a non-DUSD loan when their vault has the following collaterals:

    • 100% DUSD
    • <50% DFI + other tokens
  • Others:

    • Removing collateral from a vault that has an active DUSD loan -> verify that this is not allowed
    • Adding of non-DUSD collateral to a vault with an active DUSD loan, containing 100% DUSD collateral -> verify that this is not allowed

Developer Checklist:

  • Read your code changes at least once
  • Tested on iOS/Android device (e.g, No crashes, library supported etc.)
  • No console errors on web
  • Tested on Light mode and Dark mode*
  • Your UI implementation visually matched the rendered design*
  • Unit tests*
  • Added e2e tests*
  • Added translations*

@linear
Copy link

linear bot commented Oct 26, 2023

DFC-394 DUSD Loops in Vaults

Summary

Based on:

DeFiCh/ain#1971

**What happened based on **[Issue #4034](https://Allow DUSD loops in Vault in changi network #4034)

"DVM: Enable DUSD loops in vaults #1971 was integrated in June in a beta release for changi. Now I want test it in the Lightwallet with a new vault and DUSD as collateral. But the Lightwallet does not allow it: Continue Button is disabled with message: 'Insuffucient DFI in vault. Add more to borrow DUSD"

What you expected to happen:

Allow borrowing of DUSD in a Vault with DUSD collateral.

How to reproduce it (as minimally and precisely as possible):

  • Switch to Changi network
  • Create Vault
  • Add DUSD as collateral
  • try to borrow DUSD

@github-actions github-actions bot added area/ui-ux kind/feature New feature request labels Oct 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

Missing Translations Report

The following translations are missing for this pull request.

{
    "missingLanguageItems": {
        "zh-Hans": {
            "missingCount": 0,
            "labels": {},
            "totalCount": 2029,
            "allLabels": "{}"
        },
        "zh-Hant": {
            "missingCount": 0,
            "labels": {},
            "totalCount": 2029,
            "allLabels": "{}"
        },
        "fr": {
            "missingCount": 0,
            "labels": {},
            "totalCount": 2029,
            "allLabels": "{}"
        },
        "es": {
            "missingCount": 0,
            "labels": {},
            "totalCount": 2029,
            "allLabels": "{}"
        },
        "it": {
            "missingCount": 0,
            "labels": {},
            "totalCount": 2029,
            "allLabels": "{}"
        }
    },
    "totalMissingCount": 0
}

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

Build preview for DeFiChain Wallet is ready!

Built with commit 6a7c145

https://expo.io/@defichain/wallet?release-channel=pr-preview-4114

@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2023

Codecov Report

Merging #4114 (d6f737e) into main (b320ad0) will decrease coverage by 19.26%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##             main    #4114       +/-   ##
===========================================
- Coverage   56.13%   36.87%   -19.26%     
===========================================
  Files         438      438               
  Lines       12312    12352       +40     
  Branches     4069     4089       +20     
===========================================
- Hits         6911     4555     -2356     
- Misses       5317     7746     +2429     
+ Partials       84       51       -33     
Files Coverage Δ
...or/screens/Loans/screens/BorrowLoanTokenScreen.tsx 0.00% <0.00%> (-79.77%) ⬇️
...ultDetail/components/VaultDetailCollateralsRow.tsx 0.00% <0.00%> (-65.46%) ⬇️
...r/screens/Loans/hooks/ValidateLoanAndCollateral.ts 0.00% <0.00%> (ø)
...eens/Loans/screens/AddOrRemoveCollateralScreen.tsx 0.00% <0.00%> (-58.95%) ⬇️

... and 141 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Krysh90
Copy link

Krysh90 commented Oct 26, 2023

One thing came to my mind regarding your comment about my PR. You absolutely correct that adding/removing collateral was missing. I found one case, where it was especially needed. If you have a vault with DFI + DUSD as collateral and DUSD as loan. If you then try to remove all the DFI while still having enough DUSD to cover the loan, it should be possible to take out the DFI.

As before the 50% DFI requirement was valid and afterwards the 100% DUSD. I added a comment to the PR, where I think this check is missing. As it is not working on your current feature branch.

I have tested a few of my tests, which all passed.

@lykalabrada
Copy link
Contributor Author

One thing came to my mind regarding your comment about my PR. You absolutely correct that adding/removing collateral was missing. I found one case, where it was especially needed. If you have a vault with DFI + DUSD as collateral and DUSD as loan. If you then try to remove all the DFI while still having enough DUSD to cover the loan, it should be possible to take out the DFI.

As before the 50% DFI requirement was valid and afterwards the 100% DUSD. I added a comment to the PR, where I think this check is missing. As it is not working on your current feature branch.

I have tested a few of my tests, which all passed.

Thanks for checking! You are correct, I updated the PR to enable the removal of all DFI in the presence of only DUSD.

*
* Note: DUSD loops in vaults are now allowed - https://github.com/DeFiCh/ain/pull/1971
*
* @returns
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @returns
* @param props - An object containing the collateral and loan information.
* @returns { isLoanAllowed: boolean } - Indicates whether the loan is allowed.

@lykalabrada lykalabrada enabled auto-merge (squash) November 1, 2023 02:13
@lykalabrada lykalabrada disabled auto-merge November 1, 2023 02:14
@lykalabrada lykalabrada merged commit 9b52cb0 into main Nov 1, 2023
11 of 15 checks passed
@lykalabrada lykalabrada deleted the lyka/allow-dusd-loops branch November 1, 2023 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow DUSD loops in Vault in changi network
5 participants