Add `accountToAccount` RPC #284

jingyi2811 · 2021-05-24T12:18:23Z

What kind of PR is this?:

/kind feature

What this PR does / why we need it:

Add accounttoaccount rpc and assertion to prevent sending DFI in account form

Which issue(s) does this PR fixes?:

Fixes #228

Additional comments?:

defichain-bot · 2021-05-24T12:18:38Z

@jingyi2811: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.

/kind feature
/kind fix
/kind chore
/kind docs
/kind refactor
/kind dependencies

Details

I am a bot created to help the DeFiCh developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the DeFiCh/oss-governance-bot repository.

codeclimate · 2021-05-24T12:18:43Z

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

View more on Code Climate.

netlify · 2021-05-24T12:19:06Z

✔️ Deploy Preview for jellyfish-defi ready!

🔨 Explore the source changes: ae29d49

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

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

github-actions · 2021-05-24T12:22:25Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/jellyfish/dist/jellyfish.umd.js	19.51 KB (+0.18% 🔺)	391 ms (+0.18% 🔺)	169 ms (+2.54% 🔺)	559 ms

packages/jellyfish-api-core/__tests__/category/account.test.ts

packages/jellyfish-api-core/src/category/account.ts

packages/jellyfish-api-core/__tests__/category/account.test.ts

codecov · 2021-05-24T13:54:42Z

Codecov Report

Merging #284 (cd9982e) into main (7f1e36d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   96.87%   96.92%   +0.05%     
==========================================
  Files          88       89       +1     
  Lines        2366     2410      +44     
  Branches      303      309       +6     
==========================================
+ Hits         2292     2336      +44     
  Misses         74       74

Impacted Files	Coverage Δ
...ackages/jellyfish-api-core/src/category/account.ts	`100.00% <100.00%> (ø)`
...ages/jellyfish-api-core/src/category/blockchain.ts	`100.00% <0.00%> (ø)`
...ish-transaction/src/script/defi/dftx_masternode.ts	`100.00% <0.00%> (ø)`
...ellyfish-transaction/src/buffer/buffer_composer.ts	`99.46% <0.00%> (+0.03%)`	⬆️
...ages/jellyfish-transaction/src/script/defi/dftx.ts	`97.01% <0.00%> (+0.24%)`	⬆️
...ckages/jellyfish-transaction/src/script/mapping.ts	`96.20% <0.00%> (+0.25%)`	⬆️

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 7f1e36d...cd9982e. Read the comment docs.

packages/jellyfish-api-core/src/category/account.ts

canonbrother

minor naming issues and desc write-up

jingyi2811 · 2021-05-25T12:46:36Z

Look like one of the unit tests fails occasionally. I am now Investigating it.

jingyi2811 · 2021-05-25T17:46:52Z

Below is my finding

After adding await container.call('utxostoaccount', [{ [address]: '100@0' }])

Line 233

const account = await client.account.getAccount(accounts[0].owner.addresses[0], pagination)
expect(account.length).toStrictEqual(1)

sometimes returns 0. (Expect to get 1)

