Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/rs-platform-wallet-ffi/src/dashpay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ pub unsafe extern "C" fn platform_wallet_send_dashpay_payment(
memo: *const c_char,
core_signer_handle: *mut MnemonicResolverHandle,
out_txid: *mut [u8; 32],
out_fee_duffs: *mut u64,
) -> PlatformWalletFFIResult {
check_ptr!(core_signer_handle);
check_ptr!(out_txid);
Expand Down Expand Up @@ -592,7 +593,13 @@ pub unsafe extern "C" fn platform_wallet_send_dashpay_payment(
})
});
let result = unwrap_option_or_return!(option);
let (txid, _entry) = unwrap_result_or_return!(result);
let (txid, _entry, fee_duffs) = unwrap_result_or_return!(result);
// Exact network fee of the broadcast transaction (inputs − outputs),
// straight from the builder — nullable so callers that don't care
// can pass NULL.
if !out_fee_duffs.is_null() {
unsafe { *out_fee_duffs = fee_duffs };
}
use dashcore::hashes::Hash;
let bytes = txid.to_raw_hash().to_byte_array();
unsafe {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ impl<B: TransactionBroadcaster + ?Sized> DashPayView<'_, B> {
(
dashcore::Txid,
crate::wallet::identity::types::dashpay::payment::PaymentEntry,
u64, // network fee in duffs, from the broadcast transaction
),
PlatformWalletError,
>
Expand All @@ -575,7 +576,7 @@ impl<B: TransactionBroadcaster + ?Sized> DashPayView<'_, B> {
// re-acquires that (non-reentrant) lock internally.
self.drain_pending_contact_crypto(provider).await;

let (payment_address, tx) = {
let (payment_address, tx, fee) = {
let mut wm = self.wallet_manager.write().await;

// Resolve the external account's xpub so we can derive addresses.
Expand Down Expand Up @@ -664,14 +665,14 @@ impl<B: TransactionBroadcaster + ?Sized> DashPayView<'_, B> {
// `impl<S: Signer> TransactionSigner for S`) rather than the
// resident `wallet`, so funding-input signatures are produced
// from Keychain-derived keys without a resident seed.
let (tx, _fee) = builder
let (tx, fee) = builder
.build_signed(signer, |addr| {
managed_account.address_derivation_path(&addr)
})
.await
.map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?;

(payment_address, tx)
(payment_address, tx, fee)
Comment on lines +668 to +675

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Piped-through fee is a size-based recomputation, not the actual inputs-minus-outputs fee

I traced this through to TransactionBuilder::build_signed in the pinned key-wallet crate (rev 1ee1c94, key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs). The value now surfaced by this PR is fee_rate.calculate_fee(encoded_size(&signed_tx)) — a fee-rate × final-serialized-size recomputation done after signing.

That is provably not the same quantity as the fee the broadcast transaction actually pays. During assemble_unsigned, the change output (if any) is sized as total_input - total_output - selection.estimated_fee, so the true on-chain fee (inputs minus outputs) always equals selection.estimated_fee — a value computed before signing from an estimated per-input size (148 bytes) and the coin-selector's dust-folding rules, not from the final signed size. The two numbers only match by coincidence:

  • Dust case: when change_amount falls below the dust threshold, accumulate_coins_with_size folds the dust into estimated_fee (total_value - target_amount) so the true paid fee is inflated by up to 546 duffs. build_signed's returned value is oblivious to this — it just does size × rate — so it undercounts the actual fee by exactly the folded-in dust amount whenever no change output is created.
  • General case: the 148-byte/input estimate used for coin selection and the real encoded size of a signed P2PKH input (variable-length DER signatures) aren't guaranteed to match, so the two fee numbers can diverge even when a change output exists.

This PR's whole purpose is to let wallets show the exact fee instead of re-estimating after the fact — but the plumbed-through value inherits an estimate that's decoupled from what the change output / dust folding actually paid. Since send_payment previously discarded this value entirely (_fee), this inaccuracy had no consumer before; this PR is what turns it into an externally-relied-upon contract, which is why it's in scope here even though the root computation lives in the external key-wallet crate.

source: ['codex']

};

// --- 3. Broadcast the transaction, releasing the build's UTXO
Expand Down Expand Up @@ -726,7 +727,7 @@ impl<B: TransactionBroadcaster + ?Sized> DashPayView<'_, B> {
})?;
}

Ok((txid, entry))
Ok((txid, entry, fee))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,8 @@ extension ManagedPlatformWallet {

/// Send a Dash payment to an established DashPay contact.
/// `amountDuffs` is in duffs (1 DASH = 100_000_000 duffs).
/// Returns the 32-byte transaction id.
/// Returns the 32-byte transaction id plus the exact network fee
/// (duffs) of the broadcast transaction, straight from the builder.
///
/// Prerequisite: `register_external_contact_account` must have
/// run for the `(fromIdentityId, toContactIdentityId)` pair on
Expand All @@ -2113,7 +2114,7 @@ extension ManagedPlatformWallet {
toContactIdentityId: Identifier,
amountDuffs: UInt64,
memo: String? = nil
) async throws -> Data {
) async throws -> (txid: Data, feeDuffs: UInt64) {
let handle = self.handle
let fromBytes: [UInt8] = fromIdentityId.withFFIBytes { ptr in
Array(UnsafeBufferPointer(start: ptr, count: 32))
Expand All @@ -2128,7 +2129,8 @@ extension ManagedPlatformWallet {
// derived, digest signed, buffers zeroed) — the seed never becomes
// resident and no private key leaves Swift.
let coreSigner = MnemonicResolver()
return try await Task.detached(priority: .userInitiated) { () -> Data in
return try await Task.detached(priority: .userInitiated) { () -> (txid: Data, feeDuffs: UInt64) in
var feeDuffs: UInt64 = 0
var txidTuple: (
UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8,
UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8,
Expand All @@ -2153,7 +2155,8 @@ extension ManagedPlatformWallet {
amountDuffs,
memoPtr,
coreSigner.handle,
&txidTuple
&txidTuple,
&feeDuffs
)
}
if let memoCopy {
Expand All @@ -2165,7 +2168,8 @@ extension ManagedPlatformWallet {
}
}
try result.check()
return Swift.withUnsafeBytes(of: &txidTuple) { Data($0) }
let txid = Swift.withUnsafeBytes(of: &txidTuple) { Data($0) }
return (txid: txid, feeDuffs: feeDuffs)
}.value
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ struct SendDashPayPaymentSheet: View {
// useful to pass. The Rust-side
// `PaymentEntry.memo` slot stays available for
// future local-note wiring.
let txid = try await wallet.sendDashPayPayment(
let (txid, _) = try await wallet.sendDashPayPayment(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: In-repo reference implementation discards the newly-exposed fee

This PR's stated motivation is that wallets could previously only re-estimate the fee after the fact. SendDashPayPaymentSheet is the in-tree DashPay send flow and the natural place to demonstrate the new capability, but it discards feeDuffs (let (txid, _) = try await wallet.sendDashPayPayment(...)) instead of surfacing it in the confirmation UI. Not a defect — the real consumer is dashwallet-ios — but the example app doesn't showcase the feature this PR adds.

source: ['claude']

fromIdentityId: senderIdentity.identityId,
toContactIdentityId: contact.identityId,
amountDuffs: duffs,
Expand Down
Loading