From cbc6098b8e0a00c44022b1b145e4b271bbf3b601 Mon Sep 17 00:00:00 2001 From: theborakompanioni Date: Mon, 21 Feb 2022 13:20:24 +0100 Subject: [PATCH 1/5] fix: respect minimum_maker conf value on Send page --- src/components/Send.jsx | 63 ++++++++++++++++++++++++++++------------- src/libs/JmWalletApi.js | 18 ++++++++++++ 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/components/Send.jsx b/src/components/Send.jsx index 58a5ffa0f..1eb5235a9 100644 --- a/src/components/Send.jsx +++ b/src/components/Send.jsx @@ -27,9 +27,9 @@ const isValidAmount = (candidate) => { return !isNaN(parsed) && parsed > 0 } -const isValidNumCollaborators = (candidate) => { +const isValidNumCollaborators = (candidate, minNumCollaborators) => { const parsed = parseInt(candidate, 10) - return !isNaN(parsed) && parsed >= 1 && parsed <= 99 + return !isNaN(parsed) && parsed >= minNumCollaborators && parsed <= 99 } const extractErrorMessage = async (response, fallbackReason = 'Unknown Error - No exact reasons are available : (') => { @@ -53,20 +53,20 @@ const extractErrorMessage = async (response, fallbackReason = 'Unknown Error - N return message || fallbackReason } -const CollaboratorsSelector = ({ numCollaborators, setNumCollaborators }) => { +const CollaboratorsSelector = ({ numCollaborators, setNumCollaborators, minNumCollaborators }) => { const settings = useSettings() const [usesCustomNumCollaborators, setUsesCustomNumCollaborators] = useState(false) const validateAndSetCustomNumCollaborators = (candidate) => { - if (isValidNumCollaborators(candidate)) { + if (isValidNumCollaborators(candidate, minNumCollaborators)) { setNumCollaborators(candidate) } else { setNumCollaborators(null) } } - const defaultCollaboratorsSelection = [3, 5, 6, 7, 9] + const defaultCollaboratorsSelection = [0, 2, 3, 4, 6].map((val) => val + minNumCollaborators) return ( @@ -101,9 +101,9 @@ const CollaboratorsSelector = ({ numCollaborators, setNumCollaborators }) => { })} { /> {usesCustomNumCollaborators && ( - Please use between 1 and 99 collaborators. + Please use between {minNumCollaborators} and 99 collaborators. )} @@ -139,15 +139,17 @@ export default function Send({ makerRunning, coinjoinInProcess }) { const location = useLocation() const [alert, setAlert] = useState(null) + const [isLoading, setIsLoading] = useState(true) const [isSending, setIsSending] = useState(false) const [isCoinjoin, setIsCoinjoin] = useState(false) const [isCoinjoinOptionEnabled, setIsCoinjoinOptionEnabled] = useState(!makerRunning && !coinjoinInProcess) + const [minNumCollaborators, setMinNumCollaborators] = useState(4) // default value from an unchanged joinmarket.cfg (last check 2022-02-20) const initialDestination = null const initialAccount = 0 const initialAmount = null const initialNumCollaborators = () => { - return pseudoRandomNumber(5, 7) + return pseudoRandomNumber(minNumCollaborators + 1, minNumCollaborators + 3) } const [destination, setDestination] = useState(initialDestination) @@ -171,29 +173,46 @@ export default function Send({ makerRunning, coinjoinInProcess }) { isValidAddress(destination) && isValidAccount(account) && isValidAmount(amount) && - (isCoinjoin ? isValidNumCollaborators(numCollaborators) : true) + (isCoinjoin ? isValidNumCollaborators(numCollaborators, minNumCollaborators) : true) ) { setFormIsValid(true) } else { setFormIsValid(false) } - }, [destination, account, amount, numCollaborators, isCoinjoin]) + }, [destination, account, amount, numCollaborators, minNumCollaborators, isCoinjoin]) useEffect(() => { - // Reload wallet info if not already available. - if (walletInfo) return - const abortCtrl = new AbortController() setAlert(null) + setIsLoading(true) - Api.getWalletDisplay({ walletName: wallet.name, token: wallet.token, signal: abortCtrl.signal }) - .then((res) => (res.ok ? res.json() : Promise.reject(new Error(res.message || 'Loading wallet failed.')))) - .then((data) => setWalletInfo(data.walletinfo)) + const requestContext = { walletName: wallet.name, token: wallet.token, signal: abortCtrl.signal } + // Reload wallet info if not already available. + const loadingWalletInfo = walletInfo + ? Promise.resolve() + : Api.getWalletDisplay(requestContext) + .then((res) => (res.ok ? res.json() : Promise.reject(new Error(res.message || 'Loading wallet failed.')))) + .then((data) => setWalletInfo(data.walletinfo)) + .catch((err) => { + !abortCtrl.signal.aborted && setAlert({ variant: 'danger', message: err.message }) + }) + + const loadingMinimumMakerConfig = Api.postConfigGet(requestContext, { section: 'POLICY', field: 'minimum_makers' }) + .then((res) => (res.ok ? res.json() : Promise.reject(new Error(res.message || 'Loading config value failed.')))) + .then((data) => setMinNumCollaborators(parseInt(data.configvalue, 10))) .catch((err) => { !abortCtrl.signal.aborted && setAlert({ variant: 'danger', message: err.message }) }) + Promise.all([loadingWalletInfo, loadingMinimumMakerConfig]) + .then((_) => { + if (minNumCollaborators && numCollaborators < minNumCollaborators) { + setNumCollaborators(minNumCollaborators) + } + }) + .finally(() => setIsLoading(false)) + return () => abortCtrl.abort() }, [wallet, setWalletInfo, walletInfo]) @@ -261,7 +280,7 @@ export default function Send({ makerRunning, coinjoinInProcess }) { const isValid = formIsValid if (isValid) { - const counterparties = parseInt(numCollaborators) + const counterparties = parseInt(numCollaborators, 10) const success = isCoinjoin ? await startCoinjoin(account, destination, amount, counterparties) @@ -280,7 +299,7 @@ export default function Send({ makerRunning, coinjoinInProcess }) { return ( <> - {!walletInfo ? ( + {isLoading ? (
@@ -374,7 +393,11 @@ export default function Send({ makerRunning, coinjoinInProcess }) { )} {isCoinjoin && ( - + )} { }) } +/** + * Get the value of a specific config setting. Note values are always returned as string. + * + * @returns an object with property `configvalue` as string + */ +const postConfigGet = async ({ walletName, token, signal }, { section, field }) => { + return await fetch(`/api/v1/wallet/${walletName}/configget`, { + method: 'POST', + headers: { ...Authorization(token) }, + signal, + body: JSON.stringify({ + section, + field, + }), + }) +} + export { postMakerStart, getMakerStop, @@ -170,4 +187,5 @@ export { getWalletUtxos, getYieldgenReport, postFreeze, + postConfigGet, } From dae4db766f0e51003c2016940183cf40461d30db Mon Sep 17 00:00:00 2001 From: theborakompanioni Date: Mon, 21 Feb 2022 13:42:43 +0100 Subject: [PATCH 2/5] cleanup: remove extracting error message from text/html --- src/components/Send.jsx | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/src/components/Send.jsx b/src/components/Send.jsx index 1eb5235a9..b236edac3 100644 --- a/src/components/Send.jsx +++ b/src/components/Send.jsx @@ -32,27 +32,6 @@ const isValidNumCollaborators = (candidate, minNumCollaborators) => { return !isNaN(parsed) && parsed >= minNumCollaborators && parsed <= 99 } -const extractErrorMessage = async (response, fallbackReason = 'Unknown Error - No exact reasons are available : (') => { - // The server will answer with a html response instead of json on certain errors. - // The situation is mitigated by parsing the returned html till a fix is available. - // Tracked here: https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/1170 (last checked: 2022-02-11) - const isHtmlErrorMessage = response.headers.get('content-type') === 'text/html' - - if (isHtmlErrorMessage) { - return await response - .text() - .then((html) => { - var parser = new DOMParser() - var doc = parser.parseFromString(html, 'text/html') - return doc.title || fallbackReason - }) - .then((reason) => `The server reported a problem: ${reason}`) - } - - const { message } = await response.json() - return message || fallbackReason -} - const CollaboratorsSelector = ({ numCollaborators, setNumCollaborators, minNumCollaborators }) => { const settings = useSettings() @@ -235,7 +214,7 @@ export default function Send({ makerRunning, coinjoinInProcess }) { }) success = true } else { - const message = await extractErrorMessage(res) + const { message } = await res.json() setAlert({ variant: 'danger', message }) } } catch (e) { @@ -261,7 +240,7 @@ export default function Send({ makerRunning, coinjoinInProcess }) { setAlert({ variant: 'success', message: 'Collaborative transaction started' }) success = true } else { - const message = await extractErrorMessage(res) + const { message } = await res.json() setAlert({ variant: 'danger', message }) } } catch (e) { From e99e2426e23d9d285c03d200dc0c4a6eb8493882 Mon Sep 17 00:00:00 2001 From: theborakompanioni Date: Mon, 21 Feb 2022 14:36:45 +0100 Subject: [PATCH 3/5] fix: adapt initial val for collaborators based on min val --- src/components/Send.jsx | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/components/Send.jsx b/src/components/Send.jsx index b236edac3..31bc5eaee 100644 --- a/src/components/Send.jsx +++ b/src/components/Send.jsx @@ -127,15 +127,15 @@ export default function Send({ makerRunning, coinjoinInProcess }) { const initialDestination = null const initialAccount = 0 const initialAmount = null - const initialNumCollaborators = () => { - return pseudoRandomNumber(minNumCollaborators + 1, minNumCollaborators + 3) + const initialNumCollaborators = (minValue) => { + return minValue + pseudoRandomNumber(1, 3) } const [destination, setDestination] = useState(initialDestination) const [account, setAccount] = useState(parseInt(location.state?.account, 10) || initialAccount) const [amount, setAmount] = useState(initialAmount) // see https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/docs/USAGE.md#try-out-a-coinjoin-using-sendpaymentpy - const [numCollaborators, setNumCollaborators] = useState(initialNumCollaborators()) + const [numCollaborators, setNumCollaborators] = useState(initialNumCollaborators(minNumCollaborators)) const [formIsValid, setFormIsValid] = useState(false) useEffect(() => { @@ -179,18 +179,16 @@ export default function Send({ makerRunning, coinjoinInProcess }) { const loadingMinimumMakerConfig = Api.postConfigGet(requestContext, { section: 'POLICY', field: 'minimum_makers' }) .then((res) => (res.ok ? res.json() : Promise.reject(new Error(res.message || 'Loading config value failed.')))) - .then((data) => setMinNumCollaborators(parseInt(data.configvalue, 10))) + .then((data) => { + const minimumMakers = parseInt(data.configvalue, 10) + setMinNumCollaborators(minimumMakers) + setNumCollaborators(initialNumCollaborators(minimumMakers)) + }) .catch((err) => { !abortCtrl.signal.aborted && setAlert({ variant: 'danger', message: err.message }) }) - Promise.all([loadingWalletInfo, loadingMinimumMakerConfig]) - .then((_) => { - if (minNumCollaborators && numCollaborators < minNumCollaborators) { - setNumCollaborators(minNumCollaborators) - } - }) - .finally(() => setIsLoading(false)) + Promise.all([loadingWalletInfo, loadingMinimumMakerConfig]).finally(() => setIsLoading(false)) return () => abortCtrl.abort() }, [wallet, setWalletInfo, walletInfo]) @@ -269,7 +267,7 @@ export default function Send({ makerRunning, coinjoinInProcess }) { setDestination(initialDestination) setAccount(initialAccount) setAmount(initialAmount) - setNumCollaborators(initialNumCollaborators()) + setNumCollaborators(initialNumCollaborators(minNumCollaborators)) setIsCoinjoin(false) form.reset() } From 57206fe1fd156d69934c29fed9035f9ba93a8d89 Mon Sep 17 00:00:00 2001 From: Thebora Kompanioni Date: Mon, 21 Feb 2022 17:22:29 +0100 Subject: [PATCH 4/5] Update src/libs/JmWalletApi.js Co-authored-by: Daniel <10026790+dnlggr@users.noreply.github.com> --- src/libs/JmWalletApi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/JmWalletApi.js b/src/libs/JmWalletApi.js index 323611104..6bacdd813 100644 --- a/src/libs/JmWalletApi.js +++ b/src/libs/JmWalletApi.js @@ -156,7 +156,7 @@ const postFreeze = async ({ walletName, token }, { utxo, freeze = true }) => { } /** - * Get the value of a specific config setting. Note values are always returned as string. + * Get the value of a specific config setting. Note that values are always returned as string. * * @returns an object with property `configvalue` as string */ From dbfd2b5947e547e2bbfbfe8e3c3dc27ebfb9c5b8 Mon Sep 17 00:00:00 2001 From: theborakompanioni Date: Mon, 21 Feb 2022 19:44:32 +0100 Subject: [PATCH 5/5] review: suggest reasonable default number of collaborators --- src/components/Send.jsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/Send.jsx b/src/components/Send.jsx index 31bc5eaee..1c8b50644 100644 --- a/src/components/Send.jsx +++ b/src/components/Send.jsx @@ -8,6 +8,9 @@ import { useCurrentWalletInfo, useSetCurrentWalletInfo, useCurrentWallet } from import { useSettings } from '../context/SettingsContext' import * as Api from '../libs/JmWalletApi' +// initial value for `minimum_markers` from the default joinmarket.cfg (last check on 2022-02-20 of v0.9.5) +const MINIMUM_MAKERS_DEFAULT_VAL = 4 + // not cryptographically random const pseudoRandomNumber = (min, max) => { return Math.round(Math.random() * (max - min)) + min @@ -122,13 +125,14 @@ export default function Send({ makerRunning, coinjoinInProcess }) { const [isSending, setIsSending] = useState(false) const [isCoinjoin, setIsCoinjoin] = useState(false) const [isCoinjoinOptionEnabled, setIsCoinjoinOptionEnabled] = useState(!makerRunning && !coinjoinInProcess) - const [minNumCollaborators, setMinNumCollaborators] = useState(4) // default value from an unchanged joinmarket.cfg (last check 2022-02-20) + const [minNumCollaborators, setMinNumCollaborators] = useState(MINIMUM_MAKERS_DEFAULT_VAL) const initialDestination = null const initialAccount = 0 const initialAmount = null const initialNumCollaborators = (minValue) => { - return minValue + pseudoRandomNumber(1, 3) + // always suggest a reasonably large number even when the configured minimum is unreasonably low + return Math.max(MINIMUM_MAKERS_DEFAULT_VAL, minValue) + pseudoRandomNumber(1, 3) } const [destination, setDestination] = useState(initialDestination)