-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactoring single source of truth for dapp state #510
Merged
itaven
merged 62 commits into
develop
from
feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
Nov 4, 2024
Merged
Refactoring single source of truth for dapp state #510
itaven
merged 62 commits into
develop
from
feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
Nov 4, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
Jeday
changed the title
Refactoring single source of truth
Refactoring single source of truth for dapp state
Oct 16, 2024
Preview stand statusStand was demolished |
…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
modules/web3/web3-provider/connect-wallet-modal/connect-wallet-modal.tsx
Outdated
Show resolved
Hide resolved
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
solidovic
reviewed
Oct 18, 2024
…ch-debtimprovement refactor: l2 tech debt improvement
solidovic
previously approved these changes
Oct 30, 2024
…-of-truth-for-chainid-and-rpc Signed-off-by: Evgeny Taktarov <[email protected]>
solidovic
previously approved these changes
Oct 30, 2024
…-of-truth-for-chainid-and-rpc Signed-off-by: Anton Shalimov <[email protected]>
solidovic
approved these changes
Nov 4, 2024
DiRaiks
approved these changes
Nov 4, 2024
itaven
deleted the
feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc
branch
November 4, 2024 12:10
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Also #524
problem is SUPPORTED_CHAINS inlcude L2 chains but other pages only support ethereum chains and not l2 chains
Demo
Code review notes
Web3 Module
Now we have
modules/web3
. This is the first module in ESW so I will explain how and what.It will contain all hooks/utils/providers needed to enable web3 in our app. Slowly migrating code from old-sdk/ethers in
shared/
to new-sdk/wagmi/viem inmodules/web3
.Main thing is that module(and sub-modules inside) only export and expose to other code bare minimum e.g.
Web3Provider
handles everything inside itself (new sdk, legacy sdk, reef-knot) and only exposes itself anduseLidoSDK
to outside code.If you find yourself using deep import liked
modules/web3/something/else
, either this is an util that should be exposed OR the thing that using it should itself migrate intomodules/web3
.Old sdk used to be setup in async/hacky way and it actually covered out problem with LL connector and wagmi. Now derives it state from wagmi and new sdk. And when sees L2 chain, acts like wallet is not connected and we are on default chain. This prevents accidental L1 tx interaction.
useDappStatus
This hooks will be slowly consuming all state/flag related stuff that we have. It exposes all things needed to know out dapp state: current chainId, is user wallet chain supported on this page, is wallet connected and etc.
This is better than having to rely directly on wagmi hooks, which are confusing and could experience changes with version update.
Example:
useChainId
returns chainId that in wagmi supported chains(but could an l2 chain and is not supported on stake) anduseAccount
returns chainId that in user wallet, which could be any chain at all and if passed to our code will break invaraints.Chain Type
So with introduction of optimism to widget two main problems arose:
mainnet
chains with each set oftestnet
that should act independentlyThis made our lives very hard and code very complex.
So
chainType
is used to distinguish between those chains, expectingEthereum
(mainnet + testnets) andOptimism
(mainnet+testnet) to act independently.I used a cool trick with default context value, that allows us to supply dummy only L1 value by default on all pages. And only on
wrap
we will have L2 support viaSupportsL2Chains
provider. This brings no changes to current behavior and only uses it nicely in code.Testing notes
Unfortunately such big refactors bring a lot of regress. Visible changes are:
But actually tx flow stuff is not touched, so it would be enough just to see that 1 tx of each type goes trough.
More attention is needed on chain/wallet connection edgecases: unsupported chain, multichain chain, not connected, connected but on L2, connected but chose different chain in chain picker(wrap). Change pages. Disconnected, auto connected.
I also added extra warning in console when request fails. This will allow us to see if and how rpc request fails even when using wallet RPC.
Please check WC, because it will use primarily our RPC and monitor network to requests to other chainIds.
Checklist: