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

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions