-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add simulations & wagmi hooks #69
Conversation
1c20476
to
dc5c4b6
Compare
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.
can't we let the user choose the method to use? either with multicall or with deployless
774c8a2
to
cf029fe
Compare
cf029fe
to
cf2ca37
Compare
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.
Since we already know that deployless fails for mainnet weth, wdyt about always skipping deployless with a set of address? It can prevent known rpc fails
Else everything looks good
…viem-deployless-reads
…ho-org/sdks into feat/blue-sdk-viem-deployless-reads
…ho-org/sdks into feat/blue-sdk-viem-deployless-reads
…viem-deployless-reads
…viem-deployless-reads
…ho-org/sdks into feat/blue-sdk-viem-deployless-reads
…ho-org/sdks into feat/blue-sdk-viem-deployless-reads
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.
I don't have reviewed logic yet but few remarks from your Thursday presentation:
blue-sdk-viem-simulation should not depend on react imo. Ex if we want to use it in liquidation SDK. A solution would be to have a dedicated package for the hook useSimulationState (which is ok I think)
- I think it's not necessary to have "viem" in the SDK name for a non primitive SDK like blue-sdk. underthehood, as soon as you want to use simulation, you use viem.
…viem-deployless-reads
Requests are faster because each of them is now independent (no need to fetch the config before fetching the rest of each market/vault). But deployless reads are not multicall-able, meaning they can't be batched in a single onchain RPC call via multicall.
This state is arguable and does not seems better out-of-the-box. However, here is why it is better:
So the only matter is the request count for RPC backends, which will be the cost paid by the RPC user in the end. It should stay relatively low considering that now, each entity costs at most 1 RPC request (either through the multicall = 0, or through a single deployless read = 1)
Fixes INTER-1016