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

chore(jellyfish): add negative interest rate tests #1755

Merged
merged 10 commits into from
Sep 18, 2022

Conversation

DieHard073055
Copy link
Contributor

@DieHard073055 DieHard073055 commented Sep 14, 2022

What this PR does / why we need it:

Tests the negative interest rate update that was released.

Which issue(s) does this PR fixes?:

Fixes part of #1749

Additional comments?:

I will be pushing smaller changes at a time, while i get reviews on the work.

TODO:

  • should take a loan with negative interest rate and accrue DUSD
  • should fully pay back a vault with negative interest rate
  • should fully pay back a vault with multiple loans including negative interest rate loan
  • should pay back a vault partially with negative interest rate
  • should pay back a vault partially with multiple loans including negative interest rate loan

@codeclimate
Copy link

codeclimate bot commented Sep 14, 2022

Code Climate has analyzed commit 970cfa6 and detected 0 issues on this pull request.

View more on Code Climate.

@netlify
Copy link

netlify bot commented Sep 14, 2022

Deploy Preview for jellyfishsdk ready!

Name Link
🔨 Latest commit 970cfa6
🔍 Latest deploy log https://app.netlify.com/sites/jellyfishsdk/deploys/6322d704c6c82f000926b9aa
😎 Deploy Preview https://deploy-preview-1755--jellyfishsdk.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Sep 14, 2022

Docker build preview for jellyfish/apps is ready!

Built with commit 26bb487

  • ghcr.io/jellyfishsdk/legacy-api:pr-1755
  • ghcr.io/jellyfishsdk/ocean-api:pr-1755
  • ghcr.io/jellyfishsdk/playground-api:pr-1755
  • ghcr.io/jellyfishsdk/status-api:pr-1755
  • ghcr.io/jellyfishsdk/whale-api:pr-1755

You can also get an immutable image with the commit hash

  • ghcr.io/jellyfishsdk/legacy-api:26bb487cafd450dcf9bbc3ac1e362b8f05e14486
  • ghcr.io/jellyfishsdk/ocean-api:26bb487cafd450dcf9bbc3ac1e362b8f05e14486
  • ghcr.io/jellyfishsdk/playground-api:26bb487cafd450dcf9bbc3ac1e362b8f05e14486
  • ghcr.io/jellyfishsdk/status-api:26bb487cafd450dcf9bbc3ac1e362b8f05e14486
  • ghcr.io/jellyfishsdk/whale-api:26bb487cafd450dcf9bbc3ac1e362b8f05e14486

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Base: 93.51% // Head: 93.06% // Decreases project coverage by -0.45% ⚠️

Coverage data is based on head (970cfa6) compared to base (86e5e2c).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1755      +/-   ##
==========================================
- Coverage   93.51%   93.06%   -0.46%     
==========================================
  Files         361      361              
  Lines       10416    10416              
  Branches     1304     1304              
==========================================
- Hits         9741     9694      -47     
- Misses        647      693      +46     
- Partials       28       29       +1     
Impacted Files Coverage Δ
...ontainers/src/containers/RegTestContainer/index.ts 100.00% <ø> (ø)
...playground-api/src/controllers/WalletController.ts 26.19% <0.00%> (-73.81%) ⬇️
packages/playground-api-client/src/apis/Wallet.ts 16.66% <0.00%> (-66.67%) ⬇️
...egacy-api/src/controllers/stats/StatsController.ts 25.92% <0.00%> (-62.97%) ⬇️
...y-api/src/controllers/stats/LegacyStatsProvider.ts 47.36% <0.00%> (-45.62%) ⬇️
packages/jellyfish-network/src/BlockSubsidy.ts 74.54% <0.00%> (-25.46%) ⬇️
...ckages/jellyfish-api-core/src/category/poolpair.ts 92.59% <0.00%> (-7.41%) ⬇️
apps/whale-api/src/module.api/poolpair.service.ts 81.17% <0.00%> (+0.41%) ⬆️
packages/jellyfish-testing/src/icxorderbook.ts 100.00% <0.00%> (+1.61%) ⬆️
...api/src/module.api/poolswap.pathfinding.service.ts 96.99% <0.00%> (+3.00%) ⬆️
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DieHard073055
Copy link
Contributor Author

Now when I take a loan with

    const dusdLoanAmount = 5000
    const txid = await testing.rpc.loan.takeLoan({
      vaultId: bobVaultId,
      amounts: `${dusdLoanAmount}@DUSD`
    })

And then check the interest rate with

    const interests = await testing.rpc.loan.getInterest('scheme')

