bugfix: Unique caches for simple_cache_middleware instances

docs/middleware.rst

@@ -31,7 +31,7 @@ are likely to change regularly, so this list may not include the latest version'
 You can find the latest defaults in the constructor in ``web3/manager.py``
 
 AttributeDict
-~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~
 
 .. py:method:: web3.middleware.attrdict_middleware
                web3.middleware.async_attrdict_middleware
@@ -185,7 +185,7 @@ the order of: ``[2, 1, 0]``.
 See "Internals: :ref:`internals__middlewares`" for a deeper dive to how middlewares work.
 
 Middleware Stack API
-~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~
 
 To add or remove items in different layers, use the following API:
 
@@ -282,7 +282,7 @@ To add or remove items in different layers, use the following API:
 
 
 Optional Middleware
------------------------
+-------------------
 
 Web3 ships with non-default middleware, for your custom use. In addition to the other ways of
 :ref:`Modifying_Middleware`, you can specify a list of middleware when initializing Web3, with:
@@ -325,27 +325,52 @@ Stalecheck
 
 
 Cache
-~~~~~~~~~~~
+~~~~~
 
-All of the caching middlewares accept these common arguments.
+Simple Cache Middleware
+'''''''''''''''''''''''
 
-* ``cache_class`` must be a callable which returns an object which implements the dictionary API.
-* ``rpc_whitelist`` must be an iterable, preferably a set, of the RPC methods that may be cached.
-* ``should_cache_fn`` must be a callable with the signature ``fn(method, params, response)`` which returns whether the response should be cached.
+.. py:method:: web3.middleware.construct_simple_cache_middleware(cache, rpc_whitelist, should_cache_fn)
+               web3.middleware.async_construct_simple_cache_middleware(cache, rpc_whitelist, should_cache_fn)
 
+    These simple cache constructor methods accept the following arguments:
 
-.. py:method:: web3.middleware.construct_simple_cache_middleware(cache_class, rpc_whitelist, should_cache_fn)
-               web3.middleware.async_construct_simple_cache_middleware(cache_class, rpc_whitelist, should_cache_fn)
+    :param cache: Must be an instance of the ``web3.utils.caching.SimpleCache`` class.
+        If a cache instance is not provided, a new instance will be created.
+    :param rpc_whitelist: Must be an iterable, preferably a set, of the RPC methods that
+        may be cached. A default list is used if none is provided.
+    :param should_cache_fn: Must be a callable with the signature
+        ``fn(method, params, response)`` which returns whether the response should
+        be cached.
 
     Constructs a middleware which will cache the return values for any RPC
     method in the ``rpc_whitelist``.
 
     Ready to use versions of this middleware can be found at
-    ``web3.middleware.simple_cache_middleware`` and ``web3.middleware.async_simple_cache_middleware``.
+    ``web3.middleware.simple_cache_middleware`` and
+    ``web3.middleware.async_simple_cache_middleware``. These are the equivalent of using
+    the constructor methods with the default arguments.
 
+Time-based Cache Middleware
+'''''''''''''''''''''''''''
 
 .. py:method:: web3.middleware.construct_time_based_cache_middleware(cache_class, cache_expire_seconds, rpc_whitelist, should_cache_fn)
 
+    The time-based cache constructor method accepts the following arguments:
+
+    :param cache_class: Must be a callable which returns an object which implements the
+        dictionary API.
+    :param rpc_whitelist: Must be an iterable, preferably a set, of the RPC methods that
+        may be cached. A default list is used if none is provided.
+    :param should_cache_fn: Must be a callable with the signature
+        ``fn(method, params, response)`` which returns whether the response should
+        be cached.
+
+    .. warning::
+      The ``cache_class`` argument is slated to change to the ``cache``
+      argument with ``web3.utils.caching.SimpleCache`` instance in web3.py ``v7``,
+      as is the current state of the simple cache middleware above.
+
     Constructs a middleware which will cache the return values for any RPC
     method in the ``rpc_whitelist`` for an amount of time defined by
     ``cache_expire_seconds``.
