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(jellyfish-api-core): Add account getPendingFutureSwaps RPC for dfip2203 futures #1321

Merged
merged 22 commits into from
Apr 9, 2022

Conversation

jingyi2811
Copy link
Contributor

@jingyi2811 jingyi2811 commented Apr 5, 2022

What this PR does / why we need it:

Add getPendingFutureSwaps RPC

Which issue(s) does this PR fixes?:

Fixes part of #1268

Additional comments?:

@codeclimate
Copy link

codeclimate bot commented Apr 5, 2022

Code Climate has analyzed commit 71d4b76 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@netlify
Copy link

netlify bot commented Apr 5, 2022

Deploy Preview for jellyfish-defi ready!

Name Link
🔨 Latest commit 71d4b76
🔍 Latest deploy log https://app.netlify.com/sites/jellyfish-defi/deploys/62514bfe45fe3c00086ada62
😎 Deploy Preview https://deploy-preview-1321--jellyfish-defi.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.

@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #1321 (71d4b76) into main (fa355c4) will decrease coverage by 0.85%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1321      +/-   ##
==========================================
- Coverage   90.47%   89.62%   -0.86%     
==========================================
  Files         348      348              
  Lines       10008    10010       +2     
  Branches     1222     1222              
==========================================
- Hits         9055     8971      -84     
- Misses        905      987      +82     
- Partials       48       52       +4     
Impacted Files Coverage Δ
...ackages/jellyfish-api-core/src/category/account.ts 100.00% <100.00%> (+2.81%) ⬆️
apps/whale/src/module.health/probe.indicator.ts 37.50% <0.00%> (-62.50%) ⬇️
apps/whale/src/module.model/pool.swap.ts 38.46% <0.00%> (-61.54%) ⬇️
...s/whale/src/module.indexer/model/dftx/pool.swap.ts 36.95% <0.00%> (-60.87%) ⬇️
apps/whale/src/module.defid/defid.probes.ts 27.27% <0.00%> (-59.10%) ⬇️
...le/src/module.indexer/model/dftx/composite.swap.ts 32.55% <0.00%> (-53.49%) ⬇️
apps/whale/src/module.model/_model.probes.ts 33.33% <0.00%> (-50.01%) ⬇️
...kages/playground-api-client/src/apis/Playground.ts 50.00% <0.00%> (-50.00%) ⬇️
apps/legacy-api/src/modules/WhaleApiModule.ts 45.83% <0.00%> (-45.84%) ⬇️
apps/whale/src/module.api/fee.controller.ts 60.00% <0.00%> (-40.00%) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa355c4...71d4b76. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

Docker build preview for jellyfish/apps is ready!

Built with commit 2840b35

  • ghcr.io/defich/legacy-api:pr-1321
  • ghcr.io/defich/ocean-api:pr-1321
  • ghcr.io/defich/playground-api:pr-1321
  • ghcr.io/defich/status-api:pr-1321

@jingyi2811
Copy link
Contributor Author

The final review of this PR is going to determine how the design of listPendingFutureSwaps PR looks like as both PRs are highly related.

})

it('Should getPendingFutureSwaps if futureswap DUSD for TSLA', async () => {
// Call getpendingfutureswaps before performing futureswap
Copy link
Contributor Author

@jingyi2811 jingyi2811 Apr 6, 2022

Choose a reason for hiding this comment

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

It looks redundant as Line 142 does the same thing.
However, I believe every code in every Jest Item (It) should be independent.

@@ -35,7 +35,7 @@ export abstract class DeFiDContainer extends DockerContainer {
if (process?.env?.DEFICHAIN_DOCKER_IMAGE !== undefined) {
return process.env.DEFICHAIN_DOCKER_IMAGE
}
return 'defi/defichain:master-2a236bb79'
return 'defi/defichain:master-57c099c1a'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be the latest docker, if not reviewer please change this for me.

@jingyi2811 jingyi2811 marked this pull request as ready for review April 6, 2022 09:17
@jingyi2811 jingyi2811 changed the title Add getPendingFutureSwaps RPC feat(jellyfish-api-core): Add account getPendingFutureSwaps RPC for dfip2203 futures Apr 6, 2022
@defichain-bot defichain-bot added the kind/feature New feature request label Apr 6, 2022
Copy link
Contributor

@canonbrother canonbrother left a comment

Choose a reason for hiding this comment

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

check the listing logic against the gov

futureswap
check the list 
deactivate the DFIP2203 
check the list again

await testing.generate(1)
}

beforeEach(async () => {
Copy link
Contributor Author

@jingyi2811 jingyi2811 Apr 8, 2022

Choose a reason for hiding this comment

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

Reset container to remove GOV attributes

@@ -43,6 +43,11 @@ export class TestingToken {
const to = { [address]: [account] }
return await this.rpc.account.sendTokensToAddress({}, to)
}

async getTokenId (symbol: string): Promise<string> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new function created by Suranga

})
})

describe('Multiple futureswaps', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't duplicate the negative test units for setGov in multiple futureswaps

…utureSwaps

# Conflicts:
#	docs/node/CATEGORIES/08-account.md
#	packages/jellyfish-api-core/src/category/account.ts
#	packages/testcontainers/src/containers/DeFiDContainer.ts
@fuxingloh fuxingloh merged commit fdfb3d2 into main Apr 9, 2022
@fuxingloh fuxingloh deleted the jimmy/rpc-getPendingFutureSwaps branch April 9, 2022 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants