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

feat(rpc): impl uncle count handlers #1511

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Feb 22, 2023

impls

  • eth_getUncleCountByBlockHash
  • eth_getUncleCountByBlockNumber

change return type to option. should return null for not found

ref #1225

@mattsse mattsse requested a review from rkrasiuk February 22, 2023 14:42
@mattsse mattsse added C-enhancement New feature or request A-rpc Related to the RPC implementation labels Feb 22, 2023
block_id: impl Into<BlockId>,
) -> EthResult<Option<Vec<reth_primitives::Header>>> {
let block_id = block_id.into();
Ok(self.client().ommers(block_id)?)
Copy link
Member

Choose a reason for hiding this comment

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

Starting to think we should have the API operate an LRU cache of all of these historical queries, so we avoid going to the db

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we have something like LRU per table?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if we add this to the Database/StateProvider directly, we'd need a sync primitive for the cache which could do more harm than good.

for RPC, I was thinking about adding a Task that has an LRU but communicates via a channel.

So the workflow would be something like:

  1. RPC request comes in
  2. needs block x
  3. sends Command to fetch block x to the cache task
  4. cache task looks up LRU or fetches from DB sends response back

@gakonst gakonst merged commit 17dffba into main Feb 22, 2023
@gakonst gakonst deleted the matt/add-uncle-count-impls branch February 22, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants