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

sendTokensToAddress RPC #323

Merged
merged 30 commits into from
Jun 22, 2021
Merged

sendTokensToAddress RPC #323

merged 30 commits into from
Jun 22, 2021

Conversation

siradji
Copy link
Contributor

@siradji siradji commented May 31, 2021

What kind of PR is this?:

/kind feature
What this PR does / why we need it:

Add sendTokensToAddress RPC
Which issue(s) does this PR fixes?:

Fixes part of #48
Additional comments?

@codeclimate
Copy link

codeclimate bot commented May 31, 2021

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

View more on Code Climate.

@github-actions
Copy link

github-actions bot commented May 31, 2021

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/jellyfish/dist/index.umd.js 20.48 KB (+0.56% 🔺) 410 ms (+0.56% 🔺) 168 ms (-0.8% 🔽) 578 ms

@siradji
Copy link
Contributor Author

siradji commented May 31, 2021

Behavior

Test fails when source address is provided but passes when an empty object is passed.

Fails at

  it('should create a transaction with source address provided', async () => {
    const to = await client.wallet.getNewAddress()
    const from = await client.wallet.getNewAddress()
    const transactionHex = await client.account.sendTokensToAddress({ [from]: '2@DFI' }, { [to]: '10@DBTC' })

    expect(typeof transactionHex).toStrictEqual('string')
  })

Error

image

@netlify
Copy link

netlify bot commented May 31, 2021

✔️ Deploy Preview for jellyfish-defi ready!

🔨 Explore the source changes: 1d81213

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

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

@fuxingloh fuxingloh changed the title Quick save sendTokensToAddress RPC May 31, 2021
@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #323 (4f66bb1) into main (116bcdb) will increase coverage by 0.15%.
The diff coverage is 99.42%.

❗ Current head 4f66bb1 differs from pull request most recent head 65b56b0. Consider uploading reports for the commit 65b56b0 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   97.06%   97.21%   +0.15%     
==========================================
  Files          92       94       +2     
  Lines        2589     2696     +107     
  Branches      329      346      +17     
==========================================
+ Hits         2513     2621     +108     
+ Misses         76       75       -1     
Impacted Files Coverage Δ
...ckages/jellyfish-api-core/src/category/poolpair.ts 100.00% <ø> (ø)
packages/jellyfish-transaction/src/index.ts 100.00% <ø> (ø)
...ckages/jellyfish-transaction/src/script/mapping.ts 96.66% <ø> (ø)
packages/jellyfish-transaction/src/tx_segwit.ts 100.00% <ø> (ø)
packages/jellyfish/src/index.ts 100.00% <ø> (ø)
...tcontainers/src/chains/reg_test_container/index.ts 100.00% <ø> (ø)
...kages/testcontainers/src/chains/defid_container.ts 93.06% <85.71%> (-0.69%) ⬇️
...ackages/jellyfish-api-core/src/category/account.ts 100.00% <100.00%> (ø)
...ages/jellyfish-api-core/src/category/blockchain.ts 100.00% <100.00%> (ø)
packages/jellyfish-api-core/src/category/oracle.ts 100.00% <100.00%> (ø)
... and 30 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 116bcdb...65b56b0. Read the comment docs.

@siradji siradji marked this pull request as ready for review June 1, 2021 16:21
@siradji siradji requested a review from thedoublejay as a code owner June 1, 2021 16:21
@fuxingloh
Copy link
Contributor

Take note of conflicts too.

@fuxingloh fuxingloh marked this pull request as draft June 3, 2021 01:39
@jingyi2811 jingyi2811 marked this pull request as ready for review June 17, 2021 04:27
jingyi2811
jingyi2811 previously approved these changes Jun 17, 2021
@siradji siradji requested a review from canonbrother June 17, 2021 12:33
@canonbrother
Copy link
Contributor

i'm good overall

Copy link
Contributor

@fuxingloh fuxingloh left a comment

Choose a reason for hiding this comment

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

  1. take note of small details
  2. improve on code testing, don't declare and use a global variable if you don't have to, use additional describe blocks if you need to.

@fuxingloh
Copy link
Contributor

refactor your test

@fuxingloh fuxingloh marked this pull request as draft June 21, 2021 03:33
@siradji siradji marked this pull request as ready for review June 21, 2021 22:14
@siradji siradji requested a review from fuxingloh June 21, 2021 22:14
Copy link
Contributor

@fuxingloh fuxingloh left a comment

Choose a reason for hiding this comment

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

thanks this looks great ❤️

@fuxingloh fuxingloh enabled auto-merge (squash) June 22, 2021 04:43
@fuxingloh fuxingloh requested a review from jingyi2811 June 22, 2021 04:43
@fuxingloh fuxingloh merged commit 3efc13e into main Jun 22, 2021
@fuxingloh fuxingloh deleted the siradji/rcp-sendTokensToAddress branch June 22, 2021 04:52
eli-lim pushed a commit that referenced this pull request Mar 27, 2022
* Added basic APR

* Fix client tests

* PR suggestions

* PR suggestions

* Fix code smell

* PR suggestions

* Update packages/whale-api-client/src/api/poolpairs.ts

* Add error for non dfi

* Refactor code smell
Refactor types to be cleaner by referencing instead of declaring
Error should not fail but return undefined
Should return fractional number not percentages
Updated ttl to 60 minutes

Co-authored-by: Fuxing Loh <[email protected]>
Co-authored-by: Fuxing Loh <[email protected]>
canonbrother pushed a commit that referenced this pull request Jun 1, 2022
* Added basic APR

* Fix client tests

* PR suggestions

* PR suggestions

* Fix code smell

* PR suggestions

* Update packages/whale-api-client/src/api/poolpairs.ts

* Add error for non dfi

* Refactor code smell
Refactor types to be cleaner by referencing instead of declaring
Error should not fail but return undefined
Should return fractional number not percentages
Updated ttl to 60 minutes

Co-authored-by: Fuxing Loh <[email protected]>
Co-authored-by: Fuxing Loh <[email protected]>
canonbrother pushed a commit that referenced this pull request Jun 1, 2022
* Added basic APR

* Fix client tests

* PR suggestions

* PR suggestions

* Fix code smell

* PR suggestions

* Update packages/whale-api-client/src/api/poolpairs.ts

* Add error for non dfi

* Refactor code smell
Refactor types to be cleaner by referencing instead of declaring
Error should not fail but return undefined
Should return fractional number not percentages
Updated ttl to 60 minutes

Co-authored-by: Fuxing Loh <[email protected]>
Co-authored-by: Fuxing Loh <[email protected]>
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.

5 participants