The class only stores the file path, reading it from a global. Globals
are confusing and make testing harder.
The method reading from a stream does not even use any class members, so
putting it in a class is also confusing.
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
CSubNet serialization code that was removed in
fa4e6afdae was needed by multiprocess code
to share ban map between gui and node processes.
Rather than adding it back, use suggestion from MarcoFalke
<falke.marco@gmail.com>
https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to
use JSON serialization. This requires making BanMapToJson /
BanMapFromJson functions public.
b11a195ef4 refactor: Detach wallet transaction methods (followup for move-only) (Russell Yanofsky)
Pull request description:
This makes `CWallet` and `CWalletTx` methods in `spend.cpp` and `receive.cpp` files into standalone functions.
It's a followup to [#21207 MOVEONLY: CWallet transaction code out of wallet.cpp/.h](https://github.com/bitcoin/bitcoin/pull/21207), which moved code from `wallet.cpp` to new `spend.cpp` and `receive.cpp` files.
There are no changes in behavior. This is just making methods into functions and removing circular dependencies created by #21207. There are no comment or documentation changes, either. Removed comments from `transaction.h` are just migrated to `spend.h`, `receive.h`, and `wallet.h`.
---
This commit was split off from #21206 so there are a few earlier review comments there
ACKs for top commit:
achow101:
ACK b11a195ef4
Sjors:
utACK b11a195ef4
meshcollider:
light ACK b11a195ef4
Tree-SHA512: 75ce818d3f03b728b14b12e2d21bd20b7be73978601989cb37ff98254393300d1bb7823281449cd3d9e40756d67d42bd9a46bbdafd2e8baa95aaf2cb1c84549f
ea98d9c2ef rpc: fix/add missing RPCExamples for "Util" RPCs (Sebastian Falbesoner)
Pull request description:
Similar to https://github.com/bitcoin/bitcoin/pull/18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains `createmultisig`, `deriveaddresses`, `estimatesmartfee`, `getdescriptorinfo`, `signmessagewithprivkey`, `validateaddress`, `verifymessage`) some love by fixing one broken and adding three missing examples:
- fixed `HelpExampleRpc` for `createmultisig` (disturbing escape characters and quotation marks)
- added missing `HelpExampleRpc` for
- `deriveaddresses` (also put descriptor in a new string constant)
- `estimatesmartfee`
- `getdescriptorinfo` (also put descriptor in a new string constant)
Output for `createmultisig` example on the master branch:
```
$ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Enter host password for user '__cookie__':
{"result":null,"error":{"code":-1,"message":"JSON value is not an array as expected"},"id":"curltest"}
```
Output for `createmultisig` example on the PR branch:
```
$ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Enter host password for user '__cookie__':
{"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"}
```
ACKs for top commit:
jonatack:
ACK ea98d9c2ef looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples
Tree-SHA512: d6ecb6da66f19517065453357d210102e2cc9f1f8037aeb6a9177ff036d0c21773dddf5e0acdbc71edbbde3026e4d1e7ce7c0935cd3e023c60f34e1b173b3299
218862a018 Display peers in -netinfo that we don't relay addresses to (Jon Atack)
3834e23b25 Display peers in -netinfo that request we not relay transactions (Jon Atack)
0a9ee3a2c7 Simplify a few conditionals in -netinfo (Jon Atack)
5eeea8e257 Add addr_processed and addr_rate_limited stats to -netinfo (Jon Atack)
Pull request description:
Update CLI -netinfo to display the getpeerinfo `addr_processed`, `addr_rate_limited`, `addr_relay_enabled` and `relaytxes` data with auto-adjusting column widths.
```
$ ./src/bitcoin-cli -netinfo help
txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes
"*" - the peer requested we not relay transactions to it (relaytxes is false)
addrp Total number of addresses processed, excluding those dropped due to rate limiting
"." - we do not relay addresses to this peer (addr_relay_enabled is false)
addrl Total number of addresses dropped due to rate limiting
```
ACKs for top commit:
0xB10C:
Code review and tested ACK 218862a018
Zero-1729:
re-tACK 218862a018
vasild:
ACK 218862a018
jarolrod:
tACK 218862a018
Tree-SHA512: bb9da4bdd71859b234f6e4c2c46257a57ef0d0e0b363d2b8fded128bcaa28132f64a0a4651c622e1de1e3b7c05c7587a4369e9e79799895884fda9745c63409d
fa2e9de59f test: Check that non-signaling BIP125 tx can be replaced via parent (MarcoFalke)
Pull request description:
While `optout_child_tx` in the `test_no_inherited_signaling` test is reported as "bip125-replaceable", it is not *directly* replaceable. For example by bumping the fee of `optout_child_tx`. However, it is still replaceable *indirectly* via it's BIP-125 signalling parent.
Clarify this by extending the test.
ACKs for top commit:
mjdietzx:
Tested ACK fa2e9de59f
josibake:
ACK fa2e9de59f
Tree-SHA512: b3608beae743dcb6152df4d2cfe1c0af6b4404ba3837f73e1d1431bd7c637f0c7fab0379aaab2218d5cd63e71070a079c0595ec031056058e8d3c933c2bae0a9
ab9c34237a release: remove gitian (fanquake)
Pull request description:
Note that this doesn't yet touch any glibc back compat related code.
ACKs for top commit:
laanwj:
Code review ACK ab9c34237a
Tree-SHA512: 8e2fe3ec1097f54bb11ab9136b43818d90eab5dbb0a663ad6a552966ada4bdb49cc12ff4e66f0ec0ec5400bda5c81f3a3ce70a9ebb6fe1e0db612da9f00a51a7
ae4ad1a125 msvc: update bitcoin_config.h defines (fanquake)
Pull request description:
While not many of these are used, some, like `HAVE_DLLEXPORT_ATTRIBUTE`, are used when creating libbitcoinconsensus.
Given this file is static, also just remove everything that is commented out.
ACKs for top commit:
sipsorcery:
tACK ae4ad1a125.
Tree-SHA512: fe9bb3381f9219617092fde57c7db25e8f9a5a2ecb9a77a8d61d9ae74ffecd93024aaa17fb6685cda74d20efc5975feb12f9817bceb895f91b79fc5cbee895ca
97cea1a93a policy: unit test Segwit dust thresholds (Antoine Poinsot)
Pull request description:
This is the unit testing part of #22779, hence without the threshold modification.
ACKs for top commit:
MarcoFalke:
cr ACK 97cea1a93a
benthecarman:
crACK 97cea1a93a
Tree-SHA512: 96fb194709ae44364455eb920ed3ecff2e11e5327e0a72b9eeec9f9445894302099a0c4ffb1e0c8d4d523c0bfe06c57f1ebb0c03cf3389a73f518e3b174c45aa
5559cf1460 doc: Add packages that provide Qt Wayland plugin for Linux (Hennadii Stepanov)
Pull request description:
When building on Linux using system packages (without depends) the support of Wayland protocol for modern desktop environments (e.g., GNOME, KDE Plasma) depends on the presence of the installed Qt Wayland plugin which is loaded dynamically at the GUI startup.
1. On Debian/Ubuntu, the [`qtwayland5`](https://packages.ubuntu.com/focal/qtwayland5) package is required (also see this [patch](https://codereview.qt-project.org/c/qt/qtbase/+/231227), and this [doc](https://wiki.debian.org/Wayland#Qt_.28supported_since_5.29)):
- with `qtwayland5` installed:
```
$ QT_QPA_PLATFORM="wayland;xcb" ./src/qt/bitcoin-qt -printtoconsole
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
2021-08-05T09:51:31Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
2021-08-05T09:51:31Z Qt 5.11.3 (dynamic), plugin=wayland (dynamic)
2021-08-05T09:51:31Z No static plugins.
2021-08-05T09:51:31Z Style: fusion / QFusionStyle
2021-08-05T09:51:31Z System: Debian GNU/Linux 10 (buster), x86_64-little_endian-lp64
...
```
- without `qtwayland5`:
```
$ QT_QPA_PLATFORM="wayland;xcb" ./src/qt/bitcoin-qt -printtoconsole
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
qt.qpa.plugin: Could not find the Qt platform plugin "wayland" in ""
2021-08-05T09:48:55Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
2021-08-05T09:48:55Z Qt 5.11.3 (dynamic), plugin=xcb (dynamic)
2021-08-05T09:48:55Z No static plugins.
2021-08-05T09:48:55Z Style: fusion / QFusionStyle
2021-08-05T09:48:55Z System: Debian GNU/Linux 10 (buster), x86_64-little_endian-lp64
2021-08-05T09:48:55Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0
...
```
2. On Fedora, the [`qt5-qtwayland`](https://fedora.pkgs.org/34/fedora-x86_64/qt5-qtwayland-5.15.2-4.fc34.x86_64.rpm.html) package is required:
- with `qt5-qtwayland` installed:
```
$ ./src/qt/bitcoin-qt -printtoconsole
QSocketNotifier: Can only be used with threads started with QThread
2021-08-05T08:41:03Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
2021-08-05T08:41:03Z Qt 5.15.2 (dynamic), plugin=wayland (dynamic)
2021-08-05T08:41:03Z No static plugins.
2021-08-05T08:41:03Z Style: fusion / QFusionStyle
2021-08-05T08:41:03Z System: Fedora 34 (Workstation Edition), x86_64-little_endian-lp64
...
```
- without `qt5-qtwayland`:
```
$ ./src/qt/bitcoin-qt -printtoconsole
qt.qpa.plugin: Could not find the Qt platform plugin "wayland" in ""
2021-08-05T07:50:41Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
2021-08-05T07:50:41Z Qt 5.15.2 (dynamic), plugin=xcb (dynamic)
2021-08-05T07:50:41Z No static plugins.
2021-08-05T07:50:41Z Style: fusion / QFusionStyle
2021-08-05T07:50:41Z System: Fedora 34 (Workstation Edition), x86_64-little_endian-lp64
2021-08-05T07:50:41Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0
...
```
ACKs for top commit:
fanquake:
ACK 5559cf1460 - I don't think there's any harm to point this out in our Linux build docs. It's not changing our binaries or dependencies in any way.
Tree-SHA512: e26856586b29540b55c12905a091408e95ce59ea2c952520086b41138c955fba1b78e95e868f75205af07c6eccae51644177f7165d837ae058aaf0c0abf3ccf5
While not many of these are used, some, like HAVE_DLLEXPORT_ATTRIBUTE, are used
when creating libbitcoinconsensus.
Given this file is static, also just remove everything that is commented out.
fa18553d38 fuzz: Remove addrdb fuzz target (MarcoFalke)
Pull request description:
The target has several issues:
* It is named incorrectly (`addrdb`, but it constructs a `CBanEntry`)
* It doesn't do anything meaningful, other than consuming one integer and passing it to a constructor
* It consumes CPU time that can be used for the other targets
* It is redundant with the banman fuzz target
Fix all by removing it.
ACKs for top commit:
amitiuttarwar:
ACK fa18553d38, thanks for the cleanup
Tree-SHA512: 3f8944d3f80913bf466c03062fed070e96073fb72d0938b2bc9a2586960c86879d6f251e16fd81cfeb4e6685ff9eef6bccb25cd3901b218a100c90f25a3c9240
56a42f10f4 Stricter BIP32 decoding and test vector 5 (Pieter Wuille)
Pull request description:
This adds detection for various edge cases when decoding BIP32 extended pubkeys/privkeys, and tests them using the proposed https://github.com/bitcoin/bips/pull/921 BIP32 test vector 5.
ACKs for top commit:
darosior:
utACK 56a42f10f4 -- Had to implement essentially the same fix in python-bip32.
kristapsk:
ACK 56a42f10f4. Checked that test vectors are the same as in BIP32 and that tests pass.
Tree-SHA512: 5cc800cc9dc10e43ae89b659ce4f44026d04ec3cabac4eb5122d2e72ec2ed66cd5ace8c7502259e469a9ecaa5ecca2457e55dfe5fedba59948ecbf6673af67a7
96cc6bb04f guix/prelude: Override VERSION with FORCE_VERSION (Carl Dong)
Pull request description:
```
Previously, if the builder exported $VERSION in their environment (as
past Gitian-building docs told them to), but their HEAD does not
actually point to v$VERSION, their build outputs will differ from those
of other builders.
This is because the contrib/guix/guix-* scripts only ever act on the
current git worktree, and does not try to check out $VERSION if $VERSION
is set in the environment.
Setting $VERSION only makes the scripts pretend like the current
worktree is $VERSION.
This problem was seen in jonatack's attestation for all.SHA256SUMS,
where only his bitcoin-22.0rc3-osx-signed.dmg differed from everyone
else's.
Here is my deduced sequence of events:
1. Aug 27th: He guix-builds 22.0rc3 and uploads his attestations up to
guix.sigs
2. Aug 30th, sometime after POSIX time 1630310848: he pulls the latest
changes from master in the same worktree where he guix-built 22.0rc3
and ends up at 7be143a960
3. Aug 30th, sometime before POSIX time 1630315907: With his worktree
still on 7be143a960, he guix-codesigns. Normally, this would result
in outputs going in guix-build-7be143a960e2, but he had
VERSION=22.0rc3 in his environment, so the guix-* scripts pretended
like he was building 22.0rc3, and used 22.0rc3's guix-build directory
to locate un-codesigned outputs and dump codesigned ones.
However, our SOURCE_DATE_EPOCH defaults to the POSIX time of HEAD
(7be143a960), which made all timestamps in the resulting codesigned
DMG 1630310848, 7be143a960e2's POSIX timestamp. This differs from the
POSIX timestamp of 22.0rc3, which is 1630348517. Note that the
windows codesigning procedure does not consider SOURCE_DATE_EPOCH.
We resolve this by only allowing VERSION overrides via the FORCE_VERSION
environment variable.
```
Please ignore the branch name, it's not relevant to the change.
ACKs for top commit:
fanquake:
ACK 96cc6bb04f - Also makes sense given there are Guix build guides recommending to set `VERSION` as part of the process. i.e https://gist.github.com/hebasto/7293726cbfcd0b58e1cfd5418316cee3.
Tree-SHA512: 9dca3fc637ce11049286a3ebee3cd61cce2125fc51d31cf472fbed7f659e1846fc44062753e0e71bfaec9e7fbab6f040bb88d9d4bc4f8acb28c6890563584acf
d9d3ec07cf Consolidate XOnlyPubKey lookup hack (Andrew Chow)
Pull request description:
The places where we need to lookup information for a XOnlyPubKey
currently implement a hack which makes both serializations of the full
pubkey in order to try the CKeyIDs for the lookup functions. Instead of
duplicating this everywhere it is needed, we can consolidate the CKeyID
generation into a function, and then have wrappers around GetPubKey,
GetKey, and GetKeyOrigin which takes the XOnlyPubKey, retrieves all of
the CKeyIDs (using the new GetKeyIDs() function in XOnlyPubKey), and
tries their respective underlying lookup function.
Split from #22364
ACKs for top commit:
S3RK:
Code Review reACK d9d3ec0
Zero-1729:
re-crACK d9d3ec0
theStack:
re-ACK d9d3ec07cf
meshcollider:
Code review + functional test run ACK d9d3ec07cf
Tree-SHA512: 21a7f6d37fad74483a38006f82b3558337fe9ed30e0b4392e6fff82c22251a42ac996b43f06cdaa9289ee34a768e181d87aa4208b5538e36ae4977954e1fa6a0
d2a09c8355 doc: mention bech32m/BIP350 in doc/descriptors.md (Pieter Wuille)
Pull request description:
As it says on the tin.
ACKs for top commit:
0xB10C:
ACK d2a09c8355
fanquake:
ACK d2a09c8355 - can be backported for coherent documentation. Bech32m support was backported into branches back to the 0.19.
Tree-SHA512: a75d8bbf1ab062ec19e14cbe38dd0405a08c58e5dd4e9d94881f2c5efd52cc22dc755eae8d21bbda066feb099d93312b65557900bc2e65a70ba0412614cf0135
387355bb94 test, refactor: rpc_rawtransaction PEP8 (Jon Atack)
7d5cec2e49 refactor: separate the rpc_rawtransaction tests into functions (Jon Atack)
409779df95 move-only: regroup similar rpc_rawtransaction tests together (Jon Atack)
d861040dd2 test: remove no longer needed (ASCII art) comments (Jon Atack)
14398b30d6 test: add and harmonize getrawtransaction logging (Jon Atack)
85d8869cf8 test: run 2nd getrawtransaction section with/without -txindex (Jon Atack)
0097740773 refactor: txid to constant in rpc_rawtransaction to isolate tests (Jon Atack)
8c19d1329f refactor: dedup/reorg createrawtransaction sequence number tests (Jon Atack)
7f073594c9 Test src/node/transaction::GetTransaction() without -txindex (Jon Atack)
Pull request description:
Following up on https://github.com/bitcoin/bitcoin/pull/22383#pullrequestreview-698583510, this pull adds missing `src/node/transaction::GetTransaction()` test coverage for combinations of `-txindex` and `blockhash` and does some refactoring of the test file.
ACKs for top commit:
mjdietzx:
reACK 387355bb94
josibake:
reACK 387355bb94
MarcoFalke:
Approach ACK 387355bb94🔆
Tree-SHA512: b47c4ff87d69c61434e5729c954b338bc13744eddaba0879ca9f5f42243ba2cb4640d94c5f74de9f2735a8bf5e66b3d1c3bd3b7c26cd7324da7d3270ce87c6fd
in microseconds.
Change the function name in order to print "LockContention" instead
of "PrintLockContention" to the log. Add Doxygen documentation.
With this change, the lock contention log prints:
2021-09-01T11:29:03Z LockContention: pnode->cs_vSend, net.cpp:1373 started
2021-09-01T11:29:03Z LockContention: pnode->cs_vSend, net.cpp:1373 completed (31μs)
2021-09-01T11:29:03Z LockContention: cs_vNodes, net.cpp:2277 started
2021-09-01T11:29:03Z LockContention: cs_vNodes, net.cpp:2277 completed (6μs)
2021-09-01T11:29:04Z LockContention: cs_vNodes, net.cpp:2242 started
2021-09-01T11:29:04Z LockContention: cs_vNodes, net.cpp:2242 completed (3μs)
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
