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

Add new getBlockHeader rpc #163

Merged
merged 2 commits into from
Apr 27, 2021
Merged

Add new getBlockHeader rpc #163

merged 2 commits into from
Apr 27, 2021

Conversation

jingyi2811
Copy link
Contributor

/kind feature
Add new getBlockHeader rpc

@netlify
Copy link

netlify bot commented Apr 27, 2021

Deploy preview for jellyfish-defi ready!

Built with commit 003e4aa

https://deploy-preview-163--jellyfish-defi.netlify.app

@github-actions
Copy link

github-actions bot commented Apr 27, 2021

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/jellyfish/dist/jellyfish.umd.js 18.43 KB (+0.08% 🔺) 369 ms (+0.08% 🔺) 174 ms (+0.91% 🔺) 542 ms

@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #163 (003e4aa) into main (365cb35) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #163   +/-   ##
=======================================
  Coverage   95.47%   95.47%           
=======================================
  Files          52       52           
  Lines        1105     1106    +1     
  Branches      137      137           
=======================================
+ Hits         1055     1056    +1     
  Misses         48       48           
  Partials        2        2           
Impacted Files Coverage Δ
...ages/jellyfish-api-core/src/category/blockchain.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 365cb35...003e4aa. Read the comment docs.

interface blockchain {
getBlockHeader (hash: string, verbosity: true): Promise<BlockHeader>
getBlockHeader (hash: string, verbosity: false): Promise<string>

Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty line


/**
* Get block header data with particular header hash.
* Returns an Object with information about block header.
Copy link
Contributor

Choose a reason for hiding this comment

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

Returns a string that is serialized, hex-encoded data for blockheader.

it('should getBlockHeader with verbosity false and return just a string that is serialized, hex-encoded data for block', async () => {
const blockHash = await waitForBlockHash(1)
const hash: string = await client.blockchain.getBlockHeader(blockHash, false)
expect(hash).not.toBeNull()
Copy link
Contributor

Choose a reason for hiding this comment

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

assert test this is a string.

Comment on lines 161 to 172
/**
* Wait for block hash to reach a certain height
*/

async function waitForBlockHash (height: number): Promise<string> {
await waitForExpect(async () => {
const info = await client.blockchain.getBlockchainInfo()
expect(info.blocks).toBeGreaterThan(height)
})

return await client.blockchain.getBlockHash(height)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing that everyone is using this utility for waitForBlockHash, you should refactor and move it towards the top level and remove all duplicates.

@fuxingloh fuxingloh merged commit 8489cb1 into main Apr 27, 2021
@fuxingloh fuxingloh deleted the getBlockHeader branch April 27, 2021 11:33
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.

3 participants