-
Notifications
You must be signed in to change notification settings - Fork 274
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
The return of simple_cache_middleware #3436
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.
🎸 .
Not sure if there is a test we can add, but looks good nonetheless.
@@ -259,6 +259,9 @@ def attach_middleware(self): | |||
self.log.debug('Injecting POA middleware at layer 0') | |||
self.client.inject_middleware(geth_poa_middleware, layer=0) | |||
|
|||
self.log.debug("Adding simple_cache_middleware") | |||
self.client.add_middleware(simple_cache_middleware) |
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.
Welcome back old friend.
Yeah, I thought about that too, but we'd be testing web3py. There's also no middlewares at the current TesterBlockchain level. |
Type of PR:
Required reviews:
What this does:
simple_cache_middleware
Issues fixed/closed:
Why it's needed:
Currently, TACo nodes exhibit a relatively high RPC requests consumption (see #3370). This is just a small improvement, mainly reducing repeated calls to
eth_chainId
. For some reason, with v7.1.0 I've seen ~5,000 of these per day.Notes for reviewers:
We had to temporarily remove
simple_cache_middleware
when @KPrasch and I detected that it shared state between multiple Web3 instances (see ethereum/web3.py#2977 ). This was fixed by ethereum/web3.py#2979, and released in v6.5.0 (https://web3py.readthedocs.io/en/stable/releases.html#web3-py-v6-5-0-2023-06-15). We currently pin v6.14.0.