9c5775c331e02dab06c78ecbb58488542d16dda7 addrman: cap the `max_pct` to not exceed the maximum number of addresses (brunoerg)
Pull request description:
Fixes#31234
This PR fixes a bad alloc issue in `GetAddresses` by capping the value `max_pct`. In practice, values greater than 100 should be treated as 100 since it's the percentage of addresses to return. Also, it limites the value `max_pct` in connman target to exercise values between 0 and 100.
ACKs for top commit:
adamandrews1:
Code Review ACK 9c5775c331
marcofleon:
Tested ACK 9c5775c331e02dab06c78ecbb58488542d16dda7. Reproduced the crash on master and checked that this fixed it. The checks added to `GetAddr_` look reasonable.
mzumsande:
Code Review ACK 9c5775c331e02dab06c78ecbb58488542d16dda7
vasild:
ACK 9c5775c331e02dab06c78ecbb58488542d16dda7
Tree-SHA512: 2957ae561ccc37df71f43c1863216d2e563522ea70b9a4baee6990e0b4a1ddadccabdcb9115c131a9a57480367b5ebdd03e0e3d4c8583792e2b7d1911a0a06d3
5c2e291060cca3be500f3af0f6f2d3fd2177a7c9 bench: Add basic CheckEphemeralSpends benchmark (Greg Sanders)
3f6559fa581b1f78cd9a9ef4dc0169e315ffa6b3 Add release note for ephemeral dust (Greg Sanders)
71a6ab4b33df383642cca49397a88b1606171225 test: unit test for CheckEphemeralSpends (Greg Sanders)
21d28b2f362708dd9206feb9ddc11a352063ef0c fuzz: add ephemeral_package_eval harness (Greg Sanders)
127719f516a6a8bbfb65f09827bbe22190df3a58 test: Add CheckMempoolEphemeralInvariants (Greg Sanders)
e2e30e89ba4b9bdbcabaf5b4346610922f0728bb functional test: Add ephemeral dust tests (Greg Sanders)
4e68f901390d512a9dfaf0de34daf822449e9bd2 rpc: disallow in-mempool prioritisation of dusty tx (Greg Sanders)
e1d3e81ab4d34485f1b82cb9c3b967e92a4e1f15 policy: Allow dust in transactions, spent in-mempool (Greg Sanders)
04b2714fbbc4a019d23743a488b9f9b42652617b functional test: Add new -dustrelayfee=0 test case (Greg Sanders)
Pull request description:
A replacement for https://github.com/bitcoin/bitcoin/pull/29001
Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type, and simple focusing on the feature of allowing temporary dust in the mempool.
Users of this can immediately use dust outputs as:
1. Single keyed anchor (can be shared by multiple parties)
2. Single unkeyed anchor, ala P2A
Which is useful when the parent transaction cannot have fees for technical or accounting reasons.
What I'm calling "keyed" anchors would be used anytime you don't want a third party to be able to run off with the utxo. As a motivating example, in Ark there is the concept of a "forfeit transaction" which spends a "connector output". The connector output would ideally be 0-value, but you would not want that utxo spend by anyone, because this would cause financial loss for the coordinator of the service: https://arkdev.info/docs/learn/concepts#forfeit-transaction
Note that this specific use-case likely doesn't work as it involves a tree of dust, but the connector idea in general demonstrates how it could be used.
Another related example is connector outputs in BitVM2: https://bitvm.org/bitvm2.html .
Note that non-TRUC usage will be impractical unless the minrelay requirement on individual transactions are dropped in general, which should happen post-cluster mempool.
Lightning Network intends to use this feature post-29.0 if available: https://github.com/lightning/bolts/issues/1171#issuecomment-2373748582
It's also useful for Ark, ln-symmetry, spacechains, Timeout Trees, and other constructs with large presigned trees or other large-N party smart contracts.
ACKs for top commit:
glozow:
reACK 5c2e291060c via range-diff. Nothing but a rebase and removing the conflict.
theStack:
re-ACK 5c2e291060cca3be500f3af0f6f2d3fd2177a7c9
Tree-SHA512: 88e6a6b3b91dc425de47ccd68b7668c8e98c5683712e892c588f79ad639ae95c665e2d5563dd5e5797983e7542cbd1d4353bc90a7298d45a1843b05a417f09f5
Also known as Ephemeral Dust.
We try to ensure that dust is spent in blocks by requiring:
- ephemeral dust tx is 0-fee
- ephemeral dust tx only has one dust output
- If the ephemeral dust transaction has a child,
the dust is spent by by that child.
0-fee requirement means there is no incentive to mine
a transaction which doesn't have a child bringing its
own fees for the transaction package.
83fab3212c91d91fc5502f940c901a07772ff747 test: Add combinerawtransaction test to rpc_createmultisig (Ava Chow)
Pull request description:
The only coverage of combinerawtransaction is in a legacy wallet only test. So also use it in rpc_createmultisig so that this RPC remains tested after the legacy wallet is removed.
Split from #28710
ACKs for top commit:
maflcko:
re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
BrandonOdiwuor:
Re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
Abdulkbk:
ACK 83fab3212c91d91fc5502f940c901a07772ff747
brunoerg:
code review ACK 83fab3212c91d91fc5502f940c901a07772ff747
rkrux:
tACK 83fab3212c91d91fc5502f940c901a07772ff747
Tree-SHA512: 383d88ff6c9b54337ed81c714026e527b0fed41d976959fd5c6863b49d0defa4ea13fdc3d984885c86a2b6380825cd66c17842cc31f20fbec4bc42d86aecbbfa
5a96767e3f531ba9e8a676eec47727421f9f589f depends, libevent: Do not install *.pc files and remove patches for them (Hennadii Stepanov)
ffda355b5a2113fa0f7db8015f7b08bf1351e245 cmake, refactor: Move `HAVE_EVHTTP_...` to `libevent` interface (Hennadii Stepanov)
b619bdc3303217f4415342fe60e586e18fa48308 cmake: Revamp `FindLibevent` module (Hennadii Stepanov)
Pull request description:
This PR generalizes the use of `find_package` / `pkg_check_modules`, prioritizing the former.
Addresses https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2444700876:
> We should also follow up with refactoring the libevent module, to more generically use CMake/pkg-config, rather than restricting the CMake usage to `vcpkg`. At that point, we'd likely be able to dump pkg-config for the depends path entirely.
Similar to https://github.com/bitcoin/bitcoin/pull/30903.
ACKs for top commit:
fanquake:
ACK 5a96767e3f531ba9e8a676eec47727421f9f589f
Tree-SHA512: 181020c16ccd2821e718c73f264badcdc5e62980c4a8d9691e759efe2ea00da2326e26308d1dcfdeac01e9e27930428ecace9f36941deee951b751b138d7266c
ec375de39ff8e0d44fc026618a234e37019e46c1 doc: Add missing 'blank=true' option in offline-signing-tutorial.md (secp512k2)
Pull request description:
Issue:
The text mentions that the `createwallet` command should use the options `disable_private_keys=true, blank=true`, but the provided command only includes `disable_private_keys=true`, missing the `blank=true` option.
Correction:
Added `blank=true` to the command to match the options described in the text.
Explanation:
The `blank=true` option is necessary to create a blank wallet. Including this option ensures the command matches the options specified in the text.
ACKs for top commit:
fanquake:
ACK ec375de39ff8e0d44fc026618a234e37019e46c1
Tree-SHA512: 8c145e3ef1598c5e13f2aa89e496f76bfe2fc6f47d5e740963acad18dd1f782655a822dc234862af8e5a08060ab7fe1039a3dcfa68455a9143fe2d849975927c
5e3b444022c354ad4d69e3142f7bc98d723c9e29 doc: Fix missing comma in JSON example in REST-interface.md (secp512k2)
Pull request description:
This pull request addresses a minor issues in the REST-interface.md documentation:
Missing Comma in JSON Example: In the "Query UTXO set" section, a missing comma after the "desc" field in the JSON example has been added to ensure valid JSON syntax.
ACKs for top commit:
maflcko:
lgtm ACK 5e3b444022c354ad4d69e3142f7bc98d723c9e29
Abdulkbk:
ACK 5e3b444022c354ad4d69e3142f7bc98d723c9e29
Tree-SHA512: d2d479c8a991d3380d16b7b140a375a90dca0fce0a024a4b8ccf842d703398fde14ae972349f5fbd2e0ce26aa6cd6d07c0262d9c09ddc4c6c466527cfbe0e1f1
fa729ab4a276c3462e390bf2fab6cad93d3a590d doc: Fixup bitcoin-wallet manpage chain selection args (MarcoFalke)
Pull request description:
The sentence is missing `-testnet4` and `-chain`. Instead of duplicating the full list (and having to keep it in sync), just refer to them as `(test)chain selection arguments`.
ACKs for top commit:
willcl-ark:
utACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
tdb3:
Code Review ACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
rkrux:
crACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
Tree-SHA512: e2cb6e2dd778a34e6c7e8ccde9794ab601e68bad68fe110f41cd73ac12ac3c5d0632fb59a48355f03ef0909f77ec5afd7ea50f301a998cb3ec76e115969f3e7e
4120c7543ee32efed7396d7153411ecbbd588ad3 scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp} (Sebastian Falbesoner)
Pull request description:
The confusing "command" terminology for the 12-byte field in the (v1) p2p message header was replaced with the more proper term "message type" in other modules already years ago, see eg #18533, #18937, #24078, #24141. This PR does the same for the protocol.{h,cpp} module to complete the replacements. Note that "GetCommand" is a method name also used in the `ArgsManager` (there it makes much more sense), so the scripted-diff lists for this replacement the files explicitly, rather than using `$(git grep -l ...)`.
ACKs for top commit:
maflcko:
review ACK 4120c7543ee32efed7396d7153411ecbbd588ad3 🛒
fjahr:
Code review ACK 4120c7543ee32efed7396d7153411ecbbd588ad3
rkrux:
tACK 4120c7543ee32efed7396d7153411ecbbd588ad3
Tree-SHA512: 7b4dd30136392a145da95d2f3ba181c18c155ba6f3158e49e622d76811c6a45ef9b5c7539a979a04d8404faf18bb27f11457aa436d4e2998ece3deb2c9e59748
36a22e5683375b7925767de9daa9df4c48831c15 ci: make ctest stop on failure (furszy)
Pull request description:
Make `ctest` stops when the first failure happens.
Wasting less resources and notifying the developer faster when a failure occurs.
ACKs for top commit:
maflcko:
lgtm ACK 36a22e5683375b7925767de9daa9df4c48831c15
tdb3:
code review and test ACK 36a22e5683375b7925767de9daa9df4c48831c15
Tree-SHA512: 3abdb330e76aa312f7a5432e3d447a654e6689fc56e067b8e4d07ed8d677fc92f836e603aab0b2f175a6c039a5d50e5fd1160d503164321c1af44ad902f09605
0de3e96e333090548a43e5e870c4cb8941d6baf1 tracing: use bitcoind pid in bcc tracing examples (0xb10c)
411c6cfc6c2e488e407f057b646730e63806ed8a tracing: only prepare tracepoint args if attached (0xb10c)
d524c1ec06643208c189089089e84f6e1cd0abad tracing: dedup TRACE macros & rename to TRACEPOINT (0xb10c)
Pull request description:
Currently, if the tracepoints are compiled (e.g. in depends and release builds), we always prepare the tracepoint arguments regardless of the tracepoints being used or not. We made sure that the argument preparation is as cheap as possible, but we can almost completely eliminate any overhead for users not interested in the tracepoints (the vast majority), by gating the tracepoint argument preparation with an `if(something is attached to this tracepoint)`. To achieve this, we use the optional semaphore feature provided by SystemTap.
The first commit simplifies and deduplicates our tracepoint macros from 13 TRACEx macros to a single TRACEPOINT macro. This makes them easier to use and also avoids more duplicate macro definitions in the second commit.
The Linux tracing tools I'm aware of (bcc, bpftrace, libbpf, and systemtap) all support the semaphore gating feature. Thus, all existing tracepoints and their argument preparation is gated in the second commit. For details, please refer to the commit messages and the updated documentation in `doc/tracing.md`.
Also adding unit tests that include all tracepoint macros to make sure there are no compiler problems with them (e.g. some varadiac extension not supported).
Reviewers might want to check:
- Do the tracepoints still work for you? Do the examples in `contrib/tracing/` run on your system (as bpftrace frequently breaks on every new version, please test master too if it should't work for you)? Do the CI interface tests still pass?
- Is the new documentation clear?
- The `TRACEPOINT_SEMAPHORE(event, context)` macros places global variables in our global namespace. Is this something we strictly want to avoid or maybe move to all `TRACEPOINT_SEMAPHORE`s to a separate .cpp file or even namespace? I like having the `TRACEPOINT_SEMAPHORE()` in same file as the `TRACEPOINT()`, but open for suggestion on alternative approaches.
- Are newly added tracepoints in the unit tests visible when using `readelf -n build/src/test/test_bitcoin`? You can run the new unit tests with `./build/src/test/test_bitcoin --run_test=util_trace_tests* --log_level=all`.
<details><summary>Two of the added unit tests demonstrate that we are only processing the tracepoint arguments when attached by having a test-failure condition in the tracepoint argument preparation. The following bpftrace script can be used to demonstrate that the tests do indeed fail when attached to the tracepoints.</summary>
`fail_tests.bt`:
```c
#!/usr/bin/env bpftrace
usdt:./build/src/test/test_bitcoin:test:check_if_attached {
printf("the 'check_if_attached' test should have failed\n");
}
usdt:./build/src/test/test_bitcoin:test:expensive_section {
printf("the 'expensive_section' test should have failed\n");
}
```
Run the unit tests with `./build/src/test/test_bitcoin` and start `bpftrace fail_tests.bt -p $(pidof test_bitcoin)` in a separate terminal. The unit tests should fail with:
```
Running 594 test cases...
test/util_trace_tests.cpp(31): error: in "util_trace_tests/test_tracepoint_check_if_attached": check false has failed
test/util_trace_tests.cpp(51): error: in "util_trace_tests/test_tracepoint_manual_tracepoint_active_check": check false has failed
*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
```
</details>
These links might provide more contextual information for reviewers:
- [How SystemTap Userspace Probes Work by eklitzke](https://eklitzke.org/how-sytemtap-userspace-probes-work) (actually an example on Bitcoin Core; mentions that with semaphores "the overhead for an untraced process is effectively zero.")
- [libbpf comment on USDT semaphore handling](1596a09b5d/src/usdt.c (L83-L92)) (can recommend the whole comment for background on how the tracepoints and tracing tools work together)
- https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation#Semaphore_Handling
ACKs for top commit:
willcl-ark:
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
laanwj:
re-ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
jb55:
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
vasild:
ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
Tree-SHA512: 0e5e0dc5e0353beaf5c446e4be03d447e64228b1be71ee9972fde1d6fac3fac71a9d73c6ce4fa68975f87db2b2bf6eee2009921a2a145e24d83a475d007a559b
This pull request addresses a minor issues in the REST-interface.md documentation:
Missing Comma in JSON Example: In the "Query UTXO set" section, a missing comma after the "desc" field in the JSON example has been added to ensure valid JSON syntax.
c189eec848e3c31f438151d4d3422718a29df3a3 doc: release note for mempoolrullrbf removal (Greg Sanders)
d47297c6aaba44672fdd19d817d9b11d2dc90bb7 rpc: Mark fullrbf and bip125-replaceable as deprecated (Greg Sanders)
04a5dcee8ab56f2089ab08192b97b67bc15bc3ba docs: remove requirement to signal bip125 (Greg Sanders)
111a23d9b3615094fbfdf6cc8c996adc3db2782c Remove -mempoolfullrbf option (Greg Sanders)
Pull request description:
Given https://github.com/bitcoin/bitcoin/pull/30493 and the related discussion on network uptake it's probably not helpful to have an option for a feature that will not be respected by the network in any meaningful way.
Wallet changes can be done in another PR on its own cadence to account for possible fingerprinting, waiting for fullrbf logic to permeate the network, etc.
ACKs for top commit:
stickies-v:
re-ACK c189eec848e3c31f438151d4d3422718a29df3a3
achow101:
ACK c189eec848e3c31f438151d4d3422718a29df3a3
murchandamus:
ACK c189eec848e3c31f438151d4d3422718a29df3a3
rkrux:
reACK c189eec848e3c31f438151d4d3422718a29df3a3
Tree-SHA512: 9447f88f8f291c56c5bde70af0a91b0a4f5163aaaf173370fbfdaa3c3fd0b44120b14d3a1977f7ee10e27ffe9453f8a70dd38aad0ffb8c39cf145049d2550730
47f50c7af5572520fd986b313a63a44a76d3c859 doc: add bitcoin-qt man description (willcl-ark)
40b82e3ab0a1889ddb816ca66f80512bdba93ef6 doc: add bitcoin-util man description (willcl-ark)
a7bf80f3a2db17ca6cbbaf45c49fbd6011633fa5 doc: add bitcoin-tx man description (willcl-ark)
3f9a5168323070a18b6c8b7bffeafe1d2965f50b doc: add bitcoin-wallet man description (willcl-ark)
d8c0bb23ef83f207394a3f714f4498ab8abf88c1 doc: add bitcoin-cli man description (willcl-ark)
09abccfa7729b53d057c9c0a7836367d6cc0233d doc: add bitcoind man description (willcl-ark)
Pull request description:
Closes#29552
Add better descriptions to help string for all binaries. Use format which is correctly detected by help2man when generating manpages.
Examples:
Before:
After:
Demonstration using `bitcoin-cli` also highlights removal of inline usage explanations which were being incorrectly formatted by `help2man`. This results in the following changed format to `bitcoin-cli --help`:
ACKs for top commit:
achow101:
ACK 47f50c7af5572520fd986b313a63a44a76d3c859
tdb3:
re ACK 47f50c7af5572520fd986b313a63a44a76d3c859
rkrux:
tACK 47f50c7af5572520fd986b313a63a44a76d3c859
maflcko:
ACK 47f50c7af5572520fd986b313a63a44a76d3c859 📠
Tree-SHA512: 124a8877077b7d47758ea970949d472b2444e3ba65d2bfeb47ebbdb1f5f8d3bf0abe2c88714bb6c92ba0e36583f0b36aa6f016ea88b65f011c610096ea872182
The only coverage of combinerawtransaction is in a legacy wallet only
test. So also use it in rpc_createmultisig so that this RPC remains
tested after the legacy wallet is removed.
e2ba8236715ee4530d08312b075d8b41cb592257 depends: Specify CMake generator explicitly (Hennadii Stepanov)
Pull request description:
Building packages in depends implies using GNU Make. However, this assumption can be wrong in environments where the [`CMAKE_GENERATOR`](https://cmake.org/cmake/help/latest/envvar/CMAKE_GENERATOR.html) variable is set.
This change explicitly makes CMake use the "Unix Makefiles" generator.
Can be tested as follows:
```
$ env CMAKE_GENERATOR=Ninja make -C depends
```
ACKs for top commit:
fanquake:
ACK e2ba8236715ee4530d08312b075d8b41cb592257 - Going forward I think we should look at making this work without having to hard code anything.
Tree-SHA512: e14ed1cec192434fe089d36a83e1e150727a3b299fada80a61fa5b44b0c50e014a774ef1e6cd6df189e25f7a13042a20d4f9605f6ccd32e7782f10adaf5e788f
fa461d7a43aa5a321278c8f734e80fb7aa79bfdb fuzz: Limit wallet_notifications iterations (MarcoFalke)
Pull request description:
I don't think the fuzz target has ever found a real issue. The closest being https://github.com/bitcoin/bitcoin/pull/25869
It is also, by far, the slowest fuzz target. For example, looking at https://cirrus-ci.com/task/5533338067271680?logs=ci#L3974, it takes more than one hour:
```
Run wallet_notifications with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_corpora/wallet_notifications')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1096115652
INFO: Loaded 1 modules (625824 inline 8-bit counters): 625824 [0x5628396d9138, 0x562839771dd8),
INFO: Loaded 1 PC tables (625824 PCs): 625824 [0x562839771dd8,0x56283a0fe7d8),
INFO: 1287 files found in /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/wallet_notifications
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1047827 bytes
INFO: seed corpus: files: 1287 min: 1b max: 1047827b total: 11616898b rss: 172Mb
#16pulse cov: 14328 ft: 25341 corp: 14/239b exec/s: 5 rss: 204Mb
#64pulse cov: 19179 ft: 58412 corp: 61/3587b exec/s: 5 rss: 320Mb
#128pulse cov: 19692 ft: 85738 corp: 125/16Kb exec/s: 3 rss: 544Mb
#256pulse cov: 19923 ft: 107490 corp: 253/72Kb exec/s: 2 rss: 556Mb
#512pulse cov: 20107 ft: 124704 corp: 509/330Kb exec/s: 2 rss: 590Mb
Slowest unit: 10 s:
artifact_prefix='./'; Test unit written to ./slow-unit-9fa5f7d7e4afa1626622ef1b3c70a7563eecf11d
#1024pulse cov: 20360 ft: 136324 corp: 1009/2488Kb exec/s: 0 rss: 726Mb
Slowest unit: 23 s:
artifact_prefix='./'; Test unit written to ./slow-unit-5d99a20de2c2b6bedb0cbaf0ba3743ae3ba13c7c
Slowest unit: 26 s:
artifact_prefix='./'; Test unit written to ./slow-unit-8889ecb61bdc0650355e0d0d27c012f3239d07a4
Slowest unit: 42 s:
artifact_prefix='./'; Test unit written to ./slow-unit-d16c084282ac1a85fcdc43c48e49836b08446686
#1289INITED cov: 20409 ft: 138281 corp: 1245/10323Kb exec/s: 0 rss: 880Mb
#1289DONE cov: 20409 ft: 138281 corp: 1245/10323Kb lim: 1047827 exec/s: 0 rss: 880Mb
Done 1289 runs in 3813 second(s)
```
Looking at the flame graphs, it looks like the slow runs spend most of their time in the Knapsack solver. This seems reasonable, because it may run 1000 inner Knapsack iterations 200 times. So reduce the fuzz iterations from 200 to 20 to avoid fuzz timeouts and wasted resources.
ACKs for top commit:
brunoerg:
code review ACK fa461d7a43aa5a321278c8f734e80fb7aa79bfdb
dergoegge:
lgtm ACK fa461d7a43aa5a321278c8f734e80fb7aa79bfdb
Tree-SHA512: bee707a3398ab0c729f335f00d8cad63135939831454dd863830fc957b4b51b27064224be0ed15eb76cfcc39de972e4e79b0802940934fbac516840ddc475ab9
d22a234ed270286b483aec2db1e2f716b9756231 net: Use actual memory size in receive buffer accounting (laanwj)
047b5e2af1f03549d85926aa02fed0dfa00d449f streams: add DataStream::GetMemoryUsage (laanwj)
c3a6722f34a575834b282894d441d7bf802bd467 net: Use DynamicUsage(m_type) in CSerializedNetMsg::GetMemoryUsage (laanwj)
c6594c0b142133535c1d2d5b8d8084cf9e57592b memusage: Add DynamicUsage for std::string (laanwj)
7596282a556accb84fbcec3f31a3e2017d8ade0c memusage: Allow counting usage of vectors with different allocators (laanwj)
Pull request description:
Add a method `CNetMessage::GetMemoryUsage` and use this for accounting of the size of the process receive queue instead of the raw message size (like we already do for the send buffer and `CSerializedNetMsg`).
This ensures that allocation and deserialization overhead is better taken into account.
On average, this counts about ~100 extra bytes per packet on x86_64:
```
2024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
2024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes
2024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes
2024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes
```
ACKs for top commit:
l0rinc:
ACK d22a234ed270286b483aec2db1e2f716b9756231
achow101:
ACK d22a234ed270286b483aec2db1e2f716b9756231
i-am-yuvi:
ACK d22a234ed270286b483aec2db1e2f716b9756231
danielabrozzoni:
Light ACK d22a234ed270286b483aec2db1e2f716b9756231 - code looks good to me, but I'm not very familiar with C++ memory management specifics
Tree-SHA512: ef09707e77b67bdbc48e9464133e4fccfa5c05051c1022e81ad84f20ed41db83ac5a9b109ebdb8d38f70785c03c5d6bfe51d32dc133d49e52d1e6225f6f8e292
Issue:
The text mentions that the `createwallet` command should use the options `disable_private_keys=true, blank=true`, but the provided command only includes `disable_private_keys=true`, missing the `blank=true` option.
Correction:
Added `blank=true` to the command to match the options described in the text.
Explanation:
The `blank=true` option is necessary to create a blank wallet. Including this option ensures the command matches the options specified in the text.
5161c2618cd36cd2d64c64d42d6f6d3b1929cb28 ci: add second_deadlock_stack=1 to TSAN options (fanquake)
Pull request description:
This is mentioned in the developer notes, but isn't present in `TSAN_OPTIONS`, resulting in:
```bash
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=60508)
Cycle in lock order graph: M0 (0xffff98e02208) => M1 (0xffff98e0cbe8) => M2 (0xffff98e0cd98) => M0
<snip>
Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message
```
Add it, for (potentially) more informative output, when failures occur. Checked that adding does output more information.
ACKs for top commit:
maflcko:
lgtm ACK 5161c2618cd36cd2d64c64d42d6f6d3b1929cb28
hebasto:
ACK 5161c2618cd36cd2d64c64d42d6f6d3b1929cb28.
Tree-SHA512: 57bfa24d248ed76ba0db537edff425453a0765c4768fc1b6f59a87876d4acf63ed38bb3c20f369a008ae256472d9d24e58d76729d423f662dfdb2952afc46cb0
Previously this assertion checked MAX_PEER_TX_REQUEST_IN_FLIGHT was not
exceeded. However, this property is not actually enforced; it is just
used to determine when a peer is overloaded.
9e5089dbb02ef8a32f484aab682ad06748e1a532 build, msvc: Enable `libqrencode` vcpkg package (Hennadii Stepanov)
30089b0cb61c1c40d6338877ffd5d72b31024271 cmake: Add `FindQRencode` module (Hennadii Stepanov)
Pull request description:
This PR introduces the `FindQRencode` CMake module, following the official CMake [guidelines](https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#find-modules) for managing [upstream libraries](https://github.com/fukuchi/libqrencode) that lack a config file package. This module enhances flexibility in locating the `libqrencode` library by making the use of `pkg-config` optional.
With this update, `libqrencode` can be detected on systems where either `pkg-config` or the `libqrencode.pc` file is unavailable, such as Windows environments using the vcpkg package manager. However, if `libqrencode.pc` is available, it remains beneficial as the only direct source of the library's version information.
Additionally, the `libqrencode` vcpkg package is enabled for MSVC builds.
Here is a diff for configuration output on Ubuntu 24.10:
```diff
-- Detecting CXX compile features - done
-- Found SQLite3: /usr/include (found suitable version "3.46.1", minimum required is "3.7.17")
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.8.1")
--- Checking for module 'libqrencode'
--- Found libqrencode, version 4.1.1
+-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so (found version "4.1.1")
-- Found Qt: /usr/lib/x86_64-linux-gnu/cmake/Qt5 (found suitable version "5.15.15", minimum required is "5.11.3")
-- Performing Test CXX_SUPPORTS__WERROR
-- Performing Test CXX_SUPPORTS__WERROR - Success
```
ACKs for top commit:
fanquake:
ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532
Tree-SHA512: bb9baca64386772f2f4752b1cbff1230792562ca6b2e37c56ad28580b55b1ae6ff65c2cf0d8ab026111d7b5a056d7ac672496a3cfd1a81e4fdd2b84c8cf75fff
