REST: add blockhash call, fetch blockhash by height #14353

jonasschnelli · 2018-09-29T19:57:54Z

Completes the REST interface for trivial block exploring by adding a call that allows to fetch the blockhash in the main chain by a given height.

src/rest.cpp

test/functional/interface_rest.py

jgarzik · 2018-09-30T01:03:40Z

concept ACK

src/rest.cpp

jimmysong

Thanks @jonasschnelli !

src/rest.cpp

promag · 2018-10-01T17:13:45Z

Tested ACK ae0a5a1.

What is the use case? Query block hash by height and then query the block? If so could overload /rest/block instead to avoid 2nd round trip?

jonasschnelli · 2018-10-01T19:01:58Z

@promag: the use case is exactly that (find a block by giving a height) (ex: https://bitcointools.jonasschnelli.ch/explorer/height/500000).

You could build it into /rest/block, but IMO we should keep it modular. I guess that is also the reason why there is a "getblock" and a "getblockhash" call on RPC.

gmaxwell · 2018-10-01T19:33:09Z

PR is mistitled, not a "blockheader" call. This PR doesn't really seem to indicate the benefit this provides to the user. Making multiple round trips also has bad performance, if the purpose really is just querying blocks by height this indirection seems like pretextual modularity.

jonasschnelli · 2018-10-01T20:02:13Z

Changed the title (was wrong, thank @gmaxwell)

I'm happy to add a per-height accessing to /rest/block, but this would mean accessing that new per-height-fetching would always return a block (optionally reduced to only contain txids) which would always access the disk.

Of course, we could then add it to /rest/headers/ as well so one would fetch a single blockheader (or range of headers) per height.

I haven't made a study about use cases but the extra roundtrip made sense to me (costs for http mem only retrieval roundtrips are usually tiny).

ch4ot1c · 2018-10-02T20:58:39Z

Ack, seems marginally beneficial

doc/REST-interface.md

Requires bitcoin/bitcoin#14353

promag · 2018-10-18T01:38:20Z

utACK, but fix first commit message.

DrahtBot · 2018-11-09T18:50:03Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#11770 ([REST] add a rest endpoint for estimatesmartfee, docs, and test by joemphilips)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

src/rest.cpp

luke-jr · 2018-12-20T02:59:33Z

See also #11765

jonasschnelli · 2019-01-03T22:05:42Z

Fixed @luke-jr point (invalid JSON respons)
Renamed from blockhash to blockhashbyheight.

laanwj · 2019-01-04T09:43:08Z

test/functional/interface_rest.py

nit: this is /blockhashbyheight now

laanwj · 2019-01-04T09:43:47Z

utACK (-nit), I'm surprised this didn't exist yet.

laanwj · 2019-01-04T09:48:45Z

doc/REST-interface.md

The help for RPC getblockhash is Returns hash of block in best-block-chain at height provided. I think that's slightly better terminology to use here than "main chain" which could be confused with testnet/mainnet/....

jonasschnelli · 2019-01-04T18:55:51Z

Fixed documentation and the test comment inconsistency (reported by @laanwj)

jnewbery

looks good. A few comments, mostly around better test coverage :)

test/functional/interface_rest.py

src/rest.cpp

test/functional/interface_rest.py

src/rest.cpp

jonasschnelli · 2019-01-19T06:11:11Z

Fixed points reported by @jnewbery (thanks for the review).

promag · 2019-01-20T10:28:36Z

@jonasschnelli commit messages should mention /rest/blockhashbyheight.

jonasschnelli · 2019-01-21T21:55:56Z

Fixed the commit message

jnewbery · 2019-01-22T22:19:51Z

utACK 42ff30e

promag · 2019-01-22T23:58:52Z

utACK 42ff30e.

42ff30e [Docs] add short documentation for /rest/blockhashbyheight (Jonas Schnelli) 579d418 [QA] add rest tests for /rest/blockhashbyheight/<HEIGHT>.<FORMAT> (Jonas Schnelli) eb9ef04 REST: add "blockhashbyheight" call, fetch blockhash by height (Jonas Schnelli) Pull request description: Completes the REST interface for trivial block exploring by adding a call that allows to fetch the blockhash in the main chain by a given height. Tree-SHA512: 94be9e56718f857279b11cc16dfa8d04f3b5a762e87ae54281b4d87247c71c844895f4944d5a47f09056bf851f4c4761ac4fbdbaaee957265d14de5c1c73e8d2

