Skip to content

Commit 7e315af

Browse files
ZocoLiniclaude
andauthored
fix(key-wallet): ask every account whether a transaction is new, not just the matched ones (#1000)
The transaction total a sync reports disagreed between runs whose wallet state was identical — 6800 against 6801 over the same 6787 transactions. The counter is fed `new_txids`, so a transaction counted twice inflates it, and which ones get counted twice depends on delivery order. `is_new` polled only `affected_accounts`. That set is not stable across deliveries: an account that matched solely because the transaction spent its coin drops out of it on every later delivery, the coin having left `utxos` when the spend was recorded. Once the account holding the record stops matching, the remaining candidates have no record, and a transaction the wallet has held all along is announced as new a second time. Asking every account instead is the semantics the code already documented — `test_instantsend_backfills_missing_record_in_other_account` describes "the wallet-level `is_new`", which the old loop only delivered while the holding account kept matching. It sits behind the relevance early-return, so it runs only for transactions that matched, and costs nothing measurable. Measured over full mainnet restores: 13 to 14 transactions per sync counted twice, always on the second delivery of a block whose first delivery had already recorded them. With this the reported total is 6787 on every run, equal to the number of distinct transactions the wallet actually holds. Claude-Session: https://claude.ai/code/session_019jkiLt3KdPr5jG4ruYELjT Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 93260bf commit 7e315af

1 file changed

Lines changed: 112 additions & 12 deletions

File tree

‎key-wallet/src/transaction_checking/wallet_checker.rs‎

Lines changed: 112 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -184,19 +184,8 @@ impl WalletTransactionChecker for ManagedWalletInfo {
184184
// `unconfirmed` bucket.
185185
let external_final_parents = self.accounts.final_parents_of(tx);
186186

187-
// Check if this transaction already exists in any affected account
188187
let txid = tx.txid();
189-
let mut is_new = true;
190-
for account_match in &result.affected_accounts {
191-
if let Some(account) =
192-
self.accounts.get_by_account_type_match(&account_match.account_type_match)
193-
{
194-
if account.has_transaction(&txid) {
195-
is_new = false;
196-
break;
197-
}
198-
}
199-
}
188+
let is_new = !self.accounts.all_accounts().into_iter().any(|a| a.has_transaction(&txid));
200189
result.is_new_transaction = is_new;
201190

202191
if !is_new {
@@ -1319,6 +1308,117 @@ mod tests {
13191308
assert_eq!(ctx.managed_wallet.balance.spendable(), 150_000);
13201309
}
13211310

1311+
/// A transaction the wallet already holds must not be reported new again
1312+
/// when the account holding it stops matching.
1313+
#[tokio::test]
1314+
async fn known_transaction_is_not_new_again_when_its_holder_stops_matching() {
1315+
let mut wallet =
1316+
Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default)
1317+
.expect("Should create wallet");
1318+
wallet
1319+
.add_account(
1320+
AccountType::Standard {
1321+
index: 1,
1322+
standard_account_type: StandardAccountType::BIP44Account,
1323+
},
1324+
None,
1325+
)
1326+
.expect("Should add second BIP44 account");
1327+
1328+
let mut managed_wallet =
1329+
ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0);
1330+
1331+
let xpub0 =
1332+
wallet.accounts.standard_bip44_accounts.get(&0).expect("account 0").account_xpub;
1333+
let address0 = managed_wallet
1334+
.bip44_managed_account_at_index_mut(0)
1335+
.expect("managed account 0")
1336+
.next_receive_address(Some(&xpub0), true)
1337+
.expect("address for account 0");
1338+
let xpub1 =
1339+
wallet.accounts.standard_bip44_accounts.get(&1).expect("account 1").account_xpub;
1340+
let address1 = managed_wallet
1341+
.bip44_managed_account_at_index_mut(1)
1342+
.expect("managed account 1")
1343+
.next_receive_address(Some(&xpub1), true)
1344+
.expect("address for account 1");
1345+
1346+
let mut wallet_mut = wallet;
1347+
let in_block = |h: u32, tag: u8| {
1348+
TransactionContext::InBlock(BlockInfo::new(
1349+
h,
1350+
BlockHash::from_slice(&[tag; 32]).expect("hash"),
1351+
1_650_000_000 + h,
1352+
))
1353+
};
1354+
1355+
let funding = Transaction::dummy(&address0, 0..1, &[100_000]);
1356+
let funded = managed_wallet
1357+
.check_core_transaction(&funding, in_block(100, 1), &mut wallet_mut, true, true)
1358+
.await;
1359+
assert!(funded.is_relevant);
1360+
1361+
let spend = Transaction {
1362+
version: 2,
1363+
lock_time: 0,
1364+
input: vec![TxIn {
1365+
previous_output: OutPoint::new(funding.txid(), 0),
1366+
script_sig: ScriptBuf::new(),
1367+
sequence: 0xffffffff,
1368+
witness: dashcore::Witness::new(),
1369+
}],
1370+
output: vec![TxOut {
1371+
value: 90_000,
1372+
script_pubkey: address1.script_pubkey(),
1373+
}],
1374+
special_transaction_payload: None,
1375+
};
1376+
let txid = spend.txid();
1377+
1378+
let first = managed_wallet
1379+
.check_core_transaction(&spend, in_block(200, 2), &mut wallet_mut, true, true)
1380+
.await;
1381+
assert!(first.is_new_transaction, "first sight of the spend is new");
1382+
assert_eq!(first.affected_accounts.len(), 2, "account 0 by input, account 1 by output");
1383+
1384+
managed_wallet
1385+
.bip44_managed_account_at_index_mut(1)
1386+
.expect("managed account 1")
1387+
.transactions_mut()
1388+
.remove(&txid);
1389+
1390+
assert!(
1391+
managed_wallet
1392+
.bip44_managed_account_at_index(0)
1393+
.expect("managed account 0")
1394+
.utxos
1395+
.is_empty(),
1396+
"account 0 matched only through the coin this spend consumed"
1397+
);
1398+
assert!(
1399+
managed_wallet
1400+
.bip44_managed_account_at_index(0)
1401+
.expect("managed account 0")
1402+
.has_transaction(&txid),
1403+
"account 0 still holds the record"
1404+
);
1405+
assert!(
1406+
!managed_wallet
1407+
.bip44_managed_account_at_index(1)
1408+
.expect("managed account 1")
1409+
.has_transaction(&txid),
1410+
"account 1 does not"
1411+
);
1412+
1413+
let again = managed_wallet
1414+
.check_core_transaction(&spend, in_block(200, 2), &mut wallet_mut, true, true)
1415+
.await;
1416+
assert!(
1417+
!again.is_new_transaction,
1418+
"the wallet already holds this transaction in account 0"
1419+
);
1420+
}
1421+
13221422
/// Test that the InstantSend branch backfills a `TransactionRecord` on accounts
13231423
/// that match the transaction but have no prior record. This mirrors the
13241424
/// confirmation path's backfill: a tx pays outputs to two accounts but only

0 commit comments

Comments
 (0)