@@ -359,6 +384,21 @@ All of the caching middlewares accept these common arguments.
 
 .. py:method:: web3.middleware.construct_latest_block_based_cache_middleware(cache_class, average_block_time_sample_size, default_average_block_time, rpc_whitelist, should_cache_fn)
 
+    The latest-block-based cache constructor method accepts the following arguments:
+
+    :param cache_class: Must be a callable which returns an object which implements the
+        dictionary API.
+    :param rpc_whitelist: Must be an iterable, preferably a set, of the RPC methods that
+        may be cached. A default list is used if none is provided.
+    :param should_cache_fn: Must be a callable with the signature
+        ``fn(method, params, response)`` which returns whether the response should
+        be cached.
+
+    .. warning::
+      The ``cache_class`` argument is slated to change to the ``cache``
+      argument with ``web3.utils.caching.SimpleCache`` instance in web3.py ``v7``,
+      as is the current state of the simple cache middleware above.
+
     Constructs a middleware which will cache the return values for any RPC
     method in the ``rpc_whitelist`` for the latest block.
     It avoids re-fetching the current latest block for each

newsfragments/2979.bugfix.rst

@@ -0,0 +1 @@
+Properly create a fresh cache for each instance of ``simple_cache_middleware`` if no cache is provided. Fixes a bug when using this middleware with multiple instances of ``Web3``.

tests/core/middleware/test_simple_cache_middleware.py

@@ -11,9 +11,11 @@
     generate_cache_key,
 )
 from web3.middleware import (
+    async_simple_cache_middleware,
     construct_error_generator_middleware,
     construct_result_generator_middleware,
     construct_simple_cache_middleware,
+    simple_cache_middleware,
 )
 from web3.middleware.async_cache import (
     async_construct_simple_cache_middleware,
@@ -155,6 +157,40 @@ def test_simple_cache_middleware_does_not_cache_endpoints_not_in_whitelist(w3):
     assert result_a != result_b
 
 
+def test_simple_cache_middleware_does_not_share_state_between_providers():
+    result_generator_a = construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 11111}
+    )
+    result_generator_b = construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 22222}
+    )
+    result_generator_c = construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 33333}
+    )
+
+    w3_a = Web3(provider=BaseProvider(), middlewares=[result_generator_a])
+    w3_b = Web3(provider=BaseProvider(), middlewares=[result_generator_b])
+    w3_c = Web3(  # instantiate the Web3 instance with the cache middleware
+        provider=BaseProvider(),
+        middlewares=[
+            result_generator_c,
+            simple_cache_middleware,
+        ],
+    )
+
+    w3_a.middleware_onion.add(simple_cache_middleware)
+    w3_b.middleware_onion.add(simple_cache_middleware)
+
+    result_a = w3_a.manager.request_blocking("eth_chainId", [])
+    result_b = w3_b.manager.request_blocking("eth_chainId", [])
+    result_c = w3_c.manager.request_blocking("eth_chainId", [])
+
+    assert result_a != result_b != result_c
+    assert result_a == 11111
+    assert result_b == 22222
+    assert result_c == 33333
+
+
 # -- async -- #
 
 
@@ -275,3 +311,42 @@ async def test_async_simple_cache_middleware_does_not_cache_non_whitelist_endpoi
     result_b = await async_w3.manager.coro_request("not_whitelisted", [])
 
     assert result_a != result_b
