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

node/http: get header by hash/height #210

Merged
merged 3 commits into from
May 1, 2020

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Jul 2, 2019

It can be useful to query for block headers directly without needing to query for full blocks.
Less disk i/o and less bandwidth would be needed when querying for just the block header.
It will also work for nodes in SPV mode.

This is a port of a PR to bcoin here https://github.com/bcoin-org/bcoin/pull/808/files

@codecov-io
Copy link

codecov-io commented Jul 2, 2019

Codecov Report

Merging #210 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #210      +/-   ##
=========================================
+ Coverage   52.99%   53.1%   +0.11%     
=========================================
  Files         129     129              
  Lines       35773   35782       +9     
  Branches     6032    6033       +1     
=========================================
+ Hits        18957   19003      +46     
+ Misses      16816   16779      -37
Impacted Files Coverage Δ
lib/node/http.js 52.86% <100%> (+3.56%) ⬆️
lib/covenants/rules.js 73.04% <0%> (-0.15%) ⬇️
lib/node/rpc.js 24.79% <0%> (+0.06%) ⬆️
lib/script/script.js 58% <0%> (+0.07%) ⬆️
lib/wallet/txdb.js 78.14% <0%> (+0.25%) ⬆️
lib/blockchain/chain.js 60.75% <0%> (+0.3%) ⬆️
lib/coins/coinview.js 76.08% <0%> (+0.54%) ⬆️
lib/blockchain/chaindb.js 64.58% <0%> (+0.85%) ⬆️
lib/blockchain/chainentry.js 59.74% <0%> (+1.29%) ⬆️
... and 1 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 cc1ef7a...77be96b. Read the comment docs.

@tynes
Copy link
Contributor Author

tynes commented Jul 8, 2019

Updated to match latest changes https://github.com/bcoin-org/bcoin/pull/808/files

@tynes tynes closed this Oct 15, 2019
@tynes tynes deleted the get-header-by-hash-or-height branch October 15, 2019 03:13
@tynes tynes restored the get-header-by-hash-or-height branch October 15, 2019 16:16
@tynes tynes reopened this Oct 15, 2019
@tynes
Copy link
Contributor Author

tynes commented Oct 15, 2019

Accidentally closed when pruning old git branches. Still open for review, but not critical to have merged now.

@tynes
Copy link
Contributor Author

tynes commented Apr 30, 2020

PR to docs here handshake-org/handshake-org.github.io#68


it('should fetch null for block header that does not exist', async () => {
// many blocks in the future
const header = await nclient.get(`/header/${40000}`);
Copy link
Member

Choose a reason for hiding this comment

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

Curious why is 40000 in braces?
This could be foolproof by getting chain tip height and adding any value > 1
But no change needed really.

Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

ACK, would be nice to have CLI update in hs-client as well.

@tynes
Copy link
Contributor Author

tynes commented May 1, 2020

ACK, would be nice to have CLI update in hs-client as well.

Here is the method added to hs-client. Do you think there should also be a command added like:

$ hsd-cli header <heightOrHash>

handshake-org/hs-client#28

@boymanjor boymanjor merged commit 361826f into handshake-org:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants