Skip to content

kotlin-sdk: bridge the change-address reservation primitive to FFI (self-transfer destinations collide with ordinary payment change) #5276

Open

Description

@HashEngineering

Problem

ManagedCoreWallet.nextChangeAddress(accountIndex) returns the next unused internal address without claiming it. The transaction builder's change selector draws from the same pool with the same call, and AddressPool::next_unused does not change state. So while an address C is unused, an app-selected self-transfer destination is the next ordinary payment's change address — deterministically, not as a race.

Consumer impact in dash-wallet (dashpay/dash-wallet#1570): unshield, paper-wallet sweep and CoinJoin combine each obtain a destination via nextChangeAddress, and an ordinary payment made while one of those is in flight can pay the same address. The payment's recipient can identify its own change output, and therefore associate the self-transfer — and its amount — with the paying customer.

The window is not build-time. The destination stops being shared only when the engine marks it used on network observation of the spending transaction. For a shielded withdrawal, which runs through the platform withdrawal queue, that can be ~10 minutes, and the app's proving overlay is explicitly dismissable so the user can keep transacting meanwhile.

Why this is cheap to fix

The primitive already exists in key-wallet and already composes with the change selector:

  • AddressState { Available, Reserved { at }, Used } — address_pool.rs (serde + bincode derives present)
  • AddressPool::next_unused_and_reserve(key_source, now), release_reservation(index), sweep_expired_reservations(now, ttl)
  • is_available() is Available only, and next_unused already skips Reserved

So a reservation is honoured by TransactionBuilder::fund_from with no change to the change selector and no cooperation from competing builders.

The gap is exactly two things: there is no internal-pool equivalent of the receive-side trio (next_receive_address_and_reserve / release_receive_reservation / sweep_expired_receive_reservations on ManagedCoreFundsAccount), and none of it is bridged to FFI.

Proposed change

1. key-wallet (rust-dashcore) — mirror the receive-side trio onto internal_addresses, ~60 lines:

ManagedCoreFundsAccount::next_change_address_and_reserve(&mut self, xpub: Option<&ExtendedPubKey>, now: u64) -> Result<Address, &'static str>
ManagedCoreFundsAccount::release_change_reservation(&mut self, address: &Address) -> bool
ManagedCoreFundsAccount::sweep_expired_change_reservations(&mut self, now: u64, ttl: u64) -> usize

2. rs-platform-wallet + rs-platform-wallet-ffi — bridge it, beside core_wallet_next_change_address in src/core_wallet/addresses.rs:

core_wallet_next_change_address_and_reserve(handle, account_index, now_secs, out_address)
core_wallet_release_change_reservation(handle, account_index, address)

plus the corresponding CoreWallet::*_for_account wrappers.

3. kotlin-sdk — ManagedCoreWallet.nextChangeAddressAndReserve(accountIndex: Int): String and releaseChangeReservation(accountIndex: Int, address: String).

Durability requirement — please treat as part of the ask

The reservation must survive process death and be TTL-swept. Without persistence, an unreserved-on-restart address reopens the hole; without a TTL sweep, a killed app pins gap-limit headroom forever.

AddressInfo.state carries serde + bincode derives, but that is not evidence it is actually persisted and reloaded. Worth stating because there is precedent for exactly this gap: set_gap_limit turned out to be in-memory only, which was discovered by field measurement rather than from the signature.

A strictly better variant, if you are open to it

A persisted ISSUED watermark distinct from USED, with next_unused drawing from max(used, issued), would fix this and the fresh/current receive-address collapse — one upstream change, two defects. The second is what dashpay/dash-wallet#1570 exists to work around: with the dashj wallet held post-cutover its key chain never observes a block, so markPubKeyAsUsed never runs, and both currentReceiveAddress() and freshReceiveAddress() walked forward from the restore index through the already-used range, serving addresses the wallet had already been paid on.

Low-effort stopgap

Making CoreTransactionBuilder.setChangeAddress public — or adding an optional changeAddress: String? to sendToAddresses / buildSignedPayment — would let an app implement a soft reservation without reimplementing the send path. It pins rather than reserves, so it is a mitigation rather than a fix, but it is small and unblocks consumers before the above lands.

Unrelated, noticed in passing

core_wallet_tx_builder_add_inputs_from_outpoints exists at the FFI but is not bound in Kotlin. It is the natural basis for the UTXO lock/exclusion capability consumers currently lack.

Versions

Observed against org.dashj:dash-sdk-android:0.1.0-v42int27-SNAPSHOT. Rust mechanism read from platform @ v5.0-dev and the vendored rust-dashcore; those are not proven to be the exact revisions the pinned AAR was built from, so please confirm line-level details against the build revision.

Activity

  1. HashEngineering commented on Oct 5, 2026

    @HashEngineering
    CollaboratorAuthor

    Widening the ask: iOS has the same defect on the receive chain

    I scoped this to change-address reservation because that is where Android hit it. Having now checked dashpay/dashwallet-ios @ develop (HEAD 7c0064d), that scoping is wrong — please read the ask as covering both chains.

    iOS never calls nextChangeAddress, and still has the bug

    Zero call sites for nextChangeAddress / getNextChangeAddress / next_change_address anywhere in the app (.swift, .m, .h). The SDK picks change internally in the transaction builder and nothing competes for that pointer. The one change-address touch is SwiftDashSDKTransactionSender.swift:433, pinning change back to the source address in a send-from-address flow.

    Instead, one function — SwiftDashSDKReceiveAddressReader.receiveAddress() → wallet.coreWallet().nextReceiveAddress(accountIndex: 0) (SwiftDashSDKReceiveAddressReader.swift:137) — serves both the advertised address and the self-transfer destination:

    Advertised: Receive QR (PaymentsLandingViewModel.swift:452), Coinbase (CBAccount.swift:111), Uphold (UpholdAmountModel.swift:86), Topper (TopperViewModel.swift:49), CrowdNode account identity (CrowdNodeModel.swift:148), SwapKit (SwapKitSwapProvider.swift:989, RefundAddressViewModel.swift:158), ZenLedger (ZenLedgerViewModel.swift:34), BIP70 refund (BIP70PaymentService+App.swift:18), Explore, Apple Watch, home shortcut.

    Self-transfer destinations: unshield / shielded→core withdraw (ShieldedTransferCoordinator.swift:919, :1223), CoinJoin consolidation (WalletSendService.swift:440), identity credit withdrawal (IdentityWithdrawViewModel.swift:138).

    Since next_unused leaves state untouched, all of these return the same string between two chain events. Concretely: consolidating freshly-mixed CoinJoin funds pays the address the user has already given to a KYC'd exchange. That is a worse outcome than the Android case in the issue body — there the colliding address is one a payment counterparty can infer is change; here it is one the user handed over.

    Why this makes the fix narrower to specify, not broader

    core_wallet_next_receive_address and core_wallet_next_change_address both land on AddressPool::next_unused, which returns the first is_available() entry and does not change its state. So this is one defect with two consumers, not two defects.

    The receive-side half of the fix already exists: ManagedCoreFundsAccount::next_receive_address_and_reserve on top of AddressPool::next_unused_and_reserve / release_reservation / sweep_expired_reservations. It is simply not exposed. So the request becomes:

    1. Add the change-chain wrapper (next_change_address_and_reserve and friends) — the ~60 lines in the issue body.
    2. Bridge BOTH through rs-platform-wallet → rs-platform-wallet-ffi → Kotlin and Swift: core_wallet_next_receive_address_and_reserve and core_wallet_next_change_address_and_reserve, plus release and TTL sweep.

    One change fixes Android's change-chain collision and iOS's receive-chain collision.

    The reference implementation already works around this

    swift-sdk/SwiftExampleApp/.../WithdrawPlatformAddressView.swift:696-705 hand-rolls a per-sheet address cache, with a comment at :584-591 asserting that nextReceiveAddress advances the external pool and that repeated probes "burned receive addresses". That comment cites rs-platform-wallet/src/wallet/core/wallet.rs:103, which no longer matches, and the code I read does not advance — so I suspect it is stale. Flagging it either way for two reasons: if it is stale it should be corrected before someone copies the workaround, and if it is accurate then the behaviour is worse than described here and the example app is the only thing mitigating it. Either way, when the reference app needs a cache to use an address API safely, the API is the thing to fix.

    Caveat, unchanged

    Rust citations are from platform @ v5.0-dev and vendored rust-dashcore; neither is proven to be the revision the pinned artifacts were built from. The iOS app pins only a relative path (project.pbxproj:14056, relativePath = "../platform/packages/swift-sdk") with no commit, so confirm line-level details against your build revisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions