Commit graph

37245 commits

Author SHA1 Message Date
Luke Dashjr
5a4fe55270
GUI: Intro: Support translating caption of data directory chooser 2023-03-31 12:21:35 +01:00
Luke Dashjr
3868ba3a27
GUI: Support translating peer network names 2023-03-31 12:21:23 +01:00
Luke Dashjr
f1f9811198
GUI: Support translating address type dropdown entries 2023-03-31 12:21:11 +01:00
dergoegge
3fa4c54ac5 [net processing] Pass TxRelay to FindTxForGetData instead of Peer 2023-03-31 13:19:20 +02:00
dergoegge
c85ee76a36 [net processin] Don't take cs_main in FindTxForGetData
Taking cs_main is no longer necessary since we moved
`m_recently_announced_invs` to `Peer` and `mapRelay` is actually only
accessed from the message processing thread.
2023-03-31 13:18:39 +02:00
fanquake
47184cfa2f
Merge bitcoin/bitcoin#27362: test: remove GetRNGState lsan suppression
71b3e9b0ad sanitizers: remove GetRNGState lsan suppression (fanquake)

Pull request description:

  I am no-longer seeing this, testing with the native_asan job over `x86_64` (Ubuntu 22.04) and `aarch64` (Fedora 37).

  Can anyone recreate the false-positive?

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 71b3e9b0ad
  hebasto:
    ACK 71b3e9b0ad, tested on Ubuntu 22.04 x86_64.

Tree-SHA512: 63020327d61acd6c94c6c278c9c4d72aedc10253fa172bcf9353bcad4c28d068bee824969eb3ce92152244831df8fe92cffae536453c8073a4fda74dfdfbcefa
2023-03-31 11:16:35 +01:00
MarcoFalke
fafe3a8e38
ci: Remove second user account 2023-03-31 08:28:38 +02:00
fanquake
6b9fedd221
Merge bitcoin/bitcoin#27361: guix: use python-minimal (3.9)
d0e571ebb1 guix: use python-minimal (3.9) (fanquake)

Pull request description:

  This further minifies the Guix release build environment.

ACKs for top commit:
  TheCharlatan:
    ACK d0e571ebb1
  hebasto:
    ACK d0e571ebb1

Tree-SHA512: 0a8aa9ae861107f106c3b9c41f78ffbaf0e71e3c61f6d96e5c82415b4570b8ac85d6578d37cd0df0ec315c1c9f35fc90b281f139271ccfd15a1495ba76166789
2023-03-30 18:56:19 +01:00
fanquake
45be6af694
Merge bitcoin/bitcoin#27333: ci: cleanup of CI_EXEC & CI_EXEC_ROOT
b5ef1419ec ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321) (Vasil Stoyanov)

Pull request description:

  Basically it removes the above-mentioned env-vars as per MarcoFalke's instructions. The only deviation from the plan laid out there was that I double-quoted the last instance of $ANDROID_HOME for the sake of consistency and future-proofing and the rest of the non-quoted vars due to lint failing the build.

  Fixes #27321.

ACKs for top commit:
  josibake:
    ACK b5ef1419ec
  hernanmarino:
    untested ACK b5ef1419ec. LGTM

Tree-SHA512: a79776bf64a2fa8b38195cc84445e171fd689f156aac5a1e5d39040300567eb9f4c2ebd00fbf3fa0e55b68793f8f752d94f7d817f6097ed9dd3a8ea57651b981
2023-03-30 18:55:27 +01:00
glozow
328087d16f
Merge bitcoin/bitcoin#27350: test: refactor: dedup mempool_package_limits.py subtests via decorator
e669833943 test: dedup package limit checks via decorator in mempool_package_limits.py (Sebastian Falbesoner)
72f25e238c test: refactor: use Satoshis for fees in mempool_package_limits.py (Sebastian Falbesoner)

Pull request description:

  The subtests in the functional test mempool_package_limits.py all follow the same pattern:
  1. first, check that the mempool is currently empty
  2. create and submit certain single txs to the mempool, prepare list of hex transactions
  3. check that `testmempoolaccept` on the package hex fails with a "package-mempool-limits" error on each tx result
  4. after mining a block, check that submitting the package succeeds

  Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again.

  In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (`COIN` and `Decimal`) and a comment for the `test_desc_size_limits` subtest is fixed (s/25KvB/21KvB/).

