This commit documents our assumption about
TxRelay::m_tx_inventory_to_send being empty prior to version handshake
completion.
The added Assume acts as testing oracle for our fuzzing tests to
potentially detect if the assumption is violated.
Github-Pull: #26569
Rebased-From: ce63fca13e
If migratewallet fails, we do a cleanup which removes the watchonly and
solvables wallets if they were created. However, if they were not, their
pointers are nullptr and we don't check for that, which causes a
segfault during the cleanup. So check that they aren't nullptr before
cleaning them up.
Github-Pull: #26594
Rebased-From: 86ef7b3c7b
MacOS 13 sends a window focus change notification after the main
window has been destroyed but before the QTApplication has been
destroyed. This results in the menu bar receiving a notification
despite it no longer existing. The solution is to pass the main
window as context when subscribing to the notifications. Qt
automatically unsubscribes to notifications if the sender OR
context is destroyed.
Github-Pull: bitcoin-core/gui#680
Rebased-From: 8a5014cd8a
In addition to the pubkeys in hd_keypaths and tap_bip32_keypaths, also
see if the descriptor can produce a SigningProvider for the output
pubkey.
Also slightly refactors this area to reduce code duplication.
Github-Pull: #26418
Rebased-From: 8781a1b6bb
Taproot pubkey info was not being added for multi_a signing. The filling
of this info is moved into the common function CreateTaprootScriptSig so
that any signing of taproot scripts will include the pubkey info.
Github-Pull: #26418
Rebased-From: 323890d0d7
e049fd76f0 Bugfix: Check for readlink buffer overflow and handle gracefully (Luke Dashjr)
Pull request description:
Identical commit taken as-is from https://github.com/bitcoin/bitcoin/pull/25548 for backport
ACKs for top commit:
hebasto:
ACK e049fd76f0
Tree-SHA512: 37e63d570de898187c1bc8dd311c299c527adea51faa08aa6a3923bdb9390e3263902ace3d52a1cfc34ac2ba84e9358961574f886be1f64b5749a62e3c50ad57
d5701900fc rpc: make `address` field optional (w0xlt)
e4b8c9b2bf rpc: add non-regression test about deriveaddresses crash when index is 2147483647 (muxator)
bf2bf73bcb rpc: fix crash in deriveaddresses when derivation index is 2147483647 (muxator)
b04f5f9608 test: Test for out of bounds vout in sendall (Andrew Chow)
dedee6af57 wallet: Check utxo prevout index out of bounds in sendall (Andrew Chow)
931db785ee test: Test that sendall works with watchonly spending specific utxos (Andrew Chow)
bbe864a13a wallet: Correctly check ismine for sendall (Andrew Chow)
4b7d30d026 Adjust `.tx/config` for new Transifex CLI (Hennadii Stepanov)
Pull request description:
Backports:
* https://github.com/bitcoin/bitcoin/pull/26321
* https://github.com/bitcoin/bitcoin/pull/26344
* https://github.com/bitcoin/bitcoin/pull/26275
* https://github.com/bitcoin/bitcoin/pull/26349
ACKs for top commit:
instagibbs:
ACK d5701900fc
hebasto:
ACK d5701900fc, I've cherry-picked commits manually and got zero diff with this PR branch.
Tree-SHA512: dad64f4074b4f06d666c0f2d804eda92df241bcce0a49c28486311a151f2e9d46b75e1bce02de570dcc85957c9ce936debb2a4faa045800c9757c6c495115d7c
2147483647 is the maximum positive value of a signed int32, and - currently -
the maximum value that the deriveaddresses bitcoin RPC call accepts as
derivation index due to its input validation routines.
Before this change, when the derivation index (and thus range_end) reached
std::numeric_limits<int_32_t>::max(), the "i" variable in the for cycle (which
is declared as int, and as such 32 bits in size on most platforms) would be
incremented at the end of the first iteration and then warp back to
-2147483648. This caused SIGABRT in bitcoind and a core dump.
This change assigns "i" an explicit size of 64 bits on every platform,
sidestepping the problem.
Fixes#26274.
Github-Pull: #26275
Rebased-From: addf9d6502
sendall should be using a bitwise AND for sendall's IsMine check rather
than an equality as IsMine will never return ISMINE_ALL.
Github-Pull: #26344
Rebased-From: 6bcd7e2a3b
Instead of having an entire TaprootBuilder which may or may not be
complete, and could potentially have future changes that interact oddly
with taproot tree tuples, have m_tap_tree be just the tuples.
When needed in other a TaprootBuilder for actual use, the tuples will be
added to a a TaprootBuilder that, in the future, can take in whatever
other data is needed as well.
Github-Pull: #25858
Rebased-From: 0577d423ad
Merging should be checking that the current PSBTOutput doesn't have a
taptree and the other one's is copied over. The original merging had
this inverted and would remove m_tap_tree if the other did not have it.
Github-Pull: #25858
Rebased-From: 7df6e1bb77
Since m_next_resend is now only called from MaybeResendWalletTxs()
we don't have any potential race conditions anymore, so the usage
of std::atomic can be reverted.
Github-Pull: #26205
Rebased-From: b01682a812
We only want to relay our resubmitted transactions once every 12-36h.
By separating the timer update logic out of ResubmitWalletTransactions
and into MaybeResendWalletTxs we avoid non-relay calls (previously in
the separate ReacceptWalletTransactions function) from resetting that
timer.
Github-Pull: #26205
Rebased-From: 9245f45670
Moves the logic of whether or not transactions should actually be
resent out of the function that's resending them. This reduces
responsibilities of ResubmitWalletTransactions and allows
carving out the updating of m_next_resend in a future commit.
Github-Pull: #26205
Rebased-From: 7fbde8af5c
ReacceptWalletTransactions is replaced by ResubmitWalletTransactions
which already handles acquiring the necessary locks internally.
Github-Pull: #26205
Rebased-From: 01f3534632
Since commit f08c9fb0c6 from PR
https://github.com/bitcoin/bitcoin/pull/21726, index
`BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has
also been a race condition in the `coinstatsindex_initial_sync` unit test.
It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the
last connected block to be fully processed, than to be able to return before
prune locks are set, so this switches the order of `m_best_block_index =
block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more
reliable.
Also since commit f08c9fb0c6, there has been a
race condition in the `coinstatsindex_initial_sync` test. Before that commit,
the atomic index best block pointer `m_best_block_index` was updated as the
last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain`
could safely be used in tests to wait for the last `BlockConnected`
notification to be finished before stopping and destroying the index. But
after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer
sufficient, and there is a race between the test shutdown code which destroys
the index object and the new code introduced in that commit calling
`AllowPrune()` and `GetName()` on the index object. Reproducibility
instructions for this are in
https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133
This commit fixes the `coinstatsindex_initial_sync` race condition, even though
it will require an additional change to silence TSAN false positives,
https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this
partially addresses but does not resolve the bug reporting TSAN errors
https://github.com/bitcoin/bitcoin/issues/25365.
There is no known race condition outside of test code currently, because the
bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not
`BlockUntilSyncedToCurrentChain` to safely shut down.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: MacroFake <falke.marco@gmail.com>
Github-Pull: #26215
Rebased-From: 8891949bdc
Follow-up to #25717. The commit "Utilize anti-DoS headers download
strategy" changed how this bool variable is computed, so that its value
is now the opposite of what it should be.
GitHub-Pull: #26172
Rebased-From: bdcafb9133
c1860341a7 qt: 24.0rc2 translations update (Hennadii Stepanov)
Pull request description:
This PR pulls the recent translations from the [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-core/bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool, and it is supposed to be merged just before `v24.0rc2` tagging.
Top commit has no ACKs.
Tree-SHA512: 4c31452dd36509b0c1f0f5f499b9a3add53409a592d70625c14d7e249de48e7fce65777c9a78882bd37dc345362f45fbae117aa80cec342e6352fc43ad9306c3
a60d9eb9e6 Bugfix: Wallet: Lock cs_wallet for SignMessage (Luke Dashjr)
Pull request description:
(Clean merge of #26130 to 24.x branch)
Top commit has no ACKs.
Tree-SHA512: 821e19d222cc1eb9a6b957ec87d48cfb00b2c5b8182682ac57d9c76785b667ad9c71444e6bf0f53177c06d5fb39e72dbfc82d7debe4b1597699eefaf3001d08d
fa4ba04c15 fuzz: Remove no-op call to get() (MacroFake)
fa642286b8 fuzz: Avoid timeout in bitdeque fuzz target (MacroFake)
Pull request description:
Identical commit from https://github.com/bitcoin/bitcoin/pull/26012
Not strictly required for 24.x, but I guess it can't hurt to avoid timeouts.
Top commit has no ACKs.
Tree-SHA512: 4d4bfb645e3513bf22cc9c64bdcbde2ad9e28b5a07ab07a02fbfa19df02147b371d2ca794ab3a095c22b66781832055e0de3af908aaead4c26ea12189e05cbe3
68209a7b5c rpc: make addpeeraddress work with cjdns addresses (Martin Zumsande)
a8a9ed67cc init: Abort if i2p/cjdns are chosen via -onlynet but unreachable (Martin Zumsande)
Pull request description:
Identical commit from https://github.com/bitcoin/bitcoin/pull/25989
ACKs for top commit:
fanquake:
ACK 68209a7b5c
Tree-SHA512: eec335df06b4c209cfe3473cb623828effd00c45a5dd605bb920edd265de1c789627482b005a51e89b8fc79cc4c5d26ff1fc306f2e4573897c5c7f083aa22861
fad61573ed Fix nNextResend data race in ResubmitWalletTransactions (MacroFake)
Pull request description:
Identical commit id from https://github.com/bitcoin/bitcoin/pull/26132
Top commit has no ACKs.
Tree-SHA512: 9404e2e10ba059c412e282abbf9bef581cf5ddcac36cf05da1dff3927b5015e12469238c402c28308a774fdd969d1039e595d5e2caca0902977ae0a72746ff43
faf5bb87da doc: Move -permitbaremultisig to the relay help category (MacroFake)
Pull request description:
Identical commit from https://github.com/bitcoin/bitcoin/pull/26119
ACKs for top commit:
glozow:
ACK faf5bb87da
jarolrod:
ACK faf5bb87da
Tree-SHA512: 42d3fd541703cbea7d2afff54dc7a42dac475c70c59ed124aba59cae6a87898040d964201e6cc6098e202f7b87bfa98513b3efd3c25d9fe52dc0ef55f3540bef
cs_desc_main is typically locked within scope of a cs_wallet lock, but:
CWallet::IsLocked locks cs_wallet
...called from DescriptorScriptPubKeyMan::GetKeys
...called from DescriptorScriptPubKeyMan::GetSigningProvider which locks cs_desc_main first, but has no access to cs_wallet
...called from DescriptorScriptPubKeyMan::SignMessage
...called from CWallet::SignMessage which can access and lock cs_wallet
Resolve the out of order locks by grabbing cs_wallet in CWallet::SignMessage first
c3e536555a Bugfix: Wallet: Return util::Error rather than non-error nullptr when CreateWallet/LoadWallet/RestoreWallet fail (Luke Dashjr)
335ff98c8a Bugfix: Wallet: Wrap RestoreWallet content in a try block to ensure exceptions become returned errors and incomplete wallet directory is removed (Luke Dashjr)
Pull request description:
Bug 1: `copy_file` can throw exceptions, but `RestoreWallet` is expected to return a nullptr with a populated `errors` parameter. This is fixed by wrapping `copy_file` and `LoadWallet` (for good measure) in a `try` block, and converting any exceptions to the intended return style.
Bug 2: `util::Result` turns what would have been a `false` unique_ptr into a `true` nullptr result, which leads to nullptr dereferences in at least the 3 cases of wallet creation/loading/restoring. This is fixed by keeping the pointer as a plain `std::unique_ptr` until actually returning it (ie, after the nullptr check).
Fixes https://github.com/bitcoin-core/gui/issues/661
ACKs for top commit:
achow101:
ACK c3e536555a
Tree-SHA512: 4291b3dbbb147acea2e63a704324c9371bc16ecb4237f8753729b0b0a6e55c9758ad61bfe8bd432fd7b0bae95d8b63a9831e61ac8b8d5c0197b550a2e0f4a105
