From 0fce8fa17d2c4f32a0411f34a0267b94a6573d3c Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 27 May 2026 11:13:28 +0200 Subject: [PATCH] fix(swift-sdk): sort transfer outputs lexicographically before ReduceOutput MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ManagedPlatformAddressWallet.transfer()` targeted the fee-reduction step by Swift insertion index, telling Rust to `ReduceOutput(N)` where N was `ffiOutputs.count - 1` (the change row appended last). The Rust side collects those outputs into `BTreeMap`, which canonicalizes them lexicographically by address — variant byte first (`P2pkh = 0 < P2sh = 1`), then the 20-byte hash. DPP's fee-deduction layer indexes that canonicalized list when resolving `ReduceOutput(N)`. When the resolved change address sorted before any recipient, the wrong row got decremented: the recipient was underpaid by the fee amount, and the change came back full. Hits randomly based on which address gets generated as change, so it passed integration tests most runs. Mirror the Rust ordering in Swift via a new `buildSortedFFIOutputs` helper: build `(addressType, hash, balance)` rows for all outputs + change, sort by `(addressType, hash)` using `Data.lexicographicallyPrecedes`, then look up the change row's actual position in the sorted list. Marshal into `AddressBalanceEntryFFI` only after the sort completes. Add 2 regression tests on the pure helper: - #3738 scenario (change `0x00…`, recipient `0xFF…`) — change at sorted index 0 - multi-recipient with change in middle, crossing the 0x7F/0x80 byte boundary to catch any signed-byte comparator regression Fixes #3738 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ManagedPlatformAddressWallet.swift | 77 ++++++++++------- .../ManagedPlatformAddressWalletTests.swift | 82 +++++++++++++++++++ 2 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ManagedPlatformAddressWalletTests.swift diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift index 1240e4630d9..db354c5f298 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift @@ -262,40 +262,15 @@ public final class ManagedPlatformAddressWallet: @unchecked Sendable { ) } - // Marshal recipient outputs. - var ffiOutputs: [AddressBalanceEntryFFI] = [] - ffiOutputs.reserveCapacity(outputs.count + 1) - for out in outputs { - let outTuple = Self.hashTuple(from: out.hash) - ffiOutputs.append( - AddressBalanceEntryFFI( - address: PlatformAddressFFI(address_type: out.addressType, hash: outTuple), - balance: out.credits, - nonce: 0, - account_index: 0, - address_index: 0 - ) - ) - } - - // Append the change output to the resolved change address — - // guaranteed to be distinct from every input and recipient - // (the protocol rejects outputs that also appear as inputs). + // Build the FFI output list in the same lexicographic order Rust's + // BTreeMap canonicalizes to, so the fee-reduction + // index we hand it lines up with the row Rust will actually decrement. let changeAmount = totalInputs - totalRecipientCredits - let changeTuple = Self.hashTuple(from: resolvedChange.hash) - ffiOutputs.append( - AddressBalanceEntryFFI( - address: PlatformAddressFFI(address_type: resolvedChange.addressType, hash: changeTuple), - balance: changeAmount, - nonce: 0, - account_index: 0, - address_index: 0 - ) + let (ffiOutputs, changeIndex) = Self.buildSortedFFIOutputs( + recipients: outputs, + change: (resolvedChange.addressType, resolvedChange.hash, changeAmount) ) - // Fee strategy: take the fee out of the change output (last index) - // so recipients get their requested amounts unchanged. - let changeIndex = UInt16(ffiOutputs.count - 1) let feeStrategy: [FeeStrategyStepFFI] = [ FeeStrategyStepFFI(step_type: 1, index: changeIndex) // 1 = ReduceOutput ] @@ -353,6 +328,46 @@ public final class ManagedPlatformAddressWallet: @unchecked Sendable { }.value } + /// Build the FFI output array (recipients + change) in the same + /// lexicographic order Rust's `BTreeMap` uses, and + /// return the change row's index in that sorted list. + /// + /// Mirrors `derive(Ord)` on + /// `enum PlatformAddress { P2pkh([u8;20]), P2sh([u8;20]) }`: variant + /// discriminant first (`P2pkh = 0 < P2sh = 1`), then 20-byte hash + /// compared lexicographically. Load-bearing because + /// `FeeStrategyStep::ReduceOutput(N)` on the Rust side indexes the + /// post-canonicalization output list — not Swift's insertion order. + /// See https://github.com/dashpay/platform/issues/3738. + internal static func buildSortedFFIOutputs( + recipients: [TransferOutput], + change: (addressType: UInt8, hash: Data, balance: UInt64) + ) -> (rows: [AddressBalanceEntryFFI], changeIndex: UInt16) { + var rows: [(addressType: UInt8, hash: Data, balance: UInt64)] = + recipients.map { (addressType: $0.addressType, hash: $0.hash, balance: $0.credits) } + rows.append(change) + rows.sort { a, b in + if a.addressType != b.addressType { return a.addressType < b.addressType } + return a.hash.lexicographicallyPrecedes(b.hash) + } + let changeIdx = UInt16(rows.firstIndex { + $0.addressType == change.addressType && $0.hash == change.hash + }!) + let ffiRows = rows.map { row in + AddressBalanceEntryFFI( + address: PlatformAddressFFI( + address_type: row.addressType, + hash: hashTuple(from: row.hash) + ), + balance: row.balance, + nonce: 0, + account_index: 0, + address_index: 0 + ) + } + return (ffiRows, changeIdx) + } + /// Copy a 20-byte `Data` into the fixed-size tuple shape the FFI expects. private static func hashTuple( from data: Data diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ManagedPlatformAddressWalletTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ManagedPlatformAddressWalletTests.swift new file mode 100644 index 00000000000..a34c0701ba2 --- /dev/null +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ManagedPlatformAddressWalletTests.swift @@ -0,0 +1,82 @@ +import XCTest +@testable import SwiftDashSDK + +final class ManagedPlatformAddressWalletTests: XCTestCase { + + /// Convert the FFI's 20-byte tuple back to Data for assertion. + private func hashData(_ entry: AddressBalanceEntryFFI) -> Data { + withUnsafeBytes(of: entry.address.hash) { Data($0) } + } + + // Pre-fix this returned changeIndex == 1 (insertion order, change was + // last). Rust then indexed sorted row 1 (the recipient) and carved the + // fee out of them. Regression scenario from issue #3738. + func test_buildSortedFFIOutputs_changeSortsBeforeRecipient_indexIsZero() { + let recipientHash = Data(repeating: 0xFF, count: 20) + let changeHash = Data(repeating: 0x00, count: 20) + let recipient = ManagedPlatformAddressWallet.TransferOutput( + addressType: 0, + hash: recipientHash, + credits: 100 + ) + let change = ( + addressType: UInt8(0), + hash: changeHash, + balance: UInt64(50) + ) + + let (rows, changeIndex) = ManagedPlatformAddressWallet.buildSortedFFIOutputs( + recipients: [recipient], + change: change + ) + + XCTAssertEqual(changeIndex, 0) + XCTAssertEqual(rows.count, 2) + XCTAssertEqual(hashData(rows[0]), changeHash, "row 0 = change address (0x00…)") + XCTAssertEqual(rows[0].balance, 50) + XCTAssertEqual(hashData(rows[1]), recipientHash, "row 1 = recipient address (0xFF…)") + XCTAssertEqual(rows[1].balance, 100) + } + + // Multi-recipient: change address sorts into the MIDDLE of the + // output list. Defends against an off-by-one or + // last-position-assumption regression in the helper, and crosses + // the 0x7F/0x80 byte boundary so that any accidental signed-byte + // comparison would flip the order and fail the test. + func test_buildSortedFFIOutputs_multipleRecipients_changeInMiddle() { + let lowRecipientHash = Data(repeating: 0x10, count: 20) + let changeHash = Data(repeating: 0x80, count: 20) + let highRecipientHash = Data(repeating: 0xF0, count: 20) + let recipients = [ + ManagedPlatformAddressWallet.TransferOutput( + addressType: 0, + hash: lowRecipientHash, + credits: 100 + ), + ManagedPlatformAddressWallet.TransferOutput( + addressType: 0, + hash: highRecipientHash, + credits: 200 + ), + ] + let change = ( + addressType: UInt8(0), + hash: changeHash, + balance: UInt64(75) + ) + + let (rows, changeIndex) = ManagedPlatformAddressWallet.buildSortedFFIOutputs( + recipients: recipients, + change: change + ) + + XCTAssertEqual(rows.count, 3) + XCTAssertEqual(changeIndex, 1, "change at 0x80… sorts between 0x10… and 0xF0…") + XCTAssertEqual(hashData(rows[0]), lowRecipientHash) + XCTAssertEqual(rows[0].balance, 100) + XCTAssertEqual(hashData(rows[1]), changeHash) + XCTAssertEqual(rows[1].balance, 75) + XCTAssertEqual(hashData(rows[2]), highRecipientHash) + XCTAssertEqual(rows[2].balance, 200) + } +}