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 the /headers[/:from_hash[/:count]] endpoint #41

Draft
wants to merge 1 commit into
base: new-index
Choose a base branch
from

Conversation

afilini
Copy link

@afilini afilini commented Nov 1, 2021

Opening this as a draft to gather some feedback. I'm pretty sure this won't compile on liquid right now, but I didn't want to invest too much time getting it to run in case there's no interest for this from your end.


Add a new endpoint to download headers in bulk, up to 2000 with a single
request.

Headers are returned in binary form, with the VarInt-encoded number of
items in the body preceeding them.

By default from_hash is the current best hash, but a different
starting block can be specified.

The returned list goes "backwards" returning the count - 1 blocks
before from_hash plus the header of from_hash itself. This allows
caching the response indefinitely, since it's guaranteed that the
headers that come before a given block will never change.

Returns an error if from_hash is not a valid block or it isn't found
in the blockchain.

If count is greater than the limit of 2000 it will be silently
capped to said value.

Copy link
Collaborator

@shesek shesek left a comment

Choose a reason for hiding this comment

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

Concept ACK, this looks very useful. Thanks for the contribution!

Added some review comments.

src/rest.rs Outdated Show resolved Hide resolved
src/rest.rs Outdated
(&Method::GET, Some(&"headers"), hash, count, None, None) => {
let count = count
.and_then(|c| c.parse::<usize>().ok())
.and_then(|c| match c {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about c.max(1).min(2000) instead?

src/rest.rs Outdated
raw.append(&mut encode::serialize(
&encode::VarInt(headers.len() as u64),
));
for header in headers.iter() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be better to consume the vector with into_iter.

None => return Err(HttpError::not_found("Block not found".to_string())),
};

let mut raw = Vec::with_capacity(8 + 80 * headers.len());
Copy link
Collaborator

Choose a reason for hiding this comment

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

The header size is different in elements. I think this should be the only difference there.

.unwrap()
.iter()
.rev()
.skip(self.best_height() - from_header.height())
Copy link
Collaborator

Choose a reason for hiding this comment

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

The best height might change between calling best_height() and starting the iteration itself. get_headers should grab one read lock and reuse it to ensure the state is consistent.

@afilini afilini force-pushed the feat/add-headers-download-endpoint branch from 755925d to 018c094 Compare December 10, 2021 11:25
Add a new endpoint to download headers in bulk, up to 2000 with a single
request.

Headers are returned in binary form, with the VarInt-encoded number of
items in the body preceeding them.

By default `from_hash` is the current best hash, but a different
starting block can be specified.

The returned list goes "backwards" returning the `count - 1` blocks
*before* `from_hash` plus the header of `from_hash` itself. This allows
caching the response indefinitely, since it's guaranteed that the
headers that come before a given block will never change.

Returns an error if `from_hash` is not a valid block or it isn't found
in the blockchain.

If `count` is greater than the limit of `2000` it will be silently
capped to said value.
@afilini afilini force-pushed the feat/add-headers-download-endpoint branch from 018c094 to bf26302 Compare December 10, 2021 11:28
junderw pushed a commit to junderw/electrs that referenced this pull request Dec 12, 2023
Include tx confirmation status in bulk /block/txs response
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.

2 participants