Skip to content

Commit 82cc476

Browse files
fix(sweep): validate destination addresses against wallet network (#1355)
1 parent 8789e63 commit 82cc476

8 files changed

Lines changed: 72 additions & 14 deletions

File tree

src/components/sweep/SweepForm.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useMemo, useState } from 'react'
22
import { yupResolver } from '@hookform/resolvers/yup'
3+
import type { Network } from 'bitcoin-address-validation'
34
import { AlertTriangleIcon } from 'lucide-react'
45
import { useFieldArray, useForm, useWatch, type SubmitHandler } from 'react-hook-form'
56
import { useTranslation } from 'react-i18next'
@@ -53,6 +54,7 @@ interface SweepFormProps {
5354
initialValues?: Partial<SweepFormValues>
5455
jars: Jar[]
5556
addressSummary: AddressSummary
57+
network: Network
5658
disabled?: boolean
5759
debug?: boolean
5860
}
@@ -62,6 +64,7 @@ export const SweepForm = ({
6264
onSubmit,
6365
jars,
6466
addressSummary,
67+
network,
6568
initialValues,
6669
disabled,
6770
debug,
@@ -82,10 +85,11 @@ export const SweepForm = ({
8285
minNumberOfDestinations,
8386
maxNumberOfDestinations,
8487
addressSummary,
88+
network,
8589
},
8690
t,
8791
),
88-
[minNumberOfDestinations, maxNumberOfDestinations, addressSummary, t],
92+
[minNumberOfDestinations, maxNumberOfDestinations, addressSummary, network, t],
8993
)
9094
const { formState, reset, register, control, setValue, handleSubmit, trigger } = useForm<
9195
SweepFormValues,

src/components/sweep/SweepFormSchema.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Network } from 'bitcoin-address-validation'
12
import type { TFunction } from 'i18next'
23
import { describe, expect, it } from 'vitest'
34
import type { AddressSummary } from '@/context/JamWalletInfoContext'
@@ -12,13 +13,19 @@ import {
1213

1314
const t = ((key: string) => key) as unknown as TFunction<'translation', undefined>
1415
const validRegtestAddress = 'bcrt1qrnz0thqslhxu86th069r9j6y7ldkgs2tzgf5wx'
16+
const validMainnetAddress = '1BoatSLRHtKNngkdXEeobR76b53LETtpyT'
1517

16-
const validate = async (values: SweepFormValues, addressSummary = {} as AddressSummary) => {
18+
const validate = async (
19+
values: SweepFormValues,
20+
addressSummary = {} as AddressSummary,
21+
network: Network = Network.regtest,
22+
) => {
1723
return await sweepFormSchema(
1824
{
1925
minNumberOfDestinations: 1,
2026
maxNumberOfDestinations: 5,
2127
addressSummary,
28+
network,
2229
},
2330
t,
2431
).validate(values, { abortEarly: false })
@@ -49,6 +56,22 @@ describe('sweepFormSchema', () => {
4956
})
5057
})
5158

59+
it('rejects an address from the wrong network', async () => {
60+
await expect(
61+
validate({
62+
...buildSweepFormValuesDefaultValues(),
63+
destinations: [{ address: validMainnetAddress }],
64+
}),
65+
).rejects.toMatchObject({
66+
inner: [
67+
expect.objectContaining({
68+
path: 'destinations[0].address',
69+
message: 'scheduler.feedback_destination_network_mismatch',
70+
}),
71+
],
72+
})
73+
})
74+
5275
it('rejects duplicate destination addresses', async () => {
5376
await expect(
5477
validate({

src/components/sweep/SweepFormSchema.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { Network } from 'bitcoin-address-validation'
12
import type { TFunction } from 'i18next'
23
import * as yup from 'yup'
34
import {
@@ -15,7 +16,7 @@ import {
1516
} from '@/constants/jam'
1617
import { JM_NG_DEFAULT_TUMBLER_PARAMS, type TumblerParameters } from '@/constants/jm'
1718
import type { AddressSummary } from '@/context/JamWalletInfoContext'
18-
import { isValidAddress } from '@/lib/formValidation'
19+
import { isAddressOnNetwork, isValidAddress } from '@/lib/formValidation'
1920
import { factorToPercentage, isValidNumber, percentageToFactor, pseudoRandomInteger } from '@/lib/utils'
2021
import type { Seconds } from '@/types/global'
2122
import { buildDestinationErrors, normalizeDestinationAddresses } from './destinationValidation'
@@ -115,14 +116,17 @@ export const sweepFormSchema = (
115116
minNumberOfDestinations,
116117
maxNumberOfDestinations,
117118
addressSummary,
119+
network,
118120
}: {
119121
minNumberOfDestinations: number
120122
maxNumberOfDestinations: number
121123
addressSummary: AddressSummary
124+
network: Network
122125
},
123126
t: TFunction<'translation', undefined>,
124127
): yup.ObjectSchema<SweepFormValues> => {
125128
const invalidDestinationAddressMessage = t('scheduler.feedback_invalid_destination_address')
129+
const networkMismatchDestinationAddressMessage = t('scheduler.feedback_destination_network_mismatch')
126130
const invalidNumberOfDestinationsMessage = t('send.feedback_invalid_number_of_destination_addresses', {
127131
// TODO: i18n
128132
defaultValue: 'Please provide between {{ min }} and {{ max }} destination addresses.',
@@ -137,20 +141,20 @@ export const sweepFormSchema = (
137141
.of(
138142
yup
139143
.object({
140-
// TODO: use formValidation#destinationAddressField ?
141144
address: yup
142145
.string()
143146
.transform((_, originalValue: unknown) =>
144147
typeof originalValue === 'string' ? normalizeDestinationAddresses([originalValue])[0] : '',
145148
)
146149
.defined()
147-
.test('valid-sweep-destination', invalidDestinationAddressMessage, function (value) {
148-
if (!isValidAddress(value)) {
149-
return false
150-
}
151-
152-
return true
153-
}),
150+
.test('valid-sweep-destination', invalidDestinationAddressMessage, (value) => isValidAddress(value))
151+
// Only run once the address itself is valid, so an invalid address surfaces a
152+
// single, correct error instead of also reporting a network mismatch.
153+
.test(
154+
'sweep-destination-network-mismatch',
155+
networkMismatchDestinationAddressMessage,
156+
(value) => !isValidAddress(value) || isAddressOnNetwork(value, network),
157+
),
154158
})
155159
.required(),
156160
)

src/components/sweep/SweepPage.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ vi.mock('@/components/ui/jam/PageLoading', () => ({
264264

265265
vi.mock('@/context/JamWalletInfoContext', () => ({
266266
useJamWalletInfoContext: () => mocks.walletInfo,
267+
useDetectNetwork: () => ({ network: 'regtest' }),
267268
}))
268269

269270
vi.mock('@/hooks/useApiClient', () => ({

src/components/sweep/SweepPage.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import PageTitle from '@/components/ui/jam/PageTitle'
2828
import { isDevMode } from '@/constants/debugFeatures'
2929
import type { TumblerParameters } from '@/constants/jm'
3030
import { useJamSessionInfoContext } from '@/context/JamSessionInfoContext'
31-
import { useJamWalletInfoContext } from '@/context/JamWalletInfoContext'
31+
import { useDetectNetwork, useJamWalletInfoContext } from '@/context/JamWalletInfoContext'
3232
import { useApiClient } from '@/hooks/useApiClient'
3333
import { useFeeConfigValidation } from '@/hooks/useFeeConfigValidation'
3434
import { useRefreshSession } from '@/hooks/useRefreshSession'
@@ -65,6 +65,7 @@ export const SweepPage = ({ walletFileName }: SweepPageProps) => {
6565
const { rescanInfo, takerInfo, makerInfo } = useJamSessionInfoContext()
6666
const jmSession = useStore(jmSessionStore, (state) => state.state)
6767
const walletInfo = useJamWalletInfoContext()
68+
const { network } = useDetectNetwork()
6869
const { enabled: isDeveloperMode } = useDeveloperMode()
6970

7071
const [showFeeConfigDialog, setShowFeeConfigDialog] = useState(false)
@@ -452,6 +453,7 @@ export const SweepPage = ({ walletFileName }: SweepPageProps) => {
452453
<SweepForm
453454
jars={walletInfo.jars}
454455
addressSummary={walletInfo.addressSummary}
456+
network={network}
455457
disabled={isOperationDisabled || isWaitingSchedulerStart || isWaitingSchedulerStop}
456458
debug={isDeveloperMode}
457459
onSubmit={async (values) => {

src/i18n/locales/en/translation.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@
783783
"complete_wallet_subtitle": "This will use all of your non-frozen funds.",
784784
"description_destination_addresses": "A Scheduled Sweep will send all available funds to multiple destinations, splitting them up in random chunks.",
785785
"feedback_invalid_destination_address": "Please enter a valid destination address.",
786+
"feedback_destination_network_mismatch": "$t(send.feedback_destination_network_mismatch)",
786787
"feedback_reused_destination_address": "This address is already used. To preserve your privacy please choose another one.",
787788
"label_destination_input": "Destination {{ destination }}",
788789
"placeholder_destination_input": "Enter destination address...",

src/lib/formValidation.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111

1212
const mainnetAddress = '1BoatSLRHtKNngkdXEeobR76b53LETtpyT'
1313
const testnetAddress = 'mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn'
14+
const regtestBech32Address = 'bcrt1q6rz28mcfaxtmd6v789l9rrlrusdprr9pz3cppk'
15+
const regtestLegacyAddressLabeledTestnet = 'mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV'
1416

1517
const addressSummary = {
1618
[mainnetAddress]: { address: mainnetAddress, used: false },
@@ -36,6 +38,19 @@ describe('isAddressOnNetwork', () => {
3638
it('returns false for unparseable input instead of throwing', () => {
3739
expect(isAddressOnNetwork('not-an-address', Network.mainnet)).toBe(false)
3840
})
41+
42+
it('treats testnet and regtest as interchangeable for base58 addresses ambiguous between the two', () => {
43+
// bech32 regtest addresses are unambiguous and match regtest directly...
44+
expect(isAddressOnNetwork(regtestBech32Address, Network.regtest)).toBe(true)
45+
expect(isAddressOnNetwork(regtestBech32Address, Network.testnet)).toBe(true) // TODO: can be detected and differentiated for `p2wpkh`
46+
// ...but a legacy address on a regtest wallet is labeled "testnet" by the library, so it
47+
// must still be accepted when the wallet's detected network is regtest.
48+
expect(isAddressOnNetwork(regtestLegacyAddressLabeledTestnet, Network.regtest)).toBe(true)
49+
expect(isAddressOnNetwork(regtestLegacyAddressLabeledTestnet, Network.testnet)).toBe(true)
50+
// mainnet is never ambiguous with testnet/regtest.
51+
expect(isAddressOnNetwork(mainnetAddress, Network.regtest)).toBe(false)
52+
expect(isAddressOnNetwork(regtestBech32Address, Network.mainnet)).toBe(false)
53+
})
3954
})
4055

4156
describe('isReusedAddress', () => {

src/lib/formValidation.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getAddressInfo, validate as isValidBitcoinAddress, type Network } from 'bitcoin-address-validation'
1+
import { getAddressInfo, Network, validate as isValidBitcoinAddress } from 'bitcoin-address-validation'
22
import * as yup from 'yup'
33
import type { AddressSummary } from '@/context/JamWalletInfoContext'
44
import type { BitcoinAddress, BlockHeight, JarIndex } from '@/types/global'
@@ -11,9 +11,17 @@ import { isValidInteger } from './utils'
1111
export const isValidAddress = (value: unknown): value is BitcoinAddress =>
1212
typeof value === 'string' && isValidBitcoinAddress(value)
1313

14+
// Legacy (base58) addresses share the same version bytes on testnet and regtest, so
15+
// bitcoin-address-validation can't tell them apart and always labels them "testnet" -
16+
// only bech32 addresses carry a distinct "bcrt1" prefix. Treat the two as interchangeable
17+
// so a regtest wallet doesn't reject its own legacy-style addresses as "wrong network".
18+
const AMBIGUOUS_TESTNET_REGTEST_NETWORKS: ReadonlySet<Network> = new Set([Network.testnet, Network.regtest])
19+
1420
export const isAddressOnNetwork = (value: string, network: Network): boolean => {
1521
try {
16-
return getAddressInfo(value).network === network
22+
const addressNetwork = getAddressInfo(value).network
23+
if (addressNetwork === network) return true
24+
return AMBIGUOUS_TESTNET_REGTEST_NETWORKS.has(addressNetwork) && AMBIGUOUS_TESTNET_REGTEST_NETWORKS.has(network)
1725
} catch (_ignoredOnPurpose) {
1826
return false
1927
}

0 commit comments

Comments
 (0)