wallet: Automatically add receiving destinations to the address book #22929
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
b12df19 to
aa7e438
Compare
|
ACK c97c2a9 |
2912d04 to
a66fd0b
Compare
|
Code review ACK 3d71d16 |
…inations to the address book 87245f1 test: listtranscations with externally generated addresses (S3RK) 6902d3d Add to spends only transcations from me (S3RK) 5ff3bfe Automatically add labels to detected receiving addresses (S3RK) 955ea13 Return used destinations from ScriptPubKeyMan::MarkUnusedAddresses (S3RK) 8a617fc Add CWallet::IsInternalScriptPubKeyMan (S3RK) aab4d11 wallet: resolve ambiguity of two ScriptPubKey managers providing same script (S3RK) Pull request description: This PR fixes certain use-cases when **send-to-self** transactions are missing from `listtransactions` output. 1. When a receiving address is generated externally to the wallet (e.g. same wallet running on two nodes, or by 3rd party from xpub) 2. When restoring backup with lost metadata, but keypool gap is not exceeded yet When the block is connected or tx added to mempool we already mark used keys. This PR extends this logic to determine whether the destination is a receiving one and if yes add it to the address book with empty label. Works both for legacy and descriptors wallets. - For legacy it uses the internal flag from the keypool entry. Caveat: because we don't know which script type would be used we add all possible destinations for such keys. - For descriptor wallets it uses internal flag for the script pub key manager. Caveat: it only works for active descriptors. fixes #19856 fixes #20293 ACKs for top commit: laanwj: Code review ACK 87245f1 Tree-SHA512: 03fafd5548ead0c4ffe9ebcc9eb2849f1d2fa7270fda4166419b86877d4e57dcf04460e465fbb9c90b42031f3c05d1b83f1b67a9f82c2a42980825ed1e7b52e6
|
This breaks "walletconflicts" Note: My |
|
@luke-jr I'm not sure what exactly is the broken scenario, happy to debug or write a test for it. It seems to me that |
"walletconflicts" no longer returns other transactions that conflict with the transaction. This is an issue in master, not just my branch. I only mentioned my branch because it's how I found it.
IIRC, it's not supposed to only be spends from this wallet, but also spends to this wallet. I'm not sure if that's why "walletconflicts" is broken now, or not (though the comment for mapTxSpends suggests it may be) |
|
Created #24083 with a revert |
3ee6d07 test: add more wallet conflicts assertions (S3RK) 3b98bf9 Revert "Add to spends only transcations from me" (S3RK) Pull request description: This reverts commit d045664 from #22929. This commit was based on invalid assumption that `mapTxSpends` should contain only outgoing txs and broke wallet conflicts feature. ACKs for top commit: achow101: ACK 3ee6d07 Tree-SHA512: bf5a77ced6bac57d5eb85771d9189c53e1edc295d179ed5a1bdce18e365794a9101b4cecf35387b27f67260db3b47f7214e7876e490494529b748cceeb95632d
This PR fixes certain use-cases when send-to-self transactions are missing from
listtransactionsoutput.(e.g. same wallet running on two nodes, or by 3rd party from xpub)
When the block is connected or tx added to mempool we already mark used keys. This PR extends this logic to determine whether the destination is a receiving one and if yes add it to the address book with empty label.
Works both for legacy and descriptors wallets.
fixes #19856
fixes #20293