Summary: [QA] add rest tests for /rest/blockhashbyheight/<HEIGHT>.<FORMAT> [Docs] add short documentation for /rest/blockhashbyheight This is a backport of Core [[bitcoin/bitcoin#14353 | PR14353]] Test Plan: `ninja && ninja check-functional` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7681

42ff30e [Docs] add short documentation for /rest/blockhashbyheight (Jonas Schnelli) 579d418 [QA] add rest tests for /rest/blockhashbyheight/<HEIGHT>.<FORMAT> (Jonas Schnelli) eb9ef04 REST: add "blockhashbyheight" call, fetch blockhash by height (Jonas Schnelli) Pull request description: Completes the REST interface for trivial block exploring by adding a call that allows to fetch the blockhash in the main chain by a given height. Tree-SHA512: 94be9e56718f857279b11cc16dfa8d04f3b5a762e87ae54281b4d87247c71c844895f4944d5a47f09056bf851f4c4761ac4fbdbaaee957265d14de5c1c73e8d2

jonasschnelli added the RPC/REST/ZMQ label Sep 29, 2018

promag reviewed Sep 29, 2018

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

test/functional/interface_rest.py Outdated Show resolved Hide resolved

jgarzik reviewed Sep 30, 2018

View reviewed changes

test/functional/interface_rest.py Outdated Show resolved Hide resolved

jimmysong reviewed Sep 30, 2018

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from 1477ecb to ae0a5a1 Compare September 30, 2018 09:34

jimmysong approved these changes Sep 30, 2018

View reviewed changes

promag reviewed Sep 30, 2018

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

jonasschnelli changed the title ~~REST: add blockheader call, fetch blockhash by height~~ REST: add blockhash call, fetch blockhash by height Oct 1, 2018

ch4ot1c reviewed Oct 2, 2018

View reviewed changes

doc/REST-interface.md Outdated Show resolved Hide resolved

jonasschnelli added a commit to jonasschnelli/simple-block-explorer that referenced this pull request Oct 7, 2018

Add support for block height

5505b19

Requires bitcoin/bitcoin#14353

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from ae0a5a1 to 8e9cb6b Compare October 17, 2018 19:44

practicalswift mentioned this pull request Oct 22, 2018

Enable flake8 rule E225 which checks for missing whitespace around op… #14534

Closed

luke-jr reviewed Dec 20, 2018

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from 8e9cb6b to 3a93411 Compare January 3, 2019 22:04

laanwj reviewed Jan 4, 2019

View reviewed changes

test/functional/interface_rest.py Outdated

Copy link

Member

laanwj Jan 4, 2019

Choose a reason for hiding this comment

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

nit: this is /blockhashbyheight now

laanwj reviewed Jan 4, 2019

View reviewed changes

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from 3a93411 to d63921b Compare January 4, 2019 18:55

jnewbery reviewed Jan 18, 2019

View reviewed changes

test/functional/interface_rest.py Outdated Show resolved Hide resolved

src/rest.cpp Outdated Show resolved Hide resolved

test/functional/interface_rest.py Show resolved Hide resolved

src/rest.cpp Show resolved Hide resolved

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from d63921b to c8937f4 Compare January 19, 2019 06:10

jonasschnelli added 3 commits January 21, 2019 11:54

REST: add "blockhashbyheight" call, fetch blockhash by height

eb9ef04

[QA] add rest tests for /rest/blockhashbyheight/<HEIGHT>.<FORMAT>

579d418

[Docs] add short documentation for /rest/blockhashbyheight

42ff30e

jonasschnelli force-pushed the 2018/09/rest_blockhash branch from c8937f4 to 42ff30e Compare January 21, 2019 21:55

jonasschnelli merged commit 42ff30e into bitcoin:master Jan 23, 2019

adamjonas mentioned this pull request Oct 21, 2019

[REST] added blockhash api, tests and documentation #11765

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

REST: add blockhash call, fetch blockhash by height #14353

REST: add blockhash call, fetch blockhash by height #14353

Uh oh!

Conversation

jonasschnelli commented Sep 29, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jgarzik commented Sep 30, 2018

Uh oh!

Uh oh!

jimmysong left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

promag commented Oct 1, 2018

Uh oh!

jonasschnelli commented Oct 1, 2018

Uh oh!

gmaxwell commented Oct 1, 2018

Uh oh!

jonasschnelli commented Oct 1, 2018

Uh oh!

ch4ot1c commented Oct 2, 2018

Uh oh!

Uh oh!

promag commented Oct 18, 2018

Uh oh!

DrahtBot commented Nov 9, 2018

Conflicts

Uh oh!

Uh oh!

luke-jr commented Dec 20, 2018

Uh oh!

jonasschnelli commented Jan 3, 2019

Uh oh!

laanwj Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jan 4, 2019

Uh oh!

laanwj Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jan 4, 2019

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonasschnelli commented Jan 19, 2019

Uh oh!

promag commented Jan 20, 2019

Uh oh!

jonasschnelli commented Jan 21, 2019

Uh oh!

jnewbery commented Jan 22, 2019

Uh oh!

promag commented Jan 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants