BirthdayResearch / defichain-wallet

DeFiChain Wallet. The DeFi Blockchain Light Wallet for iOS, Android & Web.
MIT License
125 stars 46 forks source link

feat(ui-ux): allow dusd loops in vaults #4114

Closed lykalabrada closed 9 months ago

lykalabrada commented 10 months ago

What this PR does / why we need it:

In line with ain changes (https://github.com/DeFiCh/ain/pull/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:

Developer Checklist:

linear[bot] commented 10 months ago
DFC-394 DUSD Loops in Vaults

**Summary** Based on: [https://github.com/DeFiCh/ain/pull/1971](https://github.com/DeFiCh/ain/pull/1971) **What happened based on **[**Issue #4034**](https://Allow DUSD loops in Vault in changi network #4034) "[DVM: Enable DUSD loops in vaults #1971](https://github.com/DeFiCh/ain/pull/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[bot] commented 10 months ago

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[bot] commented 10 months ago

Build preview for DeFiChain Wallet is ready!

Built with commit 6a7c145c34499efd51d5340a7242876faa22f24f

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

codecov-commenter commented 10 months ago

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%) :arrow_down:
...ultDetail/components/VaultDetailCollateralsRow.tsx 0.00% <0.00%> (-65.46%) :arrow_down:
...r/screens/Loans/hooks/ValidateLoanAndCollateral.ts 0.00% <0.00%> (ø)
...eens/Loans/screens/AddOrRemoveCollateralScreen.tsx 0.00% <0.00%> (-58.95%) :arrow_down:

... and 141 files with indirect coverage changes

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

Krysh90 commented 10 months ago

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 commented 10 months ago

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.