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

Some Infura RPC methods should be cached but are not #143

Open
mcmire opened this issue Sep 9, 2022 · 4 comments
Open

Some Infura RPC methods should be cached but are not #143

mcmire opened this issue Sep 9, 2022 · 4 comments

Comments

@mcmire
Copy link
Contributor

mcmire commented Sep 9, 2022

The following RPC methods take block parameters and yet are not covered in blockTagParamIndex and so are not being cached the same as other RPC methods which take block parameters:

  • eth_getBlockTransactionCountByNumber
  • eth_getTransactionByBlockNumberAndIndex
  • eth_getUncleByBlockNumberAndIndex
  • eth_getUncleCountByBlockNumber
@mcmire
Copy link
Contributor Author

mcmire commented Oct 3, 2022

Look into eth_getUncleByBlockNumberAndIndex and eth_getUncleCountByBlockNumber — how are these computed and are these cacheable?

@mcmire
Copy link
Contributor Author

mcmire commented Oct 3, 2022

I did some research and it looks like when a block is attested to be a part of the canonical chain, if there are any other competing blocks that should be marked as uncles, those are associated with the canonical block. For instance, see this block which became an uncle after this block became the canonical (the uncle is listed on the block page).

Interestingly, according to the Ethereum docs, uncles (ommers) are not possible in Ethereum 2.0. In fact, according to Etherscan, the last uncle was 18 days ago. So caching uncle-related RPC methods isn't going to be as important in the future as it is now.

In any case, in terms of whether these methods are safe to cache in the first place, my thought is that the possibility that a block that was once canonical then can be an uncle now if the network says so is a consequence of the overall ephemeralness of Ethereum that affects all kinds of blocks, not just uncle blocks. In other words, if we think that eth_getUncleByBlockNumberAndIndex is unreliable because the information this method returns may change depending on when we call it, I feel like the same thing would apply for eth_getBlockByNumber, because the block in question may change if the block gets replaced in the canonical chain. But it seems like in allowing eth_getBlockByNumber to be cached, we assume that this will never happen. So we ought to apply the same assumption to eth_getUncleByBlockNumberAndIndex and eth_getUncleCountByBlockNumber and go ahead and cache them.

Or, we ignore these methods and only cache eth_getBlockTransactionCountByNumber and eth_getTransactionByBlockNumberAndIndex, as it is increasingly likely that the uncle-related methods will no longer be used.

Thoughts?

@Gudahtt
Copy link
Member

Gudahtt commented Oct 4, 2022

Interesting; I hadn't realized this changed post-merge. I guess it's still an issue for PoW chains though.

Ideally we would detect chain reorganizations and refresh any affected caches, across the board. Until then, chain reorganizations might be very confusing.

In the meantime, I'm unsure whether caching these would make these situations more confusing or not. Maybe better to err on the side of caution and leave them uncached for now.

@mcmire
Copy link
Contributor Author

mcmire commented Oct 4, 2022

Okay, so I'll just focus on caching eth_getBlockTransactionCountByNumber and eth_getTransactionByBlockNumberAndIndex then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants