Skip to content

Commit 8dd5f1b

Browse files
feat(key-wallet): add keep-finalized-transactions Cargo feature (#733)
* feat(key-wallet): add `keep-finalized-transactions` feature By default, records of chainlocked transactions are now dropped from each managed account's in-memory `transactions` map; only their txids are retained (in a new `finalized_txids` set on `ManagedCoreKeysAccount`) to keep dedup, `has_transaction`, and finality queries working. The opt-in `keep-finalized-transactions` Cargo feature reverts to the old behavior — every processed transaction stays in the map for the wallet's lifetime. The drop is driven off `TransactionContext::is_finalized_in_block` (chainlock only), not `is_finalized` (chainlock or IS-lock), so IS-locked records survive long enough to absorb the surrounding block-confirmation event (xdustinface review on #709). `confirm_transaction` now returns `Option<TransactionRecord>` so callers always observe the record even when it's about to be dropped. The feature is forwarded through `key-wallet-manager` and `key-wallet-ffi`. Three FFI accessors that walk the full `transactions` map (`managed_core_account_get_transaction_count`, `managed_core_account_get_transactions`, `managed_core_account_free_transactions`) are gated to the feature because they would otherwise return a partial history. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dash-spv-ffi): enable `keep-finalized-transactions` for tests Integration tests under `dash-spv-ffi/tests/dashd_sync/` walk the full per-account transaction history (`managed_core_account_get_transactions`, `managed_core_account_get_transaction_count`, `managed_core_account_free_transactions`) to verify end-to-end wallet sync against a regtest dashd. These accessors are gated behind the `keep-finalized-transactions` feature on `key-wallet-ffi`, so under the default feature set they aren't compiled in and the test build fails to resolve the imports. Add `key-wallet-ffi` as a dev-dependency with the feature enabled. Cargo unifies features across the build graph at test time, which makes the gated accessors available in the test binaries without expanding the lib crate's default feature surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(key-wallet): align finalization with chainlock-only semantics Address xdustinface review feedback on PR #733: - Drop `TransactionContext::is_finalized()` (the soft IS-or-chainlock check). The wallet now treats only a chainlock as finality. - Rename `TransactionContext::is_finalized_in_block()` to `is_chain_locked()` so the predicate name describes the variant rather than overloading "finalized" — same naming style as the existing `is_instant_send()` helper. - Drop `ManagedAccountTrait::transaction_is_finalized()` (the unused soft-finality variant) and rename `transaction_is_finalized_in_block()` to `transaction_is_finalized()`. Now that there's a single finalization concept, the trait method name aligns with the feature name. - Replace runtime `if cfg!(...) { ... } else { ... }` branches in `ManagedCoreKeysAccount::has_transaction` and `transaction_is_finalized` with two `#[cfg]`-gated function bodies — one per feature configuration. Same for the chainlock-driven drop call in `confirm_transaction` / `record_transaction`: the `let drop_now = …` binding now only exists when the feature is off, removing the `#[allow(unused_variables)]` workaround. - Rewrite the doc on `has_transaction` so it's clear what it actually reports (live record OR finalized-txid marker) and how callers use it (distinguish brand-new sightings from re-processings) instead of the misleading "dedup signal in `confirm_transaction`" wording. Drop logic still triggers on the strict `is_chain_locked()` check — IS-locked-but-not-yet-chainlocked records still survive so the surrounding block-confirmation event can populate height / block hash before the chainlock catches up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(key-wallet): fix stale `transaction_is_finalized_in_block` mention The previous commit renamed the trait method to `transaction_is_finalized` but missed this Cargo.toml feature-doc reference. Caught by CodeRabbit on PR #733. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dd7937a commit 8dd5f1b

13 files changed

Lines changed: 451 additions & 42 deletions

File tree

dash-spv-ffi/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ clap = { version = "4.5", features = ["derive"] }
2828

2929
[dev-dependencies]
3030
dash-spv = { path = "../dash-spv", features = ["test-utils"] }
31+
# Tests inspect per-account transaction history end-to-end (including
32+
# chainlocked transactions), which requires the `keep-finalized-transactions`
33+
# feature on `key-wallet-ffi`. Cargo unifies features across the build graph at
34+
# test time, so this enables the gated FFI accessors in test builds without
35+
# changing the lib's default feature surface.
36+
key-wallet-ffi = { path = "../key-wallet-ffi", features = ["keep-finalized-transactions"] }
3137
serial_test = "3.0"
3238
tempfile = "3.8"
3339

key-wallet-ffi/Cargo.toml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ bip38 = ["key-wallet/bip38"]
1818
bincode = ["key-wallet/bincode", "key-wallet-manager/bincode"]
1919
eddsa = ["dashcore/eddsa", "key-wallet/eddsa"]
2020
bls = ["dashcore/bls", "key-wallet/bls"]
21+
# Forward to `key-wallet/keep-finalized-transactions` (via key-wallet-manager).
22+
# With this on, every processed transaction (including chainlocked ones)
23+
# stays in the in-memory `transactions` map for the wallet's lifetime.
24+
# With it off (the default), records of chainlocked transactions are
25+
# dropped and only their txids are kept (in `finalized_txids`) for dedup.
26+
# See `key-wallet`'s feature documentation for details.
27+
keep-finalized-transactions = [
28+
"key-wallet/keep-finalized-transactions",
29+
"key-wallet-manager/keep-finalized-transactions",
30+
]
2131

2232
[dependencies]
2333
key-wallet = { path = "../key-wallet" }

key-wallet-ffi/FFI_API.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,15 @@ Functions: 108
230230
| `managed_account_collection_summary_data` | Get structured account collection summary data for managed collection ... | managed_account_collection |
231231
| `managed_account_collection_summary_free` | Free a managed account collection summary and all its allocated memory #... | managed_account_collection |
232232
| `managed_core_account_free` | Free a managed account handle # Safety - `account` must be a valid pointer... | managed_account |
233-
| `managed_core_account_free_transactions` | Free transactions array returned by managed_core_account_get_transactions #... | managed_account |
233+
| `managed_core_account_free_transactions` | Free transactions array returned by managed_core_account_get_transactions ... | managed_account |
234234
| `managed_core_account_get_account_type` | Get the account type of a managed account # Safety - `account` must be a... | managed_account |
235235
| `managed_core_account_get_address_pool` | Get an address pool from a managed account by type This function returns... | managed_account |
236236
| `managed_core_account_get_balance` | Get the balance of a managed account # Safety - `account` must be a valid... | managed_account |
237237
| `managed_core_account_get_external_address_pool` | Get the external address pool from a managed account This function returns... | managed_account |
238238
| `managed_core_account_get_index` | Get the account index from a managed account Returns the primary account... | managed_account |
239239
| `managed_core_account_get_internal_address_pool` | Get the internal address pool from a managed account This function returns... | managed_account |
240240
| `managed_core_account_get_network` | Get the network of a managed account # Safety - `account` must be a valid... | managed_account |
241-
| `managed_core_account_get_transaction_count` | Get the number of transactions in a managed account # Safety - `account`... | managed_account |
241+
| `managed_core_account_get_transaction_count` | Get the number of transactions in a managed account Only available with the... | managed_account |
242242
| `managed_core_account_get_transactions` | Get all transactions from a managed account Returns an array of... | managed_account |
243243
| `managed_core_account_get_utxo_count` | Get the number of UTXOs in a managed account # Safety - `account` must be... | managed_account |
244244
| `managed_platform_account_free` | Free a managed platform account handle # Safety - `account` must be a... | managed_account |
@@ -3047,7 +3047,7 @@ managed_core_account_free_transactions(transactions: *mut FFITransactionRecord,
30473047
```
30483048

30493049
**Description:**
3050-
Free transactions array returned by managed_core_account_get_transactions # Safety - `transactions` must be a pointer returned by `managed_core_account_get_transactions` - `count` must be the count returned by `managed_core_account_get_transactions` - This function must only be called once per allocation
3050+
Free transactions array returned by managed_core_account_get_transactions Only available with the `keep-finalized-transactions` Cargo feature, in which configuration `managed_core_account_get_transactions` is also available — the two functions are paired. # Safety - `transactions` must be a pointer returned by `managed_core_account_get_transactions` - `count` must be the count returned by `managed_core_account_get_transactions` - This function must only be called once per allocation
30513051

30523052
**Safety:**
30533053
- `transactions` must be a pointer returned by `managed_core_account_get_transactions` - `count` must be the count returned by `managed_core_account_get_transactions` - This function must only be called once per allocation
@@ -3175,7 +3175,7 @@ managed_core_account_get_transaction_count(account: *const FFIManagedCoreAccount
31753175
```
31763176

31773177
**Description:**
3178-
Get the number of transactions in a managed account # Safety - `account` must be a valid pointer to an FFIManagedCoreAccount instance
3178+
Get the number of transactions in a managed account Only available with the `keep-finalized-transactions` Cargo feature. With the feature off (the default), records of chainlocked transactions are dropped from the in-memory map, so the count would not reflect the full history — the function is intentionally not exposed. # Safety - `account` must be a valid pointer to an FFIManagedCoreAccount instance
31793179

31803180
**Safety:**
31813181
- `account` must be a valid pointer to an FFIManagedCoreAccount instance
@@ -3191,7 +3191,7 @@ managed_core_account_get_transactions(account: *const FFIManagedCoreAccount, tra
31913191
```
31923192

31933193
**Description:**
3194-
Get all transactions from a managed account Returns an array of FFITransactionRecord structures. # Safety - `account` must be a valid pointer to an FFIManagedCoreAccount instance - `transactions_out` must be a valid pointer to receive the transactions array pointer - `count_out` must be a valid pointer to receive the count - The caller must free the returned array using `managed_core_account_free_transactions`
3194+
Get all transactions from a managed account Returns an array of FFITransactionRecord structures. Only available with the `keep-finalized-transactions` Cargo feature. With the feature off (the default), records of chainlocked transactions are dropped from the in-memory map, so this would only return a partial history — the function is intentionally not exposed. # Safety - `account` must be a valid pointer to an FFIManagedCoreAccount instance - `transactions_out` must be a valid pointer to receive the transactions array pointer - `count_out` must be a valid pointer to receive the count - The caller must free the returned array using `managed_core_account_free_transactions`
31953195

31963196
**Safety:**
31973197
- `account` must be a valid pointer to an FFIManagedCoreAccount instance - `transactions_out` must be a valid pointer to receive the transactions array pointer - `count_out` must be a valid pointer to receive the count - The caller must free the returned array using `managed_core_account_free_transactions`

key-wallet-ffi/src/managed_account.rs

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use dash_network::ffi::FFINetwork;
88
use dashcore::hashes::Hash;
99
use std::os::raw::{c_char, c_uint};
10+
#[cfg(feature = "keep-finalized-transactions")]
1011
use std::ptr::slice_from_raw_parts_mut;
1112
use std::sync::Arc;
1213

@@ -614,8 +615,14 @@ pub unsafe extern "C" fn managed_core_account_get_balance(
614615
true
615616
}
616617

618+
#[cfg(feature = "keep-finalized-transactions")]
617619
/// Get the number of transactions in a managed account
618620
///
621+
/// Only available with the `keep-finalized-transactions` Cargo feature. With
622+
/// the feature off (the default), records of chainlocked transactions are
623+
/// dropped from the in-memory map, so the count would not reflect the full
624+
/// history — the function is intentionally not exposed.
625+
///
619626
/// # Safety
620627
///
621628
/// - `account` must be a valid pointer to an FFIManagedCoreAccount instance
@@ -914,10 +921,16 @@ impl Drop for FFITransactionRecord {
914921
}
915922
}
916923

924+
#[cfg(feature = "keep-finalized-transactions")]
917925
/// Get all transactions from a managed account
918926
///
919927
/// Returns an array of FFITransactionRecord structures.
920928
///
929+
/// Only available with the `keep-finalized-transactions` Cargo feature. With
930+
/// the feature off (the default), records of chainlocked transactions are
931+
/// dropped from the in-memory map, so this would only return a partial
932+
/// history — the function is intentionally not exposed.
933+
///
921934
/// # Safety
922935
///
923936
/// - `account` must be a valid pointer to an FFIManagedCoreAccount instance
@@ -951,8 +964,13 @@ pub unsafe extern "C" fn managed_core_account_get_transactions(
951964
true
952965
}
953966

967+
#[cfg(feature = "keep-finalized-transactions")]
954968
/// Free transactions array returned by managed_core_account_get_transactions
955969
///
970+
/// Only available with the `keep-finalized-transactions` Cargo feature, in
971+
/// which configuration `managed_core_account_get_transactions` is also
972+
/// available — the two functions are paired.
973+
///
956974
/// # Safety
957975
///
958976
/// - `transactions` must be a pointer returned by `managed_core_account_get_transactions`
@@ -1547,10 +1565,13 @@ pub unsafe extern "C" fn managed_platform_account_result_free_error(
15471565
mod tests {
15481566
use super::*;
15491567
use crate::address_pool::address_pool_free;
1568+
use crate::types::{FFIAccountCreationOptionType, FFIWalletAccountCreationOptions};
1569+
// These types are only used by the FFITransactionRecord tests, which run
1570+
// only when transactions stay in memory.
1571+
#[cfg(feature = "keep-finalized-transactions")]
15501572
use crate::types::{
1551-
FFIAccountCreationOptionType, FFIBlockInfo, FFIInputDetail, FFIOutputDetail, FFIOutputRole,
1552-
FFITransactionContext, FFITransactionContextType, FFITransactionDirection,
1553-
FFITransactionType, FFIWalletAccountCreationOptions,
1573+
FFIBlockInfo, FFIInputDetail, FFIOutputDetail, FFIOutputRole, FFITransactionContext,
1574+
FFITransactionContextType, FFITransactionDirection, FFITransactionType,
15541575
};
15551576
use crate::wallet_manager::{
15561577
wallet_manager_add_wallet_from_mnemonic_with_options, wallet_manager_create,
@@ -1828,9 +1849,15 @@ mod tests {
18281849
assert_eq!(balance_out.locked, 0);
18291850
assert_eq!(balance_out.total, 0);
18301851

1831-
// Test get_transaction_count
1832-
let tx_count = managed_core_account_get_transaction_count(account);
1833-
assert_eq!(tx_count, 0); // Initially no transactions
1852+
// Test get_transaction_count (only available with the
1853+
// `keep-finalized-transactions` feature; without it the function
1854+
// is not exposed because chainlocked records are pruned and the
1855+
// count would be incomplete)
1856+
#[cfg(feature = "keep-finalized-transactions")]
1857+
{
1858+
let tx_count = managed_core_account_get_transaction_count(account);
1859+
assert_eq!(tx_count, 0); // Initially no transactions
1860+
}
18341861

18351862
// Test get_utxo_count
18361863
let utxo_count = managed_core_account_get_utxo_count(account);
@@ -1858,8 +1885,11 @@ mod tests {
18581885
let account_type = managed_core_account_get_account_type(ptr::null(), &mut index_out);
18591886
assert_eq!(account_type, FFIAccountKind::StandardBIP44); // Default type
18601887

1861-
let tx_count = managed_core_account_get_transaction_count(ptr::null());
1862-
assert_eq!(tx_count, 0);
1888+
#[cfg(feature = "keep-finalized-transactions")]
1889+
{
1890+
let tx_count = managed_core_account_get_transaction_count(ptr::null());
1891+
assert_eq!(tx_count, 0);
1892+
}
18631893

18641894
let utxo_count = managed_core_account_get_utxo_count(ptr::null());
18651895
assert_eq!(utxo_count, 0);
@@ -2080,6 +2110,7 @@ mod tests {
20802110
}
20812111
}
20822112

2113+
#[cfg(feature = "keep-finalized-transactions")]
20832114
#[test]
20842115
fn test_free_transactions_null_safety() {
20852116
unsafe {
@@ -2088,6 +2119,7 @@ mod tests {
20882119
}
20892120
}
20902121

2122+
#[cfg(feature = "keep-finalized-transactions")]
20912123
#[test]
20922124
fn test_ffi_transaction_record_roundtrip() {
20932125
let mut records = Vec::new();

key-wallet-manager/Cargo.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ bincode = ["key-wallet/bincode", "dep:bincode"]
1414
test-utils = ["key-wallet/test-utils"]
1515
bls = ["key-wallet/bls"]
1616
eddsa = ["key-wallet/eddsa"]
17+
# Forward to `key-wallet/keep-finalized-transactions`. With this on,
18+
# every processed transaction (including chainlocked ones) stays in
19+
# the in-memory `transactions` map for the wallet's lifetime. With
20+
# it off (the default), records of chainlocked transactions are
21+
# dropped and only their txids are kept (in `finalized_txids`) for
22+
# dedup. See `key-wallet`'s feature documentation for details.
23+
keep-finalized-transactions = ["key-wallet/keep-finalized-transactions"]
1724

1825
[dependencies]
1926
key-wallet = { path = "../key-wallet", default-features = false }

key-wallet/Cargo.toml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,19 @@ bip38 = ["scrypt", "aes", "bs58", "rand"]
1616
eddsa = ["dashcore/eddsa"]
1717
bls = ["dashcore/bls"]
1818
test-utils = ["dashcore/test-utils"]
19+
# Keep the full `TransactionRecord` for transactions that have reached a
20+
# "finalized in block" state — i.e. they have a ChainLock confirming the
21+
# block they were mined in. With this feature ON, every processed
22+
# transaction (including ones already chainlocked) stays in the
23+
# `transactions` map for the wallet's lifetime; finalization is implicit
24+
# in the stored `TransactionContext`. With it OFF (the default), records
25+
# of chainlocked transactions are dropped from the map and a per-account
26+
# `finalized_txids: HashSet<Txid>` retains only their txids so
27+
# `has_transaction` / `transaction_is_finalized` still answer
28+
# correctly. An InstantSend lock alone does NOT trigger record dropping
29+
# — we keep the record around so the surrounding block confirmation can
30+
# still write its height / block hash before the chainlock arrives.
31+
keep-finalized-transactions = []
1932

2033
[dependencies]
2134
internals = { path = "../internals", package = "dashcore-private" }

key-wallet/src/managed_account/managed_account_trait.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,26 @@ pub trait ManagedAccountTrait {
4444
/// Get mutable transactions
4545
fn transactions_mut(&mut self) -> &mut BTreeMap<Txid, TransactionRecord>;
4646

47+
/// Returns `true` if this account has already processed `txid`,
48+
/// whether it is still represented as a full record in `transactions`
49+
/// or has been pruned (under the default feature configuration) and
50+
/// is now only retained as a finalized-txid marker. Used by callers
51+
/// that need to distinguish a brand-new sighting from a re-processing
52+
/// (mempool → block, IS-lock arrival, chainlock, …).
53+
fn has_transaction(&self, txid: &Txid) -> bool;
54+
55+
/// Returns `true` if `txid` has been mined in a block that is itself
56+
/// chainlocked — the only finality signal we treat as terminal
57+
/// (mirrors [`crate::transaction_checking::TransactionContext::is_chain_locked`]).
58+
///
59+
/// `InBlock` alone is not enough (the block can still be reorganized
60+
/// out), and `InstantSend` alone is not enough either (the
61+
/// surrounding block confirmation may still arrive and write the
62+
/// height / block hash before the chainlock catches up). Only
63+
/// `InChainLockedBlock` qualifies. This is the trigger for dropping
64+
/// the full record under the default feature configuration.
65+
fn transaction_is_finalized(&self, txid: &Txid) -> bool;
66+
4767
/// Return the current monitor revision.
4868
///
4969
/// Bumped whenever the monitored address set changes (e.g. new addresses

0 commit comments

Comments
 (0)