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

Added getBalances RPC #362

Merged
merged 24 commits into from
Jun 23, 2021
Merged

Added getBalances RPC #362

merged 24 commits into from
Jun 23, 2021

Conversation

aikchun
Copy link
Contributor

@aikchun aikchun commented Jun 9, 2021

What kind of PR is this?:

/kind feature

What this PR does / why we need it:

Implemented RPC for #48

  1. wallet.getbalances

Which issue(s) does this PR fixes?:

Additional comments?:

@codeclimate
Copy link

codeclimate bot commented Jun 9, 2021

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

View more on Code Climate.

@netlify
Copy link

netlify bot commented Jun 9, 2021

✔️ Deploy Preview for jellyfish-defi ready!

🔨 Explore the source changes: b999820

🔍 Inspect the deploy log: https://app.netlify.com/sites/jellyfish-defi/deploys/60d1e3d7c6dfe30007c144d6

😎 Browse the preview: https://deploy-preview-362--jellyfish-defi.netlify.app

@github-actions
Copy link

github-actions bot commented Jun 9, 2021

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/jellyfish/dist/index.umd.js 20.51 KB (+0.16% 🔺) 411 ms (+0.16% 🔺) 152 ms (+2.87% 🔺) 563 ms

@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #362 (b999820) into main (d26d5e1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #362   +/-   ##
=======================================
  Coverage   97.27%   97.27%           
=======================================
  Files          94       94           
  Lines        2750     2752    +2     
  Branches      346      346           
=======================================
+ Hits         2675     2677    +2     
  Misses         75       75           
Impacted Files Coverage Δ
packages/jellyfish-api-core/src/category/wallet.ts 100.00% <100.00%> (ø)

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 d26d5e1...b999820. Read the comment docs.

@aikchun aikchun force-pushed the aikchun/getbalances branch 2 times, most recently from 27bb821 to 270ef03 Compare June 11, 2021 03:02
@canonbrother canonbrother marked this pull request as draft June 11, 2021 05:10
@aikchun aikchun force-pushed the aikchun/getbalances branch from 5b0d10a to c6283bc Compare June 12, 2021 08:19
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.

missing withTokens = true test and minor refactors

@aikchun
Copy link
Contributor Author

aikchun commented Jun 15, 2021

There's a strange behavior regarding the balances when withTokens is set to true.

balances.mine.trusted and balances.mine.untrusted_pending are objects, when console.log, looks like:

{ '0': BigNumber {s: 1, e: 3, c: [20000036] } }

But they can't be referenced like a normal object nor are they instances of BigNumber

Not too sure if this is the intended behavior. If it is, could require some insight on how to test this.

@fuxingloh

@fuxingloh
Copy link
Contributor

There's a strange behavior regarding the balances when withTokens is set to true.

balances.mine.trusted and balances.mine.untrusted_pending are objects, when console.log, looks like:

{ '0': BigNumber {s: 1, e: 3, c: [20000036] } }

But they can't be referenced like a normal object nor are they instances of BigNumber

Not too sure if this is the intended behavior. If it is, could require some insight on how to test this.

@fuxingloh

0x00 aka 0 stands for DFI in token form also.

aikchun added 23 commits June 22, 2021 21:21
…hOnlyBalance to WalletBalances, WalletMineBalances and WalletWatchOnlyBalances respectively.
@aikchun aikchun force-pushed the aikchun/getbalances branch from 1079fb5 to b999820 Compare June 22, 2021 13:21
@aikchun aikchun marked this pull request as ready for review June 22, 2021 14:02
@fuxingloh fuxingloh merged commit baf4782 into main Jun 23, 2021
@fuxingloh fuxingloh deleted the aikchun/getbalances branch June 23, 2021 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants