From 1ae9f7864913627df60fdec571b8b53bc07c5568 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Fri, 13 Nov 2026 02:50:59 +0100 Subject: [PATCH] fix: spark exit redo hold and overlapping spark sends --- Bitkit/Managers/SparkExitManager.swift | 10 +- Bitkit/Services/LightningService.swift | 82 ++++++++++-- .../Services/SparkExit/SparkExitDriver.swift | 23 +++- .../Services/SparkExit/SparkExitRecord.swift | 3 + BitkitTests/ChannelFreeRailTests.swift | 121 +++++++++++++++++- BitkitTests/SparkExitReviewTests.swift | 14 ++ BitkitTests/SparkExitTests.swift | 49 +++++++ .../restored-wallet-pre-restore-invoice.xml | 33 +++++ 8 files changed, 314 insertions(+), 21 deletions(-) create mode 100644 journeys/send/restored-wallet-pre-restore-invoice.xml diff --git a/Bitkit/Managers/SparkExitManager.swift b/Bitkit/Managers/SparkExitManager.swift index b4dcd3d7a..4e555b50d 100644 --- a/Bitkit/Managers/SparkExitManager.swift +++ b/Bitkit/Managers/SparkExitManager.swift @@ -1576,10 +1576,14 @@ final class SparkExitManager { UserDefaults.standard.dictionary(forKey: remindersKey) as? [String: TimeInterval] ?? [:] } - /// An unverified rebuild's fee ask the tick from `previous` withdrew: the valid generation needs nothing more. + /// A fee ask the tick from `previous` withdrew: an unverified rebuild's, as the valid generation needs nothing more, + /// or one of a Redo rebuild, asked before or during it, that runs with no new funding, on the Redo's tick or a later + /// one, as the held funding pays for it. nonisolated static func isFundingAskWithdrawn(_ record: SparkExitRecord, previous: SparkExitRecord) -> Bool { - guard previous.isUnverifiedAsk == true, case .needsFunding = previous.phase else { return false } - return record.phase == .running + guard case .needsFunding = previous.phase, record.phase == .running else { return false } + let isRedoAsk = (previous.isRedoAsk == true || record.isRedoAsk == true || previous.isRedoRebuild == true) + && record.funding == previous.funding + return previous.isUnverifiedAsk == true || isRedoAsk } /// Takes back the exit's delivered fee asks, which no longer hold. diff --git a/Bitkit/Services/LightningService.swift b/Bitkit/Services/LightningService.swift index 4e512d8b4..3cb2b07aa 100644 --- a/Bitkit/Services/LightningService.swift +++ b/Bitkit/Services/LightningService.swift @@ -1078,7 +1078,12 @@ class LightningService: NodeLifecycleService { return paid } guard beginRailSend(paymentHash: paymentHash) else { throw Self.pendingDuplicate(paymentHash: paymentHash) } - defer { endRailSend(paymentHash: paymentHash) } + var isRailSendEnded = false + defer { + if !isRailSendEnded { + endRailSend(paymentHash: paymentHash) + } + } // Spark takes no route parameters, so a caller's routing fee cap is checked against its estimate instead. if let maxFeeMsat = params?.maxTotalRoutingFeeMsat { let feeSats: UInt64 @@ -1096,6 +1101,7 @@ class LightningService: NodeLifecycleService { throw ChannelFreeRailSendRefused(reason: t("wallet__max_possible_fee_rate")) } } + var isRefusedAtAdmission = false do { let sent = try await rail.send( bolt11: bolt11, @@ -1104,7 +1110,12 @@ class LightningService: NodeLifecycleService { ?? Self.paymentAmountSats(bolt11: bolt11, sats: sats).map(Self.defaultMaxFeeSats), beforeSubmission: { [weak self] in try beforeSubmission() - try self?.admitSend(paymentHash: paymentHash, on: .channelFree, since: mark) + do { + try self?.admitSend(paymentHash: paymentHash, on: .channelFree, since: mark) + } catch { + isRefusedAtAdmission = true + throw error + } // Noted once admitted, so a refused attempt never takes the place of the one that went out. self?.noteAttempt(on: .channelFree, bolt11: bolt11) self?.recordRailAttempt(paymentHash: paymentHash, expiresAt: expiresAt) @@ -1125,6 +1136,16 @@ class LightningService: NodeLifecycleService { releaseRailAttempt(paymentHash: paymentHash, unlessInFlight: false) } throw refusal + } catch where isRefusedAtAdmission { + // Nothing of this send left. Refused for a Spark send recorded since the checks, its caller gets that + // send's outcome, as it may miss its completion. + guard let paymentHash, eventCallbackLock.withLock({ + attemptsRecorded(since: mark.records, key: Self.railAttemptsKey).contains(paymentHash.lowercased()) + }) else { throw error } + endRailSend(paymentHash: paymentHash) + isRailSendEnded = true + guard let paid = try await settledRecordedRailSend(paymentHash: paymentHash, since: mark) else { throw error } + return paid } catch { // A send that failed once submitted may still go out, so its outcome waits for a later sync. if let paymentHash { @@ -1221,8 +1242,8 @@ class LightningService: NodeLifecycleService { /// When the wallet was restored, in seconds since 1970. Spark can report a sync while its payment history failed to /// sync, so its list can lack a send it made before then, and a still-valid invoice created before it that Spark - /// hasn't listed as pending or paid is refused on either rail until it expires, unless Spark's operators confirm - /// no send of it. + /// hasn't listed as pending or paid is refused on either rail until it expires, unless Spark's operators show no + /// live or paid send of it. static let sparkRestoredAtKey = "sparkRestoredAt" var sparkRestoredAt: Int? { @@ -1458,7 +1479,8 @@ class LightningService: NodeLifecycleService { /// duplicate, as LDK does its own, and a settled one returns its hash as paid. A failed or unknown one lets the /// payment go. While either rail can't list its payments, an invoice it was sent over is refused as pending too. /// After a restore, a still-valid invoice created (`invoiceCreatedAt`) before it that Spark's list doesn't show - /// pending or paid is refused unless Spark's operators confirm no send of it (`refuseUnverifiedRestoredInvoice`). + /// pending or paid is refused unless Spark's operators show no live or paid send of it + /// (`refuseUnverifiedRestoredInvoice`). private func settledEarlierAttempt( paymentHash: String?, on rails: [LightningPaymentRail], @@ -1546,8 +1568,9 @@ class LightningService: NodeLifecycleService { } /// After a restore, refuses a still-valid invoice created before it, as Spark's list can lack a send it made then, - /// unless Spark's operators show no Lightning send of it and it can't have been paid by a direct Spark transfer, - /// which they can't see. Any send they show, sent, pending or failed, or no answer, refuses it until it expires. + /// unless Spark's operators show no live or paid Lightning send of it and it can't have been paid by a direct Spark + /// transfer, which they can't see. A send they show sent or pending, or no answer, refuses it until it expires. A + /// failed one counts as unsent, as they show it failed only once every attempt at the hash was returned. private func refuseUnverifiedRestoredInvoice( paymentHash: String, bolt11: String, @@ -1566,7 +1589,7 @@ class LightningService: NodeLifecycleService { guard sparkRuns else { return } do { guard let rail else { throw ChannelFreeRailUnavailable() } - if let status = try await rail.lightningSendStatus(bolt11: bolt11) { + if let status = try await rail.lightningSendStatus(bolt11: bolt11), status != .failed { Logger.warn("Refused paying '\(paymentHash)', a pre-restore invoice Spark shows as '\(status)'", context: "LightningService") throw ChannelFreeRailSendRefused.restoredInvoiceUnverified() } @@ -1574,7 +1597,7 @@ class LightningService: NodeLifecycleService { Logger.warn("Refused paying '\(paymentHash)', a pre-restore invoice Spark may have paid by transfer", context: "LightningService") throw ChannelFreeRailSendRefused.restoredInvoiceUnverified() } - Logger.info("Spark's operators show no send of pre-restore invoice '\(paymentHash)'", context: "LightningService") + Logger.info("Found no live or paid Spark operator send of pre-restore invoice '\(paymentHash)'", context: "LightningService") } catch let refusal as ChannelFreeRailSendRefused { throw refusal } catch { @@ -1673,7 +1696,8 @@ class LightningService: NodeLifecycleService { /// The last check before a send leaves on `rail`, under the same lock that marks the other rail's sends in flight, /// so of two sends of one invoice only the first admitted goes out. Refuses while the other rail has one in flight, - /// or recorded one since `mark`. An admitted node send stays in flight until `endLdkSend`; a Spark send already is. + /// or recorded one since `mark`, and a Spark send also once Spark recorded one since `mark`, as Spark doesn't refuse + /// a repeat. An admitted node send stays in flight until `endLdkSend`; a Spark send already is. func admitSend(paymentHash: String?, on rail: LightningPaymentRail, since mark: AttemptMark) throws { guard let hash = paymentHash?.lowercased() else { return } try eventCallbackLock.withLock { @@ -1683,12 +1707,50 @@ class LightningService: NodeLifecycleService { Logger.warn("Refused paying '\(hash)', the other rail sent it meanwhile", context: "LightningService") throw Self.pendingDuplicate(paymentHash: hash) } + if rail == .channelFree, attemptsRecorded(since: mark.records, key: Self.railAttemptsKey).contains(hash) { + Logger.warn("Refused paying '\(hash)', Spark sent it meanwhile", context: "LightningService") + throw Self.pendingDuplicate(paymentHash: hash) + } if rail == .ldk { ldkSendsInFlight.insert(hash) } } } + /// The outcome of the Spark send of `paymentHash` recorded since `mark` that refused this one at admission: its hash + /// once Spark lists it paid, the failure once it lists it failed with no send of the invoice live before or after + /// the listing is read and none recorded while it is, otherwise nil, as it may still pay. + private func settledRecordedRailSend(paymentHash: String, since mark: AttemptMark) async throws -> PaymentHash? { + let hash = paymentHash.lowercased() + // Read before the liveness check, so a send recorded right after that check still counts as recorded meanwhile. + let listingMark = attemptMark + let wasLive = isSendLive(paymentHash: hash, since: mark) + guard let payments = await readableRailPayments(), + let send = ChannelFreeRailPayment.latestOutboundAttempt(payments, paymentHash: hash) + else { return nil } + let isLive = wasLive || isSendLive(paymentHash: hash, since: mark) || eventCallbackLock.withLock { + attemptsRecorded(since: listingMark.records, key: Self.railAttemptsKey).contains(hash) + } + switch send.status { + case .succeeded: + Logger.info("Settled the refused Spark send '\(hash)' as paid", context: "LightningService") + return paymentHash + case .failed where !isLive: + Logger.info("Settled the refused Spark send '\(hash)' as failed", context: "LightningService") + throw ChannelFreeRailSendRefused(reason: t("wallet__payment_failed_description")) + default: + return nil + } + } + + /// Whether a send of `hash` is in flight on either rail, or the node recorded one since `mark`. + private func isSendLive(paymentHash hash: String, since mark: AttemptMark) -> Bool { + eventCallbackLock.withLock { + railSendsInFlight.contains(hash) || ldkSendsInFlight.contains(hash) + || attemptsRecorded(since: mark.records, key: Self.ldkAttemptsKey).contains(hash) + } + } + func beginLdkSend(paymentHash: String?) { guard let paymentHash else { return } _ = eventCallbackLock.withLock { ldkSendsInFlight.insert(paymentHash.lowercased()) } diff --git a/Bitkit/Services/SparkExit/SparkExitDriver.swift b/Bitkit/Services/SparkExit/SparkExitDriver.swift index ae45c6fa5..48ee78a9e 100644 --- a/Bitkit/Services/SparkExit/SparkExitDriver.swift +++ b/Bitkit/Services/SparkExit/SparkExitDriver.swift @@ -77,6 +77,10 @@ struct SparkExitDriver { @discardableResult func tick(_ record: SparkExitRecord) async throws -> SparkExitRecord { var record = record + // A Redo's fee ask ended on an earlier tick, so its withdrawal was already seen. + if record.isRedoAsk == true, record.phase.requiredFundingSats == nil { + record.isRedoAsk = nil + } switch record.phase { case .done, .failed: guard record.needsDriving, !Task.isCancelled else { return record } @@ -338,6 +342,7 @@ struct SparkExitDriver { record.isRebuilding = true record.isRedoRebuild = true record.isUnverifiedAsk = nil + record.isRedoAsk = true } } @@ -446,7 +451,7 @@ struct SparkExitDriver { // Only cooperative leaves the operators could not co-sign: kept as the exit's recovery, so the next check // puts them through the bounded failed-leaf retry (about a week), with the operators-waiting line. if !built.failed.isEmpty { - if holdsRedo(&record, waiting: waitingLeafIds, tip: tip) { + if holdsRedo(&record, waiting: waitingLeafIds, tip: tip, isBuilt: true) { return } record.adopt(built, at: now()) @@ -479,7 +484,7 @@ struct SparkExitDriver { } // A diverged generation is never sent again, so a Redo rebuild that leaves out a waiting leaf it recovers would // leave that leaf unsent until the rest of the exit is done: it tries again at its usual pace for a few blocks. - if holdsRedo(&record, waiting: waitingLeafIds, tip: tip) { + if holdsRedo(&record, waiting: waitingLeafIds, tip: tip, isBuilt: true) { return } record.adopt(built, at: now()) @@ -754,6 +759,10 @@ struct SparkExitDriver { utxos = try? await chain.utxos(address: address) } record.phase = .needsFunding(requiredSats: SparkExitMath.neededFunding(requiredSats: requiredSats, record: record, utxos: utxos)) + // A Redo rebuild's ask ends once the funding it holds pays for a later build. + if record.isRedoRebuild == true { + record.isRedoAsk = true + } } /// Leaves a build skipped while they may still move (`waitingLeafIds`), prepared again on their own once the rest of @@ -1009,8 +1018,9 @@ struct SparkExitDriver { } /// A Redo rebuild that leaves out a waiting leaf the diverged generation recovers is not adopted for - /// `redoHoldBlocks` from the first such build: it stays rebuilding, at the rebuild pace. - private func holdsRedo(_ record: inout SparkExitRecord, waiting: [String], tip: UInt32?) -> Bool { + /// `redoHoldBlocks` from the first such build: it stays rebuilding, at the rebuild pace. `isBuilt` when the funding + /// the exit holds paid for the build. + private func holdsRedo(_ record: inout SparkExitRecord, waiting: [String], tip: UInt32?, isBuilt: Bool = false) -> Bool { guard record.isRedoRebuild == true, record.recovery?.response.leaves.contains(where: { waiting.contains($0.leafId) }) == true else { return false } @@ -1022,8 +1032,9 @@ struct SparkExitDriver { Logger.info("Holding the Spark exit's Redo rebuild for waiting leaves \(waiting)", context: "SparkExitDriver") record.redoHeldSince = since record.lastBuildAt = now() - // Funding a top-up already covered runs the hold at the rebuild pace, with no fee ask on screen. - if record.phase != .running, record.fundingShortfallSats == nil { + // A build the held funding paid for, or funding a top-up already covered, runs the hold at the rebuild pace, with + // no fee ask on screen; a later build that is short asks again. + if record.phase != .running, isBuilt || record.fundingShortfallSats == nil { record.phase = .running } return true diff --git a/Bitkit/Services/SparkExit/SparkExitRecord.swift b/Bitkit/Services/SparkExit/SparkExitRecord.swift index c507f90b8..a03265f20 100644 --- a/Bitkit/Services/SparkExit/SparkExitRecord.swift +++ b/Bitkit/Services/SparkExit/SparkExitRecord.swift @@ -254,6 +254,9 @@ struct SparkExitRecord: Codable, Equatable { /// The fee funding asked for came from a rebuild over a step the SDK could not read, while the generation out there /// is valid: funding it resumes that rebuild with its guard, and the ask ends once every step reads again. var isUnverifiedAsk: Bool? + /// The fee funding asked for came while a Redo rebuild was pending: that rebuild may run on the funding the exit + /// holds, and the ask then ends. Kept until a tick starts past that ask. + var isRedoAsk: Bool? /// The tip of the last rebuild over a step the SDK could not read, so it rebuilds at most once a block. var unverifiedRebuiltAtHeight: UInt32? /// A higher fee rate the steps out there are not built at yet: a fee bump whose build kept the valid generation diff --git a/BitkitTests/ChannelFreeRailTests.swift b/BitkitTests/ChannelFreeRailTests.swift index 1e2dadad2..56f222a97 100644 --- a/BitkitTests/ChannelFreeRailTests.swift +++ b/BitkitTests/ChannelFreeRailTests.swift @@ -1416,8 +1416,19 @@ extension ChannelFreeRailTests { try await assertOperatorsStatusRefusesBothRails(.pending) } - func testAPreRestoreInvoiceTheOperatorsShowFailedIsRefusedOnBothRails() async throws { - try await assertOperatorsStatusRefusesBothRails(.failed) + func testAPreRestoreInvoiceRetriesOnBothRailsAfterItsSparkSendFailed() async throws { + let hash = try String(Bolt11Invoice.fromStr(invoiceStr: Self.invoice).paymentHash()) + try await holdForTheOperators(sparkFunded: true) + rail.sendStatus = .success(nil) + try await sendOverSpark() + // The send failed, for example with no route: Spark lists it failed, and the operators show it failed once + // every attempt at its hash was returned. + rail.payments = [Bitkit.ChannelFreeRailPayment(paymentHash: hash, direction: .outbound, status: .failed, preimage: nil)] + rail.sendStatus = .success(.failed) + + try await sendOverSpark() + rail.balanceSats = 0 + await assertReachesTheNode() } func testAPreRestoreInvoiceTheOperatorsShowNoSendOfPaysOverTheNode() async throws { @@ -1440,6 +1451,29 @@ extension ChannelFreeRailTests { XCTAssertEqual(rail.sparkTransferChecks, 1) } + func testAPreRestoreInvoiceTheOperatorsShowNoSendOfPaysOverTheNodeWhileSparkIsUnsynced() async throws { + try restore(secondsAfterTheInvoice: 60) + try pinTheClockWhileTheInvoiceIsValid() + leaveSparkUnsyncedAndEmpty() + rail.sendStatus = .success(nil) + + await assertReachesTheNode() + XCTAssertEqual(rail.sendStatusFetches, 1) + } + + func testAPreRestoreInvoiceTheOperatorsShowNoSendOfPaysOverAFundedSparkWhileItIsUnsynced() async throws { + try restore(secondsAfterTheInvoice: 60) + try pinTheClockWhileTheInvoiceIsValid() + leaveSparkUnsyncedAndEmpty() + rail.balanceSats = 100_000 + rail.sendStatus = .success(nil) + + _ = try await Bitkit.LightningService.shared.send(bolt11: Self.invoice) + + XCTAssertEqual(rail.sentBolt11, Self.invoice) + XCTAssertEqual(rail.sendStatusFetches, 1) + } + func testAPreRestoreInvoiceSparkMayHavePaidByTransferIsRefusedOnBothRails() async throws { for sparkFunded in [false, true] { try await holdForTheOperators(sparkFunded: sparkFunded) @@ -1482,6 +1516,81 @@ extension ChannelFreeRailTests { } } + // MARK: - Overlapping Spark sends + + /// Sends the invoice over a funded Spark while a second Spark send of it runs whole during this send's operator + /// check, after which Spark lists that send as `status`, and `afterCompeting` runs. + private func sendOverACompetingSparkSend( + listedAs status: LDKNode.PaymentStatus, + afterCompeting: @escaping () -> Void = {} + ) async throws -> PaymentHash { + let hash = try String(Bolt11Invoice.fromStr(invoiceStr: Self.invoice).paymentHash()) + let service = Bitkit.LightningService.shared + try await holdForTheOperators(sparkFunded: true) + rail.sendStatus = .success(nil) + rail.onSendStatus = { [rail] in + _ = try? await service.send(bolt11: Self.invoice) + rail?.payments = [Bitkit.ChannelFreeRailPayment(paymentHash: hash, direction: .outbound, status: status, preimage: nil)] + afterCompeting() + } + return try await service.send(bolt11: Self.invoice) + } + + func testOfTwoOverlappingSparkSendsOfOneInvoiceOnlyOneReachesSparkAndTheOtherReturnsItsPaidOutcome() async throws { + let hash = try String(Bolt11Invoice.fromStr(invoiceStr: Self.invoice).paymentHash()) + + let sent = try await sendOverACompetingSparkSend(listedAs: .succeeded) + + XCTAssertEqual(rail.sends, 1, "Spark sent an invoice it sent meanwhile") + XCTAssertEqual(sent, hash, "a paid send left its caller pending") + } + + func testASparkSendRefusedForACompetingOneThatFailedReturnsTheFailure() async throws { + do { + _ = try await sendOverACompetingSparkSend(listedAs: .failed) + XCTFail("A send refused for a competing one that failed reported success") + } catch { + XCTAssertTrue(error is Bitkit.ChannelFreeRailSendRefused, "\(error)") + } + XCTAssertEqual(rail.sends, 1) + } + + func testASparkSendRefusedForAFailedCompetingOneStaysPendingWhenASendEndsDuringItsRead() async throws { + let hash = try String(Bolt11Invoice.fromStr(invoiceStr: Self.invoice).paymentHash()) + let service = Bitkit.LightningService.shared + defer { service.endLdkSend(paymentHash: hash) } + + // A node retry in flight when the refused send settles ends while Spark's list is read. + await assertPendingOverACompetingSparkSend { [rail] in + service.beginLdkSend(paymentHash: hash) + rail?.onList = { service.endLdkSend(paymentHash: hash) } + } + } + + func testASparkSendRefusedForAFailedCompetingOneStaysPendingWhenASendStartsDuringItsRead() async throws { + let hash = try String(Bolt11Invoice.fromStr(invoiceStr: Self.invoice).paymentHash()) + let service = Bitkit.LightningService.shared + + // A Spark retry is recorded while Spark's list is read. + await assertPendingOverACompetingSparkSend { [rail] in + rail?.onList = { service.recordRailAttempt(paymentHash: hash) } + } + } + + private func assertPendingOverACompetingSparkSend( + afterCompeting: @escaping () -> Void, + file: StaticString = #filePath, + line: UInt = #line + ) async { + do { + _ = try await sendOverACompetingSparkSend(listedAs: .failed, afterCompeting: afterCompeting) + XCTFail("A send refused for a competing one reported success", file: file, line: line) + } catch { + XCTAssertTrue(PrivatePaykitService.isDuplicatePaymentError(error), "\(error)", file: file, line: line) + } + XCTAssertEqual(rail.sends, 1, file: file, line: line) + } + func testOnlyASparkPreferringSendPaysAnInvoicesSparkAddressByTransfer() { XCTAssertTrue(Bitkit.SparkManager.paysBySparkTransfer(prefersSpark: true, sparkTransferFeeSats: 0)) XCTAssertFalse(Bitkit.SparkManager.paysBySparkTransfer(prefersSpark: false, sparkTransferFeeSats: 0)) @@ -1771,6 +1880,7 @@ private final class FakeRail: Bitkit.ChannelFreeLightningRail { var estimateError: Error? var listError: Error? var sentBolt11: String? + var sends = 0 var sentFeeLimitSats: UInt64? var sendGate: (() async -> Void)? var refusal: Bitkit.ChannelFreeRailSendRefused? @@ -1786,6 +1896,8 @@ private final class FakeRail: Bitkit.ChannelFreeLightningRail { /// What the operators show of the invoice's sends; unreachable unless a test sets it. var sendStatus: Result = .failure(NSError(domain: "spark operators", code: 1)) var sendStatusFetches = 0 + /// Runs once, on the next read of the operators' answer, before it returns `sendStatus`. + var onSendStatus: (() async -> Void)? /// Whether the invoice carries a Spark address a Spark-preferring send pays directly. var paysBySparkTransfer: Result = .success(false) var sparkTransferChecks = 0 @@ -1809,6 +1921,7 @@ private final class FakeRail: Bitkit.ChannelFreeLightningRail { await sendGate?() sentFeeLimitSats = maxFeeSats sentBolt11 = bolt11 + sends += 1 return "ee" } @@ -1825,6 +1938,10 @@ private final class FakeRail: Bitkit.ChannelFreeLightningRail { func lightningSendStatus(bolt11: String) async throws -> LDKNode.PaymentStatus? { sendStatusFetches += 1 + if let onSendStatus { + self.onSendStatus = nil + await onSendStatus() + } return try sendStatus.get() } diff --git a/BitkitTests/SparkExitReviewTests.swift b/BitkitTests/SparkExitReviewTests.swift index b05084581..793409dfc 100644 --- a/BitkitTests/SparkExitReviewTests.swift +++ b/BitkitTests/SparkExitReviewTests.swift @@ -763,6 +763,20 @@ final class SparkExitReviewTests: XCTestCase { XCTAssertFalse(record.isRebuilding) } + /// A Redo on a fee ask whose rebuild the held funding pays that same tick withdraws the ask's notification. + func testARedoRebuiltFromTheHeldFundingOnItsOwnTickWithdrawsTheFeeAsk() async throws { + let asked = bumpedRecord() + sdk.checks = try [CheckRecoverFundsResponse(recovery: XCTUnwrap(asked.recovery?.response), verdict: .redo(reason: .onChainStateDiverged))] + sdk.builds = [SparkExitFixtures.recovery([SparkExitFixtures.tx("m1")])] + + var record = try await driver().tick(asked) + + XCTAssertEqual(record.phase, .running) + XCTAssertTrue(SparkExitManager.isFundingAskWithdrawn(record, previous: asked), "the fee ask's notification stayed") + record = try await driver().tick(record) + XCTAssertNil(record.isRedoAsk, "the ended ask stayed marked") + } + /// A Redo rebuild the held funding pays too little for still follows the awaited top-up: one that confirmed short /// asks for the rest, as while waiting for it. func testARedoRebuildShortOfFundingAsksForTheRestOfAShortTopUp() async throws { diff --git a/BitkitTests/SparkExitTests.swift b/BitkitTests/SparkExitTests.swift index a823e4ce6..9dd11fafe 100644 --- a/BitkitTests/SparkExitTests.swift +++ b/BitkitTests/SparkExitTests.swift @@ -1495,8 +1495,10 @@ final class SparkExitDriverTests: XCTestCase { sdk.builds = Array(repeating: SparkExitFixtures.recovery([SparkExitFixtures.tx("n1b")]), count: 2) clock = clock.addingTimeInterval(SparkExitDriver.rebuildInterval) + let asked = record record = try await driver().tick(record) XCTAssertEqual(record.funding.compactMap(\.txid), ["f0", "topup"]) + XCTAssertFalse(SparkExitManager.isFundingAskWithdrawn(record, previous: asked), "a top-up answered the ask") XCTAssertEqual(record.generation, 1, "the Redo rebuild without leaf-2 was adopted") XCTAssertEqual(record.phase, .running, "a funded hold still asks for fees") XCTAssertEqual(record.redoHeldSince, 1000) @@ -1526,6 +1528,53 @@ final class SparkExitDriverTests: XCTestCase { XCTAssertEqual(record.retrySince, chain.tip, "the waiting leaf's retry clock starts once the hold ends") } + /// A Redo hold whose build the held funding paid for runs at the rebuild pace, whatever the old shortfall, and its + /// fee ask is withdrawn. A later build of the hold that comes up short asks again. + func testARedoHoldBuiltFromTheHeldFundingWithdrawsTheFeeAsk() async throws { + var record = try await heldRedoRebuild() + record.phase = .needsFunding(requiredSats: 23000) + let asked = record + chain.utxosByAddress["bcrt1qfunding"] = [SparkExitFixtures.utxo("f0", vout: 1, value: 20000)] + sdk.builds = [SparkExitFixtures.recovery([SparkExitFixtures.tx("n1b")])] + clock = clock.addingTimeInterval(SparkExitDriver.rebuildInterval) + + record = try await driver().tick(record) + + XCTAssertEqual(sdk.recoverRequests.count, 2, "the held funding did not build the rebuild") + XCTAssertEqual(record.generation, 1, "the Redo rebuild without leaf-2 was adopted") + XCTAssertEqual(record.phase, .running, "a hold the held funding paid for still asks for fees") + XCTAssertEqual(record.redoHeldSince, 1000) + XCTAssertTrue(SparkExitManager.isFundingAskWithdrawn(record, previous: asked), "the fee ask's notification stayed") + + sdk.recoverError = SdkError.InsufficientCpfpFunds(requiredSats: 30000, docsUrl: "") + clock = clock.addingTimeInterval(SparkExitDriver.rebuildInterval) + record = try await driver().tick(record) + + guard case .needsFunding = record.phase else { return XCTFail("a later short build of the hold did not ask: \(record.phase)") } + XCTAssertEqual(record.generation, 1) + } + + /// A Redo hold's own build that comes up short asks for fees; a later build the held funding pays withdraws the ask. + func testARedoHoldThroughAShortBuildWithdrawsTheAskOnceTheHeldFundingPays() async throws { + let held = try await heldRedoRebuild() + XCTAssertEqual(held.phase, .running) + chain.utxosByAddress["bcrt1qfunding"] = [SparkExitFixtures.utxo("f0", vout: 1, value: 20000)] + sdk.recoverErrors = [SdkError.InsufficientCpfpFunds(requiredSats: 30000, docsUrl: "")] + clock = clock.addingTimeInterval(SparkExitDriver.rebuildInterval) + let asked = try await driver().tick(held) + guard case .needsFunding = asked.phase else { return XCTFail("the hold's short build did not ask: \(asked.phase)") } + + // The next build needs less, which the held funding pays. + sdk.builds = [SparkExitFixtures.recovery([SparkExitFixtures.tx("n1b")])] + clock = clock.addingTimeInterval(SparkExitDriver.rebuildInterval) + let record = try await driver().tick(asked) + + XCTAssertEqual(record.phase, .running, "a hold the held funding paid for still asks for fees") + XCTAssertEqual(record.generation, 1) + XCTAssertEqual(record.funding, asked.funding) + XCTAssertTrue(SparkExitManager.isFundingAskWithdrawn(record, previous: asked), "the fee ask's notification stayed") + } + /// A Redo over a generation that recovers both leaves, whose rebuild skips leaf-2 as unread, at tip 1000. private func heldRedoRebuild() async throws -> SparkExitRecord { var record = twoLeafRecord() diff --git a/journeys/send/restored-wallet-pre-restore-invoice.xml b/journeys/send/restored-wallet-pre-restore-invoice.xml new file mode 100644 index 000000000..bd1f8533d --- /dev/null +++ b/journeys/send/restored-wallet-pre-restore-invoice.xml @@ -0,0 +1,33 @@ + + + After a restore, a still-valid invoice created before it pays over Spark once Spark's operators show no send of + it, and one the wallet paid before the restore never goes out again. Spark's list can lack a send the wallet made + before the restore, so a pre-restore invoice is checked with the operators, also while Spark has not synced since + the restore; a send they show completed or pending refuses it with "Bitkit can't confirm this invoice wasn't paid + before you restored your wallet. Please request a new invoice." A failed one counts as unsent. Once Spark's first + sync lists the earlier payment, the app reads it as already paid instead, with no new send: either outcome passes + the last check, as long as nothing leaves the wallet. The operator answers, the unsynced case and the failed-send + retry are covered by ChannelFreeRailTests. + + Preconditions: two Debug builds on two simulators, each with Spark connected on its regtest (no key needed), no + Lightning channel and QuickPay disabled. The payer has a Spark balance of at least 5,000 sats from Lightspark's + regtest faucet, sent to the deposit address its Spark connection screen shows (id "SparkDepositAddress"). Bitkit's + own invoices carry no Spark address, so the payer pays them over Lightning through Spark's provider. The journey + wipes the payer half way through, so do not run it against a wallet whose funds matter. Do all steps within the + invoices' expiry, a day for an invoice from Receive. + + + On the payee, tap Receive (id "Receive"), create a 1,000-sat Lightning invoice and copy it as invoice A + On the payee, create a second 1,500-sat Lightning invoice and copy it as invoice B + On the payer, record the Spending balance, open invoice A with xcrun simctl openurl and a lightning: link, and swipe to pay + Verify the payer shows the payment as sent and the payee received 1,000 sats + On the payer, open the menu, navigate to Settings, then the "Security" tab (id "Tab-security"), then tap "Back Up Your Money" (id "BackupWallet"), continue past the intro (id "BackupIntroViewContinue"), tap to reveal the phrase (id "TapToReveal") and record the phrase from the word grid (id "SeedContainer") + Open Settings, then the "Security" tab, then "Reset And Restore" (id "ResetAndRestore"), tap "Reset Wallet" and confirm with "Yes, Reset" in the alert; neither carries an id, so tap them by label + On the onboarding screen tap "Restore" (id "RestoreWallet"), then "MultipleDevices-button", enter the recorded words into the word inputs (id "Word-0" onwards) and tap Restore (id "RestoreButton") + Wait for the restore to finish, tap "Get Started" (id "GetStartedButton") and wait until the Spending balance shows the balance recorded before the payment, less invoice A and its fee + On the payer, open invoice B with a lightning: link and swipe to pay + Verify the payer shows the payment as sent, with no "Bitkit can't confirm this invoice wasn't paid before you restored your wallet" error, and the payee received 1,500 sats + On the payer, record the Spending balance, then open invoice A again with a lightning: link and swipe to pay if the confirmation opens + Verify invoice A did not go out again: the payer shows the error "Bitkit can't confirm this invoice wasn't paid before you restored your wallet. Please request a new invoice." or reads the invoice as already paid, the payer's Spending balance is unchanged and the payee received nothing new + +