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

Daemon RPC: high_height_ok req boolean field /getblocks.bin #9382

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

j-berman
Copy link
Collaborator

Behavior before: when req.start_height > chain tip, response fails and the client is dropped and blocked

Behavior after: when req.high_height_ok && req.start_height > chain tip, the server returns a successful empty block response that includes the chain height

This is useful when firing parallel requests to scan the chain and the client doesn't initially know what the chain tip is. This avoids the client getting itself dropped when firing too high of a request.

The reason I implemented this new optional high_height_ok field rather than changing the default behavior to return a successful response when the request includes too high of a height is to maintain backwards compatibility with clients that perhaps currently rely on a failure response when requesting too high a height. Perhaps this is overkill and I could be convinced that making "high height ok" the server's default behavior (and not implementing the request field) is the better call.

Used in the Seraphis lib async scanner: UkoeHB#23

Behavior before: when start_height > chain tip, response fails

Behavior after: when req.high_height_ok is true && req.start_height
> chain tip, server rerturns a successful response that includes
chain height
Copy link
Contributor

@vtnerd vtnerd left a comment

Choose a reason for hiding this comment

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

This seems ok, but I'm not seeing why it would be needed by the new async scanner. Why won't the scanner know the top block height?

@j-berman
Copy link
Collaborator Author

E.g. you re-open your wallet and it's 3 blocks behind the chain tip. Client fires 10 parallel requests:

req 0 start height: chain tip - 3
req 1 start height: (chain tip - 3) + 20
req 2 start height: (chain tip - 3) + 40
...
req 9 start height: (chain tip - 3) + 180

Technically reqs 1-9 are unnecessary, but the client won't know this until it gets its first response. This approach also avoids the need to make a round trip to the server to request the current chain tip before starting to scan.

@selsta selsta added the daemon label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants