Skip to content

Commit 14ab27c

Browse files
fix: address corey hw transfer review items
Tighten broadcast connectivity detection, restore HW fee grid with offline estimate, and clear pending broadcast from updateHwLimits to keep signing cancellation in sync. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 6a9af96 commit 14ab27c

9 files changed

Lines changed: 210 additions & 17 deletions

Bitkit/Extensions/BroadcastError+Connectivity.swift

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,45 @@
11
import BitkitCore
22
import Foundation
33

4+
extension BroadcastError {
5+
/// Whether this broadcast failure is likely transient connectivity (retry without re-signing).
6+
/// Core 0.4.1 maps both Electrum connect failures and node rejections to `ElectrumError`; rejections
7+
/// must not be treated as connectivity until core splits the variant (see bitkit-core).
8+
var isConnectivityFailure: Bool {
9+
switch self {
10+
case .InvalidHex, .InvalidTransaction:
11+
return false
12+
case .TaskError:
13+
return true
14+
case let .ElectrumError(errorDetails):
15+
return Self.isElectrumConnectivityDetails(errorDetails)
16+
}
17+
}
18+
19+
private static func isElectrumConnectivityDetails(_ details: String) -> Bool {
20+
let lower = details.lowercased()
21+
if lower.hasPrefix("broadcast failed:") {
22+
return false
23+
}
24+
if lower.hasPrefix("failed to connect to electrum:") {
25+
return true
26+
}
27+
if lower.contains("offline")
28+
|| lower.contains("timeout")
29+
|| lower.contains("connection refused")
30+
|| lower.contains("dns")
31+
|| lower.contains("network")
32+
{
33+
return true
34+
}
35+
return false
36+
}
37+
}
38+
439
extension Error {
540
func isBroadcastConnectivityFailure() -> Bool {
641
if let broadcastError = self as? BroadcastError {
7-
if case .ElectrumError = broadcastError {
8-
return true
9-
}
10-
return false
42+
return broadcastError.isConnectivityFailure
1143
}
1244

1345
if let appError = self as? AppError, let underlyingError = appError.underlyingError {

Bitkit/Managers/HwWalletManager.swift

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,10 +547,47 @@ final class HwWalletManager {
547547
sats: UInt64,
548548
satsPerVByte: UInt64,
549549
addressType: AddressScriptType = hwFundingDefaultAddressType
550+
) async throws -> HwFundingTransaction {
551+
let fingerprint = try await TrezorService.shared.getDeviceFingerprint()
552+
return try await composeFundingTransactionInternal(
553+
deviceId: deviceId,
554+
address: address,
555+
sats: sats,
556+
satsPerVByte: satsPerVByte,
557+
fingerprint: fingerprint,
558+
addressType: addressType
559+
)
560+
}
561+
562+
/// Offline coin-selection for the exact funding amount; returns the mining fee only.
563+
func estimateOfflineFundingMiningFee(
564+
deviceId: String,
565+
address: String,
566+
sats: UInt64,
567+
satsPerVByte: UInt64,
568+
addressType: AddressScriptType = hwFundingDefaultAddressType
569+
) async throws -> UInt64 {
570+
let funding = try await composeFundingTransactionInternal(
571+
deviceId: deviceId,
572+
address: address,
573+
sats: sats,
574+
satsPerVByte: satsPerVByte,
575+
fingerprint: nil,
576+
addressType: addressType
577+
)
578+
return funding.miningFeeSats
579+
}
580+
581+
private func composeFundingTransactionInternal(
582+
deviceId: String,
583+
address: String,
584+
sats: UInt64,
585+
satsPerVByte: UInt64,
586+
fingerprint: String?,
587+
addressType: AddressScriptType
550588
) async throws -> HwFundingTransaction {
551589
let account = try getFundingAccount(deviceId: deviceId, addressType: addressType)
552590
let network = networkProvider()
553-
let fingerprint = try await TrezorService.shared.getDeviceFingerprint()
554591
let params = ComposeParams(
555592
wallet: WalletParams(
556593
extendedKey: account.xpub,

Bitkit/ViewModels/HwFundingSigner.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ struct HwFundingSigner {
9898
connecting.warmUpConnection(deviceId: deviceId)
9999
}
100100

101+
/// Offline compose for the exact order amount; does not require a connected device.
102+
func estimateOfflineFundingMiningFee(deviceId: String, address: String, sats: UInt64) async throws -> UInt64 {
103+
let satsPerVByte = await resolvedSatsPerVByte()
104+
return try await funding.estimateOfflineFundingMiningFee(
105+
deviceId: deviceId,
106+
address: address,
107+
sats: sats,
108+
satsPerVByte: satsPerVByte,
109+
addressType: hwFundingDefaultAddressType
110+
)
111+
}
112+
101113
private func ensureConnected(deviceId: String) async throws {
102114
do {
103115
try await withTimeout(timeouts.reconnect) {

Bitkit/ViewModels/TransferViewModel.swift

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ protocol HwTransferFunding: Sendable {
7777
satsPerVByte: UInt64,
7878
addressType: AddressScriptType
7979
) async throws -> HwFundingTransaction
80+
/// Offline coin-selection estimate for the exact funding amount (`fingerprint: nil`); fee only.
81+
func estimateOfflineFundingMiningFee(
82+
deviceId: String,
83+
address: String,
84+
sats: UInt64,
85+
satsPerVByte: UInt64,
86+
addressType: AddressScriptType
87+
) async throws -> UInt64
8088
func signFunding(deviceId: String, funding: HwFundingTransaction) async throws -> HwFundingSignedTx
8189
func broadcastFunding(serializedTx: String) async throws -> String
8290
}
@@ -500,7 +508,8 @@ class TransferViewModel: ObservableObject {
500508
-> (networkFeeSat: UInt64, serviceFeeSat: UInt64)
501509
) async {
502510
guard let hwSigner else { return }
503-
hwSpending = HwSpendingState(isLoading: true)
511+
clearPendingHwFundingBroadcast()
512+
hwSpending.isLoading = true
504513

505514
let availability: HwFundingSigner.Availability
506515
do {
@@ -530,6 +539,22 @@ class TransferViewModel: ObservableObject {
530539
hwSpending.isLoading = false
531540
}
532541

542+
/// Best-effort offline mining-fee estimate for the Sign screen (`fingerprint: nil` compose).
543+
func updateHwFundingFeeEstimate(order: IBtOrder, deviceId: String) async {
544+
guard let hwSigner else { return }
545+
guard !hwSpending.hasPendingBroadcast else { return }
546+
guard let address = order.payment?.onchain?.address, !address.isEmpty else { return }
547+
do {
548+
hwSpending.miningFeeSats = try await hwSigner.estimateOfflineFundingMiningFee(
549+
deviceId: deviceId,
550+
address: address,
551+
sats: order.feeSat
552+
)
553+
} catch {
554+
Logger.debug("Skipped offline hardware funding fee estimate for '\(deviceId)'", context: "TransferViewModel")
555+
}
556+
}
557+
533558
/// Pay for the order by composing and signing the funding send on the Trezor (via the signer),
534559
/// then record and watch it. Coordination only — the device orchestration lives in `HwFundingSigner`.
535560
func onTransferToSpendingHwConfirm(order: IBtOrder, deviceId: String) {
@@ -564,8 +589,8 @@ class TransferViewModel: ObservableObject {
564589
order: order,
565590
deviceId: deviceId,
566591
address: address
567-
) { funding in
568-
self.hwSpending.miningFeeSats = funding.miningFeeSats
592+
) { [weak self] funding in
593+
self?.hwSpending.miningFeeSats = funding.miningFeeSats
569594
}
570595
pendingHwFundingBroadcast = PendingHwFundingBroadcast(
571596
orderId: order.id,

Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ struct SpendingHwSign: View {
4444
.navigationBarHidden(true)
4545
.padding(.horizontal, 16)
4646
.bottomSafeAreaPadding()
47-
.task {
48-
// Warm up the BLE connection ahead of the user tapping Open Trezor Connect.
47+
.task(id: order.id) {
4948
transfer.warmUpHardwareConnection(deviceId: deviceId)
49+
await transfer.updateHwFundingFeeEstimate(order: order, deviceId: deviceId)
5050
}
5151
.onChange(of: transfer.hwSignedEvent) {
5252
navigation.navigate(.spendingHwSigned)
@@ -77,7 +77,7 @@ struct SpendingHwSign: View {
7777
accentColor: .purpleAccent
7878
)
7979

80-
SpendingHwFeeGrid(order: order)
80+
SpendingHwFeeGrid(order: order, miningFeeSats: transfer.hwSpending.miningFeeSats)
8181
.padding(.top, 16)
8282

8383
HStack(spacing: 16) {
@@ -151,19 +151,28 @@ struct SpendingHwSign: View {
151151
/// Blocktank order fee summary shared by the hardware Sign and Signed screens.
152152
struct SpendingHwFeeGrid: View {
153153
let order: IBtOrder
154+
var miningFeeSats: UInt64 = 0
155+
156+
private var lspFee: UInt64 {
157+
order.feeSat - order.clientBalanceSat
158+
}
159+
160+
private var total: UInt64 {
161+
order.feeSat + miningFeeSats
162+
}
154163

155164
var body: some View {
156165
VStack(spacing: 16) {
157166
HStack {
158167
FeeDisplayRow(
159168
label: t("lightning__spending_confirm__network_fee"),
160-
amount: order.networkFeeSat
169+
amount: miningFeeSats
161170
)
162171
.frame(maxWidth: .infinity)
163172

164173
FeeDisplayRow(
165174
label: t("lightning__spending_confirm__lsp_fee"),
166-
amount: order.serviceFeeSat
175+
amount: lspFee
167176
)
168177
.frame(maxWidth: .infinity)
169178
}
@@ -177,7 +186,7 @@ struct SpendingHwFeeGrid: View {
177186

178187
FeeDisplayRow(
179188
label: t("lightning__spending_confirm__total"),
180-
amount: order.feeSat
189+
amount: total
181190
)
182191
.frame(maxWidth: .infinity)
183192
}

Bitkit/Views/Transfer/Hardware/SpendingHwSigned.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ struct SpendingHwSigned: View {
2727

2828
DisplayText(t("lightning__transfer_hw__signed_title"), accentColor: .purpleAccent)
2929

30-
SpendingHwFeeGrid(order: order)
30+
SpendingHwFeeGrid(order: order, miningFeeSats: transfer.hwSpending.miningFeeSats)
3131
.padding(.top, 16)
3232

3333
Image("check")

BitkitTests/BroadcastConnectivityTests.swift

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,36 @@ import BitkitCore
33
import XCTest
44

55
final class BroadcastConnectivityTests: XCTestCase {
6-
func testElectrumBroadcastErrorIsConnectivityFailure() {
6+
func testElectrumConnectFailureIsConnectivityFailure() {
7+
XCTAssertTrue(
8+
BroadcastError.ElectrumError(errorDetails: "Failed to connect to Electrum: offline")
9+
.isBroadcastConnectivityFailure()
10+
)
11+
}
12+
13+
func testElectrumOfflineDetailsAreConnectivityFailure() {
714
XCTAssertTrue(BroadcastError.ElectrumError(errorDetails: "offline").isBroadcastConnectivityFailure())
815
}
916

10-
func testWrappedElectrumBroadcastErrorIsConnectivityFailure() {
17+
func testElectrumBroadcastRejectionIsNotConnectivityFailure() {
18+
XCTAssertFalse(
19+
BroadcastError.ElectrumError(errorDetails: "Broadcast failed: bad-txns-inputs-missingorspent")
20+
.isBroadcastConnectivityFailure()
21+
)
22+
}
23+
24+
func testWrappedElectrumConnectFailureIsConnectivityFailure() {
1125
let error = Bitkit.AppError(error: BroadcastError.ElectrumError(errorDetails: "offline"))
1226
XCTAssertTrue(error.isBroadcastConnectivityFailure())
1327
}
1428

29+
func testWrappedElectrumBroadcastRejectionIsNotConnectivityFailure() {
30+
let error = Bitkit.AppError(
31+
error: BroadcastError.ElectrumError(errorDetails: "Broadcast failed: min relay fee not met")
32+
)
33+
XCTAssertFalse(error.isBroadcastConnectivityFailure())
34+
}
35+
1536
func testUnrelatedErrorIsNotConnectivityFailure() {
1637
XCTAssertFalse(Bitkit.AppError(message: "nope", debugMessage: nil).isBroadcastConnectivityFailure())
1738
}

BitkitTests/HwTransferMocks.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ final class MockHwFunding: HwTransferFunding {
2222
var broadcastTxId = "txid"
2323

2424
private(set) var composeCalls: [(address: String, sats: UInt64, satsPerVByte: UInt64)] = []
25+
private(set) var estimateCalls: [(address: String, sats: UInt64, satsPerVByte: UInt64)] = []
2526
private(set) var maxSpendableCalls: [(address: String, satsPerVByte: UInt64)] = []
2627
private(set) var signCalls = 0
2728
private(set) var broadcastCalls = 0
@@ -55,6 +56,18 @@ final class MockHwFunding: HwTransferFunding {
5556
return funding
5657
}
5758

59+
func estimateOfflineFundingMiningFee(
60+
deviceId _: String,
61+
address: String,
62+
sats: UInt64,
63+
satsPerVByte: UInt64,
64+
addressType _: AddressScriptType
65+
) async throws -> UInt64 {
66+
estimateCalls.append((address, sats, satsPerVByte))
67+
if let composeError { throw composeError }
68+
return funding.miningFeeSats
69+
}
70+
5871
func signFunding(deviceId _: String, funding _: HwFundingTransaction) async throws -> HwFundingSignedTx {
5972
signCalls += 1
6073
if signDelay > 0 { try await Task.sleep(nanoseconds: UInt64(signDelay * 1_000_000_000)) }

BitkitTests/TransferViewModelHwTests.swift

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,50 @@ final class TransferViewModelHwTests: XCTestCase {
274274
if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") }
275275
}
276276

277+
func testElectrumBroadcastRejectionClearsPendingState() async {
278+
let funding = MockHwFunding()
279+
funding.broadcastError = BroadcastError.ElectrumError(
280+
errorDetails: "Broadcast failed: bad-txns-inputs-missingorspent"
281+
)
282+
let vm = makeViewModel(funding: funding, connecting: MockHwConnecting())
283+
284+
vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1")
285+
await awaitSigningComplete(vm)
286+
287+
XCTAssertFalse(vm.hwSpending.hasPendingBroadcast)
288+
if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") }
289+
XCTAssertEqual(funding.signCalls, 1)
290+
}
291+
292+
func testUpdateHwLimitsClearsDesyncedPendingBroadcast() async {
293+
let funding = MockHwFunding()
294+
let vm = makeViewModel(funding: funding, connecting: MockHwConnecting())
295+
funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline")
296+
let order = IBtOrder.mock()
297+
298+
vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1")
299+
await awaitSigningComplete(vm)
300+
XCTAssertTrue(vm.hwSpending.hasPendingBroadcast)
301+
302+
await vm.updateHwLimits(deviceId: "dev1", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) })
303+
304+
XCTAssertFalse(vm.hwSpending.hasPendingBroadcast)
305+
vm.cancelHwSigning()
306+
XCTAssertEqual(funding.signCalls, 1, "cancel after limits reload must not be blocked by stale pending tx")
307+
}
308+
309+
func testUpdateHwFundingFeeEstimateSetsMiningFeeBeforeSigning() async {
310+
let funding = MockHwFunding()
311+
let vm = makeViewModel(funding: funding, connecting: MockHwConnecting())
312+
let order = IBtOrder.mock()
313+
314+
await vm.updateHwFundingFeeEstimate(order: order, deviceId: "dev1")
315+
316+
XCTAssertEqual(vm.hwSpending.miningFeeSats, funding.funding.miningFeeSats)
317+
XCTAssertEqual(funding.estimateCalls.count, 1)
318+
XCTAssertTrue(funding.composeCalls.isEmpty)
319+
}
320+
277321
func testReentrancyGuardIgnoresConcurrentConfirm() async {
278322
let funding = MockHwFunding()
279323
funding.composeError = MockHwFunding.TestError() // fail before the network-bound funding tail

0 commit comments

Comments
 (0)