diff --git a/__tests__/bitcoinUtils.test.ts b/__tests__/bitcoinUtils.test.ts index ed8b91e..383bca6 100644 --- a/__tests__/bitcoinUtils.test.ts +++ b/__tests__/bitcoinUtils.test.ts @@ -127,31 +127,52 @@ describe('decodeBitcoinAddress', () => { }) describe('isPayableBitcoinAddress', () => { - it('accepts mainnet addresses of every script type', () => { + // The flag is passed explicitly rather than left to default, so these pin BOTH sides + // of the rule regardless of what `__DEV__` happens to be under jest. + const RELEASE = {allowNonMainnet: false} + const DEBUG = {allowNonMainnet: true} + + it('accepts mainnet addresses of every script type, in any build', () => { for (const address of [P2PKH, P2SH, P2WPKH, P2WSH, P2TR]) { - expect(isPayableBitcoinAddress(address)).toBe(true) + expect(isPayableBitcoinAddress(address, RELEASE)).toBe(true) + expect(isPayableBitcoinAddress(address, DEBUG)).toBe(true) } }) /** * The loss vector this exists for: a CDK fakewallet topup hands the user a REGTEST - * deposit address, it sits in their clipboard, and Pay auto-pastes it. A regtest - * address is never a payment — it is a mistake, and an irreversible one if a mint - * broadcasts against it. + * deposit address, it sits in their clipboard, and Pay auto-pastes it. In a build a + * user can install, that is never a payment — it is a mistake. */ - it('refuses the CDK fakewallet regtest address', () => { - expect(isPayableBitcoinAddress(FAKEWALLET_REGTEST)).toBe(false) + it('refuses the CDK fakewallet regtest address in a release build', () => { + expect(isPayableBitcoinAddress(FAKEWALLET_REGTEST, RELEASE)).toBe(false) }) - it('refuses every non-mainnet address', () => { - expect(isPayableBitcoinAddress(TESTNET_P2WPKH)).toBe(false) - expect(isPayableBitcoinAddress(TESTNET_P2PKH)).toBe(false) - expect(isPayableBitcoinAddress('bcrt1qw508d6qejxtdg4y5r3zarvary0c5xw7k1234a')).toBe(false) + /** + * ...and accepts it in a debug build, because settling a melt against the fakewallet's + * regtest chain is the only way to exercise the onchain payout rail end to end. + */ + it('accepts the CDK fakewallet regtest address in a debug build', () => { + expect(isPayableBitcoinAddress(FAKEWALLET_REGTEST, DEBUG)).toBe(true) }) - it('refuses input that is not an address at all', () => { - expect(isPayableBitcoinAddress('nope')).toBe(false) - expect(isPayableBitcoinAddress('')).toBe(false) + it('refuses every non-mainnet address in a release build', () => { + expect(isPayableBitcoinAddress(TESTNET_P2WPKH, RELEASE)).toBe(false) + expect(isPayableBitcoinAddress(TESTNET_P2PKH, RELEASE)).toBe(false) + }) + + it('accepts testnet as well as regtest in a debug build', () => { + expect(isPayableBitcoinAddress(TESTNET_P2WPKH, DEBUG)).toBe(true) + expect(isPayableBitcoinAddress(TESTNET_P2PKH, DEBUG)).toBe(true) + }) + + // The network flag relaxes the NETWORK, never the checksum. A debug build must not + // become a build that pays typos. + it('refuses input that is not an address at all, even in a debug build', () => { + expect(isPayableBitcoinAddress('nope', DEBUG)).toBe(false) + expect(isPayableBitcoinAddress('', DEBUG)).toBe(false) + // Corrupted base58 checksum, mainnet prefix. + expect(isPayableBitcoinAddress('1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN3', DEBUG)).toBe(false) }) }) diff --git a/src/services/bitcoin/bitcoinUtils.ts b/src/services/bitcoin/bitcoinUtils.ts index 9133c2b..258e7db 100644 --- a/src/services/bitcoin/bitcoinUtils.ts +++ b/src/services/bitcoin/bitcoinUtils.ts @@ -134,26 +134,57 @@ export const decodeBitcoinAddress = ( export const isBitcoinAddress = (address: string): boolean => decodeBitcoinAddress(address) !== undefined +/** + * Are non-mainnet addresses payable in this build? + * + * Debug builds only. Development is the one situation where paying a regtest address is + * the POINT rather than a mistake: the CDK fakewallet backend settles onchain melts + * against a regtest chain, so a release-only guard would make the whole rail + * untestable end to end. + * + * A release build always refuses. The flag cannot be turned on by a user, and there is + * no setting for it — the only way to pay a testnet address is to be running a debug + * bundle you built yourself. + * + * `typeof` guarded because this module is also loaded by tests outside the React Native + * environment, where `__DEV__` is not defined. + */ +export const ALLOW_NON_MAINNET_PAY = + typeof __DEV__ !== 'undefined' && __DEV__ === true + /** * Is this an address Minibits is allowed to pay? * - * Mainnet only. The wallet holds mainnet-backed ecash and the mints melt to the real - * chain, so a testnet or regtest address is never a payment — it is a mistake, and an - * irreversible one if a mint broadcasts against it. + * Mainnet only, in any build a user can install. The wallet holds mainnet-backed ecash + * and the mints melt to the real chain, so a testnet or regtest address is not a + * payment — it is a mistake, and an irreversible one if a mint broadcasts against it. * * This is not hypothetical. The CDK fakewallet backend hands out REGTEST deposit * addresses (`bcrt1q…`) for onchain topup quotes, so anyone testing this wallet ends * up with one in their clipboard. Pasting it back into Pay must fail loudly, not * quietly reach the mint. * + * `allowNonMainnet` defaults to `ALLOW_NON_MAINNET_PAY` (debug builds) and is an + * explicit parameter rather than a global read so that the decision is testable and so + * that callers cannot accidentally widen it — passing `true` from production code would + * be a visible thing to review. + * * Kept separate from `decodeBitcoinAddress` on purpose: decoding tells you WHAT an * address is (and needs to recognise testnet in order to say so), while this decides - * whether we are willing to send money to it. Collapsing the two would leave us - * unable to tell "that is not an address" apart from "that is not OUR network", and - * the second deserves its own error message. + * whether we are willing to send money to it. Collapsing the two would leave us unable + * to tell "that is not an address" apart from "that is not OUR network", and the second + * deserves its own error message. */ -export const isPayableBitcoinAddress = (address: string): boolean => - decodeBitcoinAddress(address)?.network === 'mainnet' +export const isPayableBitcoinAddress = ( + address: string, + options?: {allowNonMainnet?: boolean}, +): boolean => { + const decoded = decodeBitcoinAddress(address) + if (!decoded) return false + if (decoded.network === 'mainnet') return true + + return (options?.allowNonMainnet ?? ALLOW_NON_MAINNET_PAY) === true +} export type Bip21Data = { address: string @@ -244,6 +275,7 @@ export const findBitcoinAddress = (text: string): string | undefined => { } export const BitcoinUtils = { + ALLOW_NON_MAINNET_PAY, decodeBitcoinAddress, isBitcoinAddress, isPayableBitcoinAddress, diff --git a/src/services/incomingParser.ts b/src/services/incomingParser.ts index bae3906..f51233e 100644 --- a/src/services/incomingParser.ts +++ b/src/services/incomingParser.ts @@ -40,11 +40,15 @@ export type BtcAddressData = { /** * Refuse a Bitcoin address we are not willing to pay. * - * Mainnet only, and checksum-verified. Both failures get their own message, because - * "that is not a Bitcoin address" and "that is a Bitcoin address on the wrong network" - * are completely different mistakes — and the second one is the likely one: the CDK - * fakewallet hands out REGTEST addresses (`bcrt1q…`) for onchain topup quotes, so anyone - * testing this wallet has one sitting in their clipboard. + * Checksum-verified, and mainnet-only in any build a user can install. Both failures get + * their own message, because "that is not a Bitcoin address" and "that is a Bitcoin + * address on the wrong network" are completely different mistakes — and the second is + * the likely one: the CDK fakewallet hands out REGTEST addresses (`bcrt1q…`) for onchain + * topup quotes, so anyone testing this wallet has one sitting in their clipboard. + * + * Debug builds accept non-mainnet, because settling an onchain melt against the + * fakewallet's regtest chain is the only way to exercise this rail end to end. See + * `BitcoinUtils.ALLOW_NON_MAINNET_PAY`. */ const assertPayableBtcAddress = function (address: string): string { const decoded = BitcoinUtils.decodeBitcoinAddress(address) @@ -56,7 +60,7 @@ const assertPayableBtcAddress = function (address: string): string { }) } - if (decoded.network !== 'mainnet') { + if (!BitcoinUtils.isPayableBitcoinAddress(decoded.address)) { throw new AppError( Err.VALIDATION_ERROR, `This is a ${decoded.network} Bitcoin address. Minibits can only pay to mainnet addresses.`, @@ -64,6 +68,12 @@ const assertPayableBtcAddress = function (address: string): string { ) } + if (decoded.network !== 'mainnet') { + log.warn('[assertPayableBtcAddress] Paying a NON-MAINNET address (debug build)', { + network: decoded.network, + }) + } + return decoded.address } diff --git a/src/services/wallet/operations/transferOperationApi.ts b/src/services/wallet/operations/transferOperationApi.ts index 068c66a..a669761 100644 --- a/src/services/wallet/operations/transferOperationApi.ts +++ b/src/services/wallet/operations/transferOperationApi.ts @@ -198,6 +198,11 @@ async function prepare(input: PrepareTransferInput): Promise