The interest is 0

    console.log(interests[0].totalInterest.toFixed(8)) // 0.00000000
    console.log(interests[0].interestPerBlock.toFixed(8)) // 0.00000000
    console.log(interests[0].realizedInterestPerBlock.toFixed(8)) // 0.00000000

@DieHard073055
Copy link
Contributor Author

But if I comment out the setGov function, I get some value for the interest rate

    await testing.rpc.masternode.setGov({
      ATTRIBUTES: {
        'v0/token/1/loan_minting_interest': '-5'
      }
    })
    await testing.generate(1)

the result is something like

    console.log(interests[0].totalInterest.toFixed(8)) // 0.00475647
    console.log(interests[0].interestPerBlock.toFixed(8)) // 0.00475647
    console.log(interests[0].realizedInterestPerBlock.toFixed(8)) // 0.00475647

- adding greatworldheight=14 in Container cmds fixed the issue.
- first test is half way completed.
@DieHard073055 DieHard073055 force-pushed the DieHard073055/add_negative_interest_rate_tests branch from 2337e29 to 81270f6 Compare September 14, 2022 07:48
- fix the test "it should takeLoan with negative interest and accrue
  DUSD"
- unable to payback negative interest loan.
@DieHard073055
Copy link
Contributor Author

 FAIL  packages/jellyfish-api-core/__tests__/category/loan/negativeInterest.test.ts (38.484 s)                         
  takeLoan with negative interest success                                                                              
    ✓ should takeLoan with negative interest and accrue DUSD (18044 ms)                                                
    ✕ should fully pay back a vault with negative interest rate (17752 ms)                                             
                                                                                                                       
  ● takeLoan with negative interest success › should fully pay back a vault with negative interest rate                
                                                                                                                       
    RpcApiError: 'Test PaybackLoanTx execution failed:                                                                 
    amount 0.00000000 is less than 4995.24353121', code: -32600, method: paybackloan 

I wonder what I am doing wrong here.

 // payback loan
    const loanPaybackResult = await testing.rpc.loan.paybackLoan({
      vaultId: bobVaultId,
      amounts: afterLoanAmount, // console.log shows the value is 4995.24353121@DUSD
      from: aliceAddr
    })

@DieHard073055
Copy link
Contributor Author

I have implemented all the tests.
I believe I need to calculate all the numbers in the test, rather then defining a variable with a magic number and then asserting against that ?

Comment on lines +15 to +16
// High interest rate so the change will be significant
const interestRate = 5000
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. 5000 is quite abnormal.. preferably provide a more reality interest rate.. maybe 3.3??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, i wanted to see a significant change with just one block added to the chain.

Copy link
Contributor

@canonbrother canonbrother Sep 15, 2022

Choose a reason for hiding this comment

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

noted that.. hmm.. the reason why i recommend to provide tricky figure like 3.3 or 5.6433 is to check the precision working
the significant change can be tested by c.generate(36)

@canonbrother canonbrother changed the title add negative interest rate tests chore(jellyfish): add negative interest rate tests Sep 15, 2022
@jellyfishsdk-bot jellyfishsdk-bot added the kind/chore Non feature change label Sep 15, 2022
@canonbrother
Copy link
Contributor

heyy @DieHard073055
that's some failed test.. can you check if that is regression issue??

@fuxingloh
Copy link
Contributor

fuxingloh commented Sep 15, 2022 via email

@DieHard073055
Copy link
Contributor Author

DieHard073055 commented Sep 15, 2022

One of the errors is happening in setLoanToken.test.ts

Because of the following condition interest rate cannot be less than 0!
We now allow this.

Should I remove this test? @fuxingloh

@canonbrother
Copy link
Contributor

I’ve a PR that fixes the regression but I haven’t see if it’s all fixed there

Noted that

@DieHard073055
just want make sure whether its regression or flaky <-- you can try run it local to verify...
overall the pr look good to me

@fuxingloh
Copy link
Contributor

One of the errors is happening in setLoanToken.test.ts

Because of the following condition interest rate cannot be less than 0! We now allow this.

Should I remove this test? @fuxingloh

If they are no longer valid it.skip them or fix them if possible.

@fuxingloh
Copy link
Contributor

@DieHard073055 can you also update the 2 docker-compose.yml with the new flag.

- update `updateLoanToken` to allow negative interest
- update `setLoanToken` to allow negative interest
@DieHard073055 DieHard073055 requested review from Jouzo and fuxingloh and removed request for Jouzo September 15, 2022 08:52
@fuxingloh fuxingloh merged commit 856da19 into main Sep 18, 2022
@fuxingloh fuxingloh deleted the DieHard073055/add_negative_interest_rate_tests branch September 18, 2022 17: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.

5 participants