[Fail scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 200.00000000@DFI
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 177.00000000@DBTC
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 154.00000000@DETH
2N5a9Pq7uDLkHYzDgPK5geCfb4VLR45CUsQ 23.00000000@DBTC
2NBb5ShtxDmkzTvmnmgyL3uCvW7Jcu6PBir 46.00000000@DETH

const account = await client.account.getAccount(2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G,  {start: 3,
including_start: true})

The account length will be 0

[Success scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR 46.00000000@DETH
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 200.00000000@DFI
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 177.00000000@DBTC
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 154.00000000@DETH
2N5ibMXpwieA1hAi5k9WsuEMadzsWdaNjSh 23.00000000@DBTC

const account = await client.account.getAccount(2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR,  {start: 1,
including_start: true})

The account length will be 1

@canonbrother Maybe could you help me to check why this could happen. As you are the one who works on this RPC.

fuxingloh · 2021-05-26T11:05:49Z

Below is my finding

After adding await container.call('utxostoaccount', [{ [address]: '100@0' }])

Line 247
const account = await client.account.getAccount(accounts[0].owner.addresses[0], pagination)
expect(account.length).toStrictEqual(1)
sometimes returns 0. (Expect to get 1)

[Fail scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 200.00000000@DFI
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 177.00000000@DBTC
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 154.00000000@DETH
2N5a9Pq7uDLkHYzDgPK5geCfb4VLR45CUsQ 23.00000000@DBTC
2NBb5ShtxDmkzTvmnmgyL3uCvW7Jcu6PBir 46.00000000@DETH
const account = await client.account.getAccount(2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G,  {start: 3,
including_start: true})
The account length will be 0

[Success scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR 46.00000000@DETH
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 200.00000000@DFI
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 177.00000000@DBTC
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 154.00000000@DETH
2N5ibMXpwieA1hAi5k9WsuEMadzsWdaNjSh 23.00000000@DBTC
const account = await client.account.getAccount(2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR,  {start: 1,
including_start: true})
The account length will be 1

@canonbrother Maybe could you help me to check why this could happen. As you are the one who works on this RPC.

Better to quote a code for context, hard to understand sometimes.

canonbrother · 2021-05-27T02:02:50Z

Below is my finding

After adding await container.call('utxostoaccount', [{ [address]: '100@0' }])

Line 233
const account = await client.account.getAccount(accounts[0].owner.addresses[0], pagination)
expect(account.length).toStrictEqual(1)
sometimes returns 0. (Expect to get 1)

[Fail scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 200.00000000@DFI
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 177.00000000@DBTC
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 154.00000000@DETH
2N5a9Pq7uDLkHYzDgPK5geCfb4VLR45CUsQ 23.00000000@DBTC
2NBb5ShtxDmkzTvmnmgyL3uCvW7Jcu6PBir 46.00000000@DETH
const account = await client.account.getAccount(2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G,  {start: 3,
including_start: true})
The account length will be 0

[Success scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]

2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR 46.00000000@DETH
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 200.00000000@DFI
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 177.00000000@DBTC
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 154.00000000@DETH
2N5ibMXpwieA1hAi5k9WsuEMadzsWdaNjSh 23.00000000@DBTC
const account = await client.account.getAccount(2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR,  {start: 1,
including_start: true})
The account length will be 1

@canonbrother Maybe could you help me to check why this could happen. As you are the one who works on this RPC.

nothing wrong
you have to know how getaccount work

jingyi2811 · 2021-05-27T03:39:36Z

Below is my finding
After adding await container.call('utxostoaccount', [{ [address]: '100@0' }])
Line 233
const account = await client.account.getAccount(accounts[0].owner.addresses[0], pagination)
expect(account.length).toStrictEqual(1)
sometimes returns 0. (Expect to get 1)
[Fail scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 200.00000000@DFI
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 177.00000000@DBTC
2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G 154.00000000@DETH
2N5a9Pq7uDLkHYzDgPK5geCfb4VLR45CUsQ 23.00000000@DBTC
2NBb5ShtxDmkzTvmnmgyL3uCvW7Jcu6PBir 46.00000000@DETH
const account = await client.account.getAccount(2N3asj6NyoRGSx9Niu9qeAfJL7Ymu1R1b6G,  {start: 3,
including_start: true})
The account length will be 0
[Success scenario]
if the accounts are arranged in this order [Retrieving from await client.account.listAccounts()]
2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR 46.00000000@DETH
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 200.00000000@DFI
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 177.00000000@DBTC
2N3RAgU3hBUEHUpJwxNxQYP38hUh84KaQip 154.00000000@DETH
2N5ibMXpwieA1hAi5k9WsuEMadzsWdaNjSh 23.00000000@DBTC
const account = await client.account.getAccount(2MwXSFp9jp5JtzzJ8J2GYJMRbPx6rjk5jrR,  {start: 1,
including_start: true})
The account length will be 1
@canonbrother Maybe could you help me to check why this could happen. As you are the one who works on this RPC.
nothing wrong
you have to know how getaccount work

Nevermind, this will be fixed in #295

jingyi2811 · 2021-05-27T04:25:16Z

Sorry, I need to refactor accountToAccount rpc test file into a separate folder in this PR. Doing it now.

jingyi2811 · 2021-05-27T04:51:03Z

I have separated out the code. See accountToAccount.test.ts.
Probably it is not the minimum setup. But I will optimize the code in another PR.

There is minimum code refactoring in the existing account.test.ts. Added a test. There are no changes to the existing logic.

packages/jellyfish-api-core/__tests__/category/account/accountToAccount.test.ts

packages/jellyfish-api-core/src/category/account.ts

jingyi2811 added 5 commits May 20, 2021 13:54

Add accounttoaccount rpc

115b167

Improve accountToAccount rpc unit test

6314953

Improve code

a9aa812

Add JS Doc

66a77da

Minor changes

13a8fb2

jingyi2811 requested review from canonbrother, fuxingloh and thedoublejay as code owners May 24, 2021 12:18

jingyi2811 marked this pull request as draft May 24, 2021 12:18

defichain-bot added area/jellyfish-api-core labels May 24, 2021

defichain-bot added the needs/kind Needs kind label label May 24, 2021

defichain-bot assigned jingyi2811 May 24, 2021

jingyi2811 changed the title ~~Jimmy/accounttoaccount~~ Add accounttoaccount rpc May 24, 2021

jingyi2811 commented May 24, 2021

View reviewed changes

packages/jellyfish-api-core/__tests__/category/account.test.ts Outdated Show resolved Hide resolved

jingyi2811 commented May 24, 2021

View reviewed changes

packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved

jingyi2811 commented May 24, 2021

View reviewed changes

packages/jellyfish-api-core/__tests__/category/account.test.ts Outdated Show resolved Hide resolved

jingyi2811 added 2 commits May 24, 2021 20:32

Refactor code

f00eed7

Refactor code

e1ce81d

jingyi2811 changed the title ~~Add accounttoaccount rpc~~ Add accounttoaccount rpc and assertion to prevent sending DFI in account form May 24, 2021

Fix unit test and typo

3556100

jingyi2811 commented May 24, 2021

View reviewed changes

packages/jellyfish-api-core/__tests__/category/account.test.ts Outdated Show resolved Hide resolved

Rearrange import

775e829

jingyi2811 mentioned this pull request May 24, 2021

DeFi Blockchain RPC Implementations #48

Open

Merge branch 'main' into jimmy/accounttoaccount

db494e0

canonbrother reviewed May 25, 2021

View reviewed changes

packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved

canonbrother suggested changes May 25, 2021

View reviewed changes

Apply suggestions from code review

80db4d5

jingyi2811 requested review from canonbrother and thedoublejay May 25, 2021 11:45

jingyi2811 added 3 commits May 27, 2021 11:10

Apply suggestions from code review

2d19327

Merge branch 'main' into jimmy/accounttoaccount

0c00acc

Merge branch 'main' into jimmy/accounttoaccount

ae29d49

jingyi2811 marked this pull request as ready for review May 27, 2021 04:22

jingyi2811 marked this pull request as draft May 27, 2021 04:24

Refactor code

382531b

jingyi2811 marked this pull request as ready for review May 27, 2021 04:53

canonbrother approved these changes May 27, 2021

View reviewed changes

thedoublejay requested changes May 27, 2021

View reviewed changes

packages/jellyfish-api-core/__tests__/category/account/accountToAccount.test.ts Outdated Show resolved Hide resolved

canonbrother suggested changes May 27, 2021

View reviewed changes

packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved

jingyi2811 added 2 commits May 27, 2021 13:29

Apply suggestions from code review

4770983

Apply suggestions from code review

cd9982e

thedoublejay approved these changes May 27, 2021

View reviewed changes

canonbrother approved these changes May 27, 2021

View reviewed changes

fuxingloh approved these changes May 28, 2021

View reviewed changes

fuxingloh merged commit 3f9138a into main May 28, 2021

fuxingloh deleted the jimmy/accounttoaccount branch May 28, 2021 05:32

canonbrother pushed a commit that referenced this pull request Jun 1, 2022

Bump @defichain dependencies (#284)

e4dab1a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `accountToAccount` RPC #284

Add `accountToAccount` RPC #284

jingyi2811 commented May 24, 2021 •

edited

Loading

defichain-bot commented May 24, 2021

codeclimate bot commented May 24, 2021 •

edited

Loading

netlify bot commented May 24, 2021 •

edited

Loading

github-actions bot commented May 24, 2021 •

edited

Loading

codecov bot commented May 24, 2021 •

edited

Loading

canonbrother left a comment

jingyi2811 commented May 25, 2021

jingyi2811 commented May 25, 2021 •

edited

Loading

fuxingloh commented May 26, 2021

canonbrother commented May 27, 2021

jingyi2811 commented May 27, 2021

jingyi2811 commented May 27, 2021 •

edited

Loading

jingyi2811 commented May 27, 2021 •

edited

Loading

Add accountToAccount RPC #284

Add accountToAccount RPC #284

Conversation

jingyi2811 commented May 24, 2021 • edited Loading

What kind of PR is this?:

What this PR does / why we need it:

Which issue(s) does this PR fixes?:

Additional comments?:

defichain-bot commented May 24, 2021

codeclimate bot commented May 24, 2021 • edited Loading

netlify bot commented May 24, 2021 • edited Loading

github-actions bot commented May 24, 2021 • edited Loading

size-limit report 📦

codecov bot commented May 24, 2021 • edited Loading

Codecov Report

canonbrother left a comment

Choose a reason for hiding this comment

jingyi2811 commented May 25, 2021

jingyi2811 commented May 25, 2021 • edited Loading

fuxingloh commented May 26, 2021

canonbrother commented May 27, 2021

jingyi2811 commented May 27, 2021

jingyi2811 commented May 27, 2021 • edited Loading

jingyi2811 commented May 27, 2021 • edited Loading

Add `accountToAccount` RPC #284

Add `accountToAccount` RPC #284

jingyi2811 commented May 24, 2021 •

edited

Loading

codeclimate bot commented May 24, 2021 •

edited

Loading

netlify bot commented May 24, 2021 •

edited

Loading

github-actions bot commented May 24, 2021 •

edited

Loading

codecov bot commented May 24, 2021 •

edited

Loading

jingyi2811 commented May 25, 2021 •

edited

Loading

jingyi2811 commented May 27, 2021 •

edited

Loading

jingyi2811 commented May 27, 2021 •

edited

Loading