From dad5b77806f71ace4386ce14106484693061790f Mon Sep 17 00:00:00 2001 From: Eric Corson Date: Thu, 29 Feb 2024 11:16:15 +0900 Subject: [PATCH] fix: add fetch effects and improve fetch behaviour (#641) - Add effects to handle accounts changed, chain changed, extension attached, and extension connected. Replaces fetches interspersed in App and AccountOverview - Refresh public keys on Tx completed instead of toast change - Use setter for fetching public keys to allow more control over when the fetch happens - Check for native token early in minimum gas price fetch --- .../App/AccountOverview/AccountOverview.tsx | 40 +++++++---- apps/namada-interface/src/App/App.tsx | 49 +++++-------- apps/namada-interface/src/App/fetchEffects.ts | 70 +++++++++++++++++++ .../extensionEvents/handlers/namada.ts | 17 ++++- .../src/services/extensionEvents/provider.tsx | 26 ++++++- apps/namada-interface/src/slices/fees.ts | 60 +++++++++++----- apps/namada-interface/src/slices/settings.ts | 10 +++ packages/hooks/src/index.ts | 3 +- .../hooks/src/useEffectSkipFirstRender.ts | 16 +++++ 9 files changed, 220 insertions(+), 71 deletions(-) create mode 100644 apps/namada-interface/src/App/fetchEffects.ts create mode 100644 packages/hooks/src/useEffectSkipFirstRender.ts diff --git a/apps/namada-interface/src/App/AccountOverview/AccountOverview.tsx b/apps/namada-interface/src/App/AccountOverview/AccountOverview.tsx index 69ab3f2d64..1361632eb0 100644 --- a/apps/namada-interface/src/App/AccountOverview/AccountOverview.tsx +++ b/apps/namada-interface/src/App/AccountOverview/AccountOverview.tsx @@ -7,10 +7,17 @@ import { useIntegrationConnection, useUntilIntegrationAttached, } from "@namada/integrations"; -import { Account, Chain, ExtensionKey, Extensions, TokenType, Tokens } from "@namada/types"; +import { + Account, + Chain, + ExtensionKey, + Extensions, + TokenType, + Tokens, +} from "@namada/types"; import { TopLevelRoute } from "App/types"; import { AccountsState, addAccounts, fetchBalances } from "slices/accounts"; -import { setIsConnected } from "slices/settings"; +import { namadaExtensionConnectedAtom, setIsConnected } from "slices/settings"; import { useAppDispatch, useAppSelector } from "store"; import { AccountOverviewContainer, @@ -25,9 +32,9 @@ import { } from "./AccountOverview.components"; import { DerivedAccounts } from "./DerivedAccounts"; -import { useAtomValue, useSetAtom } from "jotai"; -import { accountsAtom, balancesAtom } from "slices/accounts"; import BigNumber from "bignumber.js"; +import { useAtomValue, useSetAtom } from "jotai"; +import { balancesAtom } from "slices/accounts"; //TODO: move to utils when we have one const isEmptyObject = (object: Record): boolean => { @@ -35,6 +42,8 @@ const isEmptyObject = (object: Record): boolean => { }; export const AccountOverview = (): JSX.Element => { + const setNamadaExtensionConnected = useSetAtom(namadaExtensionConnectedAtom); + const navigate = useNavigate(); const dispatch = useAppDispatch(); const chain = useAppSelector((state) => state.chain.config); @@ -46,9 +55,6 @@ export const AccountOverview = (): JSX.Element => { metamask: false, }); - const refreshAccounts = useSetAtom(accountsAtom); - const refreshBalances = useSetAtom(balancesAtom); - const { derived } = useAppSelector((state) => state.accounts); const [integration, isConnectingToExtension, withConnection] = @@ -65,16 +71,14 @@ export const AccountOverview = (): JSX.Element => { const balances = useAtomValue(balancesAtom); const totalNativeBalance = Object.values(balances).reduce((acc, balance) => { - return acc.plus(balance[chain.currency.symbol as TokenType] || BigNumber(0)); + return acc.plus( + balance[chain.currency.symbol as TokenType] || BigNumber(0) + ); }, BigNumber(0)); const handleConnectExtension = async (): Promise => { withConnection( async () => { - // jotai - refreshAccounts(); - refreshBalances(); - const accounts = await integration?.accounts(); if (accounts) { dispatch(addAccounts(accounts as Account[])); @@ -86,12 +90,16 @@ export const AccountOverview = (): JSX.Element => { ...isExtensionConnected, [chain.extension.id]: true, }); + + setNamadaExtensionConnected(true); }, async () => { setIsExtensionConnected({ ...isExtensionConnected, [chain.extension.id]: false, }); + + setNamadaExtensionConnected(false); } ); }; @@ -108,8 +116,12 @@ export const AccountOverview = (): JSX.Element => { {!isEmptyObject(derived[chain.id]) && ( - {Tokens[chain.currency.symbol as TokenType].symbol} - {totalNativeBalance.toString()} + + {Tokens[chain.currency.symbol as TokenType].symbol} + + + {totalNativeBalance.toString()} + )} diff --git a/apps/namada-interface/src/App/App.tsx b/apps/namada-interface/src/App/App.tsx index b7091e38d5..014476dc57 100644 --- a/apps/namada-interface/src/App/App.tsx +++ b/apps/namada-interface/src/App/App.tsx @@ -1,7 +1,7 @@ /* eslint-disable max-len */ import { AnimatePresence } from "framer-motion"; import { createBrowserHistory } from "history"; -import { atom, useAtom, useAtomValue, useSetAtom } from "jotai"; +import { useAtomValue } from "jotai"; import { useEffect, useState } from "react"; import { PersistGate } from "redux-persist/integration/react"; import { ThemeProvider } from "styled-components"; @@ -23,13 +23,7 @@ import { Outlet } from "react-router-dom"; import { addAccounts, fetchBalances } from "slices/accounts"; import { setChain } from "slices/chain"; import { SettingsState } from "slices/settings"; -import { - persistor, - reduxStoreAtom, - store, - useAppDispatch, - useAppSelector, -} from "store"; +import { persistor, store, useAppDispatch, useAppSelector } from "store"; import { AppContainer, AppLoader, @@ -41,9 +35,14 @@ import { } from "./App.components"; import { TopNavigation } from "./TopNavigation"; -import { accountsAtom, balancesAtom } from "slices/accounts"; import { chainAtom } from "slices/chain"; -import { minimumGasPriceAtom } from "slices/fees"; + +import { + useOnAccountsChanged, + useOnChainChanged, + useOnNamadaExtensionAttached, + useOnNamadaExtensionConnected, +} from "./fetchEffects"; export const history = createBrowserHistory({ window }); @@ -65,11 +64,12 @@ export const AnimatedTransition = (props: { ); }; -// TODO: can be moved to slices/notifications once redux is removed -// Defining it there currently causes a unit test error related to redux-persist -const toastsAtom = atom((get) => get(reduxStoreAtom).notifications.toasts); - function App(): JSX.Element { + useOnNamadaExtensionAttached(); + useOnNamadaExtensionConnected(); + useOnAccountsChanged(); + useOnChainChanged(); + const dispatch = useAppDispatch(); const initialColorMode = loadColorMode(); const [colorMode, setColorMode] = useState(initialColorMode); @@ -79,10 +79,7 @@ function App(): JSX.Element { setColorMode((currentMode) => (currentMode === "dark" ? "light" : "dark")); }; - const [chain, refreshChain] = useAtom(chainAtom); - const refreshAccounts = useSetAtom(accountsAtom); - const refreshBalances = useSetAtom(balancesAtom); - const refreshMinimumGasPrice = useSetAtom(minimumGasPriceAtom); + const chain = useAtomValue(chainAtom); const { connectedChains } = useAppSelector( (state) => state.settings @@ -96,12 +93,9 @@ function App(): JSX.Element { const currentExtensionAttachStatus = extensionAttachStatus[chain.extension.id]; + // TODO: remove this effect once redux has been replaced by jotai useEffect(() => { const fetchAccounts = async (): Promise => { - // jotai - refreshAccounts(); - refreshBalances(); - const accounts = await integration?.accounts(); if (accounts) { dispatch(addAccounts(accounts as Account[])); @@ -113,16 +107,13 @@ function App(): JSX.Element { connectedChains.includes(chain.id) ) { fetchAccounts(); - refreshMinimumGasPrice(); } }, [chain]); + // TODO: remove this effect once redux has been replaced by jotai useEffect(() => { (async () => { if (currentExtensionAttachStatus === "attached") { - // jotai - refreshChain(); - const chain = await integration.getChain(); if (chain) { dispatch(setChain(chain)); @@ -131,12 +122,6 @@ function App(): JSX.Element { })(); }, [currentExtensionAttachStatus]); - const toasts = useAtomValue(toastsAtom); - useEffect(() => { - // TODO: this could be more conservative about how often it fetches balances - refreshBalances(); - }, [toasts]); - return ( diff --git a/apps/namada-interface/src/App/fetchEffects.ts b/apps/namada-interface/src/App/fetchEffects.ts new file mode 100644 index 0000000000..1f14d12f70 --- /dev/null +++ b/apps/namada-interface/src/App/fetchEffects.ts @@ -0,0 +1,70 @@ +import { useAtomValue, useSetAtom } from "jotai"; +import { loadable } from "jotai/utils"; + +import { useEffectSkipFirstRender } from "@namada/hooks"; +import { + Namada, + useIntegration, + useUntilIntegrationAttached, +} from "@namada/integrations"; +import { namadaExtensionConnectedAtom } from "slices/settings"; + +import { accountsAtom, balancesAtom } from "slices/accounts"; +import { chainAtom } from "slices/chain"; +import { isRevealPkNeededAtom, minimumGasPriceAtom } from "slices/fees"; + +export const useOnChainChanged = (): void => { + const chain = useAtomValue(chainAtom); + + const refreshMinimumGasPrice = useSetAtom(minimumGasPriceAtom); + const refreshPublicKeys = useSetAtom(isRevealPkNeededAtom); + + useEffectSkipFirstRender(() => { + refreshMinimumGasPrice(); + refreshPublicKeys(); + }, [chain]); +}; + +export const useOnNamadaExtensionAttached = (): void => { + const setNamadaExtensionConnected = useSetAtom(namadaExtensionConnectedAtom); + const chain = useAtomValue(chainAtom); // should always be namada + const { namada: attachStatus } = useUntilIntegrationAttached(chain); + const integration = useIntegration("namada") as Namada; + + useEffectSkipFirstRender(() => { + (async () => { + if (attachStatus === "attached") { + const connected = !!(await integration.isConnected()); + setNamadaExtensionConnected(connected); + } + })(); + }, [attachStatus]); +}; + +export const useOnNamadaExtensionConnected = (): void => { + const connected = useAtomValue(namadaExtensionConnectedAtom); + + const refreshChain = useSetAtom(chainAtom); + const refreshAccounts = useSetAtom(accountsAtom); + + useEffectSkipFirstRender(() => { + if (connected) { + refreshChain(); + refreshAccounts(); + } + }, [connected]); +}; + +export const useOnAccountsChanged = (): void => { + const accountsLoadable = useAtomValue(loadable(accountsAtom)); + + const refreshBalances = useSetAtom(balancesAtom); + const refreshPublicKeys = useSetAtom(isRevealPkNeededAtom); + + useEffectSkipFirstRender(() => { + if (accountsLoadable.state === "hasData") { + refreshBalances(); + refreshPublicKeys(); + } + }, [accountsLoadable]); +}; diff --git a/apps/namada-interface/src/services/extensionEvents/handlers/namada.ts b/apps/namada-interface/src/services/extensionEvents/handlers/namada.ts index d739ad8a27..893645a1d4 100644 --- a/apps/namada-interface/src/services/extensionEvents/handlers/namada.ts +++ b/apps/namada-interface/src/services/extensionEvents/handlers/namada.ts @@ -46,8 +46,9 @@ export const NamadaProposalsUpdatedHandler = }; export const NamadaUpdatedBalancesHandler = - (dispatch: Dispatch) => async () => { + (dispatch: Dispatch, refreshBalances: () => void) => async () => { dispatch(fetchBalances()); + refreshBalances(); }; export const NamadaUpdatedStakingHandler = @@ -67,7 +68,8 @@ export const NamadaTxStartedHandler = }; export const NamadaTxCompletedHandler = - (dispatch: Dispatch) => async (event: CustomEventInit) => { + (dispatch: Dispatch, refreshPublicKeys: () => void) => + async (event: CustomEventInit) => { const { msgId, txType, success, payload } = event.detail; if (!success) { console.warn(`${txType} failed:`, payload); @@ -80,4 +82,15 @@ export const NamadaTxCompletedHandler = error: payload || "", }) ); + refreshPublicKeys(); + }; + +export const NamadaConnectionRevokedHandler = + ( + integration: Namada, + setNamadaExtensionConnected: (connected: boolean) => void + ) => + async () => { + const connected = !!(await integration.isConnected()); + setNamadaExtensionConnected(connected); }; diff --git a/apps/namada-interface/src/services/extensionEvents/provider.tsx b/apps/namada-interface/src/services/extensionEvents/provider.tsx index 8e3bb6403a..37111546f8 100644 --- a/apps/namada-interface/src/services/extensionEvents/provider.tsx +++ b/apps/namada-interface/src/services/extensionEvents/provider.tsx @@ -10,6 +10,7 @@ import { MetamaskAccountChangedHandler, MetamaskBridgeTransferCompletedHandler, NamadaAccountChangedHandler, + NamadaConnectionRevokedHandler, NamadaNetworkChangedHandler, NamadaProposalsUpdatedHandler, NamadaTxCompletedHandler, @@ -19,8 +20,10 @@ import { } from "./handlers"; import { useSetAtom } from "jotai"; -import { accountsAtom } from "slices/accounts"; +import { accountsAtom, balancesAtom } from "slices/accounts"; import { chainAtom } from "slices/chain"; +import { isRevealPkNeededAtom } from "slices/fees"; +import { namadaExtensionConnectedAtom } from "slices/settings"; export const ExtensionEventsContext = createContext({}); @@ -32,6 +35,9 @@ export const ExtensionEventsProvider: React.FC = (props): JSX.Element => { const refreshAccounts = useSetAtom(accountsAtom); const refreshChain = useSetAtom(chainAtom); + const refreshBalances = useSetAtom(balancesAtom); + const refreshPublicKeys = useSetAtom(isRevealPkNeededAtom); + const setNamadaExtensionConnected = useSetAtom(namadaExtensionConnectedAtom); // Instantiate handlers: const namadaAccountChangedHandler = NamadaAccountChangedHandler( @@ -45,10 +51,20 @@ export const ExtensionEventsProvider: React.FC = (props): JSX.Element => { refreshChain ); const namadaTxStartedHandler = NamadaTxStartedHandler(dispatch); - const namadaTxCompletedHandler = NamadaTxCompletedHandler(dispatch); - const namadaUpdatedBalancesHandler = NamadaUpdatedBalancesHandler(dispatch); + const namadaTxCompletedHandler = NamadaTxCompletedHandler( + dispatch, + refreshPublicKeys + ); + const namadaUpdatedBalancesHandler = NamadaUpdatedBalancesHandler( + dispatch, + refreshBalances + ); const namadaUpdatedStakingHandler = NamadaUpdatedStakingHandler(dispatch); const namadaProposalsUpdatedHandler = NamadaProposalsUpdatedHandler(dispatch); + const namadaConnectionRevokedHandler = NamadaConnectionRevokedHandler( + namadaIntegration as Namada, + setNamadaExtensionConnected + ); // Keplr handlers const keplrAccountChangedHandler = KeplrAccountChangedHandler( @@ -73,6 +89,10 @@ export const ExtensionEventsProvider: React.FC = (props): JSX.Element => { useEventListenerOnce(Events.TxStarted, namadaTxStartedHandler); useEventListenerOnce(Events.TxCompleted, namadaTxCompletedHandler); useEventListenerOnce(Events.ProposalsUpdated, namadaProposalsUpdatedHandler); + useEventListenerOnce( + Events.ConnectionRevoked, + namadaConnectionRevokedHandler + ); useEventListenerOnce(KeplrEvents.AccountChanged, keplrAccountChangedHandler); useEventListenerOnce( MetamaskEvents.AccountChanged, diff --git a/apps/namada-interface/src/slices/fees.ts b/apps/namada-interface/src/slices/fees.ts index 86a566a972..b812a1c2d7 100644 --- a/apps/namada-interface/src/slices/fees.ts +++ b/apps/namada-interface/src/slices/fees.ts @@ -21,6 +21,10 @@ const minimumGasPriceAtom = (() => { const query = new Query(rpc); const promise = (async () => { + if (!nativeToken) { + throw new Error("Native token is undefined"); + } + const result = (await query.query_gas_costs()) as [string, string][]; const nativeTokenCost = result.find(([token]) => token === nativeToken); @@ -42,27 +46,45 @@ const minimumGasPriceAtom = (() => { ); })(); -const isRevealPkNeededAtom = atom(async (get) => { - const accounts = await get(accountsAtom); - const transparentAccounts = accounts.filter((account) => !account.isShielded); +const isRevealPkNeededAtom = (() => { + type RevealPkNeededMap = { [address: string]: boolean }; - const { rpc } = get(chainAtom); - const query = new Query(rpc); + const base = atom(new Promise(() => {})); - const map: Record = {}; - await Promise.all( - transparentAccounts.map(async ({ address }) => { - const publicKey = await query.query_public_key(address); - map[address] = publicKey; - }) - ); + return atom( + async (get) => { + const map = await get(base); - return (address: string): boolean => { - if (!(address in map)) { - throw new Error("address not found in public key map"); - } - return map[address] === null; - }; -}); + return (address: string): boolean => { + if (!(address in map)) { + throw new Error("address not found in public key map"); + } + return map[address]; + }; + }, + (get, set) => + set( + base, + (async (): Promise => { + const accounts = await get(accountsAtom); + const transparentAccounts = accounts.filter( + (account) => !account.isShielded + ); + + const { rpc } = get(chainAtom); + const query = new Query(rpc); + + const entries = await Promise.all( + transparentAccounts.map(async ({ address }) => { + const publicKey = await query.query_public_key(address); + return [address, !publicKey]; + }) + ); + + return Object.fromEntries(entries); + })() + ) + ); +})(); export { isRevealPkNeededAtom, minimumGasPriceAtom }; diff --git a/apps/namada-interface/src/slices/settings.ts b/apps/namada-interface/src/slices/settings.ts index d0aaed3875..090be08d0f 100644 --- a/apps/namada-interface/src/slices/settings.ts +++ b/apps/namada-interface/src/slices/settings.ts @@ -1,6 +1,8 @@ import { ChainKey } from "@namada/types"; import { createSlice, PayloadAction } from "@reduxjs/toolkit"; +import { atom } from "jotai"; + const SETTINGS_ACTIONS_BASE = "settings"; export type SettingsState = { @@ -28,3 +30,11 @@ const { actions, reducer } = settingsSlice; export const { setIsConnected } = actions; export default reducer; + +//////////////////////////////////////////////////////////////////////////////// +// JOTAI +//////////////////////////////////////////////////////////////////////////////// + +const namadaExtensionConnectedAtom = atom(false); + +export { namadaExtensionConnectedAtom }; diff --git a/packages/hooks/src/index.ts b/packages/hooks/src/index.ts index 365ffb025c..8a1f36dcd6 100644 --- a/packages/hooks/src/index.ts +++ b/packages/hooks/src/index.ts @@ -1,5 +1,6 @@ export * from "./useDebounce"; +export * from "./useEffectSkipFirstRender"; export * from "./useEvent"; -export * from "./useSanitizedParams"; export * from "./useSanitizedLocation"; +export * from "./useSanitizedParams"; export * from "./useUntil"; diff --git a/packages/hooks/src/useEffectSkipFirstRender.ts b/packages/hooks/src/useEffectSkipFirstRender.ts new file mode 100644 index 0000000000..a33eeebbcd --- /dev/null +++ b/packages/hooks/src/useEffectSkipFirstRender.ts @@ -0,0 +1,16 @@ +import { useEffect, useRef } from "react"; + +/** + * The same as useEffect, but does not run the effect on the first render. + */ +export const useEffectSkipFirstRender: typeof useEffect = (effect, deps) => { + const firstRender = useRef(true); + + useEffect(() => { + if (firstRender.current) { + firstRender.current = false; + } else { + effect(); + } + }, deps); +};