diff --git a/apps/api/src/wallet/stellar.adapter.js b/apps/api/src/wallet/stellar.adapter.js index ae36c0d..b692914 100644 --- a/apps/api/src/wallet/stellar.adapter.js +++ b/apps/api/src/wallet/stellar.adapter.js @@ -557,10 +557,13 @@ const submitPayment = async ({ const stellarMemo = buildStellarMemo({ memo, memoType }); - let transaction; - let hash; - let lastError; - for (let attempt = 1; attempt <= SEND_MAX_ATTEMPTS; attempt += 1) { + // Build and sign the envelope ONCE, before any submission attempt. + // Retries reuse the exact same signed envelope, so an ambiguous outcome + // (timeout/connection loss) is safe to re-attempt: Horizon either has + // the transaction (found by hash -> success) or it never landed (the + // same envelope is safe to resend). Only a genuine tx_bad_seq conflict + // forces a rebuild with a fresh sequence number (#197). + const buildTransaction = async () => { const sourceAccount = await server.loadAccount(sourcePublicKey); assertNativeReserve(sourceAccount, fee); @@ -579,9 +582,18 @@ const submitPayment = async ({ builder.addMemo(stellarMemo); } - transaction = builder.setTimeout(30).build(); - transaction.sign(sourceKeypair); - hash = safeHash(transaction); + const tx = builder.setTimeout(30).build(); + tx.sign(sourceKeypair); + return { tx, hash: safeHash(tx) }; + }; + + let { tx: transaction, hash } = await buildTransaction(); + let lastError; + for (let attempt = 1; attempt <= SEND_MAX_ATTEMPTS; attempt += 1) { + if (!transaction) { + // Genuine sequence conflict: rebuild with a fresh sequence number. + ({ tx: transaction, hash } = await buildTransaction()); + } try { const txResponse = await server.submitTransaction(transaction); @@ -590,50 +602,41 @@ const submitPayment = async ({ explorerUrl: getTransactionUrl(txResponse.hash), }; } catch (error) { - // Reconcile uncertain submissions before rebuilding or failing - if (isHorizonWriteUncertain(error)) { - if (hash) { - try { - const found = await server.transactions().transactionHash(hash).call(); - if (found) { - logger.info(`Recovered seemingly-timed-out payment ${hash} via Horizon lookup.`); - return { txHash: hash, explorerUrl: getTransactionUrl(hash) }; - } - } catch (_) { - // Ignore lookup errors + const ambiguous = isHorizonWriteUncertain(error) || isBadSequence(error); + + // Reconcile ambiguous submissions before rebuilding or failing: + // if the transaction actually landed, report success instead of + // risking a duplicate spend. + if (ambiguous && hash) { + try { + const found = await server.transactions().transactionHash(hash).call(); + if (found) { + logger.info(`Recovered ambiguous payment ${hash} via Horizon lookup (attempt ${attempt}).`); + return { txHash: hash, explorerUrl: getTransactionUrl(hash) }; } + } catch (_) { + // Lookup failed; fall through to the retry logic below. } + } + if (isHorizonWriteUncertain(error)) { if (attempt < SEND_MAX_ATTEMPTS) { logger.warn(`Payment uncertain (attempt ${attempt}/${SEND_MAX_ATTEMPTS}); resubmitting same envelope.`); await sleep(attempt * 250); - continue; - } else { - throw new Error( - "Transaction submission status unknown after timeout; not resubmitting to avoid a duplicate.", - ); + continue; // reuse the exact same signed envelope } + throw new Error( + "Transaction submission status unknown after timeout; not resubmitting to avoid a duplicate.", + ); } if (isBadSequence(error)) { - if (hash) { - try { - const found = await server.transactions().transactionHash(hash).call(); - if (found) { - logger.info(`Recovered tx_bad_seq payment ${hash} via Horizon lookup.`); - return { txHash: hash, explorerUrl: getTransactionUrl(hash) }; - } - } catch (_) { - // lookup failed; fall through to the standard retry path - } - } - if (attempt < SEND_MAX_ATTEMPTS) { lastError = error; logger.warn( `Payment hit tx_bad_seq (attempt ${attempt}/${SEND_MAX_ATTEMPTS}); reloading sequence and retrying.`, ); - transaction = null; + transaction = null; // force a rebuild with a fresh sequence number await sleep(attempt * 250); continue; } diff --git a/apps/api/test/stellarAdapter.test.js b/apps/api/test/stellarAdapter.test.js index 113f307..fae523f 100644 --- a/apps/api/test/stellarAdapter.test.js +++ b/apps/api/test/stellarAdapter.test.js @@ -374,3 +374,73 @@ test('getFundingAccountHealth reports fee and reserve pressure with operator thr assert.ok(Array.isArray(report.runbook)); assert.ok(report.runbook.length > 0); }); + +// #197: ambiguous outcomes must be reconciled via Horizon before any retry. +// Sequence: attempt 1 times out (status unknown, write never landed) -> the +// SAME signed envelope is resubmitted -> Horizon rejects with tx_bad_seq +// because the first transaction actually landed -> the adapter must query +// Horizon by the pre-computed hash and return success WITHOUT building a new +// envelope or spending twice. +test('submitPayment reconciles timeout + tx_bad_seq via Horizon lookup, reusing one envelope', async () => { + mockSuccessfulPaymentSetup(); + + const { HorizonWriteUncertainError } = require('../src/config/horizon'); + + let submitCalls = 0; + const submittedTxs = []; + mock.method(server, 'submitTransaction', async (tx) => { + submitCalls += 1; + submittedTxs.push(tx); + if (submitCalls === 1) { + // Attempt 1: ambiguous timeout -- the write may or may not have landed. + throw new HorizonWriteUncertainError('submission status unknown after timeout'); + } + // Attempt 2 (same envelope): Horizon says bad sequence -- proof that + // attempt 1 actually landed and consumed the sequence number. + const err = new Error('Transaction failed'); + err.response = { + data: { + extras: { + result_codes: { transaction: 'tx_bad_seq' }, + }, + }, + }; + throw err; + }); + + // Horizon lookup by hash: not found after the timeout, found after tx_bad_seq. + let lookupCalls = 0; + mock.method(server, 'transactions', () => { + lookupCalls += 1; + const callIndex = lookupCalls; + return { + transactionHash: (hash) => ({ + call: async () => { + if (callIndex === 1) { + const notFound = new Error('Not Found'); + notFound.response = { status: 404 }; + throw notFound; + } + return { hash }; + }, + }), + }; + }); + + const result = await stellarAdapter.submitPayment({ + secretKey: SOURCE_SECRET, + destination: DESTINATION_PUBLIC_KEY, + amount: '1', + asset: 'XLM', + }); + + // Success recovered via the hash lookup, using the ORIGINAL envelope's hash. + assert.equal(typeof result.txHash, 'string'); + assert.ok(result.txHash.length > 0); + assert.ok(result.explorerUrl.includes(result.txHash)); + // Exactly two submissions (timeout + bad_seq), never a third, and the SAME + // signed envelope was reused -- no rebuild, no duplicate payment. + assert.equal(submitCalls, 2); + assert.ok(submittedTxs[0] === submittedTxs[1], 'the same signed envelope must be reused'); + assert.ok(lookupCalls >= 2, 'Horizon must be queried by hash after each ambiguous outcome'); +});