a6b0c1fcc0 doc: add releases notes for 25504 (listsinceblock updates) (Antoine Poinsot)
0fd2d14454 rpc: add an include_change parameter to listsinceblock (Antoine Poinsot)
55f98d087e rpc: output parent wallet descriptors for coins in listunspent (Antoine Poinsot)
b724476158 rpc: output wallet descriptors for received entries in listsinceblock (Antoine Poinsot)
55a82eaf91 wallet: allow to fetch the wallet descriptors for a given Script (Antoine Poinsot)
Pull request description:
Wallet descriptors are useful for applications using the Bitcoin Core wallet as a backend for tracking coins, as they allow to track coins for multiple descriptors in a single wallet. However there is no information currently given for such applications to link a coin with an imported descriptor, severely limiting the possibilities for such applications of using multiple descriptors in a single wallet. This PR outputs the matching imported descriptor(s) for a given received coin in `listsinceblock` (and friends).
It comes from a need for an application i'm working on, but i think it's something any software using `bitcoind` to track multiple descriptors in a single wallet would have eventually. For instance i'm thinking about the BDK project. Currently, the way to achieve this is to import raw addresses with labels and to have your application be responsible for wallet things like the gap limit.
I'll add this to the output of `listunspent` too if this gets a few Concept ACKs.
ACKs for top commit:
instagibbs:
ACK a6b0c1fcc0
achow101:
re-ACK a6b0c1fcc0
Tree-SHA512: 7a5850e8de98b439ddede2cb72de0208944f8cda67272e8b8037678738d55b7a5272375be808b0f7d15def4904430e089dafdcc037436858ff3292c5f8b75e37
It's useful for an external application tracking coins to not be limited
by our change detection. For instance, for a watchonly wallet with two
descriptors a transaction from one to the other would be considered a
change output and not be included in the result (if the address was not
generated by this wallet).
This is a refactor, putting the burden to think about thread safety to
the caller. Otherwise, there is a risk that the caller will assume
thread safety where none exists, as is evident in the previous two
commits.
Calling ActiveHeight() and ActiveTip() subsequently without holding the
::cs_main lock over both calls may result in a height that does not
correspond to the tip due to a race.
Fix this by holding the lock.
ActiveTip() is *not* thread-safe, as the required ::cs_main lock will be
released as ActiveChainstate() returns.
ActiveTip() is an alias for ActiveChainstate().m_chain.Tip(), so m_chain
may be involved in a data-race (UB).
The test is a bit primitive as it checks the Bitcoin Core log and
assumes that if it logs that it creates a transient session, then it
does that indeed.
A more thorough test would be to check that it indeed sends the
`SESSION CREATE ... DESTINATION=TRANSIENT` command and that it uses
the returned I2P address for connecting, even for repeated connections
to the same I2P peer. That would require a mocked SAM server (proxy)
implementation in Python.
If not accepting I2P connections, then do not create
`CConnman::m_i2p_sam_session`.
When opening a new outbound I2P connection either use
`CConnman::m_i2p_sam_session` like before or create a temporary one and
store it in `CNode` for destruction later.
and destroy it when `CNode::m_sock` is closed.
I2P transient sessions are created per connection (i.e. per `CNode`) and
should be destroyed when the connection is closed. Storing the session
in `CNode` is a convenient way to destroy it together with the connection
socket (`CNode::m_sock`).
An alternative approach would be to store a list of all I2P sessions in
`CConnman` and from `CNode::CloseSocketDisconnect()` to somehow ask the
`CConnman` to destroy the relevant session.
fea75ad3ca refactor: Drop `boost/algorithm/string/replace.hpp` dependency (Hennadii Stepanov)
857526e8cb test: Add test case for `ReplaceAll()` function (Hennadii Stepanov)
Pull request description:
A new implementation of the `ReplaceAll()` seems enough for all of our purposes.
ACKs for top commit:
adam2k:
ACK Tested fea75ad3ca
theStack:
Code-review ACK fea75ad3ca
Tree-SHA512: dacfffc9d2bd1fb9f034baf8c045b1e8657b766db2f0a7f8ef7e25ee6cd888f315b0124c54aba7a29ae59186b176ef9868a8b709dc995ea215c6b4ce58e174d9
98383d6d0d doc: minor updates to guix README (Stacie)
Pull request description:
Two minor updates to the guix docs:
- `contrib/guix/README.md`: fix broken link
- `contrib/guix/INSTALL.md`: Change Ubuntu version in the section on distribution maintained packages from 21.04 (Hirsute Hippo) to 22.04 (Jammy Jellyfish). The previous link to the Ubuntu Guix package (https://packages.ubuntu.com/hirsute/guix) was for Hirsute. That link is now broken, likely because Hirsute reached EOL in January. I was unable to locate a general page for Ubuntu Guix packages so I replaced the broken link with the search results for all Ubuntu Guix packages. That page currently displays Guix packages for three different versions of Ubuntu. Happy to replace this link if there is a better option.
ACKs for top commit:
jarolrod:
ACK 98383d6d0d
Tree-SHA512: 6980f5952862773e79ca317edb4aadf6ff7c71726a0e4cb873c08bf51360c64e0498aabf4f53780f13cb06838eda93c89ba10fe35c4c8ae2b23191ab961b98f8
f6a916683d Add functional test for block announcements during initial headers sync (Suhas Daftuar)
05f7f31598 Reduce bandwidth during initial headers sync when a block is found (Suhas Daftuar)
Pull request description:
On startup, if our headers chain is more than a day behind current time, we'll pick one peer to sync headers with until our best headers chain is caught up (at that point, we'll try to sync headers with all peers).
However, if an INV for a block is received before our headers chain is caught up, we'll then start to sync headers from each peer announcing the block. This can result in doing a big headers sync with many (if not all) of our peers simultaneously, which wastes bandwidth.
This PR would reduce that overhead by picking (at most) one new peer to try syncing headers with whenever a new block is announced, prior to our headers chain being caught up.
ACKs for top commit:
LarryRuane:
ACK f6a916683d
ajtowns:
ACK f6a916683d
mzumsande:
ACK f6a916683d
dergoegge:
Code review ACK f6a916683d
achow101:
ACK f6a916683d
Tree-SHA512: 0662000bd68db146f55981de4adc2e2b07cbfda222b1176569d61c22055e5556752ffd648426f69687ed1cc203105515e7304c12b915d6270df8e41a4a0e1eaa
4584d300a4 GUI: remove now unneeded 'm_balances' field from overviewpage (furszy)
050e8b1391 GUI: 'getAvailableBalance', use cached balance if the user did not select UTXO manually (furszy)
96e3264a82 GUI: use cached balance in overviewpage and sendcoinsdialog (furszy)
321335bf02 GUI: add getter for WalletModel::m_cached_balances field (furszy)
e62958dc81 GUI: sendCoinsDialog, remove duplicate wallet().getBalances() call (furszy)
Pull request description:
As per the title says, we are recalculating the entire wallet balance on different situations calling to `wallet().getBalances()`, when should instead make use of the wallet model cached balance.
This has the benefits of (1) not spending resources calculating a balance that we already have cached, and (2) avoid blocking the main thread for a long time, in case of big wallets, walking through the entire wallet's tx map more than what it's really needed.
Changes:
1) Fix: `SendCoinsDialog` was calling `wallet().getBalances()` twice during `setModel`.
2) Use the cached balance if the user did not select any UTXO manually inside the wallet model `getAvailableBalance` call.
-----------------------
As an extra note, this work born in [#25005](https://github.com/bitcoin/bitcoin/pull/25005) but grew out of scope of it.
ACKs for top commit:
jarolrod:
ACK 4584d300a4
hebasto:
re-ACK 4584d300a4, only suggested changes and commit message formatting since my [recent](https://github.com/bitcoin-core/gui/pull/598#pullrequestreview-1071268192) review.
Tree-SHA512: 6633ce7f9a82a3e46e75aa7295df46c80a4cd4a9f3305427af203c9bc8670573fa8a1927f14a279260c488cc975a08d238faba2e9751588086fea1dcf8ea2b28
fcc0c8448b qt: Update source translations for string freeze (laanwj)
Pull request description:
Make sure transifex source translations match the ones in the source code after today's string freeze.
It looks like only one message was added: "Too many external signers
found".
ACKs for top commit:
hebasto:
ACK fcc0c8448b, `make -C src translate` produces zero diff.
jarolrod:
ACK fcc0c8448b
Tree-SHA512: 2c2383a6f1ad086e86b7c7d35b9a06c30090a8c9517f93912b64957ec58c95c982d5340f189028717dc876e14ba2344750e3f5d81b522b0cf3d7f4586b8e0cc6
38a911e107 doc: Remove my key from SECURITY.md (laanwj)
Pull request description:
As I am no longer on the `security@bitcoincore.org` alias, there is no point in asking people to GPG-encrypt to me. Remove it.
ACKs for top commit:
fanquake:
ACK 38a911e107
Tree-SHA512: 6f7a9aad9a2d1b2f5ce89b2521d103a7093b9ebb54e7388bfb2836b89ddb592878955088b4d2d8a141ee532a7b35d3a74737fb161280c029f1f2a3eacec99b73
fac09f4f7a refactor: Remove not needed empty RPC doc std::string (MacroFake)
Pull request description:
Not sure why this was put there, so remove it
ACKs for top commit:
aureleoules:
ACK fac09f4f7a.
Tree-SHA512: 7fae9c612dbb82db39e3dc6e20cc7d0c8128f4abf27e34f4854f908dc1bf3ad48ebee2dbbb864a9ce95e98c5d57119bf976793945a2084a43d137c2d95a25ea1
As I am no longer on the `security@bitcoincore.org` alias, there is no
point in asking people to GPG-encrypt to me. Remove it.
Tree-SHA512: f15640b741b908b162acf149ab27dae149fc917c15fbabf8dd9aff7bc6afb9e8b2ae956f9a0f8fbe1f05f40a48215becc1efda90a2e9c1105559cdeafb073060
When we have preselected inputs the coin selection search target is reduced
by the sum of (effective) values. This causes incorrect m_target value.
Create separate instance of SelectionResult for all the preselected inputs and
set the target equal to the sum of (effective) values. Target for preselected
SelectionResult is equal to the delta for the search target. To get the final
SelectionResult with accurate m_target we merge both SelectionResult instances.
SelectionResult::m_target should be equal to actual selection target.
Selection target is the sum of all recipient amounts plus non input fees.
So we need to remove change_fee from the m_target. It's safe because change
target is always greater than the change fee, so we can always cover fees
if change output is created.
292b1a3e9c GetExternalSigner(): fail if multiple signers are found (amadeuszpawlik)
Pull request description:
If there are multiple external signers, `GetExternalSigner()` will
just pick the first one in the list. If the user has two or more
hardware wallets connected at the same time, he might not notice this.
This PR adds a check and fails with suitable message, forcing the user to disconnect all but one external signer, so that there is no ambiguity as to which external signer was used.
ACKs for top commit:
Sjors:
tACK 292b1a3e9c
achow101:
ACK 292b1a3e9c
Tree-SHA512: e2a41d3eecc607d4f94e708614bed0f3545f7abba85f300c5a5f0d3d17d72c815259734accc5ca370953eacd290f27894ba2c18016f5e9584cd50fa1ec2fbb0b
If our headers chain is behind on startup, then if a block is found we'll try
to catch up from all peers announcing the block, in addition to our initial
headers-sync peer. This commit changes behavior so that in this situation,
we'll choose at most one peer announcing a block to additionally sync headers
from.
