-
Notifications
You must be signed in to change notification settings - Fork 13
fix(wallets): show the fee on outgoing transaction history #900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,14 +143,41 @@ pub(super) fn map_transaction_record(record: &TransactionRecord) -> WalletTransa | |
| height: record.height(), | ||
| block_hash: block_info.map(|bi| bi.block_hash()), | ||
| net_amount: record.net_amount, | ||
| fee: record.fee, | ||
| fee: transaction_fee(record), | ||
| label: Some(record.label.clone()).filter(|s| !s.is_empty()), | ||
| // Per-wallet history — every record involves our addresses. | ||
| is_ours: true, | ||
| status: status_from_context(&record.context), | ||
| } | ||
| } | ||
|
|
||
| fn transaction_fee(record: &TransactionRecord) -> Option<u64> { | ||
| if record.fee.is_some() { | ||
| return record.fee; | ||
| } | ||
| if record.transaction.input.is_empty() | ||
| || record.input_details.len() != record.transaction.input.len() | ||
| || record | ||
| .input_details | ||
| .iter() | ||
| .enumerate() | ||
| .any(|(index, detail)| detail.index as usize != index) | ||
| { | ||
| return None; | ||
| } | ||
|
|
||
| let input_total = record | ||
| .input_details | ||
| .iter() | ||
| .try_fold(0u64, |total, input| total.checked_add(input.value))?; | ||
| let output_total = record | ||
| .transaction | ||
| .output | ||
| .iter() | ||
| .try_fold(0u64, |total, output| total.checked_add(output.value))?; | ||
| input_total.checked_sub(output_total) | ||
| } | ||
|
|
||
| /// The `(transaction, [(outpoint, txout, address)])` payload the asset-lock and | ||
| /// identity-funding screens wait on, matching the | ||
| /// `CoreItem::ReceivedAvailableUTXOTransaction` contract. | ||
|
|
@@ -615,7 +642,7 @@ mod tests { | |
| use dash_sdk::dpp::dashcore::{BlockHash, Network, PublicKey, Transaction, TxOut}; | ||
| use dash_sdk::dpp::key_wallet::account::{AccountType, StandardAccountType}; | ||
| use dash_sdk::dpp::key_wallet::managed_account::transaction_record::{ | ||
| OutputDetail, OutputRole, TransactionDirection, TransactionRecord, | ||
| InputDetail, OutputDetail, OutputRole, TransactionDirection, TransactionRecord, | ||
| }; | ||
| use dash_sdk::dpp::key_wallet::transaction_checking::BlockInfo; | ||
| use dash_sdk::dpp::key_wallet::transaction_checking::transaction_router::TransactionType; | ||
|
|
@@ -851,6 +878,44 @@ mod tests { | |
| assert_eq!(snap.balance, DetWalletBalance::default()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn outgoing_transaction_fee_is_derived_from_known_inputs() { | ||
| use dash_sdk::dpp::dashcore::TxIn; | ||
|
|
||
| let source = addr(10); | ||
| let destination = addr(11); | ||
| let mut tx = tx_with(10); | ||
| tx.input.push(TxIn::default()); | ||
| tx.output.push(TxOut { | ||
| value: 9_700, | ||
| script_pubkey: destination.script_pubkey(), | ||
| }); | ||
| let record = TransactionRecord::new( | ||
| tx, | ||
| AccountType::Standard { | ||
| index: 0, | ||
| standard_account_type: StandardAccountType::BIP44Account, | ||
| }, | ||
| TransactionContext::Mempool, | ||
| TransactionType::Standard, | ||
| TransactionDirection::Outgoing, | ||
| vec![InputDetail { | ||
| index: 0, | ||
| value: 10_000, | ||
| address: source, | ||
| }], | ||
| vec![OutputDetail { | ||
| index: 0, | ||
| role: OutputRole::Sent, | ||
| address: Some(destination), | ||
| value: 9_700, | ||
| }], | ||
| -10_000, | ||
| ); | ||
|
|
||
| assert_eq!(map_transaction_record(&record).fee, Some(300)); | ||
|
Comment on lines
+882
to
+916
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Cover the fee derivation's fail-closed behavior The new test covers only a single-input happy path. The helper's user-visible correctness also depends on preserving an existing upstream fee and returning source: ['codex'] |
||
| } | ||
|
|
||
| #[test] | ||
| fn reseen_txid_upserts_in_place() { | ||
| let store = SnapshotStore::new(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Count the Platform outputs actually submitted
This branch prices every row with a non-empty address, but
send_advanced_platform_to_platformskips zero-credit rows and inserts outputs into aBTreeMap, which coalesces duplicate destinations. Two rows for the same Platform address therefore submit one transition output but are priced as two, and a populated zero-credit row is priced despite being omitted. Because the newly generalized fee calculation charges per output, derive the count from the same distinct, positive-credit destinations used by the send path.source: ['codex']