5fabde6fad wallet: AddWalletDescriptor requires cs_wallet lock (João Barbosa)
32d036e8da wallet: GetLabelAddresses requires cs_wallet lock (João Barbosa)
Pull request description:
This is another small change towards non recursive wallet lock.
ACKs for top commit:
hebasto:
ACK 5fabde6fad, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 00506f0159c56854a171e58a451db8dd9b9f735039697b1cf2ca7f54de61fb51cc1e5eff42265233e041b4b1bfd29c2247496dc4456578e1a23c323bdec2901b
d319c4dae9 qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addView (Hennadii Stepanov)
92ddc02a16 qt, refactor: Declare getWalletModel with const and noexcept qualifiers (Hennadii Stepanov)
ca0e680bdc qt, refactor: Drop redundant checks of walletModel (Hennadii Stepanov)
404373bc6a qt, refactor: Pass WalletModel object to WalletView constructor (Hennadii Stepanov)
Pull request description:
An instance of the `WalletView` class without the `walletModel` data member being set is invalid. So, it is better to set it in the constructor.
Establishing one more `WalletView` class's invariant in constructor:
- allows to drop all of checks of the`walletModel` in member functions
- makes reasoning about the code that uses instances of the `WalletView` class easier
Possible follow ups could extend this approach to other classes, e.g., `OverviewPage`, `TransactionView`, `ReceiveCoinsDialog`, `SendCoinsDialog`, `AddressBookPage`.
ACKs for top commit:
ShaMan239:
Code review ACK d319c4dae9
promag:
Code review ACK d319c4dae9.
jarolrod:
ACK d319c4dae9
Tree-SHA512: b0c61f82811bb5aba2738067b53dc9ea4439230d547ce5c8fd85c480d8d70ea15f9942dbf13842383acbce467fba1ab4e132e37c56b654b46ba897301a41066e
2445df4eb3 build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp (Hennadii Stepanov)
Pull request description:
On master (7a49fdc581) the `configure` script does not pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple Silicon:
```
% ./configure --with-miniupnpc
...
checking for miniupnpc/miniupnpc.h... no
checking for miniupnpc/upnpcommands.h... no
checking for miniupnpc/upnperrors.h... no
...
checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc."
```
```
% ./configure --with-natpmp
...
checking for natpmp.h... no
...
checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp
```
The preferred Homebrew [prefix for Apple Silicon](https://docs.brew.sh/Installation) is `/opt/homebrew`. Therefore, if we do not use `pkg-config` to detect packages, we should set the `CPPFLAGS` and `LDFLAGS` variables for them explicitly.
ACKs for top commit:
Zero-1729:
re-tACK 2445df4eb3 (re-tested on an M1 Machine running macOS 11.4).
jarolrod:
re-ACK 2445df4eb3
Tree-SHA512: d623d26492f463812bf66ca519847ff4b23d517466b6c51c3caf3642a582d02e5f03ce57915742b29f01bf9bceb731a3978ef9a5fdc82e568bcb62548eda758a
77f37f58ad doc: update enum naming in developer notes (Jon Atack)
Pull request description:
Per our current doc, the general rule is we follow the C++ Core Guidelines, which for enumerator naming stipulate:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps
```
Don’t use ALL_CAPS for enumerators
Reason: Avoid clashes with macros.
```
but our examples (and often, codebase) are contradictory to it (and perhaps to common usage), which can be confusing when a contributor needs to choose a style to use. This patch:
- updates the enumerator examples to snake_case per CPP Core Guidelines
- clarifies for contributors that in this project enumerators may be snake_case, PascalCase or ALL_CAPS, and to use what seems appropriate.
ACKs for top commit:
practicalswift:
cr ACK 77f37f58ad
ryanofsky:
ACK 77f37f58ad. I think this is good because it modernizes the example and clarifies current conventions.
promag:
ACK 77f37f58ad.
Tree-SHA512: 7facc607fe5e1abab0f635864340143f13c2e4bb074eb17eac7d829dcd0cf244c5c617fc49d35e8774e8af1fa1205eeebe0cca81f538a2a61f6a7ba200878bc6
724c497562 [fuzz] Add ConsumeAsmap() function (John Newbery)
5840476714 [addrman] Make m_asmap private (John Newbery)
f9002cb5db [net] Rename the copyStats arg from m_asmap to asmap (John Newbery)
f572f2b204 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery)
593247872d [net] Remove CConnMan::SetAsmap() (John Newbery)
50fd77045e [init] Read/decode asmap before constructing addrman (John Newbery)
Pull request description:
Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat.
The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer:
- don't reach into `CAddrMan`'s internal data from `CConnMan`
- set `m_asmap` in the initializer list and make it const
- make `m_asmap` private, and access it (as a reference to const) from a getter.
This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction.
ACKs for top commit:
mzumsande:
Tested ACK 724c497562
amitiuttarwar:
code review but utACK 724c497562
naumenkogs:
utACK 724c497562
vasild:
ACK 724c497562
MarcoFalke:
review ACK 724c497562👫
Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
6919c823cb MOVEONLY: Expose BanMapToJson / BanMapFromJson (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
CSubNet serialization code that was removed in #22570fa4e6afdae was needed by multiprocess code to share ban map between gui and node processes.
Rather than adding it back, use suggestion from MarcoFalke https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to use JSON serialization. This requires making BanMapToJson / BanMapFromJson functions public.
ACKs for top commit:
promag:
reACK 6919c823cb.
Tree-SHA512: ce909a61b7869d16cf2e9f91b643dd9d2604efc5777703d3b77a4c40cb0ccdd20396ba87b1ec85aade142e12ff9ea4c95c7155840354873579565471779f5a33
7e69873283 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack)
9b08006bc5 log, sync: improve lock contention logging and add time duration (Jon Atack)
3f4c6b87f1 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack)
b7a17444e0 log, sync: add LOCK logging category, apply it to lock contention (Jon Atack)
Pull request description:
To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used.
This patch:
- adds a `lock` logging category
- adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY`
- updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math
- improves the lock contention logging, drops the all-caps, and displays the duration in microseconds
- removes the conditional compilation directives
- allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'`
```
$ bitcoind -signet -debug=lock
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs)
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs)
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs)
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs)
$ bitcoin-cli -signet logging
"lock": true,
$ bitcoin-cli -signet logging [] '["lock"]'
"lock": false,
$ bitcoin-cli -signet logging '["lock"]'
"lock": true,
```
I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`.
ACKs for top commit:
hebasto:
re-ACK 7e69873283, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review.
theStack:
re-ACK 7e69873283🔏⏲️
Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
fa0a5fa744 ci: Fuzz with -ftrivial-auto-var-init=pattern (MarcoFalke)
Pull request description:
This makes memory bugs deterministic. `-ftrivial-auto-var-init=pattern` is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them.
`-ftrivial-auto-var-init=pattern` goes well with `-fsanitize=bool` and `-fsanitize=enum`, but those are already enabled via `-fsanitize=undefined`. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks
ACKs for top commit:
practicalswift:
cr ACK fa0a5fa744
Tree-SHA512: ed6be953cd99eadb1ba245ba30170747eff66be54d2773c8d26a3a6aee0fdcd6967c596f4f4ab1d238de6a6526623dac5211f0ba77f1986639395d7921bdc19f
e952d7557e netinfo: clarify client and server versions in header (Jon Atack)
Pull request description:
Clarify in -netinfo output that both the client and the server versions are provided.
before
```
Bitcoin Core v22.0.0rc3 - 70016/Satoshi:22.99.0/
```
after
```
Bitcoin Core client v22.0.0rc3 - server 70016/Satoshi:22.99.0/
```
Closes#22873.
ACKs for top commit:
benthecarman:
utACK e952d7557e
prayank23:
ACK e952d7557e
Zero-1729:
tACK e952d7557e
Tree-SHA512: 3e817892d398aabacb1401fd5b1816c4d4f563b4f8cf1096bdb8b53f7c4ef82d4caee09f5c7724f1fe292f837434a332acefba735152ed24a238bb6f006df909
d1267fdbb0 build: Update default platform toolset in msvc-autogen.py (Hennadii Stepanov)
Pull request description:
The platform toolset was updated from v141 to v142 in #17364.
ACKs for top commit:
sipsorcery:
ACK d1267fdbb0.
Tree-SHA512: 390dc4876948af7f6b9f26eb1e64262f6386c2541a4647a6cb7abd8f1283c63ffbf5efba5e67a8075f0085a0557fc3f1b41e70f3ede0e8cef60d0fe2d6bf3be8
2b071265c3 error if settings.json exists, but is unreadable (Tyler Chambers)
Pull request description:
If settings.json exists, but is unreadable, we should error instead of overwriting.
Fixes#22571
ACKs for top commit:
Zero-1729:
tACK 2b071265c3
ShaMan239:
tACK 2b071265c3
prayank23:
tACK 2b071265c3
ryanofsky:
Code review ACK 2b071265c3. Thanks for the fix! Note that PR https://github.com/bitcoin-core/gui/pull/379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient)
theStack:
ACK 2b071265c3📁
Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511
696c76d660 tests: Add TrimString(...) tests (practicalswift)
4bf18b089e Replace use of boost::trim_right with locale-independent TrimString (Ben Woosley)
93551862a1 Replace use of boost::trim use with locale-independent TrimString (Ben Woosley)
Pull request description:
This is [#18130 rebased](https://github.com/bitcoin/bitcoin/pull/18130#issuecomment-900158759).
> `TrimString` is an existing alternative.
> Note `TrimString` uses `" \f\n\r\t\v"` as the pattern, which is consistent with the default behavior of `std::isspace`. See: https://en.cppreference.com/w/cpp/string/byte/isspace
ACKs for top commit:
jb55:
utACK 696c76d660
practicalswift:
ACK 696c76d660
jonatack:
ACK 696c76d660
theStack:
Code-review ACK 696c76d660
Tree-SHA512: 6a70e3777602dfa65a60353e5c6874eb951e4a806844cd4bdaa4237cad980a4f61ec205defc05a29f9707776835975838f6cc635259c42adfe37ceb02ba9358d
fa7e6c56f5 Add LIFETIMEBOUND to InitializeChainstate (MarcoFalke)
fa5c896724 Add LIFETIMEBOUND to CScript where needed (MarcoFalke)
Pull request description:
Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive.
Use `LIFETIMEBOUND` to turn this error into a compile warning.
See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound
Example:
```cpp
const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)};
```
Before: (no warning)
After:
```
warning: returning reference to local temporary object [-Wreturn-stack-address]
const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)};
^~~~~~~~~
./sync.h:276:65: note: expanded from macro 'WITH_LOCK'
#define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }()
^~~~
ACKs for top commit:
theuni:
utACK fa7e6c56f5.
jonatack:
Light ACK fa7e6c56f5 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at `clang::lifetimebound` support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs
Tree-SHA512: e915acdc4532445205b7703fab61a5d682231ace78ecfb274cb8523ca2bddefd85828f50ac047cfb1afaff92a331f5f7b5a1472539f999e30f7cf8ac8c3222f3
fa1b08eb14 test: Always clear reject reason in IsStandard tx test (MarcoFalke)
Pull request description:
For some tests the reject reason wasn't cleared between runs and thus subsequent tests might (theoretically) fail to verify the correct reject reason.
ACKs for top commit:
benthecarman:
ACK fa1b08eb14
theStack:
Code-review ACK fa1b08eb14
Tree-SHA512: fcb727a690f92a4cf06127c302ba464f1e8cb997498e4f7fd9e210d193559b07e6efdb9d5c8a0bef3fe643bdfd5fedd431aaace20978dd49e56b8e770cb9f930
3633b667ff Use SelectCoinsSRD if it has less waste (Andrew Chow)
8bf789b4b4 Add SelectCoinsSRD function (Andrew Chow)
2ad3b5d2ad tests: wallet_basic lock needed unspents (Andrew Chow)
b77885f13e tests: wallet_txn explicilty specify inputs (Andrew Chow)
59ba7d2861 tests: rpc_fundrawtx better test for UTXO inclusion with include_unsafe (Andrew Chow)
a165bfbe44 tests: rpc_fundrawtx use specific inputs for unavailable change test (Andrew Chow)
df765a484d tests: rpc_fundrawtx lock to UTXO types (Andrew Chow)
Pull request description:
To ease in the use of SRD as our fallback mechanism, this PR adds it as a secondary fallback algorithm in addition to the knapsack solver. Since #22009, the solution with the least waste will be chosen. This pattern is continued with SRD simply being another solution whose waste is compared.
ACKs for top commit:
glozow:
reACK 3633b66 via `git range-diff 981b9d1...3633b66`, thanks for taking the suggestions
laanwj:
Concept and code review ACK 3633b667ff
Tree-SHA512: 895659f553fea2230990136565bdf18b1328de8b0ce47f06b64bb4d69301f6dd68cb38debe5c24fb6de1317b735fc020a987c541f00bbea65229de47e53adf92
4befc8f1c0 ci: Enable feature_asmap.py test on native Windows (Hennadii Stepanov)
Pull request description:
This PR is a [follow up](https://github.com/bitcoin/bitcoin/pull/23084#issuecomment-927077970) of #23084. It enables the `feature_asmap.py` functional test which is fixed in #23084.
ACKs for top commit:
MarcoFalke:
cr ACK 4befc8f1c0
Tree-SHA512: bdd813f0a360b7acc4be764bb402de91be519282c107ad1952f5dccd1171e39cca0e4ce11b6257de1135ddde8deb8465d1dccf29450a1cd16f6894ed86c67cb8
93b9800fec scripted-diff: Rename overloaded int GetArg to GetIntArg (Russell Yanofsky)
Pull request description:
This is meant to improve readability of code and remove guesswork needed to determine argument types and migrate to [typed arguments (#22978)](https://github.com/bitcoin/bitcoin/issues/22978) by having distinctly named `GetArg` `GetArgs` `GetBoolArg` and `GetIntArg` methods.
---
This commit was originally part of #22766 and had some review discussion there. But it was [wisely suggested](https://github.com/bitcoin/bitcoin/pull/22766#issuecomment-910001542) to be split off to make that PR smaller.
ACKs for top commit:
hebasto:
ACK 93b9800fec.
MarcoFalke:
re-ACK 93b9800fec📨
Tree-SHA512: e034bd938b2c8fbadd90bcd52213a61161965dfddf18c2cb0d68816ecf2438cde8afee6fb7e3418f5c6b35c208338da9deb99e4e418dbf68ca34552e0916a625
dc10ca346b net: switch to signet DNS seed (Sjors Provoost)
Pull request description:
I spun up a DNS seed for Signet, source: https://github.com/sipa/bitcoin-seeder/pull/94
If anyone else spins up a DNS seed, let me know in the comment and I'll add it.
Because one DNS seed is not very diverse, this PR leaves two hardcoded nodes just in case (). The one dropped node no longer exists.
Replaces #23000.
ACKs for top commit:
kallewoof:
utACK dc10ca346b
laanwj:
Concept and code review ACK dc10ca346b
jarolrod:
ACK dc10ca346b
shaavan:
ACK dc10ca346b
Tree-SHA512: 534d189becd51974042fddc3efe3df230484f05cd945e756eaf6a4a8a580e2161b3a959eb3d44dea526269eea6f87a033f7cbfe86586782c6ca4ee7c7c4097a9
b8cd2a4292 Add references for the generator/constant used in Bech32(m) (Pieter Wuille)
Pull request description:
I often find myself recreating this, or looking up references for this construction. So instead, this seems like as good a place as any to place a summary.
ACKs for top commit:
Zero-1729:
crACK b8cd2a4292
Tree-SHA512: 9d2001c5016485cea441c28fda093d67a7d4274e4c1e4dd3d357353ce6a52987e38d684d8462bad2d72ba0b6b1db2f809948e228fb02371e64b12146aace89bd
d2eccacd18 doc: Clarify that change_cost cannot be negative in GetSelectionWaste (benthecarman)
Pull request description:
We assert that the `change_cost` must be positive so we should document it in the function's docs
4f5ad43b1e/src/wallet/coinselection.cpp (L361)
ACKs for top commit:
jonatack:
ACK d2eccacd18
jarolrod:
ACK d2eccacd18
shaavan:
ACK d2eccacd18
Tree-SHA512: 089bc41d7e212b811455527ce2ac83301eae0edc03933dcbd5229b22078827395c8e3bcfda0ba029b7c60705db3119b4f80e372842a6bd8dae195dbaa0b52833
fa01f22e6e test: Add missing re.escape() to feature_addrman test (MarcoFalke)
Pull request description:
Needed to run the test on windows
ACKs for top commit:
laanwj:
Code review ACK fa01f22e6e
hebasto:
ACK fa01f22e6e, passed 2 consequential runs in my [personal CI](https://cirrus-ci.com/task/6522304080379904).
Tree-SHA512: d7ca4fb882cc6693989ddf6fc092db3259a0619cb8f87293c588484b9c62e6755e9fb1bb2c1ab85fcc8f0349d9bc155ba515e16674c0f6f56236e7fbb14655a8
Improve readability of code, simplify future scripted diff cleanup PRs, and be
more consistent with naming for GetBoolArg.
This will also be useful for replacing runtime settings type checking
with compile time checking.
-BEGIN VERIFY SCRIPT-
git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g'
-END VERIFY SCRIPT-
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
ee7891a0c4 doc: Remove outdated comments (Hennadii Stepanov)
Pull request description:
The first removed comment was introduced in #5288, the second one in #13503.
Both are outdated since #14336.
ACKs for top commit:
duncandean:
crACK ee7891a0
Tree-SHA512: a2d6071919e81c916bfc2178109bbc464417321bcc567ed0644448c5faea8e58cb08a7657afa1b6ffe1fb63e114a2a47b31c893e471839ba9d49a3986e68b2a7
faa9c19a4b doc: Add 23061 release notes (MarcoFalke)
faff17bbde Fix (inverse) meaning of -persistmempool (MarcoFalke)
Pull request description:
Passing `-persistmempool` is currently treated as `-nopersistmempool`
ACKs for top commit:
jnewbery:
reACK faa9c19a4b
hebasto:
ACK faa9c19a4b, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: f34a89a07745dabe340eb845b2a348b79c093e9056f7a21c17e1ba2e278177c9b4cf30e8095791fd645a7f90eb34850b2eee0c869b4f6ec02bf749c73b0e52ee
fad02274ba test: Remove Windows workaround in authproxy (MarcoFalke)
Pull request description:
Might no longer be needed after https://github.com/bitcoin/bitcoin/pull/23089
ACKs for top commit:
hebasto:
ACK fad02274ba, passed 3 consequential runs on my [personal CI](https://cirrus-ci.com/task/4897456077930496):
Tree-SHA512: 3c408e068ad7590dc0e5922c0b4cd3bf927e22fe624b13b8ab38db848342d310c9c934f358638fd5234c7b5f10f95d3bddc676deb925dcbba54945e2e8229275
5825b34783 test: avoid non-determinism in asmap-addrman test (Jon Atack)
Pull request description:
This is the same approach as for the addpeeraddress test in `test/functional/rpc_net.py` in commit 869f1368.
The probability of collision when adding an addrman entry is expected to be 1/2^16 = 1/65536 for an address from a different /16. This change hopes to avoid these collisions by adding 1 tried entry before adding 1 new table one, instead of 2 tried entries followed by 2 new entries, which appears to have caused a collision in the CI.
To verify the regression test still fails when expected:
- `git checkout 181a120 && git cherry-pick ef242f5`
- recompile bitcoind
- git checkout this branch and run `test/functional/feature_asmap.py`. Expected output:
```
AssertionError: Unexpected stderr bitcoind: ./addrman.h:739: void CAddrMan::Check() const: Assertion `false' failed. !=
```
Closes#23078.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
ACKs for top commit:
mzumsande:
Re-ACK 5825b34783
Tree-SHA512: eb6624a196fa5617c84aad860c8f91e8a8f60fc9fe2d7168facc298ee38db5e93b7e988e11c2ac1b399dc2c6b8fad360fb10bdbf10e598a1878300388475a200
This is the same approach as for the addpeeraddress test in
`test/functional/rpc_net.py` in commit 869f1368.
The probability of collision when adding an addrman entry is
expected to be 1/2^16 = 1/65536 for an address from a different /16.
This change hopes to avoid these collisions by adding 1 tried entry
before adding 1 new table one, instead of 2 tried entries followed
by 2 new entries, which appears to have caused a collision in the CI.
To verify the regression test stills fails when expected:
- git checkout 181a120 && git cherry-pick ef242f5
- recompile bitcoind
- git checkout this branch and run test/functional/feature_asmap.py. Expected output:
```
AssertionError: Unexpected stderr bitcoind: ./addrman.h:739: void CAddrMan::Check() const: Assertion `false' failed. !=
```
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
Try to find a solution with SelectCoinsSRD. If we do have one, add it to
the list of solutions from which we choose the one with the least waste
as the solution to use.
