From b3d485e8c440907dd5c45f9711bf5c1ac21537e6 Mon Sep 17 00:00:00 2001 From: ekzyis Date: Fri, 9 Feb 2024 16:42:26 +0100 Subject: [PATCH] Refactor default payment method setting (#803) * Refactor setting of default providers * fixed warning about component update while rendering another component * individual providers no longer need to know if they are the default or not * default setting is now handled by WebLNContext -- the same context that returns the provider. this makes a lot more sense and is a lot easier to read * default payment checkbox is now also disabled if there is only one enabled provider or if it is the default provider * Fix order lost on page reload On page reload, the providers were synced in the order they were loaded. This means that the default payment provider setting was lost. Fixed this by syncing order to local storage and on page reload, only syncing providers when they were initialized (else the order would have been lost again). --- components/webln/index.js | 140 +++++++++++++++++-------------- components/webln/lnbits.js | 10 +-- components/webln/nwc.js | 12 +-- pages/settings/wallets/lnbits.js | 11 ++- pages/settings/wallets/nwc.js | 11 ++- 5 files changed, 103 insertions(+), 81 deletions(-) diff --git a/components/webln/index.js b/components/webln/index.js index e8d4ce38..3f4d072e 100644 --- a/components/webln/index.js +++ b/components/webln/index.js @@ -1,45 +1,76 @@ -import { createContext, useContext, useEffect, useState } from 'react' +import { createContext, useCallback, useContext, useEffect, useState } from 'react' import { LNbitsProvider, useLNbits } from './lnbits' import { NWCProvider, useNWC } from './nwc' import { useToast } from '../toast' import { gql, useMutation } from '@apollo/client' const WebLNContext = createContext({}) -const storageKey = 'webln:providers' -const paymentMethodHook = (methods, { name, enabled }) => { - let newMethods - if (enabled) { - newMethods = methods.includes(name) ? methods : [...methods, name] - } else { - newMethods = methods.filter(m => m !== name) +const syncProvider = (array, provider) => { + const idx = array.findIndex(({ name }) => provider.name === name) + if (idx === -1) { + // add provider to end if enabled + return provider.enabled ? [...array, provider] : array } - savePaymentMethods(newMethods) - return newMethods + return [ + ...array.slice(0, idx), + // remove provider if not enabled + ...provider.enabled ? [provider] : [], + ...array.slice(idx + 1) + ] } -const savePaymentMethods = (methods) => { - window.localStorage.setItem(storageKey, JSON.stringify(methods)) -} +const storageKey = 'webln:providers' function RawWebLNProvider ({ children }) { const lnbits = useLNbits() const nwc = useNWC() - const providers = [lnbits, nwc] + const availableProviders = [lnbits, nwc] + const [enabledProviders, setEnabledProviders] = useState([]) - // TODO: Order of payment methods depends on user preference. - // Payment method at index 0 should be default, - // if that one fails we try the remaining ones in order as fallbacks. - // We should be able to implement this via dragging of cards. - // This list should then match the order in which the (payment) cards are rendered. - // eslint-disable-next-line no-unused-vars - const [paymentMethods, setPaymentMethods] = useState([]) - const loadPaymentMethods = () => { - const methods = window.localStorage.getItem(storageKey) - if (!methods) return - setPaymentMethods(JSON.parse(methods)) + // restore order on page reload + useEffect(() => { + const storedOrder = window.localStorage.getItem(storageKey) + if (!storedOrder) return + const providerNames = JSON.parse(storedOrder) + setEnabledProviders(providers => { + return providerNames.map(name => { + for (const p of availableProviders) { + if (p.name === name) return p + } + console.warn(`Stored provider with name ${name} not available`) + return null + }) + }) + }, []) + + // keep list in sync with underlying providers + useEffect(() => { + setEnabledProviders(providers => { + // Sync existing provider state with new provider state + // in the list while keeping the order they are in. + // If provider does not exist but is enabled, it is just added to the end of the list. + // This can be the case if we're syncing from a page reload + // where the providers are initially not enabled. + // If provider is no longer enabled, it is removed from the list. + const isInitialized = p => p.initialized + const newProviders = availableProviders.filter(isInitialized).reduce(syncProvider, providers) + const newOrder = newProviders.map(({ name }) => name) + window.localStorage.setItem(storageKey, JSON.stringify(newOrder)) + return newProviders + }) + }, [lnbits, nwc]) + + // sanity check + for (const p of enabledProviders) { + if (!p.enabled && p.initialized) { + console.warn('Expected provider to be enabled but is not:', p.name) + } } - useEffect(loadPaymentMethods, []) + + // first provider in list is the default provider + // TODO: implement fallbacks via provider priority + const provider = enabledProviders[0] const toaster = useToast() const [cancelInvoice] = useMutation(gql` @@ -50,43 +81,6 @@ function RawWebLNProvider ({ children }) { } `) - useEffect(() => { - setPaymentMethods(methods => paymentMethodHook(methods, nwc)) - if (!nwc.enabled) nwc.setIsDefault(false) - }, [nwc.enabled]) - - useEffect(() => { - setPaymentMethods(methods => paymentMethodHook(methods, lnbits)) - if (!lnbits.enabled) lnbits.setIsDefault(false) - }, [lnbits.enabled]) - - const setDefaultPaymentMethod = (provider) => { - for (const p of providers) { - if (p.name !== provider.name) { - p.setIsDefault(false) - } - } - } - - useEffect(() => { - if (nwc.isDefault) setDefaultPaymentMethod(nwc) - }, [nwc.isDefault]) - - useEffect(() => { - if (lnbits.isDefault) setDefaultPaymentMethod(lnbits) - }, [lnbits.isDefault]) - - // TODO: implement numeric provider priority using paymentMethods list - // when we have more than two providers for sending - let provider = providers.filter(p => p.enabled && p.isDefault)[0] - if (!provider && providers.length > 0) { - // if no provider is the default, pick the first one and use that one as the default - provider = providers.filter(p => p.enabled)[0] - if (provider) { - provider.setIsDefault(true) - } - } - const sendPaymentWithToast = function ({ bolt11, hash, hmac }) { let canceled = false let removeToast = toaster.warning('payment pending', { @@ -116,8 +110,21 @@ function RawWebLNProvider ({ children }) { }) } + const setProvider = useCallback((defaultProvider) => { + // move provider to the start to set it as default + setEnabledProviders(providers => { + const idx = providers.findIndex(({ name }) => defaultProvider.name === name) + if (idx === -1) { + console.warn(`tried to set unenabled provider ${defaultProvider.name} as default`) + return providers + } + return [defaultProvider, ...providers.slice(0, idx), ...providers.slice(idx + 1)] + }) + }, [setEnabledProviders]) + + const value = { provider: { ...provider, sendPayment: sendPaymentWithToast }, enabledProviders, setProvider } return ( - + {children} ) @@ -136,5 +143,10 @@ export function WebLNProvider ({ children }) { } export function useWebLN () { + const { provider } = useContext(WebLNContext) + return provider +} + +export function useWebLNConfigurator () { return useContext(WebLNContext) } diff --git a/components/webln/lnbits.js b/components/webln/lnbits.js index 5bf5cf20..fd716a51 100644 --- a/components/webln/lnbits.js +++ b/components/webln/lnbits.js @@ -64,7 +64,7 @@ export function LNbitsProvider ({ children }) { const [url, setUrl] = useState('') const [adminKey, setAdminKey] = useState('') const [enabled, setEnabled] = useState() - const [isDefault, setIsDefault] = useState() + const [initialized, setInitialized] = useState(false) const name = 'LNbits' const storageKey = 'webln:provider:lnbits' @@ -104,10 +104,9 @@ export function LNbitsProvider ({ children }) { const config = JSON.parse(configStr) - const { url, adminKey, isDefault } = config + const { url, adminKey } = config setUrl(url) setAdminKey(adminKey) - setIsDefault(isDefault) try { // validate config by trying to fetch wallet @@ -117,6 +116,8 @@ export function LNbitsProvider ({ children }) { console.error('invalid LNbits config:', err) setEnabled(false) throw err + } finally { + setInitialized(true) } }, []) @@ -124,7 +125,6 @@ export function LNbitsProvider ({ children }) { // immediately store config so it's not lost even if config is invalid setUrl(config.url) setAdminKey(config.adminKey) - setIsDefault(config.isDefault) // XXX This is insecure, XSS vulns could lead to loss of funds! // -> check how mutiny encrypts their wallet and/or check if we can leverage web workers @@ -153,7 +153,7 @@ export function LNbitsProvider ({ children }) { loadConfig().catch(console.error) }, []) - const value = { name, url, adminKey, saveConfig, clearConfig, enabled, isDefault, setIsDefault, getInfo, sendPayment } + const value = { name, url, adminKey, initialized, enabled, saveConfig, clearConfig, getInfo, sendPayment } return ( {children} diff --git a/components/webln/nwc.js b/components/webln/nwc.js index ac72209a..0a617520 100644 --- a/components/webln/nwc.js +++ b/components/webln/nwc.js @@ -11,7 +11,7 @@ export function NWCProvider ({ children }) { const [relayUrl, setRelayUrl] = useState() const [secret, setSecret] = useState() const [enabled, setEnabled] = useState() - const [isDefault, setIsDefault] = useState() + const [initialized, setInitialized] = useState(false) const [relay, setRelay] = useState() const name = 'NWC' @@ -26,9 +26,8 @@ export function NWCProvider ({ children }) { const config = JSON.parse(configStr) - const { nwcUrl, isDefault } = config + const { nwcUrl } = config setNwcUrl(nwcUrl) - setIsDefault(isDefault) const params = parseWalletConnectUrl(nwcUrl) setRelayUrl(params.relayUrl) @@ -42,14 +41,15 @@ export function NWCProvider ({ children }) { console.error('invalid NWC config:', err) setEnabled(false) throw err + } finally { + setInitialized(true) } }, []) const saveConfig = useCallback(async (config) => { // immediately store config so it's not lost even if config is invalid - const { nwcUrl, isDefault } = config + const { nwcUrl } = config setNwcUrl(nwcUrl) - setIsDefault(isDefault) if (!nwcUrl) { setEnabled(undefined) return @@ -174,7 +174,7 @@ export function NWCProvider ({ children }) { loadConfig().catch(console.error) }, []) - const value = { name, nwcUrl, relayUrl, walletPubkey, secret, saveConfig, clearConfig, enabled, isDefault, setIsDefault, getInfo, sendPayment } + const value = { name, nwcUrl, relayUrl, walletPubkey, secret, initialized, enabled, saveConfig, clearConfig, getInfo, sendPayment } return ( {children} diff --git a/pages/settings/wallets/lnbits.js b/pages/settings/wallets/lnbits.js index 2d137d16..8ad2adb2 100644 --- a/pages/settings/wallets/lnbits.js +++ b/pages/settings/wallets/lnbits.js @@ -7,11 +7,15 @@ import { useToast } from '../../../components/toast' import { useRouter } from 'next/router' import { useLNbits } from '../../../components/webln/lnbits' import { WalletSecurityBanner } from '../../../components/banners' +import { useWebLNConfigurator } from '../../../components/webln' export const getServerSideProps = getGetServerSideProps({ authRequired: true }) export default function LNbits () { - const { url, adminKey, saveConfig, clearConfig, enabled, isDefault } = useLNbits() + const { provider, enabledProviders, setProvider } = useWebLNConfigurator() + const lnbits = useLNbits() + const { name, url, adminKey, saveConfig, clearConfig, enabled } = lnbits + const isDefault = provider?.name === name const toaster = useToast() const router = useRouter() @@ -27,9 +31,10 @@ export default function LNbits () { isDefault: isDefault || false }} schema={lnbitsSchema} - onSubmit={async (values) => { + onSubmit={async ({ isDefault, ...values }) => { try { await saveConfig(values) + if (isDefault) setProvider(lnbits) toaster.success('saved settings') router.push('/settings/wallets') } catch (err) { @@ -53,7 +58,7 @@ export default function LNbits () { name='adminKey' /> { + onSubmit={async ({ isDefault, ...values }) => { try { await saveConfig(values) + if (isDefault) setProvider(nwc) toaster.success('saved settings') router.push('/settings/wallets') } catch (err) { @@ -45,7 +50,7 @@ export default function NWC () { autoFocus />