eea8114657 build: Enable unreachable-code-loop-increment (Jonathan Schoeller)
d15db4b1fc refactor: Fix unreachable code in init arg checks (Jonathan Schoeller)
Pull request description:
Closes: #19017
In #19015 it's been suggested that we add some new compiler warnings to our build. Some of these, such as `-Wunreachable-code-loop-increment`, generate warnings. We'll likely want to fix these up if we're going to turn these warnings on.
```shell
init.cpp:969:5: warning: loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment]
for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
^~~
1 warning generated.
```
aa8d76806c/src/init.cpp (L968-L972)
To fix this, collect all errors, and output them in a single error message after the loop completes. This resolves the unreachable code warning, and avoids popup hell that could result from outputting a seperate message for each error or warning one by one.
ACKs for top commit:
laanwj:
Code review ACK eea8114657
hebasto:
re-ACK eea8114657, only suggested changes applied since the [previous](https://github.com/bitcoin/bitcoin/pull/19131#pullrequestreview-421772387) review.
Tree-SHA512: 2aa3ceb7fab581b6ba2580900668388d8eba1c3001c8ff9c11c1f4a9a10fbc37f30e590249862676858446e3f4950140a252953ba1643ba3bfd772f8eae20583
fab7d954b2 test: Make valgrind.supp work on aarch64 (MarcoFalke)
Pull request description:
Was easy to fix by simply removing a line
ACKs for top commit:
practicalswift:
ACK fab7d954b2 -- diff looks correct
Tree-SHA512: d2d7c6cac453a3177c20e256ec50a03066f8dbf5ae45299077ccf4a2b45a3a40252b1b5fcaf9224a59bb5c3df5bd90ac58af27eb0f47dc87c2640df5b2b460ca
0fef60c63d build: improved output of configure for build OS (sachinkm77)
Pull request description:
The purpose of this fix is to improve output of the configure script by providing the build OS. This is done by leveraging the build_os set by the script config.sub / config.guess. #18966
ACKs for top commit:
fanquake:
ACK 0fef60c63d - thanks for following up.
Tree-SHA512: b9f49df901a9d37eb16c67c063bb3611602a84391aa54d097a52b740f474c2785c24bf405522d15d724fde25070d354bf20b885add2ee4405a71cbe9ebab5ff3
789e9dd3aa validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake)
47be28c8bc validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake)
Pull request description:
Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable.
ACKs for top commit:
MarcoFalke:
ACK 789e9dd3aa
jonatack:
ACK 789e9dd3aa
Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
bd7e530f01 This PR adds initial support for type hints checking in python scripts. (Kiminuo)
Pull request description:
This PR adds initial support for type hints checking in python scripts.
Support for type hints was introduced in Python 3.5. Type hints make it easier to read and review code in my opinion. Also an IDE may discover a potential bug sooner. Yet, as PEP 484 says: "It should also be emphasized that Python will remain a dynamically typed language, and the authors have no desire to ever make type hints mandatory, even by convention."
[Mypy](https://mypy.readthedocs.io/en/latest/index.html) is used in `lint-python.sh` to do the type checking. The package is standard so there is little chance that it will be abandoned. Mypy checks that type hints in source code are correct when they are not, it fails with an error.
**Notes:**
* [--ignore-missing-imports](https://mypy.readthedocs.io/en/latest/command_line.html#cmdoption-mypy-ignore-missing-imports) switch is passed on to `mypy` checker for now. The effect of this is that one does not need `# type: ignore` for `import zmq`. More information about import processing can be found [here](https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports). This can be changed in a follow-up PR, if it is deemed useful.
* We are stuck with Python 3.5 until 04/2021 (see https://packages.ubuntu.com/xenial/python3). When Python version is bumped to 3.6+, one can change:
```python
_opcode_instances = [] # type: List[CScriptOp]
```
to
```python
_opcode_instances:List[CScriptOp] = []
```
for type hints that are **not** function parameters and function return types.
**Useful resources:**
* https://docs.python.org/3.5/library/typing.html
* https://www.python.org/dev/peps/pep-0484/
ACKs for top commit:
fanquake:
ACK bd7e530f01 - the type checking is not the most robust (there are things it fails to detect), but I think this is worth adopting (in a limited capacity while we maintain 3.5 compat).
MarcoFalke:
ACK bd7e530f01 fine with me
Tree-SHA512: 21ef213915fb1dec6012f59ef17484e6c9e0abf542a316b63d5f21a7778ad5ebabf8961ef5fc8e5414726c2ee9c6ae07c7353fb4dd337f8fcef5791199c8987a
Migrates the CScriptNum decode tests into a unit test, and moved some
changes made in #14816. Made possible by the integration of
test_framework unit testing in #18576. Further extends the original
test with larger ints, similar to the scriptnum_tests.cpp file. Adds
test to blocktools.py testing fn create_coinbase() with CScriptNum
decode.
faf62e6ed0 ci: Remove unused workaround (MarcoFalke)
fa7c850915 ci: Install llvm to get llvm symbolizer (MarcoFalke)
fa563cef61 test: Add more tsan suppressions (MarcoFalke)
fa0cc02c0a ci: Mute depends logs completely (MarcoFalke)
fa906bf298 test: Extend tsan suppressions for clang stdlib (MarcoFalke)
fa10d85079 ci: Use libc++ instead of libstdc++ for tsan (MarcoFalke)
fa0d5ee112 ci: Set halt_on_error=1 for tsan (MarcoFalke)
fa2ffe87f7 ci: Deduplicate DOCKER_EXEC (MarcoFalke)
fac2eeeb9d cirrus: Remove no longer needed install step (MarcoFalke)
Pull request description:
According to the [ThreadSanitizer docs](https://clang.llvm.org/docs/ThreadSanitizer.html#current-status):
> C++11 threading is supported with **llvm libc++**.
For example, the thread sanitizer build is currently not checking for double lock of mutexes.
Fixes (partially) https://github.com/bitcoin/bitcoin/issues/19038#issuecomment-632138003
ACKs for top commit:
practicalswift:
ACK faf62e6ed0
fanquake:
ACK faf62e6ed0
hebasto:
ACK faf62e6ed0, maybe re-organize commits to modify suppressions in a single one?
Tree-SHA512: 98ce5154b4736dfb811ffdb6e6f63a7bc25fe50d3b73134404a8f3715ad53626c31f9c8132dbacf85de47b9409f1e17a4399e35f78b1da30b1577167ea2982ad
facede18a4 test: Check that invalid witness destinations can not be imported (MarcoFalke)
Pull request description:
ACKs for top commit:
practicalswift:
ACK facede18a4 -- thanks for adding!
Tree-SHA512: 87000606fac2e6f2780ca75cdeeb2dc1f0528d9b8f13e4156e8304ce7a6b1eb014781b6f0c59d11544bf360ba3dc5f99549470b0876132e189b9107f2c6bb38d
fab860aed4 fuzz: Stop nodes in process_message* fuzzers (MarcoFalke)
6666c828e0 fuzz: Give CNode ownership to ConnmanTestMsg in process_message fuzz harness (MarcoFalke)
Pull request description:
Background is that I saw an integer overflow in net_processing
```
#30629113 REDUCE cov: 25793 ft: 142917 corp: 3421/2417Kb lim: 4096 exec/s: 89 rss: 614Mb L: 1719/4096 MS: 1 EraseBytes-
net_processing.cpp:977:25: runtime error: signed integer overflow: 2147483624 + 100 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:977:25 in
net_processing.cpp:985:9: runtime error: signed integer overflow: -2147483572 - 100 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:985:9 in
```
Telling from the line numbers, it looks like `nMisbehavior` wrapped around.
Fix that by calling `StopNodes` after each exec, which should clear the node state and thus `nMisbehavior`.
ACKs for top commit:
practicalswift:
ACK fab860aed4
Tree-SHA512: 891c081d5843565d891aec028b6c27ef3fa39bc40ae78238e81d8f784b4d4b49cb870998574725a5159dd03aeeb2e0b9bc3d3bb51d57d1231ef42e3394b2d639
fa13c180c7 doc: Move 0.21 fragments into the main release notes (MarcoFalke)
fa1a91657f doc: Add release-notes-0.20.0.md (MarcoFalke)
faabc6e445 doc: Remove release notes of 0.20.1 release (MarcoFalke)
Pull request description:
Remove 0.20.1 fragments from master (which will become 0.21), also add the 0.20 release notes
ACKs for top commit:
laanwj:
ACK fa13c180c7
fanquake:
ACK fa13c180c7
Tree-SHA512: d5041f405cae255e6fde8d0346909a726fe41aa096478541e0ea3a45cadbb5c761b8da693aceb83f8e112575bc7e3cf896f2bf1f7b707240c6678cc48409ee2e
ActivateBestChain (ABC) is only called in the "msghand" or one of the
RPC threads, neither of which is a boost::thread. However, ABC is also
called in ThreadImport (which currently happens to be a boost::thread).
In all cases, the interruption_point is redundant with the breakpoint in
ABC that triggers when ShutdownRequested()
VerifyDB is only called in the main thread ("init") or one of the RPC
threads, neither of which is a boost::thread.
7eaf86d3bf trivial: Suggested cleanups to surrounding code (Russell Yanofsky)
b604c5c8b5 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky)
Pull request description:
This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600.
Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd and 7e89994133 from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325.
The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations.
Fixes#18325
Co-authored-by: Antoine Riard (ariard)
ACKs for top commit:
jonatack:
Re-ACK 7eaf86d3bf
ariard:
ACK 7eaf86d, reviewed, built and ran tests.
MarcoFalke:
ACK 7eaf86d3bf🍡
Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
fa1c74fd03 wallet: Remove unused boost::thread_interrupted (MarcoFalke)
fa7b885f51 walletdb: Remove unsed boost/thread (MarcoFalke)
5555d978b0 wallet: Make PeriodicFlush uninterruptible (MarcoFalke)
Pull request description:
The `boost::this_thread::interruption_point()` in the code base currently block the replacement of `boost::thread` with `std::thread`. [1]
Remove them from the wallet because they are either unused or useless.
The feature to interrupt a periodic flush is useless because all wallets have just been flushed 9ccaee1d5e/src/init.cpp (L194) and another flush should be a noop. Also, they will be flushed again shortly after 9ccaee1d5e/src/init.cpp (L285), so even if repeated flushes weren't a noop, doing 3 instead of 2 shouldn't matter too much at this point. Also, the wallet is flushed every two seconds in the worst case, so if this is an expensive operation, that period should be readjusted. (Or bdb should be removed altogether #18916)
[1] Replacement of `boost::thread` with `std::thread` should happen because:
* The boost thread dependency is slow to compile
* Boost thread is less maintained than the standard lib
* Boost thread is mostly redundant to the standard lib
* Global interruption points via exceptions are hard to keep track of during review and easy to get wrong during runtime (e.g. accidental `catch (...)`)
ACKs for top commit:
fanquake:
ACK fa1c74fd03
Tree-SHA512: b166619256de2ef4325480fa1367f68bc9371ad785ec503aed61eab41ba61f1a9807aab25451a24efda3db64855c9ba0025645b98bc58557bc3ec56c5b3297d0
152e8baf08 Use salted hasher instead of nonce in sigcache (Jeremy Rubin)
5495fa5850 Add Hash Padding Microbenchmarks (Jeremy Rubin)
Pull request description:
This PR replaces nonces in two places with pre-salted hashers.
The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step.
I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures.
ACKs for top commit:
ryanofsky:
Code review ACK 152e8baf08. No code changes, just rebase since last review and expanded commit message
Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
fa7d3a8890 test: Add missing sync_blocks to wallet_hd (MarcoFalke)
eeeed51f58 test: pep-8 wallet_hd (MarcoFalke)
Pull request description:
This fixes the ` test_framework.authproxy.JSONRPCException: non-final (-26)` error when node 1 is one block behind of node 0. (height 122 vs 123)
ACKs for top commit:
promag:
Code review ACK fa7d3a8890.
Tree-SHA512: b549dce2e08c58b949168ed2013bfa176802c963d0d7e890f643c8792da5dade14d91441dfa74372f1f1d34d696e8900a2b60b4861c0ba2dce99f2a633ab27ff
fa9c675591 Limit scope of all global std::once_flag (MarcoFalke)
Pull request description:
`once_flag` is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.
Move all the flags to function scope to
* simplify code review
* avoid mistakes where similarly named flags are accidentally exchanged
* avoid polluting the global scope
ACKs for top commit:
hebasto:
ACK fa9c675591, tested on Linux Mint 19.3 (x86_64).
promag:
Code review ACK fa9c675591.
Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
4f49d5222e gui, refactor: Register Qt meta types in application constructor (João Barbosa)
Pull request description:
Removes a warning when running `QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt`.
ACKs for top commit:
jonasschnelli:
Re utACK 4f49d5222e
hebasto:
ACK 4f49d5222e, tested on macOS 10.15.5.
Tree-SHA512: e931a022ba83cb0ef04d82544ebd9b18242f8fc2b41443afce4d5c4222f222e8b3517bdb484a1a4f61377c5dceca067d8ccf250da3a727299448e54bec33ed6e
Support for type hints was introduced in Python 3.5. Type hints make it easier to read and review code in my opinion. Also an IDE may discover a potential bug sooner. Yet, as PEP 484 says: "It should also be emphasized that Python will remain a dynamically typed language, and the authors have no desire to ever make type hints mandatory, even by convention."
Mypy is used in lint-python.sh to do the type checking. The package is standard so there is little chance that it will be abandoned. Mypy checks that type hints in source code are correct when they are not, it fails with an error.
Useful resources:
* https://docs.python.org/3.5/library/typing.html
* https://www.python.org/dev/peps/pep-0484/
Building with -Wunreachable-code-loop-increment causes a warning
due to always returning on the first iteration of the loop that
outputs errors on invalid args.
Collect all errors, and output them in a single error message
after the loop completes, resolving the warning and avoiding
popup hell by outputting a seperate message for each error.
Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in
the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up
in a deadlock.
`ClientModel::m_cached_tip_blocks` is protected by
`ClientModel::m_cached_tip_mutex`. There are two access paths that
lock the two mutexes in opposite order:
```
validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main
validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip()
ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost
...
qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex
```
and
```
qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex
qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash()
interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main
```
From `debug.log`:
```
POTENTIAL DEADLOCK DETECTED
Previous lock order was:
m_cs_chainstate validation.cpp:2851
(1) cs_main validation.cpp:2868
::mempool.cs validation.cpp:2868
(2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255
Current lock order is:
(2) m_cached_tip_mutex qt/clientmodel.cpp:119
(1) ::cs_main interfaces/node.cpp:200
```
The possible deadlock was introduced in
https://github.com/bitcoin/bitcoin/pull/17993
fac0ed16ec doc: Sync "how to upgrade" with 0.20.0 release notes (MarcoFalke)
fa861794d3 doc: Add release notes for 17219 (MarcoFalke)
Pull request description:
ACKs for top commit:
fanquake:
ACK fac0ed16ec - seems to match the relevant changes in 79606b56b2, 36c1d981d4 and 4c17c85c6c
Tree-SHA512: f04c2b3a1cd094d7f50e3f8db06d726873d2412d651c94bf38d17fd5ee0c47c84480d6b20ec18641888ee88012e7c59e7e794467a0eed1caf44c0569feecc4ca
facef3d413 doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a19 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2 doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0ca doc: Fix unrelated typos reported by codespell (MarcoFalke)
Pull request description:
Some random doc changes:
* Add sections to docs, so that they can be linked to
* Explain that anyone (even maintainers) are allowed to work on good first issues
* Expand section on Getting Started slightly
ACKs for top commit:
hebasto:
ACK facef3d413
fanquake:
ACK facef3d413
Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
f898ef65c9 tests: Add fuzzing harness for functions in script/sign.h (practicalswift)
c91d2f0615 tests: Add fuzzing harness for functions in script/sigcache.h (practicalswift)
d3d8adb79f tests: Add fuzzing harness for functions in script/interpreter.h (practicalswift)
fa80117cfd tests: Add fuzzing harness for functions in script/descriptor.h (practicalswift)
43fb8f0ca3 tests: Add fuzzing harness for functions in script/bitcoinconsensus.h (practicalswift)
8de72711c6 tests: Fill fuzzing coverage gaps for functions in script/script.h, script/script_error.h and script/standard.h (practicalswift)
c571ecb071 tests: Add fuzzing helper functions ConsumeDataStream, ConsumeTxDestination and ConsumeUInt160 (practicalswift)
Pull request description:
Add fuzzing harnesses for functions in `script/`:
* Add fuzzing helper functions `ConsumeDataStream` and `ConsumeUInt160`
* Fill fuzzing coverage gaps for functions in `script/script.h`, `script/script_error.h` and `script/standard.h`
* Add fuzzing harness for functions in `script/bitcoinconsensus.h`
* Add fuzzing harness for functions in `script/descriptor.h`
* Add fuzzing harness for functions in `script/interpreter.h`
* Add fuzzing harness for functions in `script/sigcache.h`
* Add fuzzing harness for functions in `script/sign.h`
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
ACK f898ef65c9🔉
Tree-SHA512: f6e77b34dc79f23de5fa9e38ac06e6554b5b946ec3e9a67e2bd982e60aca37ce844f785457ef427a5e3b45e31c305456bca8587cc9f4a0b50b3852e39726eb04
9e36067d8c [test] Add test for cfilters. (Jim Posen)
11106a4722 [net processing] Message handling for getcfilters. (Jim Posen)
e535670726 [indexes] Fix default [de]serialization of BlockFilter. (Jim Posen)
bb911ae7f5 [refactor] Pass CNode and CConnman by reference (John Newbery)
Pull request description:
Support `getcfilters` requests when `-peerblockfilters` is set.
Does not advertise compact filter support in version messages.
ACKs for top commit:
Empact:
re-Code Review ACK 9e36067d8c
MarcoFalke:
re-ACK 9e36067d8c , only change is adding commit "[refactor] Pass CNode and CConnman by reference" 🥑
jkczyz:
ACK 9e36067d8c
fjahr:
Code review ACK 9e36067d8c
Tree-SHA512: b45b42a25905ef0bd9e195029185300c86856c87f78cbe17921f4a25e159ae0f6f003e61714fa43779017eb97cd89d3568419be88e47d19dc8095562939e7887
9a19c9ada5 Always define the raii_event_tests test suite (Craig Andrews)
Pull request description:
The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test.
This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493
ACKs for top commit:
MarcoFalke:
ACK 9a19c9ada5🎹
Tree-SHA512: 3c42f17a9b5d56c8841f3aa9ac19da91c10aff210026266f31f7eb98a62528740d7c518c121452b68e8f801d6c80ecfb627d137ec6ed533289fa3beb08b4f176
60ed33904c tests: implement base58_decode (10xcryptodev)
Pull request description:
implements TODO: def base58_decode
ACKs for top commit:
ryanofsky:
Code review ACK 60ed33904c. Just suggested changes since last review. Thank you for taking suggestions!
Tree-SHA512: b3c06b4df041a6d88033cd077a093813a688e42d0b9aa777c715e5fd69cfba7b1bf984428bd98417d3c15232d3d48bc9c163317564f9e1d562db6611c21e2c10
9e1cb1adf1 [trivial/doc] Fix comment type (Amiti Uttarwar)
8f30260a67 [doc] Update unbroadcast description in RPC results (Amiti Uttarwar)
750456d6f2 [trivial] Remove misleading 'const' (Amiti Uttarwar)
fa32e676e5 [test] Manage node connections better in mempool persist test (Amiti Uttarwar)
1f94bb0c74 [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar)
9c8a55d9cb [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar)
ba54983182 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar)
00d44a534b [test] P2P connection behavior should meet expectations (Amiti Uttarwar)
bd093ca15d [test] updates to unbroadcast test (Amiti Uttarwar)
dab298d9ab [docs] add release notes (Amiti Uttarwar)
Pull request description:
This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions.
#18895 is another follow up to that addresses other functionality updates.
Background context:
The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool.
ACKs for top commit:
MarcoFalke:
ACK 9e1cb1adf1 👁
gzhao408:
ACK [`9e1cb1a`](9e1cb1adf1)
Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
