Skip to content

key-wallet: TransactionBuilder build/build_signed return the size-based fee, not the fee the tx actually pays (dust-drop under-report) #871

Description

@QuantumExplorer

Problem

TransactionBuilder::build() / build_signed() return a fee computed as fee_rate.calculate_fee(encoded_size(tx)) — the size-based target — rather than what the built transaction actually pays (Σ selected input values − Σ output values).

The two diverge whenever the builder drops a dust change remainder: in build() the change output is emitted only when change_amount > 546 (key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs, ~line 368 at rev 647fa98). When 0 < change ≤ 546, those duffs go to miners — the transaction's real fee is size_fee + dust, but the returned value is still just the size fee (~lines 441–451 and 485–495; the doc at ~line 456 says "The returned fee is computed from the encoded size of the signed tx"). So the returned value under-reports by up to 546 duffs exactly when the builder makes the fee larger.

Impact

Any caller surfacing the returned fee as "the network fee" shows a wrong number in the dust-drop case. Found via dashpay/platform#4049, where send_payment exposes the fee through FFI/Swift for wallet UIs — the platform side currently carries a caller-side workaround that recomputes Σinputs − Σoutputs from the account UTXO map after the build (fragile: depends on UTXO-reservation semantics and adds a lookup failure mode the builder never had).

Proposed fix

build() already has total_input and the final output set in scope when it assembles the transaction (it computes change_amount from them). Return the actual paid feetotal_input − Σ(final tx outputs) — from build() and build_signed(), and update the doc comment accordingly. Two-line change, no new lookups.

Blast-radius note: in dashpay/platform at the current pin, the only non-test consumer of the returned fee is send_payment (the standard-send path in wallet/core/broadcast.rs discards it via let (tx, _fee)), so the semantics change is safe there; internal rust-dashcore consumers should get a quick audit. key-wallet's own builder tests already assert fee ≈ 226-style values via total_input − total_output arithmetic (e.g. transaction_builder.rs ~line 823–828), which matches the corrected semantics.

Follow-up once fixed

dashpay/platform can delete the send_payment recomputation block and take build_signed's fee directly; the two end-to-end regression tests added in dashpay/platform#4049 (dust-drop + emitted-change) pin the behavior and will prove the simplification safe.

🤖 Generated with Claude Code

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