This moves the CBlockPolicyEstimator to the NodeContext, which get rids
of two globals and allows us to conditionally create the
CBlockPolicyEstimator (and to remove a circular dep).
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
1e62350ca20898189904a88dfef9ea11ddcd8626 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb854ca827a5a3925394f9e09d29b898 lint: Use c++17 std in cppcheck linter (Fabian Jahr)
Pull request description:
I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:
```
src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
```
After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.
In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.
ACKs for top commit:
practicalswift:
cr ACK 1e62350ca20898189904a88dfef9ea11ddcd8626: patch looks correct!
MarcoFalke:
review ACK 1e62350ca20898189904a88dfef9ea11ddcd8626
Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
db058efeb0821cb5022e3b29e0aff3627d7aaf83 sync: use HasReason() in double lock tests (Vasil Dimov)
a21dc469ccf076ca3b07b1adbd8bf667145f1c44 sync: const-qualify the argument of double_lock_detected() (Vasil Dimov)
6d3689fcf6cff397187028344570489db3e6ecf4 sync: print proper lock order location when double lock is detected (Vasil Dimov)
Pull request description:
Before:
```
Assertion failed: detected double lock at src/sync.cpp:153, details in debug log.
```
After:
```
Assertion failed: detected double lock for 'm' in src/test/sync_tests.cpp:40 (in thread ''), details in debug log.
```
ACKs for top commit:
jonasschnelli:
utACK db058efeb0821cb5022e3b29e0aff3627d7aaf83
ajtowns:
ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83
hebasto:
ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83, tested on Linux Mint 20 (x86_64).
Tree-SHA512: 452ddb9a14e44bb174135b39f2219c76eadbb8a6c0e80d64a25f995780d6dbc7b570d9902616db94dbfabaee197b5828ba3475171a68240ac0958fb203a7acdb
17a5f172fa9ec509b1c3f950ee8dfb6f025534d2 fuzz: Make addrman fuzzing harness deterministic (practicalswift)
Pull request description:
Make `CAddrMan` fuzzing harness deterministic.
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:
Crypt-iQ:
utACK 17a5f172fa9ec509b1c3f950ee8dfb6f025534d2
Tree-SHA512: 725f983745233e9b616782247fa18847e483c074ca4336a5beea8a9009128c3a74b4d50a12662d8ca2177c2e1fc5fc121834df6b459ac0af43c931d77ef7c4d8
f15e780b9e57554c723bc02aa41150ecf3e3a8c9 refactor: Clean up CTxMemPool initializer list (Elle Mouton)
e3310692d0e9720e960b9785274ce1f0b58b4cd7 refactor: Make CTxMemPool::m_check_ratio a const and a constructor argument (Elle Mouton)
9d4b4b2c2c49774523de740d6492ee5b1ee15e74 refactor: Avoid double to int cast for nCheckFrequency (Elle Mouton)
Pull request description:
This PR cleans up the CTxMemPool interface by including the ratio used to determine when a mempool sanity check should run in the constructor of CTxMempool instead of using nCheckFrequency which required a cast from a double to a uint32_t. Since nCheckFrequency (now called m_check_ratio) is set in the constructor and only every read from there after, it can be turned into a const and no longer needs to be guarded by the 'cs' lock.
Since nCheckFrequency/m_check_ratio no longer needs to lock the 'cs' mutux, mutex lock line in the "CTxMempool::check" function can be moved below where the m_check_ratio variable is checked. Since the variable is 0 by default (meaning that "CTxMempool::check" will most likely not run its logic) this saves us from unnecessarily grabbing the lock.
ACKs for top commit:
jnewbery:
utACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9
MarcoFalke:
ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9 👘
glozow:
utACK f15e780b9e
theStack:
Code Review ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9
Tree-SHA512: d83f3b5311ca128847b621e5e999c7e1bf0f4e6261d4cc090fb13e229a0f7eecd66ad997f654f50a838baf708d1515740aa3bffc244909a001d01fd5ae398b68
830ddf413934226d0b6ca99165916790cc52ca18 Drop noop gcc version checks (Hennadii Stepanov)
Pull request description:
Since #20413 the minimum required GCC version is 7.
ACKs for top commit:
fanquake:
ACK 830ddf413934226d0b6ca99165916790cc52ca18
Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964
95975dd08d8fdaaeaf28e0d06b861ce2748c17b6 sync: detect double lock from the same thread (Vasil Dimov)
4df6567e4cbb4677e8048de2f8008612e1b860b9 sync: make EnterCritical() & push_lock() type safe (Vasil Dimov)
Pull request description:
Double lock of the same (non-recursive) mutex from the same thread would produce an undefined behavior. Detect this from `DEBUG_LOCKORDER` and react similarly to the deadlock detection.
This came up during discussion in another, related PR: https://github.com/bitcoin/bitcoin/pull/19238#discussion_r442394521.
ACKs for top commit:
laanwj:
code review ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
hebasto:
re-ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
Tree-SHA512: 375c62db7819e348bfaecc3bd82a7907fcd8f5af24f7d637ac82f3f16789da9fc127dbd0e37158a08e0dcbba01a55c6635caf1d8e9e827cf5a3747f7690a498e
fabecce71909c984504c21fa05f91d5f1b471e8c net: Treat raw message bytes as uint8_t (MarcoFalke)
Pull request description:
Using `uint8_t` from the beginning when messages are `recv`ed has two style benefits:
* The signedness is clear from reading the code, as it does not depend on the architecture
* When passing the bytes on, the need for static signedness casts is dropped, making the code a bit less verbose and more coherent
ACKs for top commit:
laanwj:
Code review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
theStack:
Code Review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
jonatack:
Tested ACK fabecce71909c984504c21fa05f91d5f1b471e8c
Tree-SHA512: e6d9803c78633fde3304faf592afa961ff9462a7912d1da97a24720265274aa10ab4168d71b6ec2756b7448dd42585321afee0e5c889e705be778ce9a330d145
8f7b93047581c67f2133cdb8c7845471de66c30f Drop the leading 0 from the version number (Andrew Chow)
Pull request description:
Removes the leading 0 from the version number. The minor version, which we had been using as the major version, is now the major version. The revision, which we had been using as the minor version, is now the minor version. The revision number is dropped. The build number is promoted to being part of the version number. This also avoids issues where it was accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously, as the Major version was 0 so it never actually got included in it.
The user agent string formatter is updated to follow this new versioning.
***
Honestly I'm just tired of all of the people asking for "1.0" that maybe this'll shut them up. Skip the whole 1.0 thing and go straight to version 22.0!
Also, this means that the terminology we commonly use lines up with how the variables are named. So major versions are actually bumping the major version number, etc.
ACKs for top commit:
jnewbery:
Code review ACK 8f7b930475
MarcoFalke:
review ACK 8f7b93047581c67f2133cdb8c7845471de66c30f 🎻
Tree-SHA512: b5c3fae14d4c0a9c0ab3b1db7c949ecc0ac3537646306b13d98dd0efc17c489cdd16d43f0a24aaa28e9c4a92ea360500e05480a335b03f9fb308010cdd93a436
fa5ed3b4ca609426b2622cad235e107d33db7b30 net: Use Span in ReceiveMsgBytes (MarcoFalke)
Pull request description:
Pass a data pointer and a size as span in `ReceiveMsgBytes` to get the benefits of a span
ACKs for top commit:
jonatack:
ACK fa5ed3b4ca609426b2622cad235e107d33db7b30 code review, rebased to current master 12a1c3ad1a43634, debug build, unit tests, ran bitcoind/-netinfo/getpeerinfo
theStack:
ACK fa5ed3b4ca609426b2622cad235e107d33db7b30
Tree-SHA512: 89bf111323148d6e6e50185ad20ab39f73ab3a58a27e46319e3a08bcf5dcf9d6aa84faff0fd6afb90cb892ac2f557a237c144560986063bc736a69ace353ab9d
ecc6cf1a3b097b9b5b047282063a0b6779631b83 test: fix creation of std::string objects with \0s (Vasil Dimov)
Pull request description:
A string literal `"abc"` contains a terminating `\0`, so that is 4
bytes. There is no need to write `"abc\0"` unless two terminating
`\0`s are necessary.
`std::string` objects do not internally contain a terminating `\0`, so
`std::string("abc")` creates a string with size 3 and is the same as
`std::string("abc", 3)`.
In `"\01"` the `01` part is interpreted as one number (1) and that is
the same as `"\1"` which is a string like `{1, 0}` whereas `"\0z"` is a
string like `{0, 'z', 0}`. To create a string like `{0, '1', 0}` one
must use `"\0" "1"`.
Adjust the tests accordingly.
ACKs for top commit:
laanwj:
ACK ecc6cf1a3b097b9b5b047282063a0b6779631b83
practicalswift:
ACK ecc6cf1a3b097b9b5b047282063a0b6779631b83 modulo happily green CI
Tree-SHA512: 5eb489e8533a4199a9324b92f7280041552379731ebf7dfee169f70d5458e20e29b36f8bfaee6f201f48ab2b9d1d0fc4bdf8d6e4c58d6102f399cfbea54a219e
Removes the leading 0 from the version number. The minor version, which
we had been using as the major version, is now the major version. The
revision, which we had been using as the minor version, is now the minor
version. The revision number is dropped. The build number is promoted to
being part of the version number. This also avoids issues where it was
accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously,
the Major version was 0 so that was never a factor in CLIENT_VERSION.
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack)
Pull request description:
This PR builds on #11413 and #20220 to address #19543.
- replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs
- allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument
- fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values
- update the feerate error message units for these RPCs from BTC/kB to sat/vB
- update the test coverage, help docs, doxygen docs, and some of the RPC examples
- other changes to address the excellent review feedback
See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309
ACKs for top commit:
achow101:
re-ACK 05e82d8
MarcoFalke:
review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
Xekyo:
tACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Sjors:
utACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
89836a82eec63f93bbe6c3bd6a52be26e71ab54d style: minor improvements as a followup to #19845 (Vasil Dimov)
Pull request description:
Address suggestions:
https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495486760https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495488051https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495730125
ACKs for top commit:
jonatack:
re-ACK 89836a8 change since previous review is replacing std::runtime_error with std::exception, built/ran unit tests with gcc debian 10.2.0-15, then broke a few v3 net_tests involving `BOOST_CHECK_EXCEPTION`, rebuilt, ran `src/test/test_bitcoin -t net_tests -l all` and checked the error reporting.
hebasto:
re-ACK 89836a82eec63f93bbe6c3bd6a52be26e71ab54d
theStack:
ACK 89836a82eec63f93bbe6c3bd6a52be26e71ab54d
Tree-SHA512: 36477fdccabe5a8ad91fbabb4655cc363a3a7ca237a98ae6dd4a9fae4a4113762040f864d4ca13a47d081f7d16e5bd487edbfb61ab50a37e4a0424e9bec30b24
79ef8324d4c85ed16a304e98805724b8a59022ac tests: Add fuzzing harness for CConnman (practicalswift)
Pull request description:
Add fuzzing harness for `CConnman`.
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:
review ACK 79ef8324d4c85ed16a304e98805724b8a59022ac
Tree-SHA512: eb9ffae20e939b818f8b9def064544b9a8fcd127ca22d1a54af1afedf1d24143be42419f3a03d684be59a5ff07b29d8bfa34ef2aaf1d9f9f75c4c1aaa90a29a8
fa4234d877ea3193bfd0e18ff68dcb8fb84b47b5 test: Mock IBD in net_processing fuzzers (MarcoFalke)
Pull request description:
Without this the fuzzers fail to detect trivial crasher bugs, such as https://github.com/bitcoin/bitcoin/pull/20317#issuecomment-723047111
ACKs for top commit:
practicalswift:
Tested ACK fa4234d877ea3193bfd0e18ff68dcb8fb84b47b5
Tree-SHA512: ce5da5c0a604b7559805a98ffdde882b44ca4f91b003b493d6e1be230714ce4cccb11dbfc1fc175f9d8fc779551c0a4103ceb4b473552928207d7d78ae329e10
d7901ab8d2fdd2f6e68c4fa48078111bf5f0fa73 fuzz: Assert expected DecodeHexTx behaviour when using legacy decoding (practicalswift)
Pull request description:
Assert expected `DecodeHexTx` behaviour when using legacy decoding.
As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/20290#issuecomment-720989597.
ACKs for top commit:
MarcoFalke:
review ACK d7901ab8d2fdd2f6e68c4fa48078111bf5f0fa73
Tree-SHA512: 3285680059e6fa73b0fb2c52b775f6319de1ac616f731206662b742764dc888cdfd1ac1f1fcfdfd5418d2006475a852d1c1a56a7035f772f0a6b2a84f5de93bc
5cafe2b25c1d2f6825b3c8103c280020929dd645 fuzz: Add missing ECC_Start to descriptor_parse test (Ivan Metlushko)
Pull request description:
Fixes fuzzing harness.
I also observed that the corpus for this test consists only of `xprv...` keys while we are using regtest parameters. So for proper fuzzing we need either A) to update the corpus and replace `xprv...` with `tprv...` B) switch to main net in the test
ACKs for top commit:
MarcoFalke:
review ACK 5cafe2b25c1d2f6825b3c8103c280020929dd645
practicalswift:
Tested ACK 5cafe2b25c1d2f6825b3c8103c280020929dd645
Tree-SHA512: 7415a98a445ce0f96219637d2362fecfc1191ad104f55d79ca92b0c92cde165e00646be5bf3fda956385e3cb22540eca457e575048493367cdf0e00a27d7cdb8
fa3967efdb07f1d22372f4ee2e602ea1fad04a57 test: Replace ARRAYLEN with C++11 ranged for loop (MarcoFalke)
fafc5290538fde76c3780976f4b2c11dc9f24d19 test: Run AssetTest even if built --with-libs=no (MarcoFalke)
faf58ab139949ca35b33217d010b350c9a59c61d ci: Add --with-libs=no to one ci config (MarcoFalke)
Pull request description:
`script_assets_test` doesn't call libbitcoinconsensus, so it seems confusing to require it
ACKs for top commit:
fanquake:
ACK fa3967efdb07f1d22372f4ee2e602ea1fad04a57 - looks ok to me.
Tree-SHA512: 8744fef64c5d7dc19a0ef4ae9b3df5b3f356253bf000f12723064794c5e50df071824d436059985f1112d94c1b530b65cbeb6b8435114a91b195480620eafc59
c2cf8a18c25bf19ade51fedfa5c352bd7145edb0 fuzz: Check for addrv1 compatibility before using addrv1 serializer on CService (practicalswift)
Pull request description:
Check for addrv1 compatibility before using addrv1 serializer/deserializer on `CService`:
Before this patch:
```
$ src/test/fuzz/service_deserialize
service_deserialize: test/fuzz/deserialize.cpp:85:
void (anonymous namespace)::AssertEqualAfterSerializeDeserialize(const T &, const int) [T = CService]:
Assertion `Deserialize<T>(Serialize(obj, version)) == obj' failed.
```
After this patch:
```
$ src/test/fuzz/service_deserialize
…
```
Related change: #20247
ACKs for top commit:
MarcoFalke:
review ACK c2cf8a18c25bf19ade51fedfa5c352bd7145edb0
Tree-SHA512: dba6ddc60e8ef621011d844281461f1741de08c4af1a2b7156c810af44306cef7ec582de5974752db02ca085cfd23da0296d70b694e59ee262589d829fa0626e
16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751 Avoid test-before-evict evictions of current peers (Suhas Daftuar)
e8b215a086d91a8774210bb6ce8d1560aaaf0789 Refactor test for existing peer connection into own function (Suhas Daftuar)
4fe338ab3ed73b3ffb20eedf95500c56ec2920e1 Call CAddrMan::Good() on block-relay-only peer addresses (Suhas Daftuar)
daf55531260833d597ee599e2d289ea1be0b1d9c Avoid calling CAddrMan::Connected() on block-relay-only peer addresses (Suhas Daftuar)
Pull request description:
This PR does two things:
* Block-relay-only interaction with addrman.
* Calling `CAddrMan::Connected()` on an address that was a block-relay-only peer causes the time we report in `addr` messages containing that peer to be updated; particularly now that we use anchor connections with a our block-relay-only peers, this risks leaking information about those peers. So, stop this.
* Avoiding calling `CAddrMan::Good()` on block-relay-only peer addresses causes the addrman logic around maintaining the new and tried table to be less good, and in particular makes it so that block-relay-only peer addresses are more likely to be evicted from the addrman (for no good reason I can think of). So, mark those addresses as good when we connect.
* Fix test-before-evict bug. There's a bug where if we get a collision in the tried table with an existing address that is one of our current peers, and the connection is long-lived enough, then `SelectTriedCollisions()` might return that existing peer address to us as a test-before-evict connection candidate. However, our logic for new outbound connections would later prevent us from actually making a connection; the result would be that when we get a collision with a long-lived current peer, that peer's address is likely to get evicted from the tried table. Fix this by checking to see if a test-before-evict candidate is a peer we're currently connected to, and if so, mark it as `Good()`.
ACKs for top commit:
sipa:
utACK 16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751
amitiuttarwar:
code review ACK 16d9bfc417
mzumsande:
Code-Review ACK 16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751.
jnewbery:
utACK 16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751
ariard:
Code Review ACK 16d9bfc.
jonatack:
Tested ACK 16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751
Tree-SHA512: 188ccb814e436937cbb91d29d73c316ce83f4b9c22f1cda56747f0949a093e10161ae724e87e4a2d85ac40f85f5f6b4e87e97d350a1ac44f80c57783f4423324
3d0556d41087f945ed0a47a5d770076ad42ce432 Increase feature_taproot inactive test coverage (Pieter Wuille)
525cbd425e2f6a1dbd0febc53d7ada22cec4661f Only relay Taproot spends if next block has it active (Pieter Wuille)
Pull request description:
There should be no change to mempool transaction behavior for witness v1 transactions as long as no activation is defined. Until that point, we should treat the consensus rules as under debate, and for soft-fork safety, that means spends should be treated as non-standard.
It's possible to go further: don't relay them unless the consensus rules are actually active for the next block. This extends non-relay to the period where a deployment is defined, started, locked in, or failed. I see no downsides to this, and the code change is very simple.
ACKs for top commit:
Sjors:
utACK 3d0556d41087f945ed0a47a5d770076ad42ce432
MarcoFalke:
review ACK 3d0556d41087f945ed0a47a5d770076ad42ce432 🏓
jnewbery:
utACK 3d0556d41087f945ed0a47a5d770076ad42ce432
Tree-SHA512: ca625a2981716b4b44e8f3722718fd25fd04e25bf3ca1684924b8974fca49f7c1d438fdd9dcdfbc091a442002e20d441d42c41a0e2096e74a61068da6c60267a
ad5cef5dfdd5802fc187a52e74d940a52f420a51 doc: Update data directory path comments (Hennadii Stepanov)
b19e88230f0e93e95e883e65376963cb9c36f606 util: Add StripRedundantLastElementsOfPath function (Hennadii Stepanov)
Pull request description:
Wallet names in `listwalletdir` RPC are correct now, even if the `-datadir` path has any number of trailing `/`.
This PR is an alternative to #19933.
Fixes#19928.
ACKs for top commit:
MarcoFalke:
review ACK ad5cef5dfd 🔙
promag:
Code review ACK ad5cef5dfdd5802fc187a52e74d940a52f420a51.
meshcollider:
Code review + test run ACK ad5cef5dfdd5802fc187a52e74d940a52f420a51
Tree-SHA512: bccabbd6c18243d48d15b2b27201cc0f5984623dcbc635c8740cf74523f359844c36eadd40391142874fcf452a43880bb6afbf89815ae736e499f9a98143a661
A string literal `"abc"` contains a terminating `\0`, so that is 4
bytes. There is no need to write `"abc\0"` unless two terminating
`\0`s are necessary.
`std::string` objects do not internally contain a terminating `\0`, so
`std::string("abc")` creates a string with size 3 and is the same as
`std::string("abc", 3)`.
In `"\01"` the `01` part is interpreted as one number (1) and that is
the same as `"\1"` which is a string like `{1, 0}` whereas `"\0z"` is a
string like `{0, 'z', 0}`. To create a string like `{0, '1', 0}` one
must use `"\0" "1"`.
Adjust the tests accordingly.
