-
Notifications
You must be signed in to change notification settings - Fork 700
Added /v3/tenures/blocks/{consensus_hash} endpoint #6431
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
base: develop
Are you sure you want to change the base?
Added /v3/tenures/blocks/{consensus_hash} endpoint #6431
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are missing the update to the openapi spec
Note: i rollbacked the BurnHeaderHash usage as it serializes to an array of numbers by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; just a couple nits and then I'll approve
|
||
/// Stream the json block for the next block. | ||
/// Stops on non-existent block or on a block in a different tenure | ||
pub fn next_block(&mut self) -> Result<Vec<u8>, String> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for chiming in late after you’ve already done the work 🙏.
Not saying we should change this right now, but I was wondering if a recursive query could be more performant than doing ~120 queries per endpoint call.
I haven’t even tested it and is missing some of the fields (and only for nakamoto), but here’s a quick sketch of what it might look like:
WITH RECURSIVE tenure_chain AS (
SELECT
h.index_block_hash,
h.parent_block_id,
h.consensus_hash,
FROM nakamoto_block_headers h
WHERE h.index_block_hash = ?1
AND h.consensus_hash = ?2
UNION ALL
SELECT
h.index_block_hash,
h.index_block_hash,
h.consensus_hash,
FROM nakamoto_block_headers h
JOIN tenure_chain tc ON h.index_block_hash = tc.parent_block_id
WHERE h.consensus_hash = ?2
)
SELECT *
FROM tenure_chain
ORDER BY block_height DESC;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal was to "split" the I/O (both db and socket) in multiple operations. With this approach we are doing all in a single "blocking" operation. @jcnelson any thought? Eventually changing that part is not a big deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with both approaches, the reasoning behind the split makes sense as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waiting for the tests to wrap up before approving, but LGTM!
Description
This patch adds the first (of 3) new endpoints allowing getting the list of stacks blocks given a burnblock (here by consensus hash, next endpoints will cover height and block hash).
It is part (along the simulation block api #6346) of the features required by wormhole.