Refactoring single source of truth for dapp state #510

Jeday · 2024-10-16T11:21:40Z

Description

use wagmi->new sdk->old-sdk heirarchy
derive old sdk connection from wagmi and use it's transports (removed old web3 provider async hack)
all rpc requests are now routed trough wallet rpc first
supported chains context per page + better/simple l2 unsupported state for a page
all hooks only use correct chain on any page/any chain connected
web3 module to unify and structure all new/wagmi/refactored code
move most chainId related status to useDappStatus

problem is SUPPORTED_CHAINS inlcude L2 chains but other pages only support ethereum chains and not l2 chains

Demo

Code review notes

Testing notes

Checklist:

Checked the changes locally.
Created / updated analytics events.
Created / updated the technical documentation (README.md / docs / etc.).
Affects / requires changes in other services (Matomo / Sentry / CloudFlare / etc.).

…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc

preview-stands · 2024-10-16T11:23:01Z

Preview stand status

Preview stand available on testnet

…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc

solidovic · 2024-10-18T09:17:17Z

features/ipfs/security-status-banner/use-version-check.ts

@@ -19,7 +19,7 @@ const URL_CID_REGEX =
 /[/.](?<cid>Qm[1-9A-HJ-NP-Za-km-z]{44,}|b[A-Za-z2-7]{58,}|B[A-Z2-7]{58,}|z[1-9A-HJ-NP-Za-km-z]{48,}|F[0-9A-F]{50,})([./#?]|$)/;

 export const useVersionCheck = () => {


The name does not reflect the return type. I mean that this hook does not expect to receive remoteCid, currentCid, remoteCidLink (I expect to receive the status).

I can suggest renaming the hook to "useVersionControl" or "useVersionStatus"

I am all in on that, but maybe in other PR. Want to focus on daap/chain state.

Ok, lets set TODO

solidovic · 2024-10-18T09:37:47Z

modules/web3/web3-provider/dapp-chain.tsx

+ const router = useRouter();
+
+ const isChainTypeUnlocked = useMemo(
+ () => router.pathname === '/wrap/[[...mode]]',


Lets set "TODO: in the future this condition will change (will depend by router.pathname and chainType)"

solidovic · 2024-10-18T09:50:29Z

modules/web3/web3-provider/sdk-legacy.tsx

+ detectNetwork(): Promise<Network> {
+ // eslint-disable-next-line @typescript-eslint/no-misused-promises
+ if (!this._cache['detectNetwork']) {
+ this._cache['detectNetwork'] = this._uncachedDetectNetwork();


will this code break in the future?
it seems that this._cache['detectNetwork'] was not intended to be overridden

Not really, because ethers.js is being deprecated. The reason for this is override is we can only funnel ethers trough wagmi via ether's Web3Provider. But Web3Provider is built for connecting to the wallet rpc that can change chain id anytime, and it's constantly requesting it to keep it's state updated. This override removes cache reset logic for chainId and allows web3provider to act as JsonRpcProvider.

solidovic · 2024-10-18T09:54:56Z

modules/web3/index.ts

+ useDappChain,
+ SupportOnlyL1Chains,
+ DAPP_CHAIN_TYPE,
+} from './web3-provider';


Why not just use provider naming, like part of the "namespace" of web3?

To keep code structure convention:

web3-provider(folder)

web3-provider.ts (exports Web3Provider)

index.ts(exports from web3-provider.ts)

This allows us to always find root module/file by component name.

solidovic · 2024-10-18T09:57:04Z

modules/web3/web3-provider/connect-wallet-modal/connect-wallet-modal.tsx

move up in the file structure:
from 'modules/web3/web3-provider/connect-wallet-modal'
to 'modules/web3/modals'

It's only used by web3-provider that's why it's nested inside, see https://feature-sliced.design/. Things like web3/hooks are used all over the place and that's why they are grouped by type. Generally we have to avoid grouping files by type.

solidovic · 2024-10-18T10:15:18Z

modules/web3/hooks/index.ts

Adding a module breaks the current "feature-sliced" structure (I agree that there is no suitable entity within the "feature-sliced" design).
Considering that the module is a large independent part of a app, it seems we should move 'consts/chains.ts' to 'web3' module.

Modules are part of feature-sliced design. I agree though that we should move some const there. But I want to scope this PR, let's put this into roadmap.

solidovic · 2024-10-18T10:24:00Z

modules/web3/web3-provider/dapp-chain.tsx

+ chainType: DAPP_CHAIN_TYPE;
+ setChainType: React.Dispatch<React.SetStateAction<DAPP_CHAIN_TYPE>>;
+ isChainTypeUnlocked: boolean;
+ isDappChainTypeMatched: boolean;


isChainTypeMatched? (consistency naming with other fields)

or:

dappChainType

setDappChainType

isDappChainTypeUnlocked

solidovic · 2024-10-18T12:08:48Z

modules/web3/web3-provider/supported-chain.tsx

+context.displayName = 'SupportedChainsContext';
+
+export const useSupportedChain = () => {
+ const { chainId: dappChain } = useLidoSDK();


Why dappChain, not sdkChainId?
By my opinion: it may confuse and create conflict for the code reader.

solidovic · 2024-10-18T12:09:28Z

modules/web3/web3-provider/supported-chain.tsx

+
+export const useSupportedChain = () => {
+ const { chainId: dappChain } = useLidoSDK();
+ const { chainId: walletChain } = useAccount();


Why not walletChainId?

Jeday added 13 commits October 14, 2024 14:32

feat: wire legacy sdk provider trough viem+new sdk

418119d

fix: use only supported chainId

a924ee7

Merge branch 'develop' of github.com:lidofinance/ethereum-staking-wid…

3b37cce

…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc

chore: supported chains

a9b4d95

chore(refactor): web3 module

dd9f550

chore: remove unused deps

070b24d

chore: refactor dapp chain

c42688e

chore: rename capitalize

c7605a2

chore: remove unused code

c7b65ef

chore: simplify supported chains

5d3db69

chore: comments

dedc083

Merge branch 'develop' of github.com:lidofinance/ethereum-staking-wid…

69bc124

…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc

fix: use dapp status

0cf7cad

Jeday requested review from a team as code owners October 16, 2024 11:21

Jeday changed the title ~~Refactoring single source of truth~~ Refactoring single source of truth for dapp state Oct 16, 2024

Jeday added 2 commits October 16, 2024 19:21

fix: build

00da103

Merge branch 'develop' of github.com:lidofinance/ethereum-staking-wid…

0555c2d

…get into feature/si-1641-refactoring-single-source-of-truth-for-chainid-and-rpc

Jeday mentioned this pull request Oct 17, 2024

fix: unwrap and wq stats #513

Open

4 tasks

solidovic reviewed Oct 18, 2024

View reviewed changes

chore(refactor): merge supported chains and dapp chain context

77964d9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring single source of truth for dapp state #510

Refactoring single source of truth for dapp state #510

Jeday commented Oct 16, 2024 •

edited

Loading

preview-stands bot commented Oct 16, 2024 •

edited

Loading

solidovic Oct 18, 2024

Jeday Oct 18, 2024

solidovic Oct 18, 2024

solidovic Oct 18, 2024

solidovic Oct 18, 2024

Jeday Oct 18, 2024

solidovic Oct 18, 2024

Jeday Oct 18, 2024

Jeday Oct 18, 2024

solidovic Oct 18, 2024

Jeday Oct 18, 2024

solidovic Oct 18, 2024

Jeday Oct 18, 2024

solidovic Oct 18, 2024

solidovic Oct 18, 2024 •

edited

Loading

solidovic Oct 18, 2024 •

edited

Loading

solidovic Oct 18, 2024

		@@ -19,7 +19,7 @@ const URL_CID_REGEX =
		/[/.](?<cid>Qm[1-9A-HJ-NP-Za-km-z]{44,}\|b[A-Za-z2-7]{58,}\|B[A-Z2-7]{58,}\|z[1-9A-HJ-NP-Za-km-z]{48,}\|F[0-9A-F]{50,})([./#?]\|$)/;

		export const useVersionCheck = () => {

Refactoring single source of truth for dapp state #510

Are you sure you want to change the base?

Refactoring single source of truth for dapp state #510

Conversation

Jeday commented Oct 16, 2024 • edited Loading

Description

Demo

Code review notes

Testing notes

Checklist:

preview-stands bot commented Oct 16, 2024 • edited Loading

Preview stand status

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solidovic Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

solidovic Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jeday commented Oct 16, 2024 •

edited

Loading

preview-stands bot commented Oct 16, 2024 •

edited

Loading

solidovic Oct 18, 2024 •

edited

Loading

solidovic Oct 18, 2024 •

edited

Loading