search

MetaMask / eth-json-rpc-middleware

Ethereum middleware for composing an Ethereum provider using json-rpc-engine. Intended to replace provider-engine
ISC License
179 stars 88 forks source link

Response cache needs to be invalidated when `chainId` changes #138

Open lukehutch opened 2 years ago

lukehutch commented 2 years ago

If I use MetaMask through WalletConnect, then when I change the network in the wallet, the chainId is correctly updated in the provider, as is the RPC URL, but eth-json-rpc-middleware continues to return values from the old chainId, e.g. the result of getCode still refers to code deployed on the old network, not the new one that was just switched to.

eth_getCode is marked for caching as perma:

https://github.com/MetaMask/eth-json-rpc-middleware/blob/main/src/utils/cache.ts#L81

The cache does not check whether chainId has changed when evaluating whether the cache should be used.

https://github.com/MetaMask/eth-json-rpc-middleware/blob/main/src/block-cache.ts

The cache needs to be invalidated when chainId changes, otherwise stale values will be returned.

lukehutch commented 2 years ago

Actually the cause may be something different: The cache is invalidated before the block number returned by await blockTracker.getLatestBlock():

https://github.com/MetaMask/eth-json-rpc-middleware/blob/main/src/block-cache.ts#L188

However when the chainId changes, the block number changes too. You can see the problem in this screenshot, where I switched from Mainnet to Goerli in MetaMask:

image

latestBlockNumber == 0xe9f22b == 15331883 (which is the block number on Mainnet). This is the same number used as a key in blockCache.cache: blockCache.cache[15331883].

However, blockCache.cache[15331883].eth_blockNumber == 0x70d90b == 7395595 (which is the block number on Goerli).

So the Goerli block number (the response to eth_blockNumber) has been cached under the Mainnet block number.

When blockCache.clearBefore(latestBlockNumber) is given the latest block number from the wrong chain, the cache invalidation does not work as intended, depending on whether the chain switched to has a higher or a lower block number than the chain switched from.

https://github.com/MetaMask/eth-json-rpc-middleware/blob/main/src/block-cache.ts#L190

Another symptom of this problem that I have observed is warnings about large block number skews when the chain is switched:

network block skew detected; skipping block events (emitted=7395588 blockNumber15331879)

Really all caches should simply be invalidated when chainId changes.