+
+
+@pytest.mark.asyncio
+async def test_async_simple_cache_middleware_does_not_share_state_between_providers():
+    result_generator_a = await async_construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 11111}
+    )
+    result_generator_b = await async_construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 22222}
+    )
+    result_generator_c = await async_construct_result_generator_middleware(
+        {RPCEndpoint("eth_chainId"): lambda *_: 33333}
+    )
+
+    w3_a = AsyncWeb3(
+        provider=AsyncEthereumTesterProvider(), middlewares=[result_generator_a]
+    )
+    w3_b = AsyncWeb3(
+        provider=AsyncEthereumTesterProvider(), middlewares=[result_generator_b]
+    )
+    w3_c = AsyncWeb3(  # instantiate the Web3 instance with the cache middleware
+        provider=AsyncEthereumTesterProvider(),
+        middlewares=[
+            result_generator_c,
+            async_simple_cache_middleware,
+        ],
+    )
+
+    w3_a.middleware_onion.add(async_simple_cache_middleware)
+    w3_b.middleware_onion.add(async_simple_cache_middleware)
+
+    result_a = await w3_a.eth.chain_id
+    result_b = await w3_b.eth.chain_id
+    result_c = await w3_c.eth.chain_id
+
+    assert result_a != result_b != result_c
+    assert result_a == 11111
+    assert result_b == 22222
+    assert result_c == 33333

web3/middleware/async_cache.py

@@ -56,27 +56,33 @@ async def async_construct_simple_cache_middleware(
         ``response`` and returns a boolean as to whether the response should be
         cached.
     """
-    if cache is None:
-        cache = SimpleCache(256)
 
     async def async_simple_cache_middleware(
         make_request: Callable[[RPCEndpoint, Any], Any], _async_w3: "AsyncWeb3"
     ) -> AsyncMiddlewareCoroutine:
         lock = threading.Lock()
 
+        # It's not imperative that we define ``_cache`` here rather than in
+        # ``async_construct_simple_cache_middleware``. Due to the nature of async,
+        # construction is awaited and doesn't happen at import. This means separate
+        # instances would still get unique caches. However, to keep the code consistent
+        # with the synchronous version, and provide less ambiguity, we define it
+        # similarly to the synchronous version here.
+        _cache = cache if cache else SimpleCache(256)
+
         async def middleware(method: RPCEndpoint, params: Any) -> RPCResponse:
             if method in rpc_whitelist:
                 cache_key = generate_cache_key(
                     f"{threading.get_ident()}:{(method, params)}"
                 )
-                cached_request = cache.get_cache_entry(cache_key)
+                cached_request = _cache.get_cache_entry(cache_key)
                 if cached_request is not None:
                     return cached_request
 
                 response = await make_request(method, params)
                 if should_cache_fn(method, params, response):
                     async with async_lock(_async_request_thread_pool, lock):
-                        cache.cache(cache_key, response)
+                        _cache.cache(cache_key, response)
                 return response
             else:
                 return await make_request(method, params)

web3/middleware/cache.py

@@ -81,9 +81,6 @@ def construct_simple_cache_middleware(
         ``response`` and returns a boolean as to whether the response should be
         cached.
     """
-    if cache is None:
-        cache = SimpleCache(256)
-
     if rpc_whitelist is None:
         rpc_whitelist = SIMPLE_CACHE_RPC_WHITELIST
 
@@ -92,20 +89,25 @@ def simple_cache_middleware(
     ) -> Callable[[RPCEndpoint, Any], RPCResponse]:
         lock = threading.Lock()
 
+        # Setting the cache here, rather than in ``construct_simple_cache_middleware``,
+        # ensures that each instance of the middleware has its own cache. This is
+        # important for compatibility with multiple ``Web3`` instances.
+        _cache = cache if cache else SimpleCache(256)
+
         def middleware(method: RPCEndpoint, params: Any) -> RPCResponse:
             if method in rpc_whitelist:
                 cache_key = generate_cache_key(
                     f"{threading.get_ident()}:{(method, params)}"
                 )
-                cached_request = cache.get_cache_entry(cache_key)
+                cached_request = _cache.get_cache_entry(cache_key)
                 if cached_request is not None:
                     return cached_request
 
                 response = make_request(method, params)
                 if should_cache_fn(method, params, response):
                     if lock.acquire(blocking=False):
                         try:
-                            cache.cache(cache_key, response)
+                            _cache.cache(cache_key, response)
                         finally:
                             lock.release()
                 return response