ACKs for top commit:
  ismaelsadeeq:
    ACK e669833943
  glozow:
    utACK e669833943

Tree-SHA512: 84a85e739de7387391c13bd46aeb015a74302ea7c6f0ca3d4e2b1b487d38df390dc118eb5b1c11d3e4206bff316a4dab60ef6b25d8feced672345d4e36ffd205
2023-03-30 18:47:17 +01:00
glozow
d4833e9c49
Merge bitcoin/bitcoin#26140: refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer
3a060ae7b6 scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders (dergoegge)
279c53d7e4 [net processing] Move m_recently_announced_invs from CNodeState to Peer (dergoegge)
938a8e2566 [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex (dergoegge)
8a2cb1f749 [net processing] Move fPreferHeaders from CNodeState to Peer (dergoegge)
3605011e79 [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex (dergoegge)
4b84e502f5 [net processing] Move m_headers_sync_timeout from CNodeState to Peer (dergoegge)
689b747fc3 [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex (dergoegge)
d8c0d1c345 [net processing] Move nUnconnectingHeaders from CNodeState to Peer (dergoegge)
5f80d8d1ee [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex (dergoegge)
1d87137227 [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main (dergoegge)

Pull request description:

  `nUnconnectingHeaders`, `m_headers_sync_timeout`, `fPreferHeaders` and  `m_recently_announced_headers` are currently all `CNodeState` members even though they are only ever accessed from the message processing thread (therefore sufficiently guarded exclusively by `g_msgproc_mutex`). `CNodeState` exists purely to hold validation-specific state guarded by `cs_main` that is accessed by multiple threads.

  This PR adds thread-safety annotations for the above mentioned `CNodeState` members and moves them to `Peer`.

ACKs for top commit:
  glozow:
    code review ACK 3a060ae7b6, as in I am convinced these members shouldn't be guarded by cs_main and belong in Peer/TxRelay. clang checked the annotations for me.
  hebasto:
    ACK 3a060ae7b6

Tree-SHA512: 2db27c03f2c6ed36ad7dfbb4f862eeed3c3e57f845cf8abb9e7cada36f976257311892020bbcff513fbe662a881c93270e3a126946ceb0c3f94213b546bcaa81
2023-03-30 17:10:40 +01:00
fanquake
71b3e9b0ad
sanitizers: remove GetRNGState lsan suppression
I am no-longer seeing this. Can anyone recreate the false-positive?
2023-03-30 14:10:35 +01:00
dergoegge
3a060ae7b6 scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }

ren nUnconnectingHeaders     m_num_unconnecting_headers_msgs
ren fPreferHeaders           m_prefers_headers
ren MAX_UNCONNECTING_HEADERS MAX_NUM_UNCONNECTING_HEADERS_MSGS

-END VERIFY SCRIPT-
2023-03-30 14:56:34 +02:00
dergoegge
279c53d7e4 [net processing] Move m_recently_announced_invs from CNodeState to Peer 2023-03-30 14:56:19 +02:00
dergoegge
938a8e2566 [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex 2023-03-30 14:56:19 +02:00
dergoegge
8a2cb1f749 [net processing] Move fPreferHeaders from CNodeState to Peer 2023-03-30 14:56:19 +02:00
dergoegge
3605011e79 [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex 2023-03-30 14:56:19 +02:00
dergoegge
4b84e502f5 [net processing] Move m_headers_sync_timeout from CNodeState to Peer 2023-03-30 14:56:19 +02:00
dergoegge
689b747fc3 [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex 2023-03-30 14:56:19 +02:00
dergoegge
d8c0d1c345 [net processing] Move nUnconnectingHeaders from CNodeState to Peer 2023-03-30 14:56:17 +02:00
dergoegge
5f80d8d1ee [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex 2023-03-30 14:55:28 +02:00
dergoegge
1d87137227 [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main 2023-03-30 14:55:28 +02:00
fanquake
84f4ac39fd
Merge bitcoin/bitcoin#27363: ci: use LLVM/clang-16 in native_fuzz (ASAN) job
a634c288c3 ci: use LLVM/clang-16 in native_fuzz (ASAN) job (fanquake)

Pull request description:

  Similar to #27298.

ACKs for top commit:
  dergoegge:
    utACK a634c288c3

Tree-SHA512: 7a2625a3ac83710063d941dcbca42431b3b79a1380872fd2c566c0ab3041d8123d7dcddeb8a4972efd0ef6496b15bbe0b39b6d2de84df81fcdd8d68e1248fbc5
2023-03-30 13:49:06 +01:00
fanquake
5241b8bdff
Merge bitcoin/bitcoin#27368: refactor: Drop no longer used CNetMsgMaker instances
ea7ec78087 refactor: Drop no longer used `CNetMsgMaker` instances (Hennadii Stepanov)

Pull request description:

  The removed lines have been unused since the abf5d16c24 commit from https://github.com/bitcoin/bitcoin/pull/25454.

ACKs for top commit:
  dergoegge:
    utACK ea7ec78087
  Sjors:
    ACK ea7ec78087
  TheCharlatan:
    ACK ea7ec78087

Tree-SHA512: 9a2a9ff3f124b68a8cd20a637e90885096996c3aa354a4d8adbec98f5761e9e826c1c064ccd90aaf6d72beac61dd9e22c8b76d089e18bba6e0ad51e59a9c7df8
2023-03-30 13:45:24 +01:00
Hennadii Stepanov
ea7ec78087
refactor: Drop no longer used CNetMsgMaker instances 2023-03-30 11:00:44 +01:00
fanquake
a634c288c3
ci: use LLVM/clang-16 in native_fuzz (ASAN) job
Similar to #27298.
2023-03-29 16:05:59 +01:00
fanquake
d0e571ebb1
guix: use python-minimal (3.9)
This further minifies the Guix release build environment.
2023-03-29 15:47:04 +01:00
fanquake
8e7179aa13
Merge bitcoin/bitcoin#27345: guix: use GCC tool wrappers
4133c8104f guix: use gcc tool wrappers (fanquake)

Pull request description:

  This way, correct `--plugin` arguments are passed through.

  This is a prerequisite for LTO (see #25391). Split out, to try move things along, as this change is isolated, and should be straight-forward.

ACKs for top commit:
  TheCharlatan:
    ACK [4133c81](4133c8104f)
  hebasto:
    ACK 4133c8104f

Tree-SHA512: 4311a72a613cf027bd4490caa29604c985ed455589acd972285f13cbdf4806d2184a4dc6f20cb6f47c3fa751d58bfd0bacc257b87d4a804bf5ecf5b240e4a757
2023-03-29 12:09:23 +01:00
fanquake
c0311b1dda
Merge bitcoin/bitcoin#27349: test: use address_to_scriptpubkey instead of RPC call
e47ce42f67 refactor: use address_to_scriptpubkey to retrieve addresses scriptpubkey (ismaelsadeeq)
4142d19d74 refactor: move address_to_scriptpubkey to address.py (ismaelsadeeq)

Pull request description:

  PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).

  Instead of using RPC calls, this update replaces the process of obtaining an address scriptPubkey with the address_to_scriptpubkey method, resulting in improved performance for functional tests.

ACKs for top commit:
  josibake:
    re-ACK e47ce42f67
  theStack:
    ACK e47ce42f67 🌱

Tree-SHA512: 05285349a7d5ce7097b8f2582e573a5135c6deef85ea9936f68f6ce94e9ebb1d84d94f7fc7e5ed833a698e01585addd80deb52e6338f8aee985bf14db45417d2
2023-03-29 12:05:15 +01:00
Andrew Chow
7a6e7ffd06 contrib: Use machine parseable GPG output in verifybinaries
GPG has an option to provide machine parseable output. Use that instead
of trying to parse the human readable output.
2023-03-28 22:16:05 +00:00
Andrew Chow
6b2cebfa2f contrib: Add verifybinaries command for specifying files to verify
In addition to verifying the published releases with the `pub` command,
the verifybinaries script is updated to take a `bin` command where the
user specifies the local files, sums, and sigs to verify.
2023-03-28 22:16:05 +00:00
Andrew Chow
e4d5778228 contrib: Specify to GPG the SHA256SUMS file that is detached signed 2023-03-28 22:16:05 +00:00
Andrew Chow
17575c0efa contrib: Refactor verifbinaries to support subcommands
Prepares for the option to provide local binaries, sha256sums, and
signatures directly.
2023-03-28 22:16:05 +00:00
James O'Beirne
37c9fb7a59 contrib: verifybinaries: allow multisig verification
This commit adds the functionality necessary to transition from
doing binary verification on the basis of a single signature to
requiring a minimum threshold of trusted signatures.

A signature can appear as "good" from GPG output, but it may not come
from an identity the user trusts. We call these "good, untrusted"
signatures.

We report bad signatures but do not necessarily fail in their presence,
since a bad signature might coexist with enough good, trusted signatures
to fulfill our criteria.

If "--import-keys" is enabled, we will prompt the user to
optionally try to retrieve unknown keys. Marking them as trusted locally
is a WIP, but keys which are retrieved successfully and appear on the
builder-keys list will immediately count as being useful towards
fulfilling the threshold.

Logging is improved and an option to output JSON that summarizes the
whole sum signature and binary verification processes has been added.

Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
Co-authored-by: willcl-ark <will8clark@gmail.com>
2023-03-28 22:16:05 +00:00
Sebastian Falbesoner
e669833943 test: dedup package limit checks via decorator in mempool_package_limits.py 2023-03-28 22:03:31 +02:00
Sebastian Falbesoner
72f25e238c test: refactor: use Satoshis for fees in mempool_package_limits.py
This avoids having to convert from BTC to Sats and needs less imports.
Also specify the tx's target size in vsize rather than in weight, which
allows us to specify the fee-rate by a simple multiplication, rather
than having another magic number for it.
2023-03-28 22:03:27 +02:00
ismaelsadeeq
e47ce42f67 refactor: use address_to_scriptpubkey to retrieve addresses scriptpubkey
This commit updates the code by replacing the RPC call used to
decode an address and retrieve its corresponding scriptpubkey
with the address_to_scriptpubkey function. address_to_scriptpubkey
function can now decode all addresses formats, which makes
it more efficient to use.
2023-03-28 16:58:16 +01:00
ismaelsadeeq
4142d19d74 refactor: move address_to_scriptpubkey to address.py
The COINBASE_MATURITY constant in blocktools.py is imported in wallet.py.
However, importing address_to_scriptpubkey to blocktools.py will
generate a circular import error. Since the method is related to
addresses, it is best to move it to address.py, which will also
fix the circular import error.

Update imports of address_to_scriptpubkey accordingly.
2023-03-28 16:54:28 +01:00
fanquake
68828288e5
Merge bitcoin/bitcoin#27318: test: wallet_create_tx.py fix race
8aab5157c5 test: wallet_create_tx.py fix race (furszy)

Pull request description:

  Fixes #27316

  Because wallets are internally synchronized through the validation interface,
  and the interface dispatches events on a worker thread, it is possible for a
  transaction created by the first wallet to not arrive to the second wallet
  before the second wallet attempts to use one of its outputs. This is because
  we do not wait for the `BroadcastTransaction` callback during the wallet's
  "submit to mempool" process. To address this in the tests, we need to
  manually sync the validation queue.

ACKs for top commit:
  josibake:
    ACK 8aab5157c5
  theStack:
    ACK 8aab5157c5

Tree-SHA512: 76364370ab292a5c3ea1ed61cd353fc626a9e9cd6ce18464c24da1b3dcb34b65006e2bc42b84bbd25af03f9449231990bf789504728972db3217b569099eb309
2023-03-28 15:41:15 +01:00
fanquake
220008604f
Merge bitcoin/bitcoin#27298: ci: Use TSan new runtime (llvm-16, take 3)
faf4aca15a ci: Use TSan new runtime (llvm-16, take 3) (MarcoFalke)

Pull request description:

  The previous two attempts failed:
  * llvm-14: Failed in https://github.com/bitcoin/bitcoin/pull/24572
  * llvm-15: Failed in https://github.com/bitcoin/bitcoin/pull/26775

  However, now that the bug is known and fixed, it should be good to go. See also https://github.com/bitcoin/bitcoin/pull/26775#issuecomment-1380590669

ACKs for top commit:
  fanquake:
    ACK faf4aca15a - I still see [this](https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480041089) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up.

Tree-SHA512: 372b53c4d42ca7f527dae4a2b5bc5ab33c816930daf7a3479d20ea7749159a0b19cfd8d76244b95b03130e4a3d12ddbbb74668b8f7e9fc272cf1084f53b7ff9b
2023-03-28 14:17:30 +01:00
fanquake
8d31d769b7
Merge bitcoin/bitcoin#27344: fuzz: Remove legacy int parse fuzz tests
faf8dc496e fuzz: Remove legacy int parse fuzz tests (MarcoFalke)

Pull request description:

  The fuzz tests checked that the result of the new function was equal to the legacy function. (Side note: The checks were incomplete, as evident by the follow-up fix in commit b5c9bb5cb9).

  Given that they haven't found any issues in years (beside missing the above issue, that they couldn't catch), it seems time to remove them.

  They may come in handy in the rare case that someone would want to modify `LocaleIndependentAtoi()` or `Parse*Int*()`, however that seems unlikely. Also, appropriate checks can be added then.

ACKs for top commit:
  fanquake:
    ACK faf8dc496e
  dergoegge:
    ACK faf8dc496e

Tree-SHA512: 4ec88b9fa8ba49a923b0604016f0f471b3c9b9e0ba6c5c3dc4e20503c6994789921e7221d9ec467a2a37a73f21a70ba51ba3370ed5ad311dee989e218290b29a
2023-03-28 12:03:39 +01:00
fanquake
d254f942a5
Merge bitcoin/bitcoin#27324: net: #27257 follow-ups
cd0c8eeb09 [net] Pass nRecvFloodSize to CNode (dergoegge)
860402ef2e [net] Remove trivial GetConnectionType() getter (dergoegge)
b5a85b365a [net] Delete CNetMessage copy constructor/assignment op (dergoegge)

Pull request description:

  Follow-up PR for #27257

  * Deletes the copy constructor/assignment operator of `CNetMessage`
  * Removes trivial getter for the connection type
  * Avoids passing `nRecvFloodSize` to CNode methods by passing it to `CNode` on creation

ACKs for top commit:
  jnewbery:
    utACK cd0c8eeb09
  theStack:
    ACK cd0c8eeb09

Tree-SHA512: 673a758668617f69fba77e61f0eaa1538da27a4849c82c98742436692baa2d7f001129af3e7a66b160e599d12109dac08137a146f10ff9b9ebdc5c2237311d41
2023-03-28 11:48:02 +01:00
fanquake
86e7410b22
Merge bitcoin/bitcoin#27348: test: fix intermittent failure in ChainStateManager tests
f8abcb3e3b test: Fix intermittent failure in ChainStateManager tests (Martin Zumsande)

Pull request description:

  Before wiping a `ChainStateManager` and creating a new one, the `validationinterface` queue should be emptied to avoid the possibility of accessing deleted memory.
  This could lead to very rare CI failures reported in #26613 and #27320 (see [here](https://github.com/bitcoin/bitcoin/issues/27320#issuecomment-1485694691) for a more detailed explanation).

  Fixes #27320

ACKs for top commit:
  jamesob:
    crACK f8abcb3e3b
  MarcoFalke:
    lgtm ACK f8abcb3e3b

Tree-SHA512: 953fcca46ffee039ae1c8c98eed5464ba7ee0007f54e18989caab3f645f5f45e64407a21f614984fd2843a3d56219f44603086b0c305a9a5cec64a8bf76c110a
2023-03-28 11:14:02 +01:00
MarcoFalke
faf4aca15a
ci: Use TSan new runtime (llvm-16, take 3) 2023-03-28 10:01:47 +02:00
Martin Zumsande
f8abcb3e3b test: Fix intermittent failure in ChainStateManager tests
Before wiping the ChainStateManager, the validationinterface
queue must be drained to avoid accessing deleted memory.
2023-03-27 15:47:32 -04:00
Vasil Stoyanov
b5ef1419ec ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321) 2023-03-27 20:45:17 +03:00
fanquake
b759cefe93
Merge bitcoin/bitcoin#27301: depends: make fontconfig build under clang-16
9cbc1c2792 depends: make fontconfig build under clang-16 (fanquake)

Pull request description:

  Use the same workaround we've applied to qrencode, and other packages. Fontconfig not building is currently a blocker for fuzz/sanitizer infra upgrades (#27298).

  For now, this is also more straightforward than bumping the package, which introduces more complexity/usage of gperf.

  Closes: #27299.

ACKs for top commit:
  hebasto:
    ACK 9cbc1c2792

Tree-SHA512: 387ea1a73e3429f166ef5278305a56cb3c69b6e3fc8a21a66521738e313e3fe783f042759b396cd88e28c10918a4427fb836a8dfecc5a846723b6f6c6a7ade51
2023-03-27 17:49:45 +01:00
fanquake
9cbc1c2792
depends: make fontconfig build under clang-16
Use the same workaround we've applied to qrencode, and other packages.
Fontconfig not building is currently a blocker for fuzz/sanitizer infra
upgrades.

For now, this is also more straightforward than bumping the package,
which introduces more complexity/usage of gperf.
2023-03-27 16:33:15 +01:00
Hennadii Stepanov
ff26406b2b
Merge bitcoin-core/gui#693: Fix segfault when shutdown during wallet open
9a1d73fdff Fix segfault when shutdown during wallet open (John Moffett)

Pull request description:

  Fixes #689

  ## Summary

  If you open a wallet and send a shutdown signal during that process, you'll get a segfault when the wallet finishes opening. That's because the `WalletController` object gets deleted manually in bitcoin.cpp during shutdown, but copies of the pointer (and pointers to child objects) are dangling in various places and are accessed in queued events after the deletion.

  ## Details

  The issue in #689 is caused by the following sequence of events:

  1. Wallet open modal dialog is shown and worker thread does the actual work.
  2. Every 200ms, the main event loop checks to see if a shutdown has been requested, but only if a modal is not being shown.
  3. Request a shutdown while the modal window is shown.
  4. The wallet open process completes, the modal window is dismissed, and various `finish` signals are sent.
  5. During handling of one of the `finish` signals, `qApp->processEvents()` is [called](e9262ea32a/src/qt/sendcoinsdialog.cpp (L603)), which causes the main event loop to detect the shutdown (now that the modal window has been dismissed). The `WalletController` and all the `WalletModel`s are [deleted](65de8eeeca/src/qt/bitcoin.cpp (L394-L401)).
  6. Control returns to the `finish` method, which eventually tries to send a [signal](e9262ea32a/src/qt/sendcoinsdialog.cpp (L167)) from a wallet model, but it's been deleted already (and the signal is sent from a now-[dangling](d8bdee0fc8/src/qt/walletview.cpp (L65)) pointer).

  The simplest fix for that is to change the `qApp->processEvents()` into a `QueuedConnection` call. (The `qApp->processEvents() was a [workaround](https://github.com/bitcoin/bitcoin/pull/593#issuecomment-3050699) to get the GUI to scroll to the last item in a list that just got added, and this is just a safer way of doing that).

  However, once that segfault is fixed, another segfault occurs due to some queued wallet events happening after the wallet controller object is deleted here:

  65de8eeeca/src/qt/bitcoin.cpp (L394-L401)

  Since `m_wallet_controller` is a copy of that pointer in `bitcoingui.cpp`, it's now dangling and `if(null)` checks won't work correctly. For instance, this line:

  65de8eeeca/src/qt/bitcoingui.cpp (L413)

  sets up a `QueuedConnection` to `setCurrentWallet`, but by the time control reaches that method (one event cycle after shutdown deleted `m_wallet_controller` in `bitcoin.cpp`), the underlying objects have been destroyed (but the pointers are still dangling).

  Ideally, we'd use a `QPointer` or `std::shared_ptr / std::weak_ptr`s for these, but the changes would be more involved.

  This is a minimal fix for the issues. Just set `m_wallet_controller` to `nullptr` in `bitcoingui.cpp`, check its value in a couple places, and avoid a use of `qApp->processEvents`.

ACKs for top commit:
  hebasto:
    ACK 9a1d73fdff, I have reviewed the code and it looks OK.
  furszy:
    ACK 9a1d73fdff

Tree-SHA512: a1b94676eb2fcb7606e68fab443b1565b4122aab93c35382b561842a049f4b43fecc459535370d67a64d6ebc4bcec0ebcda981fff633ebd41bdba6f7093ea540
2023-03-27 15:53:42 +01:00
MarcoFalke
faf8dc496e
fuzz: Remove legacy int parse fuzz tests 2023-03-27 16:37:31 +02:00