mirror of
https://github.com/joinmarket-webui/jam.git
synced 2026-08-13 12:33:26 +02:00
fix(sweep): validate destination addresses against wallet network (#1355)
This commit is contained in:
parent
8789e63798
commit
82cc4762ff
8 changed files with 72 additions and 14 deletions
|
|
@ -1,5 +1,6 @@
|
|||
import { useMemo, useState } from 'react'
|
||||
import { yupResolver } from '@hookform/resolvers/yup'
|
||||
import type { Network } from 'bitcoin-address-validation'
|
||||
import { AlertTriangleIcon } from 'lucide-react'
|
||||
import { useFieldArray, useForm, useWatch, type SubmitHandler } from 'react-hook-form'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
|
|
@ -53,6 +54,7 @@ interface SweepFormProps {
|
|||
initialValues?: Partial<SweepFormValues>
|
||||
jars: Jar[]
|
||||
addressSummary: AddressSummary
|
||||
network: Network
|
||||
disabled?: boolean
|
||||
debug?: boolean
|
||||
}
|
||||
|
|
@ -62,6 +64,7 @@ export const SweepForm = ({
|
|||
onSubmit,
|
||||
jars,
|
||||
addressSummary,
|
||||
network,
|
||||
initialValues,
|
||||
disabled,
|
||||
debug,
|
||||
|
|
@ -82,10 +85,11 @@ export const SweepForm = ({
|
|||
minNumberOfDestinations,
|
||||
maxNumberOfDestinations,
|
||||
addressSummary,
|
||||
network,
|
||||
},
|
||||
t,
|
||||
),
|
||||
[minNumberOfDestinations, maxNumberOfDestinations, addressSummary, t],
|
||||
[minNumberOfDestinations, maxNumberOfDestinations, addressSummary, network, t],
|
||||
)
|
||||
const { formState, reset, register, control, setValue, handleSubmit, trigger } = useForm<
|
||||
SweepFormValues,
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { Network } from 'bitcoin-address-validation'
|
||||
import type { TFunction } from 'i18next'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { AddressSummary } from '@/context/JamWalletInfoContext'
|
||||
|
|
@ -12,13 +13,19 @@ import {
|
|||
|
||||
const t = ((key: string) => key) as unknown as TFunction<'translation', undefined>
|
||||
const validRegtestAddress = 'bcrt1qrnz0thqslhxu86th069r9j6y7ldkgs2tzgf5wx'
|
||||
const validMainnetAddress = '1BoatSLRHtKNngkdXEeobR76b53LETtpyT'
|
||||
|
||||
const validate = async (values: SweepFormValues, addressSummary = {} as AddressSummary) => {
|
||||
const validate = async (
|
||||
values: SweepFormValues,
|
||||
addressSummary = {} as AddressSummary,
|
||||
network: Network = Network.regtest,
|
||||
) => {
|
||||
return await sweepFormSchema(
|
||||
{
|
||||
minNumberOfDestinations: 1,
|
||||
maxNumberOfDestinations: 5,
|
||||
addressSummary,
|
||||
network,
|
||||
},
|
||||
t,
|
||||
).validate(values, { abortEarly: false })
|
||||
|
|
@ -49,6 +56,22 @@ describe('sweepFormSchema', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('rejects an address from the wrong network', async () => {
|
||||
await expect(
|
||||
validate({
|
||||
...buildSweepFormValuesDefaultValues(),
|
||||
destinations: [{ address: validMainnetAddress }],
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
inner: [
|
||||
expect.objectContaining({
|
||||
path: 'destinations[0].address',
|
||||
message: 'scheduler.feedback_destination_network_mismatch',
|
||||
}),
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects duplicate destination addresses', async () => {
|
||||
await expect(
|
||||
validate({
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import type { Network } from 'bitcoin-address-validation'
|
||||
import type { TFunction } from 'i18next'
|
||||
import * as yup from 'yup'
|
||||
import {
|
||||
|
|
@ -15,7 +16,7 @@ import {
|
|||
} from '@/constants/jam'
|
||||
import { JM_NG_DEFAULT_TUMBLER_PARAMS, type TumblerParameters } from '@/constants/jm'
|
||||
import type { AddressSummary } from '@/context/JamWalletInfoContext'
|
||||
import { isValidAddress } from '@/lib/formValidation'
|
||||
import { isAddressOnNetwork, isValidAddress } from '@/lib/formValidation'
|
||||
import { factorToPercentage, isValidNumber, percentageToFactor, pseudoRandomInteger } from '@/lib/utils'
|
||||
import type { Seconds } from '@/types/global'
|
||||
import { buildDestinationErrors, normalizeDestinationAddresses } from './destinationValidation'
|
||||
|
|
@ -115,14 +116,17 @@ export const sweepFormSchema = (
|
|||
minNumberOfDestinations,
|
||||
maxNumberOfDestinations,
|
||||
addressSummary,
|
||||
network,
|
||||
}: {
|
||||
minNumberOfDestinations: number
|
||||
maxNumberOfDestinations: number
|
||||
addressSummary: AddressSummary
|
||||
network: Network
|
||||
},
|
||||
t: TFunction<'translation', undefined>,
|
||||
): yup.ObjectSchema<SweepFormValues> => {
|
||||
const invalidDestinationAddressMessage = t('scheduler.feedback_invalid_destination_address')
|
||||
const networkMismatchDestinationAddressMessage = t('scheduler.feedback_destination_network_mismatch')
|
||||
const invalidNumberOfDestinationsMessage = t('send.feedback_invalid_number_of_destination_addresses', {
|
||||
// TODO: i18n
|
||||
defaultValue: 'Please provide between {{ min }} and {{ max }} destination addresses.',
|
||||
|
|
@ -137,20 +141,20 @@ export const sweepFormSchema = (
|
|||
.of(
|
||||
yup
|
||||
.object({
|
||||
// TODO: use formValidation#destinationAddressField ?
|
||||
address: yup
|
||||
.string()
|
||||
.transform((_, originalValue: unknown) =>
|
||||
typeof originalValue === 'string' ? normalizeDestinationAddresses([originalValue])[0] : '',
|
||||
)
|
||||
.defined()
|
||||
.test('valid-sweep-destination', invalidDestinationAddressMessage, function (value) {
|
||||
if (!isValidAddress(value)) {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}),
|
||||
.test('valid-sweep-destination', invalidDestinationAddressMessage, (value) => isValidAddress(value))
|
||||
// Only run once the address itself is valid, so an invalid address surfaces a
|
||||
// single, correct error instead of also reporting a network mismatch.
|
||||
.test(
|
||||
'sweep-destination-network-mismatch',
|
||||
networkMismatchDestinationAddressMessage,
|
||||
(value) => !isValidAddress(value) || isAddressOnNetwork(value, network),
|
||||
),
|
||||
})
|
||||
.required(),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -264,6 +264,7 @@ vi.mock('@/components/ui/jam/PageLoading', () => ({
|
|||
|
||||
vi.mock('@/context/JamWalletInfoContext', () => ({
|
||||
useJamWalletInfoContext: () => mocks.walletInfo,
|
||||
useDetectNetwork: () => ({ network: 'regtest' }),
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useApiClient', () => ({
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ import PageTitle from '@/components/ui/jam/PageTitle'
|
|||
import { isDevMode } from '@/constants/debugFeatures'
|
||||
import type { TumblerParameters } from '@/constants/jm'
|
||||
import { useJamSessionInfoContext } from '@/context/JamSessionInfoContext'
|
||||
import { useJamWalletInfoContext } from '@/context/JamWalletInfoContext'
|
||||
import { useDetectNetwork, useJamWalletInfoContext } from '@/context/JamWalletInfoContext'
|
||||
import { useApiClient } from '@/hooks/useApiClient'
|
||||
import { useFeeConfigValidation } from '@/hooks/useFeeConfigValidation'
|
||||
import { useRefreshSession } from '@/hooks/useRefreshSession'
|
||||
|
|
@ -65,6 +65,7 @@ export const SweepPage = ({ walletFileName }: SweepPageProps) => {
|
|||
const { rescanInfo, takerInfo, makerInfo } = useJamSessionInfoContext()
|
||||
const jmSession = useStore(jmSessionStore, (state) => state.state)
|
||||
const walletInfo = useJamWalletInfoContext()
|
||||
const { network } = useDetectNetwork()
|
||||
const { enabled: isDeveloperMode } = useDeveloperMode()
|
||||
|
||||
const [showFeeConfigDialog, setShowFeeConfigDialog] = useState(false)
|
||||
|
|
@ -452,6 +453,7 @@ export const SweepPage = ({ walletFileName }: SweepPageProps) => {
|
|||
<SweepForm
|
||||
jars={walletInfo.jars}
|
||||
addressSummary={walletInfo.addressSummary}
|
||||
network={network}
|
||||
disabled={isOperationDisabled || isWaitingSchedulerStart || isWaitingSchedulerStop}
|
||||
debug={isDeveloperMode}
|
||||
onSubmit={async (values) => {
|
||||
|
|
|
|||
|
|
@ -783,6 +783,7 @@
|
|||
"complete_wallet_subtitle": "This will use all of your non-frozen funds.",
|
||||
"description_destination_addresses": "A Scheduled Sweep will send all available funds to multiple destinations, splitting them up in random chunks.",
|
||||
"feedback_invalid_destination_address": "Please enter a valid destination address.",
|
||||
"feedback_destination_network_mismatch": "$t(send.feedback_destination_network_mismatch)",
|
||||
"feedback_reused_destination_address": "This address is already used. To preserve your privacy please choose another one.",
|
||||
"label_destination_input": "Destination {{ destination }}",
|
||||
"placeholder_destination_input": "Enter destination address...",
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ import {
|
|||
|
||||
const mainnetAddress = '1BoatSLRHtKNngkdXEeobR76b53LETtpyT'
|
||||
const testnetAddress = 'mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn'
|
||||
const regtestBech32Address = 'bcrt1q6rz28mcfaxtmd6v789l9rrlrusdprr9pz3cppk'
|
||||
const regtestLegacyAddressLabeledTestnet = 'mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV'
|
||||
|
||||
const addressSummary = {
|
||||
[mainnetAddress]: { address: mainnetAddress, used: false },
|
||||
|
|
@ -36,6 +38,19 @@ describe('isAddressOnNetwork', () => {
|
|||
it('returns false for unparseable input instead of throwing', () => {
|
||||
expect(isAddressOnNetwork('not-an-address', Network.mainnet)).toBe(false)
|
||||
})
|
||||
|
||||
it('treats testnet and regtest as interchangeable for base58 addresses ambiguous between the two', () => {
|
||||
// bech32 regtest addresses are unambiguous and match regtest directly...
|
||||
expect(isAddressOnNetwork(regtestBech32Address, Network.regtest)).toBe(true)
|
||||
expect(isAddressOnNetwork(regtestBech32Address, Network.testnet)).toBe(true) // TODO: can be detected and differentiated for `p2wpkh`
|
||||
// ...but a legacy address on a regtest wallet is labeled "testnet" by the library, so it
|
||||
// must still be accepted when the wallet's detected network is regtest.
|
||||
expect(isAddressOnNetwork(regtestLegacyAddressLabeledTestnet, Network.regtest)).toBe(true)
|
||||
expect(isAddressOnNetwork(regtestLegacyAddressLabeledTestnet, Network.testnet)).toBe(true)
|
||||
// mainnet is never ambiguous with testnet/regtest.
|
||||
expect(isAddressOnNetwork(mainnetAddress, Network.regtest)).toBe(false)
|
||||
expect(isAddressOnNetwork(regtestBech32Address, Network.mainnet)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isReusedAddress', () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { getAddressInfo, validate as isValidBitcoinAddress, type Network } from 'bitcoin-address-validation'
|
||||
import { getAddressInfo, Network, validate as isValidBitcoinAddress } from 'bitcoin-address-validation'
|
||||
import * as yup from 'yup'
|
||||
import type { AddressSummary } from '@/context/JamWalletInfoContext'
|
||||
import type { BitcoinAddress, BlockHeight, JarIndex } from '@/types/global'
|
||||
|
|
@ -11,9 +11,17 @@ import { isValidInteger } from './utils'
|
|||
export const isValidAddress = (value: unknown): value is BitcoinAddress =>
|
||||
typeof value === 'string' && isValidBitcoinAddress(value)
|
||||
|
||||
// Legacy (base58) addresses share the same version bytes on testnet and regtest, so
|
||||
// bitcoin-address-validation can't tell them apart and always labels them "testnet" -
|
||||
// only bech32 addresses carry a distinct "bcrt1" prefix. Treat the two as interchangeable
|
||||
// so a regtest wallet doesn't reject its own legacy-style addresses as "wrong network".
|
||||
const AMBIGUOUS_TESTNET_REGTEST_NETWORKS: ReadonlySet<Network> = new Set([Network.testnet, Network.regtest])
|
||||
|
||||
export const isAddressOnNetwork = (value: string, network: Network): boolean => {
|
||||
try {
|
||||
return getAddressInfo(value).network === network
|
||||
const addressNetwork = getAddressInfo(value).network
|
||||
if (addressNetwork === network) return true
|
||||
return AMBIGUOUS_TESTNET_REGTEST_NETWORKS.has(addressNetwork) && AMBIGUOUS_TESTNET_REGTEST_NETWORKS.has(network)
|
||||
} catch (_ignoredOnPurpose) {
|
||||
return false
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue