From 3ee077621f846d031361f0a2512efe737250ec61 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Tue, 4 Aug 2026 11:47:45 -0700 Subject: [PATCH 1/9] Fix saveTx failing on a disconnected engine UtxoEngineProcessor's updateProgressRatio threw 'No addresses to process' whenever the engine had zero subscribed addresses, which is the state of a wallet whose blockbook sockets are all down. saveTx reaches this code via processUtxos after the transaction is already saved and its inputs marked spent, so the throw turned an already-successful send into a reported failure. Skip the progress update instead; there is no denominator to compute a ratio from without subscribed addresses. --- .../utxobased/engine/UtxoEngineProcessor.ts | 8 +- test/common/utxobased/engine/saveTx.spec.ts | 168 ++++++++++++++++++ 2 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 test/common/utxobased/engine/saveTx.spec.ts diff --git a/src/common/utxobased/engine/UtxoEngineProcessor.ts b/src/common/utxobased/engine/UtxoEngineProcessor.ts index 6f6b16fa..22378251 100644 --- a/src/common/utxobased/engine/UtxoEngineProcessor.ts +++ b/src/common/utxobased/engine/UtxoEngineProcessor.ts @@ -170,8 +170,12 @@ export function makeUtxoEngineProcessor( // Increment the processed count processedCount = processedCount + 1 - // If we have no addresses, we should not have not yet began processing. - if (expectedProcessCount === 0) throw new Error('No addresses to process') + // With no subscribed addresses there is no denominator to compute a + // progress ratio from. This is a legitimate state when processing is + // driven by saveTx on a disconnected engine (no blockbook sockets, so + // nothing is subscribed), so skip the progress update rather than fail + // the caller's data write. + if (expectedProcessCount === 0) return const percent = processedCount / expectedProcessCount if (percent - processedPercent > CACHE_THROTTLE || percent === 1) { diff --git a/test/common/utxobased/engine/saveTx.spec.ts b/test/common/utxobased/engine/saveTx.spec.ts new file mode 100644 index 00000000..57759767 --- /dev/null +++ b/test/common/utxobased/engine/saveTx.spec.ts @@ -0,0 +1,168 @@ +import { assert } from 'chai' +import { makeMemoryDisklet, makeNodeDisklet } from 'disklet' +import { + EdgeCorePluginOptions, + EdgeCurrencyEngine, + EdgeCurrencyEngineCallbacks, + EdgeCurrencyEngineOptions, + EdgeCurrencyPlugin, + EdgeCurrencyTools, + EdgeTransaction, + JsonObject, + makeFakeIo +} from 'edge-core-js' +import { describe, it } from 'mocha' + +import edgeCorePlugins from '../../../../src/index' +import { noOp, testLog } from '../../../util/testLog' +import { makeFakeNativeIo } from '../../../utils' +import { fixtures } from './engine.fixtures/index' + +const [tests] = fixtures + +/** + * A transaction paying to an address that the dummy-data wallet owns + * (scriptPubkey a9142244... with a bip49 path), spending a UTXO that the + * dummy-data set holds (19e59364...:0). This makes saveTx's + * getOwnUtxosFromTx return both a spent input and a new output, which drives + * processUtxos -> processDataLayerUtxos -> updateProgressRatio. + */ +const OWN_SCRIPT_PUBKEY = 'a9142244ce86d664e85801f7eb2a56dd35afd268212587' +const SPENT_TXID = + '19e59364daf34d97ed6584e9e978f3e2375adea9a4561a83d2066d92a010ba13' +const NEW_TXID = + 'f00dbabef00dbabef00dbabef00dbabef00dbabef00dbabef00dbabef00dbabe' + +describe('saveTx on a disconnected engine', function () { + it('resolves with zero subscribed addresses', async function () { + this.timeout(10000) + + const fakeIo = makeFakeIo() + const fixtureDisklet = makeNodeDisklet(tests.dummyDataPath) + const fakeIoDisklet = makeMemoryDisklet() + const nativeIo = makeFakeNativeIo() + + // Preload the wallet's data layer with the dummy dataset so the wallet + // owns addresses and UTXOs: + const migrate = async (dir: string): Promise => { + const files = await fixtureDisklet.list(dir) + await Promise.all( + Object.entries(files).map(async ([path, type]) => { + if (type === 'folder') await migrate(path) + if (type === 'file') + await fixtureDisklet + .getText(path) + .then(async data => await fakeIoDisklet.setText(path, data)) + }) + ) + } + await migrate('tables') + + const pluginOpts: EdgeCorePluginOptions = { + initOptions: {}, + io: { + ...fakeIo, + random: () => Uint8Array.from(tests.key) + }, + log: testLog, + infoPayload: {}, + nativeIo, + pluginDisklet: fakeIoDisklet + } + const factory = edgeCorePlugins[tests.pluginId] + if (typeof factory !== 'function') + throw new Error(`Missing plugin factory for ${tests.pluginId}`) + const plugin: EdgeCurrencyPlugin = factory(pluginOpts) as any + + const tools: EdgeCurrencyTools = await plugin.makeCurrencyTools() + const privateKeys = await tools.createPrivateKey(tests.WALLET_TYPE) + Object.assign(privateKeys, { coinType: 0, format: tests.WALLET_FORMAT }) + const publicKeys = await tools.derivePublicKey({ + type: tests.WALLET_TYPE, + keys: privateKeys, + id: '!' + }) + const keys: JsonObject = { ...privateKeys, ...publicKeys } + + const callbacks: EdgeCurrencyEngineCallbacks = { + onAddressChanged: noOp, + onAddressesChecked: noOp, + onBalanceChanged: noOp, + onBlockHeightChanged: noOp, + onNewTokens: noOp, + onSeenTxCheckpoint: noOp, + onStakingStatusChanged: noOp, + onTokenBalanceChanged: noOp, + onTransactions: noOp, + onTransactionsChanged: noOp, + onTxidsChanged: noOp, + onUnactivatedTokenIdsChanged: noOp, + onWcNewContractCall: noOp + } + const engineOpts: EdgeCurrencyEngineOptions = { + callbacks, + log: testLog, + walletLocalDisklet: fakeIoDisklet, + walletLocalEncryptedDisklet: fakeIoDisklet, + customTokens: {}, + enabledTokenIds: [], + userSettings: {} + } + + // The engine is never started, so no blockbook connects and no address + // is ever subscribed. This is the same state as a running engine whose + // sockets are all down: taskCache.addressSubscribeCache is empty. + const engine: EdgeCurrencyEngine = await plugin.makeCurrencyEngine( + { type: tests.WALLET_TYPE, keys, id: '!' }, + engineOpts + ) + + const scriptPubkeyBuffer = Buffer.from(OWN_SCRIPT_PUBKEY, 'hex') + const edgeTx: EdgeTransaction = { + blockHeight: 0, + currencyCode: 'TESTBTC', + date: 1723000000, + isSend: true, + memos: [], + nativeAmount: '-50000', + networkFee: '1000', + networkFees: [], + otherParams: { + psbt: { + base64: '', + inputs: [ + { + hash: Buffer.from(SPENT_TXID, 'hex').reverse(), + index: 0, + value: 16250000, + scriptPubkey: scriptPubkeyBuffer, + sequence: 0xffffffff + } + ], + outputs: [ + { + value: 16200000, + scriptPubkey: scriptPubkeyBuffer + } + ] + } + }, + ourReceiveAddresses: [], + signedTx: '0100000000', + tokenId: null, + txid: NEW_TXID, + walletId: '!' + } + + // The regression under test: updateProgressRatio used to throw + // 'No addresses to process' here, failing saveTx AFTER the transaction + // had already been saved and its inputs marked spent. + await engine.saveTx(edgeTx) + + const txs = await engine.getTransactions({ tokenId: null }) + assert.isTrue( + txs.some(tx => tx.txid === NEW_TXID), + 'saved transaction should be listed' + ) + }) +}) From 11c8184c395c6bfc0c0385b953e6c3f5b6485998 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:02:43 -0700 Subject: [PATCH 2/9] fixup! Fix saveTx failing on a disconnected engine --- .../utxobased/engine/UtxoEngineProcessor.ts | 15 ++-- test/common/utxobased/engine/saveTx.spec.ts | 74 +++++++++++++++++-- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/common/utxobased/engine/UtxoEngineProcessor.ts b/src/common/utxobased/engine/UtxoEngineProcessor.ts index 22378251..f44bf9b4 100644 --- a/src/common/utxobased/engine/UtxoEngineProcessor.ts +++ b/src/common/utxobased/engine/UtxoEngineProcessor.ts @@ -167,16 +167,17 @@ export function makeUtxoEngineProcessor( const expectedProcessCount = Object.keys(taskCache.addressSubscribeCache).length * processesPerAddress - // Increment the processed count - processedCount = processedCount + 1 - // With no subscribed addresses there is no denominator to compute a - // progress ratio from. This is a legitimate state when processing is - // driven by saveTx on a disconnected engine (no blockbook sockets, so - // nothing is subscribed), so skip the progress update rather than fail - // the caller's data write. + // progress ratio from. The cache is empty when the engine is not running + // (never started, or stopped and its task cache cleared) while saveTx + // still drives UTXO processing. Skip the progress update, without + // counting the call as progress, rather than fail the caller's data + // write. if (expectedProcessCount === 0) return + // Increment the processed count + processedCount = processedCount + 1 + const percent = processedCount / expectedProcessCount if (percent - processedPercent > CACHE_THROTTLE || percent === 1) { log( diff --git a/test/common/utxobased/engine/saveTx.spec.ts b/test/common/utxobased/engine/saveTx.spec.ts index 57759767..e9f00d97 100644 --- a/test/common/utxobased/engine/saveTx.spec.ts +++ b/test/common/utxobased/engine/saveTx.spec.ts @@ -32,6 +32,8 @@ const SPENT_TXID = '19e59364daf34d97ed6584e9e978f3e2375adea9a4561a83d2066d92a010ba13' const NEW_TXID = 'f00dbabef00dbabef00dbabef00dbabef00dbabef00dbabef00dbabef00dbabe' +const SECOND_TXID = + 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' describe('saveTx on a disconnected engine', function () { it('resolves with zero subscribed addresses', async function () { @@ -72,7 +74,7 @@ describe('saveTx on a disconnected engine', function () { const factory = edgeCorePlugins[tests.pluginId] if (typeof factory !== 'function') throw new Error(`Missing plugin factory for ${tests.pluginId}`) - const plugin: EdgeCurrencyPlugin = factory(pluginOpts) as any + const plugin = factory(pluginOpts) as EdgeCurrencyPlugin const tools: EdgeCurrencyTools = await plugin.makeCurrencyTools() const privateKeys = await tools.createPrivateKey(tests.WALLET_TYPE) @@ -84,13 +86,22 @@ describe('saveTx on a disconnected engine', function () { }) const keys: JsonObject = { ...privateKeys, ...publicKeys } + // Track progress-side emissions: an engine that never ran a sync must + // never report itself fully synced, and must never advance the seen-tx + // checkpoint, no matter how many saveTx calls process UTXOs. + const addressesCheckedRatios: number[] = [] + const seenTxCheckpoints: string[] = [] const callbacks: EdgeCurrencyEngineCallbacks = { onAddressChanged: noOp, - onAddressesChecked: noOp, + onAddressesChecked: (ratio: number) => { + addressesCheckedRatios.push(ratio) + }, onBalanceChanged: noOp, onBlockHeightChanged: noOp, onNewTokens: noOp, - onSeenTxCheckpoint: noOp, + onSeenTxCheckpoint: (checkpoint: string) => { + seenTxCheckpoints.push(checkpoint) + }, onStakingStatusChanged: noOp, onTokenBalanceChanged: noOp, onTransactions: noOp, @@ -109,9 +120,11 @@ describe('saveTx on a disconnected engine', function () { userSettings: {} } - // The engine is never started, so no blockbook connects and no address - // is ever subscribed. This is the same state as a running engine whose - // sockets are all down: taskCache.addressSubscribeCache is empty. + // The engine is never started, so nothing populates the address + // subscribe cache (startEngine's initializeAddressSubscriptions and + // setLookAhead are what fill it, network-free). A stopped engine is in + // the same state: stop() clears the task cache. saveTx must still + // resolve there. const engine: EdgeCurrencyEngine = await plugin.makeCurrencyEngine( { type: tests.WALLET_TYPE, keys, id: '!' }, engineOpts @@ -164,5 +177,54 @@ describe('saveTx on a disconnected engine', function () { txs.some(tx => tx.txid === NEW_TXID), 'saved transaction should be listed' ) + + // A second saveTx spends the first transaction's output. The first + // call's setLookAhead derived lookahead addresses into the subscribe + // cache, so this call has a nonzero progress denominator; the first + // call's denominator-less pass must not have counted as progress. + const edgeTx2: EdgeTransaction = { + ...edgeTx, + otherParams: { + psbt: { + base64: '', + inputs: [ + { + hash: Buffer.from(NEW_TXID, 'hex').reverse(), + index: 0, + value: 16200000, + scriptPubkey: scriptPubkeyBuffer, + sequence: 0xffffffff + } + ], + outputs: [ + { + value: 16150000, + scriptPubkey: scriptPubkeyBuffer + } + ] + } + }, + txid: SECOND_TXID + } + await engine.saveTx(edgeTx2) + + const txsAfterSecond = await engine.getTransactions({ tokenId: null }) + assert.isTrue( + txsAfterSecond.some(tx => tx.txid === SECOND_TXID), + 'second saved transaction should be listed' + ) + + // Progress bookkeeping must not fabricate a completed sync out of + // saveTx-driven processing: an engine that never synced must not emit a + // fully-synced ratio and must not advance the seen-tx checkpoint. + assert.notInclude( + addressesCheckedRatios, + 1, + 'saveTx must not produce a fully-synced progress emission' + ) + assert.isEmpty( + seenTxCheckpoints, + 'saveTx must not advance the seen-tx checkpoint' + ) }) }) From 707f6490d461ae0ec3cbc704ffadcf599d841d0c Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:15:02 -0700 Subject: [PATCH 3/9] fixup! Fix saveTx failing on a disconnected engine --- src/common/utxobased/engine/UtxoEngineProcessor.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/common/utxobased/engine/UtxoEngineProcessor.ts b/src/common/utxobased/engine/UtxoEngineProcessor.ts index f44bf9b4..1c4d9b3f 100644 --- a/src/common/utxobased/engine/UtxoEngineProcessor.ts +++ b/src/common/utxobased/engine/UtxoEngineProcessor.ts @@ -376,6 +376,13 @@ export function makeUtxoEngineProcessor( serverStates.stop() clearTaskCache() clearPendingTimeouts() + // The progress counters describe the sync round that just ended. + // Clearing the cache without them would let leftover counts combine + // with a smaller refilled cache (saveTx-driven setLookAhead, or + // addGapLimitAddresses) into a bogus completed-sync emission and a + // seen-tx checkpoint advance on an engine that is not syncing. + processedCount = 0 + processedPercent = 0 running = false }, From 5989abc15ca2c5ed18f58e4c35c5e7b6626551a2 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Tue, 4 Aug 2026 11:47:58 -0700 Subject: [PATCH 4/9] Resolve broadcast ambiguity before reporting failure ServerStates.broadcastTx submits the signed transaction to every connected blockbook (or every NOWNode HTTP fallback) and rejects only when all of them fail, but a server can relay the transaction to the network and still return an error or fail to respond. Before rejecting, query the network for the txid and treat a known transaction as a successful broadcast. Also stop throwing on a mismatched broadcast-response txid in UtxoEngine.broadcastTx: the transaction is already on the network at that point, so log a warning instead of reporting a send failure. --- CHANGELOG.md | 2 + src/common/utxobased/engine/ServerStates.ts | 87 ++++++++++++++++++--- src/common/utxobased/engine/UtxoEngine.ts | 6 +- 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32d680f..ed7789af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- fixed: Failed sends after a successful broadcast: `saveTx` no longer fails on a disconnected engine, and a broadcast error is reported as a failure only after verifying the transaction is unknown to the network. + ## 3.11.0 (2026-07-13) - added: Support the `-wif:` protohandler prefix (e.g. `bch-wif:`) in `parseUri` so CashStamps private keys can be swept. diff --git a/src/common/utxobased/engine/ServerStates.ts b/src/common/utxobased/engine/ServerStates.ts index 18d289ab..6544c193 100644 --- a/src/common/utxobased/engine/ServerStates.ts +++ b/src/common/utxobased/engine/ServerStates.ts @@ -1,3 +1,4 @@ +import { asMaybe, asObject, asString } from 'cleaners' import { EdgeIo, EdgeLog, EdgeTransaction } from 'edge-core-js/types' import { parse } from 'uri-js' @@ -367,10 +368,76 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { const instance: ServerStates = { async broadcastTx(transaction: EdgeTransaction): Promise { + // Query the network for the transaction to determine whether a failed + // broadcast actually reached the network anyway. A server can relay + // the transaction and still return an error or fail to respond, so an + // error from every server does not prove the transaction wasn't sent. + const isTxidKnown = async (txid: string): Promise => { + // Ask connected blockbook instances first: + for (const uri of Object.keys(serverStatesCache)) { + const { blockbook } = serverStatesCache[uri] + if (blockbook == null || !blockbook.isConnected) continue + const known = await blockbook + .fetchTransaction(txid) + .then(() => true) + .catch(() => false) + if (known) return true + } + + // Fall back to the NOWNode HTTP API when no blockbook is connected: + const { nowNodesApiKey } = initOptions + if (nowNodesApiKey == null) return false + const nowNodeUris = serverConfigs + .filter(config => config.type === 'blockbook-nownode') + .map(config => config.uris) + .flat(1) + for (const uri of nowNodeUris) { + const known = await io + .fetchCors(`${uri}/api/v2/tx/${txid}`, { + headers: { + 'api-key': nowNodesApiKey + } + }) + .then(async response => { + if (!response.ok) return false + const json = await response.json() + return asMaybe(asTxQueryResponse)(json)?.txid === txid + }) + .catch(() => false) + if (known) return true + } + return false + } + return await new Promise((resolve, reject) => { let resolved = false let bad = 0 + // Reject with the given error only when the transaction is verifiably + // absent from the network; a transaction that reached the network + // despite the error is a successful broadcast. + const rejectUnlessTxKnown = (error?: Error): void => { + const fail = (): void => { + const msg = error != null ? `With error ${error.message}` : '' + log.error( + `broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}` + ) + reject(error) + } + isTxidKnown(transaction.txid) + .then(known => { + if (!known) return fail() + if (!resolved) { + resolved = true + log.warn( + `broadcastTx errored, but txid ${transaction.txid} is known to the network; treating broadcast as a success` + ) + resolve(transaction.txid) + } + }) + .catch(fail) + } + const wsUris = Object.keys(serverStatesCache).filter( uri => serverStatesCache[uri].blockbook != null ) @@ -394,11 +461,7 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { }) .catch((e?: Error) => { if (++bad === wsUris.length) { - const msg = e != null ? `With error ${e.message}` : '' - log.error( - `broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}` - ) - reject(e) + rejectUnlessTxKnown(e) } }) } @@ -464,11 +527,7 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { }) .catch((e?: Error) => { if (++bad === nowNodeUris.length) { - const msg = e != null ? `With error ${e.message}` : '' - log.error( - `broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}` - ) - reject(e) + rejectUnlessTxKnown(e) } }) } @@ -670,3 +729,11 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { return instance } + +/** + * Minimal shape of a Blockbook REST `/api/v2/tx/` response, used only + * to confirm that a transaction is known to the network. + */ +const asTxQueryResponse = asObject({ + txid: asString +}) diff --git a/src/common/utxobased/engine/UtxoEngine.ts b/src/common/utxobased/engine/UtxoEngine.ts index cc4ff6bd..bc238784 100644 --- a/src/common/utxobased/engine/UtxoEngine.ts +++ b/src/common/utxobased/engine/UtxoEngine.ts @@ -432,7 +432,11 @@ export async function makeUtxoEngine( throw err }) if (id !== transaction.txid) { - throw new Error('broadcast response txid does not match original') + // The transaction is on the network at this point, so a mismatched + // response txid must not be reported as a send failure. + log.warn( + `broadcast response txid mismatch: expected ${transaction.txid} received ${id}` + ) } return transaction }, From 80e9342455c238e2e048afde38a41369c5f71d24 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:02:59 -0700 Subject: [PATCH 5/9] fixup! Resolve broadcast ambiguity before reporting failure --- src/common/utxobased/engine/UtxoEngine.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/utxobased/engine/UtxoEngine.ts b/src/common/utxobased/engine/UtxoEngine.ts index bc238784..de87c8bb 100644 --- a/src/common/utxobased/engine/UtxoEngine.ts +++ b/src/common/utxobased/engine/UtxoEngine.ts @@ -433,10 +433,13 @@ export async function makeUtxoEngine( }) if (id !== transaction.txid) { // The transaction is on the network at this point, so a mismatched - // response txid must not be reported as a send failure. + // response txid must not be reported as a send failure. Track the + // txid the network actually accepted, or the wallet would watch a + // transaction that never confirms. log.warn( `broadcast response txid mismatch: expected ${transaction.txid} received ${id}` ) + return { ...transaction, txid: id } } return transaction }, From 2982e1854e288da7897d1adf9ed19c9952399848 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:27:19 -0700 Subject: [PATCH 6/9] fixup! Resolve broadcast ambiguity before reporting failure --- src/common/utxobased/engine/UtxoEngineProcessor.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/common/utxobased/engine/UtxoEngineProcessor.ts b/src/common/utxobased/engine/UtxoEngineProcessor.ts index 1c4d9b3f..0c9524b8 100644 --- a/src/common/utxobased/engine/UtxoEngineProcessor.ts +++ b/src/common/utxobased/engine/UtxoEngineProcessor.ts @@ -475,8 +475,9 @@ export function makeUtxoEngineProcessor( }, async broadcastTx(transaction: EdgeTransaction): Promise { - await serverStates.broadcastTx(transaction) - return transaction.txid + // Return the txid the network actually answered with, so the caller + // can detect a server accepting the transaction under a different id. + return await serverStates.broadcastTx(transaction) }, refillServers(): void { serverStates.refillServers() From dd22df6c4897cc2b56e8da1dbde7d4e5af9c37fc Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:03:12 -0700 Subject: [PATCH 7/9] Replace the broadcast txid query with failure classification The immediate network query for the txid was unsound: it runs under the same network conditions that made the broadcast ambiguous, so a negative result proves nothing (the relayed transaction has had no time to propagate) and a positive mostly fires when the network is healthy. It also added serial, untimed network calls to the send path. Classify the exhausted broadcast instead, with no added network calls: collect every server's failure and reject with the original error when all of them are explicit Blockbook rejections (definitively failed, safe to retry), or with BroadcastAmbiguityError when any failure is a transport error, since one of those servers may have relayed the transaction before failing to answer. The GUI can branch on the error name to lock the retry path for ambiguous failures. --- CHANGELOG.md | 2 +- src/common/utxobased/engine/ServerStates.ts | 95 +++++-------------- src/common/utxobased/engine/broadcastError.ts | 42 ++++++++ .../utxobased/engine/broadcastError.spec.ts | 60 ++++++++++++ 4 files changed, 126 insertions(+), 73 deletions(-) create mode 100644 src/common/utxobased/engine/broadcastError.ts create mode 100644 test/common/utxobased/engine/broadcastError.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ed7789af..fb1b5685 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- fixed: Failed sends after a successful broadcast: `saveTx` no longer fails on a disconnected engine, and a broadcast error is reported as a failure only after verifying the transaction is unknown to the network. +- fixed: Failed sends after a successful broadcast: `saveTx` no longer fails when the engine is not running, and an exhausted broadcast now distinguishes explicit server rejections (definitively failed, safe to retry) from transport failures where the transaction may have reached the network (`BroadcastAmbiguityError`). ## 3.11.0 (2026-07-13) diff --git a/src/common/utxobased/engine/ServerStates.ts b/src/common/utxobased/engine/ServerStates.ts index 6544c193..3a254f4a 100644 --- a/src/common/utxobased/engine/ServerStates.ts +++ b/src/common/utxobased/engine/ServerStates.ts @@ -1,4 +1,3 @@ -import { asMaybe, asObject, asString } from 'cleaners' import { EdgeIo, EdgeLog, EdgeTransaction } from 'edge-core-js/types' import { parse } from 'uri-js' @@ -21,6 +20,10 @@ import Deferred from '../network/Deferred' import { WsTask, WsTaskGenerator } from '../network/Socket' import { SocketEmitter, SocketEvent } from '../network/SocketEmitter' import { pushUpdate, removeIdFromQueue } from '../network/socketQueue' +import { + BroadcastAmbiguityError, + classifyBroadcastFailure +} from './broadcastError' import { MAX_CONNECTIONS, NEW_CONNECTIONS } from './constants' import { UtxoInitOptions } from './types' @@ -368,74 +371,28 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { const instance: ServerStates = { async broadcastTx(transaction: EdgeTransaction): Promise { - // Query the network for the transaction to determine whether a failed - // broadcast actually reached the network anyway. A server can relay - // the transaction and still return an error or fail to respond, so an - // error from every server does not prove the transaction wasn't sent. - const isTxidKnown = async (txid: string): Promise => { - // Ask connected blockbook instances first: - for (const uri of Object.keys(serverStatesCache)) { - const { blockbook } = serverStatesCache[uri] - if (blockbook == null || !blockbook.isConnected) continue - const known = await blockbook - .fetchTransaction(txid) - .then(() => true) - .catch(() => false) - if (known) return true - } - - // Fall back to the NOWNode HTTP API when no blockbook is connected: - const { nowNodesApiKey } = initOptions - if (nowNodesApiKey == null) return false - const nowNodeUris = serverConfigs - .filter(config => config.type === 'blockbook-nownode') - .map(config => config.uris) - .flat(1) - for (const uri of nowNodeUris) { - const known = await io - .fetchCors(`${uri}/api/v2/tx/${txid}`, { - headers: { - 'api-key': nowNodesApiKey - } - }) - .then(async response => { - if (!response.ok) return false - const json = await response.json() - return asMaybe(asTxQueryResponse)(json)?.txid === txid - }) - .catch(() => false) - if (known) return true - } - return false - } - return await new Promise((resolve, reject) => { let resolved = false let bad = 0 - // Reject with the given error only when the transaction is verifiably - // absent from the network; a transaction that reached the network - // despite the error is a successful broadcast. - const rejectUnlessTxKnown = (error?: Error): void => { - const fail = (): void => { - const msg = error != null ? `With error ${error.message}` : '' - log.error( - `broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}` + // Collect every server's failure so the terminal rejection can be + // classified: all explicit rejections reject with the original error + // (a definitive failure, safe to retry); any transport failure in + // the set rejects as ambiguous, because one of those servers may + // have relayed the transaction before failing to answer. + const broadcastErrors: unknown[] = [] + const rejectClassified = (error?: Error): void => { + const msg = error != null ? `With error ${error.message}` : '' + log.error(`broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}`) + if (classifyBroadcastFailure(broadcastErrors) === 'ambiguous') { + reject( + new BroadcastAmbiguityError( + broadcastErrors.map(cause => String(cause)) + ) ) + } else { reject(error) } - isTxidKnown(transaction.txid) - .then(known => { - if (!known) return fail() - if (!resolved) { - resolved = true - log.warn( - `broadcastTx errored, but txid ${transaction.txid} is known to the network; treating broadcast as a success` - ) - resolve(transaction.txid) - } - }) - .catch(fail) } const wsUris = Object.keys(serverStatesCache).filter( @@ -460,8 +417,9 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { } }) .catch((e?: Error) => { + broadcastErrors.push(e) if (++bad === wsUris.length) { - rejectUnlessTxKnown(e) + rejectClassified(e) } }) } @@ -526,8 +484,9 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { } }) .catch((e?: Error) => { + broadcastErrors.push(e) if (++bad === nowNodeUris.length) { - rejectUnlessTxKnown(e) + rejectClassified(e) } }) } @@ -729,11 +688,3 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { return instance } - -/** - * Minimal shape of a Blockbook REST `/api/v2/tx/` response, used only - * to confirm that a transaction is known to the network. - */ -const asTxQueryResponse = asObject({ - txid: asString -}) diff --git a/src/common/utxobased/engine/broadcastError.ts b/src/common/utxobased/engine/broadcastError.ts new file mode 100644 index 00000000..9a4b6f44 --- /dev/null +++ b/src/common/utxobased/engine/broadcastError.ts @@ -0,0 +1,42 @@ +/** + * Classification of an all-servers-failed broadcast. + * + * An explicit rejection is a server answering the broadcast with a Blockbook + * error response: the server received the transaction and refused it, so the + * same signed bytes will be refused again and cannot be on the network from + * this attempt. Anything else (a request timeout, a dropped connection, an + * HTTP status error whose body was never read) leaves relay possible: a + * server can relay the transaction to the network and still fail to answer. + * + * The classification is pure error-shape inspection. It adds no network + * calls, so it cannot add latency to the send path. + */ + +/** + * A broadcast that failed on every server where at least one failure was a + * transport error rather than an explicit rejection. The transaction cannot + * be assumed absent from the network, so a retry could produce a second real + * payment. Consumers branch on `name === 'BroadcastAmbiguityError'`; class + * identity does not survive the core bridge, names and properties do. + */ +export class BroadcastAmbiguityError extends Error { + readonly causes: string[] + + constructor(causes: string[]) { + super('Broadcast failed, but the transaction may have reached the network') + this.name = 'BroadcastAmbiguityError' + this.causes = causes + } +} + +export const isExplicitBroadcastRejection = (error: unknown): boolean => + String(error instanceof Error ? error.message : error).includes( + 'Blockbook Error: ' + ) + +export const classifyBroadcastFailure = ( + errors: unknown[] +): 'rejected' | 'ambiguous' => + errors.length > 0 && errors.every(isExplicitBroadcastRejection) + ? 'rejected' + : 'ambiguous' diff --git a/test/common/utxobased/engine/broadcastError.spec.ts b/test/common/utxobased/engine/broadcastError.spec.ts new file mode 100644 index 00000000..1f3132b4 --- /dev/null +++ b/test/common/utxobased/engine/broadcastError.spec.ts @@ -0,0 +1,60 @@ +import { assert } from 'chai' +import { describe, it } from 'mocha' + +import { + BroadcastAmbiguityError, + classifyBroadcastFailure, + isExplicitBroadcastRejection +} from '../../../../src/common/utxobased/engine/broadcastError' + +describe('broadcast failure classification', function () { + it('classifies all-explicit-rejection sets as rejected', function () { + assert.equal( + classifyBroadcastFailure([ + new Error('Blockbook Error: -26: dust'), + new Error('Blockbook Error: -25: missing inputs') + ]), + 'rejected' + ) + }) + + it('classifies any transport failure in the set as ambiguous', function () { + assert.equal( + classifyBroadcastFailure([ + new Error('Blockbook Error: -26: dust'), + new Error('Timeout for request 42') + ]), + 'ambiguous' + ) + assert.equal( + classifyBroadcastFailure([new Error('Timeout for request 42')]), + 'ambiguous' + ) + assert.equal( + classifyBroadcastFailure([ + new Error('Failed to broadcast transaction via Blockbook: HTTP 503') + ]), + 'ambiguous' + ) + }) + + it('treats missing or empty error information as ambiguous', function () { + assert.equal(classifyBroadcastFailure([]), 'ambiguous') + assert.equal(classifyBroadcastFailure([undefined]), 'ambiguous') + }) + + it('recognizes explicit rejections by the Blockbook error marker', function () { + assert.isTrue( + isExplicitBroadcastRejection(new Error('Blockbook Error: -26: dust')) + ) + assert.isFalse(isExplicitBroadcastRejection(new Error('socket closed'))) + assert.isFalse(isExplicitBroadcastRejection(undefined)) + }) + + it('keeps a bridge-stable name and carries its causes', function () { + const error = new BroadcastAmbiguityError(['Timeout for request 42']) + assert.equal(error.name, 'BroadcastAmbiguityError') + assert.deepEqual(error.causes, ['Timeout for request 42']) + assert.instanceOf(error, Error) + }) +}) From 3a1ab78b0ab45583534588ba17591369bb91eefc Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:27:22 -0700 Subject: [PATCH 8/9] fixup! Replace the broadcast txid query with failure classification --- src/common/utxobased/engine/broadcastError.ts | 20 +++++++++++-- .../utxobased/engine/broadcastError.spec.ts | 29 ++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/common/utxobased/engine/broadcastError.ts b/src/common/utxobased/engine/broadcastError.ts index 9a4b6f44..e8659809 100644 --- a/src/common/utxobased/engine/broadcastError.ts +++ b/src/common/utxobased/engine/broadcastError.ts @@ -34,9 +34,25 @@ export const isExplicitBroadcastRejection = (error: unknown): boolean => 'Blockbook Error: ' ) +/** + * A failure from a server that provably never accepted the payload, so it + * cannot have relayed the transaction: the Electrum stub refuses + * broadcastTx synchronously. Such failures say nothing about relay and are + * excluded from the ambiguity determination. (A not-yet-connected blockbook + * is NOT in this class: its queued request can still send once the + * connection completes, so its timeout stays ambiguous.) + */ +export const isNonRelayFailure = (error: unknown): boolean => + String(error instanceof Error ? error.message : error).includes( + 'not supported for Electrum connections' + ) + export const classifyBroadcastFailure = ( errors: unknown[] -): 'rejected' | 'ambiguous' => - errors.length > 0 && errors.every(isExplicitBroadcastRejection) +): 'rejected' | 'ambiguous' => { + if (errors.length === 0) return 'ambiguous' + const relayCapable = errors.filter(error => !isNonRelayFailure(error)) + return relayCapable.every(isExplicitBroadcastRejection) ? 'rejected' : 'ambiguous' +} diff --git a/test/common/utxobased/engine/broadcastError.spec.ts b/test/common/utxobased/engine/broadcastError.spec.ts index 1f3132b4..0736fe86 100644 --- a/test/common/utxobased/engine/broadcastError.spec.ts +++ b/test/common/utxobased/engine/broadcastError.spec.ts @@ -4,7 +4,8 @@ import { describe, it } from 'mocha' import { BroadcastAmbiguityError, classifyBroadcastFailure, - isExplicitBroadcastRejection + isExplicitBroadcastRejection, + isNonRelayFailure } from '../../../../src/common/utxobased/engine/broadcastError' describe('broadcast failure classification', function () { @@ -43,6 +44,32 @@ describe('broadcast failure classification', function () { assert.equal(classifyBroadcastFailure([undefined]), 'ambiguous') }) + it('excludes non-relay failures from the ambiguity determination', function () { + const electrumStub = new Error( + 'broadcastTx not supported for Electrum connections' + ) + assert.isTrue(isNonRelayFailure(electrumStub)) + // An Electrum stub alongside explicit rejections must not turn a + // definitively failed broadcast into an ambiguous one. + assert.equal( + classifyBroadcastFailure([ + electrumStub, + new Error('Blockbook Error: -26: dust') + ]), + 'rejected' + ) + // All-stub sets never sent anything anywhere: definitively failed. + assert.equal(classifyBroadcastFailure([electrumStub]), 'rejected') + // A transport failure still dominates. + assert.equal( + classifyBroadcastFailure([ + electrumStub, + new Error('Timeout for request 42') + ]), + 'ambiguous' + ) + }) + it('recognizes explicit rejections by the Blockbook error marker', function () { assert.isTrue( isExplicitBroadcastRejection(new Error('Blockbook Error: -26: dust')) From 51e5b66fd16234ffdb34709b8ea4095362b30700 Mon Sep 17 00:00:00 2001 From: Jonathan Tzeng Date: Fri, 14 Aug 2026 11:39:25 -0700 Subject: [PATCH 9/9] fixup! Replace the broadcast txid query with failure classification --- src/common/utxobased/engine/ServerStates.ts | 16 +++++++++++++- src/common/utxobased/engine/broadcastError.ts | 12 ++++++++++ .../utxobased/engine/broadcastError.spec.ts | 22 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/common/utxobased/engine/ServerStates.ts b/src/common/utxobased/engine/ServerStates.ts index 3a254f4a..e676eab6 100644 --- a/src/common/utxobased/engine/ServerStates.ts +++ b/src/common/utxobased/engine/ServerStates.ts @@ -22,7 +22,8 @@ import { SocketEmitter, SocketEvent } from '../network/SocketEmitter' import { pushUpdate, removeIdFromQueue } from '../network/socketQueue' import { BroadcastAmbiguityError, - classifyBroadcastFailure + classifyBroadcastFailure, + isAlreadyKnownRejection } from './broadcastError' import { MAX_CONNECTIONS, NEW_CONNECTIONS } from './constants' import { UtxoInitOptions } from './types' @@ -382,6 +383,19 @@ export function makeServerStates(config: ServerStateConfig): ServerStates { // have relayed the transaction before failing to answer. const broadcastErrors: unknown[] = [] const rejectClassified = (error?: Error): void => { + // A server refusing because it already has the transaction is + // confirmation the transaction reached the network (from this + // attempt or an earlier one with the same signed bytes): success. + if (broadcastErrors.some(isAlreadyKnownRejection)) { + if (!resolved) { + resolved = true + log.warn( + `broadcastTx: server already has txid ${transaction.txid}; treating broadcast as a success` + ) + resolve(transaction.txid) + } + return + } const msg = error != null ? `With error ${error.message}` : '' log.error(`broadcastTx fail: ${JSON.stringify(transaction)}\n${msg}`) if (classifyBroadcastFailure(broadcastErrors) === 'ambiguous') { diff --git a/src/common/utxobased/engine/broadcastError.ts b/src/common/utxobased/engine/broadcastError.ts index e8659809..95475eef 100644 --- a/src/common/utxobased/engine/broadcastError.ts +++ b/src/common/utxobased/engine/broadcastError.ts @@ -34,6 +34,18 @@ export const isExplicitBroadcastRejection = (error: unknown): boolean => 'Blockbook Error: ' ) +/** + * A rejection that means the server ALREADY HAS the transaction + * ("transaction already in block chain", "txn-already-in-mempool", + * "txn-already-known"). This is a confirmation the transaction reached the + * network, from this attempt or an earlier one with the same signed bytes, + * so the broadcast must be treated as a success: presenting it as a + * failure invites the duplicate-payment retry this work exists to stop. + */ +export const isAlreadyKnownRejection = (error: unknown): boolean => + isExplicitBroadcastRejection(error) && + /alread/i.test(String(error instanceof Error ? error.message : error)) + /** * A failure from a server that provably never accepted the payload, so it * cannot have relayed the transaction: the Electrum stub refuses diff --git a/test/common/utxobased/engine/broadcastError.spec.ts b/test/common/utxobased/engine/broadcastError.spec.ts index 0736fe86..46d4c76d 100644 --- a/test/common/utxobased/engine/broadcastError.spec.ts +++ b/test/common/utxobased/engine/broadcastError.spec.ts @@ -4,6 +4,7 @@ import { describe, it } from 'mocha' import { BroadcastAmbiguityError, classifyBroadcastFailure, + isAlreadyKnownRejection, isExplicitBroadcastRejection, isNonRelayFailure } from '../../../../src/common/utxobased/engine/broadcastError' @@ -78,6 +79,27 @@ describe('broadcast failure classification', function () { assert.isFalse(isExplicitBroadcastRejection(undefined)) }) + it('recognizes already-known rejections as network confirmation', function () { + assert.isTrue( + isAlreadyKnownRejection( + new Error('Blockbook Error: -27: transaction already in block chain') + ) + ) + assert.isTrue( + isAlreadyKnownRejection( + new Error('Blockbook Error: txn-already-in-mempool') + ) + ) + assert.isTrue( + isAlreadyKnownRejection(new Error('Blockbook Error: txn-already-known')) + ) + assert.isFalse( + isAlreadyKnownRejection(new Error('Blockbook Error: -26: dust')) + ) + // "already" in a transport error is not a Blockbook rejection. + assert.isFalse(isAlreadyKnownRejection(new Error('socket already closed'))) + }) + it('keeps a bridge-stable name and carries its causes', function () { const error = new BroadcastAmbiguityError(['Timeout for request 42']) assert.equal(error.name, 'BroadcastAmbiguityError')