feat(simulationState): avoid frequent refetching of static data #503
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dd0c26970
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| token, | ||
| chainId, | ||
| blockTag, | ||
| blockNumber, | ||
| deployless, | ||
| account, |
There was a problem hiding this comment.
The query key now omits blockNumber/blockTag, so React Query will reuse cached token data across different historical blocks even when callers pass a block-specific read. That breaks per-block accuracy for dynamic token data such as wstETH’s stEthPerToken in fetchToken() (packages/blue-sdk-viem/src/fetch/Token.ts), and can yield stale wrap/unwrap conversions in simulations or historical snapshots. Please keep block-specific parameters in the key (or explicitly document and guard against dynamic tokens) to avoid cross-block cache pollution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Hmm, I think it’s indeed preferable to keep blockNumber and blockTag in the queryKey so we preserve the previous logic if it’s needed later.
| const feeRecipient = useReadContract({ | ||
| ...parameters, | ||
| blockNumber: block?.number, | ||
| address: morpho, | ||
| abi: blueAbi, | ||
| functionName: "feeRecipient", |
There was a problem hiding this comment.
The feeRecipient read no longer includes blockNumber, so when useSimulationState is given a historical block, the fee recipient is fetched at the latest chain head instead of that block. This can mis-assign accrued fees in historical simulations because feeRecipient is mutable (see setFeeRecipient in IMorpho). Consider restoring blockNumber: block?.number or documenting that feeRecipient is always “latest” and excluding it from historical simulations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is intentional in this case, but if it doesn’t align with the general vision of the useSimulationState function, let me know @morpho-org/sdk-engineers and we can copy this function into the consumer app with these modifications.
There was a problem hiding this comment.
You will now query these data at latest block all the time, this will at best have same RPC usage (it can even increase it)
This is the opposite of what you want to achieve
UPDATE:
It wouldn't be the case if you don't rely on wagmi automatic refetching (if you use blocks to refetch)
Some data can be considered static and does not need to be fetched at a specific block on a regular basis.
Quick win: This PR introduces a small optimization by treating token metadata and the feeRecipient address as static, avoiding unnecessary refetching every 5 seconds.