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

RPC: getBlockTime returns null for slot that hasn't been purged #10089

Closed
jstarry opened this issue May 17, 2020 · 11 comments · Fixed by #11955
Closed

RPC: getBlockTime returns null for slot that hasn't been purged #10089

jstarry opened this issue May 17, 2020 · 11 comments · Fixed by #11955
Milestone

Comments

@jstarry
Copy link
Member

jstarry commented May 17, 2020

Problem

Developers expect that getBlockTime will return a non-null response when the requested block is still available via getConfimedBlock. The reason it can return null is because we only keep the last 5 epochs of stake info and stake info is needed for calculating stake-weighted timestamp for a block.

Proposed Solution

Only prune epochs from epoch_stakes when the last slot in an epoch is purged from the blockstore.

@mvines mvines modified the milestones: The Future!, v1.3.0 May 17, 2020
@mvines
Copy link
Member

mvines commented May 20, 2020

This problem is quite visible on mainnet-beta too:

$ curl -X POST -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","id":1, "method":"minimumLedgerSlot"}' http://api.mainnet-beta.solana.com
{"jsonrpc":"2.0","result":5481065,"id":1}
$ solana block-time 5481065
Error: Block Not Found: slot=5481065

@mvines
Copy link
Member

mvines commented May 20, 2020

Alternatively/additionally consider adding the block-time estimate to the results from:

@mvines
Copy link
Member

mvines commented Jun 16, 2020

At the same time, consider adding a block-time column in blockstore to reduce the compute performed by the getBlockTime RPC endpoint

@mvines
Copy link
Member

mvines commented Jul 10, 2020

Once #10093 is fixed, there only needs to be a single estimated block production time as well

@mvines
Copy link
Member

mvines commented Jul 10, 2020

Once blockTime is fully added to getConfirmedBlock, the getBlockTime endpoint could be deprecated and de-documented.

@mvines
Copy link
Member

mvines commented Jul 10, 2020

This needs #10630 to land (cc: #10984 (review))

@CriesofCarrots
Copy link
Contributor

Unless we add block_time to the ConfirmedTransaction struct, I'm thinking we'll want to keep the getBlockTime endpoint, since the explorer uses it on all /tx pages. Seems like unnecessary overhead to pull the whole block for this purpose. Does this sound right, @jstarry ?

@jstarry
Copy link
Member Author

jstarry commented Sep 1, 2020

Yeah the current form of the confirmed block endpoint is pretty expensive. I wouldn't mind deprecating getBlockTime if we made sure to have a lighter weight confirmed block endpoint. It could exclude tx details and paginate transaction signatures via options

@zfedoran
Copy link

zfedoran commented Oct 6, 2021

Once blockTime is fully added to getConfirmedBlock, the getBlockTime endpoint could be deprecated and de-documented.

@mvines I apologize, I might not be up to date with all of the other issues you guys are solving for and this is an older issue so the conversation here might be outdated.

However, my team has a need for getting just the blockTime without any other details. We could migrate to getBlock, but it seems wasteful if all we're after is the time. Is there a recommendation to migrate to getBlock or are we okay to keep using the getBlockTime endpoint?

@mvines
Copy link
Member

mvines commented Oct 6, 2021

There are no current plans to deprecate getBlockTime so seems like you'd be just fine to continue using it

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any activity in past 7 days after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants