e3d8d727031c93a68ef2e5e85c83f4718e4c209f test: Remove unnecessary block mining from importdescriptors test (Fabian Jahr)
Pull request description:
This removes generation of 6 blocks and replaces is with a `sync_all` in the `importdescriptors` test.
The generated blocks themself don't seem to serve any purpose in the test. Instead they could make the test flaky (although I did not find open issues pointing to this happening in practice in the CI). Right before the blocks being generated a transaction is created (L454) and later in the test this tx is assumed to be still in the mempool. If the nodes were to sync their mempools before the blocks are generated, the test fails. It currently only seems to work because one node sends the tx while the other generates the blocks and the mempools are not synced fast enough.
The `sync_all` is still needed to let nodes catch up at that point. Otherwise races happen further below which the generate call seems to have prevented so far.
ACKs for top commit:
laanwj:
Code review ACK e3d8d727031c93a68ef2e5e85c83f4718e4c209f
Tree-SHA512: 14f3dc2938d779d1ad43e09a7e046523fc3c92f41df012833f279a2e88e74c2fcab301fe4f3fcc038bd8460ea1360725a8d1eb5b59acd1039495bacb484fd790
05b2d9fe076233961ebf24e74d47c57cc0618bbb build: Bump default `PlatformToolset` for Visual Studio 2022 (Hennadii Stepanov)
460c6c724850f6bf1153000b060189332ebbdb35 doc: Make Windows build docs match the CI task (Hennadii Stepanov)
849cf967a3bb69db9431e993e03b6dbb04a99d8b ci: Increase CPU number for "Win64 native" task (Hennadii Stepanov)
a18c4c18714b8918c20242b8c674386bc3436cf8 ci: Bump vcpkg to the latest version (Hennadii Stepanov)
b9a5a9b68c1be9cb3b38c79c6633d8c558f56b7b ci: Limit ccache cache size properly on "Win64 native" task (Hennadii Stepanov)
156bc89788034c509051e0635ed40694b8d717c6 ci: Update Windows task image up to visualstudio2022 (Hennadii Stepanov)
Pull request description:
Besides upgrading Visual Studio, which seems [inevitable](https://github.com/bitcoin/bitcoin/pull/24531#discussion_r887854193), this PR also:
- bumps vcpkg to the latest version (previous one was in bitcoin/bitcoin#24847)
- fixes cache size limit for `ccache`
ACKs for top commit:
sipsorcery:
reACK 05b2d9fe076233961ebf24e74d47c57cc0618bbb.
hebasto:
> ACK [05b2d9f](05b2d9fe07)
jarolrod:
ACK 05b2d9fe076233961ebf24e74d47c57cc0618bbb
Tree-SHA512: 6338e74a3f1907f09ca29540e9e2cf7ac3be3b9e28271e8a20e71b67a9c3d5ebb8d34528b9677bcd1d9bc0ad723d68fd2ba7db368443ed1854cca3a3961f294b
Currently, the time it takes to get the "Win64 native" task done with
all of the caches been invalidated is very close to the 2 hours limit.
This task is the only one which runs on Windows Community Cluster,
therefore this change should not affect other CI tasks.
fabae3541ac574a1101be8dc54f1499dbbf2f231 rpc: Use steady_clock for getrpcinfo durations (MacroFake)
Pull request description:
Currently it uses `GetTimeMicros`, which is the system time. Using steady time instead, makes the code type safe and avoids spurious offsets when the system time adjusts.
ACKs for top commit:
laanwj:
Code review ACK fabae3541ac574a1101be8dc54f1499dbbf2f231
w0xlt:
Code Review ACK fabae3541a
shaavan:
Code Review ACK fabae3541ac574a1101be8dc54f1499dbbf2f231
Tree-SHA512: eb25fe3e69bf42ec8a2d4aaa69b435de7654b0d07218ce3e0c03ebaef6eb7f713128779057d012621773a34675a81f5757e7b2502c13b82adaf6e2df970d8c66
e4b4db561049c97c956e5b856713dcf63ac3e2f0 refactor: remove unused method `CDBWrapper::CompactRange` (Sebastian Falbesoner)
fb38c6e21f064e23b63a46d15adb873029463cff refactor: remove unused methods `{CDBIterator,CCoinsViewDBCursor}::GetValueSize()` (Sebastian Falbesoner)
Pull request description:
The `GetValueSize` methods haven't been used since the chainstate db cache has been switched from per-tx to per-txout model years ago (PR #10195, commit d342424301013ec47dc146a4beb49d5c9319d80a). The `CompactRange` is unused since the txindex migration code was removed (PR https://github.com/bitcoin/bitcoin/pull/22626, commit fa20f815a9).
ACKs for top commit:
fanquake:
ACK e4b4db561049c97c956e5b856713dcf63ac3e2f0
furszy:
re-ACK e4b4db56
laanwj:
Code review ACK e4b4db561049c97c956e5b856713dcf63ac3e2f0
Tree-SHA512: 77da445fb70c744046263c6f2ddb05782b68e3d4b2ea604dd7c7dc73ce7c1f2d2b48ec68db4dcb03e35fc27488b99b0a420f6fa3d5b83d325c1708ed68e99e0a
e7a9133766a2dc357a79d38cd47231cc99cdefb7 [net processing] Set CNode::m_relays_txs=true when receiving BIP37 filters (dergoegge)
Pull request description:
This line was accidentally removed in https://github.com/bitcoin/bitcoin/pull/22778.
Receiving a `filterload` message implies that we should relay txs to the sender (`CNode::m_relays_txs = true`). `CNode::m_relays_txs` is only used for the inbound eviction logic, so removing the line might have slightly changed the eviction behaviour but nothing else.
ACKs for top commit:
laanwj:
Code review ACK e7a9133766a2dc357a79d38cd47231cc99cdefb7
vasild:
ACK e7a9133766a2dc357a79d38cd47231cc99cdefb7
Tree-SHA512: 19c5df0f579f707c6c7900d12a6b71ac69e802be64f7d2fdcc40ac714c918dc4c17def164592f8836cc105a03daefefca3ca5e10423145eca8db4348c27c9cfc
This method hasn't been used since the txindex migration code has been
removed (PR #22626, commit fa20f815a9cb438c5ab61e97a453612ddd8b21b5).
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
ceec6808d331fa082407a734cd5f3c2f1c7d11b3 test: `-whitebind` and `-bind` with `-listen=0` should throw an error (brunoerg)
Pull request description:
This PR adds test coverage for the following init error:
b9122e95f0/src/init.cpp (L872-L875)
ACKs for top commit:
laanwj:
Code review ACK ceec6808d331fa082407a734cd5f3c2f1c7d11b3
Tree-SHA512: 03068abe7199b1235f029871ab87a3dd4943738c592ad62d82cdcd3e0201e627624960bd3ea1fc6fc1e7da4b8e215ba3393d1cb8130e1108049f764e51dc75c0
d8d99d041a063a1719fc1d43ecd4a6365e0657ca qt6: Do not use deprecated high DPI attributes in Qt 6 (Hennadii Stepanov)
8927bb8f063e88546fd9cf2ce595902d2ea63bb6 refactor: Fix style in `initTranslations()` function (Hennadii Stepanov)
ad73447dc2bd04120c4eb7f5757613ad35a35cf5 qt6: Do not use deprecated `QLibraryInfo::path` in Qt 6 (Hennadii Stepanov)
3f51d0b8b2b2fabf5e305bd71226a8e3e5f3ad2d qt6: Fix type registration (Hennadii Stepanov)
Pull request description:
One more step in migration to Qt 6.
Could be tested with hebasto/bitcoin#3 or bitcoin/bitcoin#24798.
No behavior change when compiling with Qt 5.
ACKs for top commit:
laanwj:
Code review ACK d8d99d041a063a1719fc1d43ecd4a6365e0657ca
jarolrod:
ACK d8d99d041a063a1719fc1d43ecd4a6365e0657ca
Tree-SHA512: e5f92a80f8622e5f95dd98a90783956a26d3c8382b9ca8e479fb6c152cfdc85a2f6084e78d463ceea1e0f0b3ac72d2b086c8ca24967b2b6070553317e9e3252e
f67b6fce3777ea8dfd23b3983d0b46a2f69d7d77 Update Arch Linux build example (Igor Bubelov)
Pull request description:
The current build example has two issues:
1. The claim that the wallet functionality will be missing is obsolete since Bitcoin Core can use SQLite, which is a hard dependency of `pacman` so we can assume that it's always present.
2. Installing package groups such as `base-devel` adds some friction and uncertainty by forcing readers to choose which packages they need, interactively. Listing required deps explicitly speeds up the whole process, makes it more transparent and cuts the number of installed packages.
ACKs for top commit:
laanwj:
re-ACK f67b6fce3777ea8dfd23b3983d0b46a2f69d7d77
Tree-SHA512: c068dac5d244044827d5d94a4b48f239180301b6870dce31b003fa111a69f7e3a483681a7ea2b3d393d6791b40043685ce2fe62c338cce1b7e37a6db0f02b1a2
faee330c7bde4d52dc18cf55166b5975b26bdecd test: Fail if connect_nodes fails (MacroFake)
Pull request description:
Currently, `connect_nodes` will return silently when the connection is disconnected while connecting. This is confusing, so fix it.
Can be tested by reverting the signet test change and observing the failure when running the test.
ACKs for top commit:
laanwj:
Tested ACK faee330c7bde4d52dc18cf55166b5975b26bdecd
Tree-SHA512: 641ca8adcb9f5ff33239b143573bddc0dfde41dbd103751ee870f1572ca2469f6a0d4bab6693102454cd3e270ef8251d87fbfac48f6d8adac70d2d6bbffaae56
a724c39606273dfe4c6f9887ef8b77d0a98f1b34 net: rename Sock::Reset() to Sock::Close() and make it private (Vasil Dimov)
e8ff3f0c52e7512a580bc907dc72e5bb141b4217 net: remove CloseSocket() (Vasil Dimov)
175fb2670a2a24220afb3eea99b7b65b0aa89c76 net: remove now unused Sock::Release() (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
* `Sock::Release()` is unused, thus remove it
* `CloseSocket()` is only called from `Sock::Reset()`, so move the body of `CloseSocket()` inside `Sock::Reset()` and remove `CloseSocket()` - this helps to hide low level file descriptor sockets inside the `Sock` class.
* Rename `Sock::Reset()` to `Sock::Close()` and make it `private` - to be used only in the destructor and in the `Sock` assignment operator. This simplifies the public API by removing one method from it.
ACKs for top commit:
laanwj:
Code review ACK a724c39606273dfe4c6f9887ef8b77d0a98f1b34
Tree-SHA512: 4b12586642b3d049092fadcb1877132e285ec66a80af92563a7703c6970e278e0f2064fba45c7eaa78eb65db94b3641fd5e5264f7b4f61116d1a6f3333868639
fa7cbc6e5c36f1e37d18ffe1084cfccf52cbc296 build: Remove negated --enable-fuzz checks from build system (MarcoFalke)
Pull request description:
It is confusing to enable the unit test binary with `ENABLE_TESTS && !ENABLE_FUZZ`, but every other binary is enabled with a simple flag. For example `ENABLE_BENCH`, or `ENABLE_FUZZ_BINARY`.
Fix that by turning `ENABLE_TESTS` back into meaning "enable unit test binary".
ACKs for top commit:
laanwj:
Code review re-ACK fa7cbc6e5c36f1e37d18ffe1084cfccf52cbc296
Tree-SHA512: 2eca26d365fe1d60ff7b87c882a441273cc64bc5eefdc10ec1a0975db24f9b47f591ce8ef53f14eb02d8e3271510e503bfd1a3d09a5d1b35fe44f531f318e87e
Outside of `Sock`, `Sock::Reset()` was used in just one place (in
`i2p.cpp`) which can use the assignment operator instead.
This simplifies the public `Sock` API by having one method less.
c01ae8f5ead6837162a2772ce792a83f66757ee4 Use consistent wording in log (Igor Bubelov)
Pull request description:
It's a trivial change, but it bothers me a bit that two log lines in a row aren't grammatically identical while following exactly the same pattern. I've read `contributing.md` and I'm aware that changes like this are usually being ignored and dropped, but I decided to leave it here anyway in case someone feels the same way about inconsistent log messages or grammar =)
ACKs for top commit:
laanwj:
Code review ACK c01ae8f5ead6837162a2772ce792a83f66757ee4
Tree-SHA512: d5b3849b3a6e3de7ea9b468c05f17cacd1dbd1aca2f3401b5138383dc8d385cea9e221db558ab472c1c4c7f6921d57dcc7af89a54776c5765fa00e429694b4e7
fa8421bc5bcd483a9501257073db17ff2e76eb46 test: Remove from_node from create_self_transfer* MiniWallet helpers (MacroFake)
Pull request description:
MiniWallet is capable to create a transaction without a node, so don't pass it in where not needed.
ACKs for top commit:
kouloumos:
ACK fa8421bc5bcd483a9501257073db17ff2e76eb46
theStack:
ACK fa8421bc5bcd483a9501257073db17ff2e76eb46
Tree-SHA512: d51e2ae6577c1e2bc80386678ff5c7974609e86317850aaec45cdbf0d23076ba1ae76342610c8f90931a6c0971c8e916864442b041a253212e6a9d476d79c541
dc1e7ad7a5713d885f70ccc6c93e7a4c07e76559 Add doc/design/libraries.md (Ryan Ofsky)
Pull request description:
Prompted by the [libbitcoinkernel issue #24303](https://github.com/bitcoin/bitcoin/issues/24303) and PRs, I started looking at existing libraries and what their dependencies are and wrote this document to describe them and where `libbitcoinkernel` fits in.
Readable link is: https://github.com/ryanofsky/bitcoin/blob/pr/libs/doc/design/libraries.md
Feedback is welcome
ACKs for top commit:
laanwj:
ACK dc1e7ad7a5713d885f70ccc6c93e7a4c07e76559
hebasto:
Approach ACK dc1e7ad7a5713d885f70ccc6c93e7a4c07e76559, using this doc as a guide in https://github.com/hebasto/bitcoin/pull/3 :)
Tree-SHA512: 7687b1847797c50de1f5ea721bd201cc8304690064743fbe6d69e2198cc239084e9da7d158be65bea948a6ec3d71d74c84122c0e523c390b389b49ea8d2cddc9
67364ebe4c499eb8effe8dac11a5e3648f30c6c7 test, qt: Add tests for `GUIUtil::extractFirstSuffixFromFilter` (w0xlt)
ace9af5688662a56d855bd26f36bc5c145b909e3 qt: Replace `QRegExp` with `QRegularExpression` (w0xlt)
c378535e28e0c54353fde9c4a66cb3134a3ee742 qt: Add a function that extracts the suffix from a filter (w0xlt)
Pull request description:
Picking up https://github.com/bitcoin-core/gui/pull/606 (labeled "Up for grabs") and applying https://github.com/bitcoin-core/gui/pull/606#pullrequestreview-984607067 and https://github.com/bitcoin-core/gui/pull/606#issuecomment-1137149907.
Replaces occurrences of `QRegExp` usage with `QRegularExpression` as part of the roadmap for Qt6 integration.
Fixes https://github.com/bitcoin-core/gui/issues/578
ACKs for top commit:
laanwj:
Code review and lightly tested ACK 67364ebe4c499eb8effe8dac11a5e3648f30c6c7
hebasto:
ACK 67364ebe4c499eb8effe8dac11a5e3648f30c6c7
Tree-SHA512: 4a17d83e557bc635cbd1a15776856e9edb7162b23a369ccbd2ac59c68b8a1ea663baaa7d5ad98e419dc03b91ef3315c768eeadc01c0b29162de109493161e814
32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 wallet: avoid extra wtx lookup in AddToSpends (furszy)
Pull request description:
As `AddToSpends` is only called from `AddToWallet` and `LoadToWallet`, places where we insert the wtx into the wallet map, we can directly feed `AddToSpends` with the `wtx` and remove another extra lookup.
ACKs for top commit:
laanwj:
Code review ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28
achow101:
ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28
theStack:
Code-review ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28
w0xlt:
Code Review ACK 32e5edc0f4
brunoerg:
crACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28
Tree-SHA512: e9fb8df44c3e3fa26c107d261bf78e45014b4755890a64817f2be62ee6b7751f5dd2813a18dcb103a21ddba1422f9d2d59c4bf186f08314e634365d36b01be8f
99b9e5f3a9fa29bbc1e45fc958470fbcc207ef23 p2p: always set nTime for self-advertisements (Martin Zumsande)
Pull request description:
This logic was recently changed in 0cfc0cd322 to overwrite `addrLocal` with the address they gave us when self-advertising to an inbound peer. But if we don't also change `nTime` again from the default `TIME_INIT`, our peer will not relay our advertised address any further.
ACKs for top commit:
naumenkogs:
ACK 99b9e5f3a9fa29bbc1e45fc958470fbcc207ef23
laanwj:
Code review ACK 99b9e5f3a9fa29bbc1e45fc958470fbcc207ef23
vasild:
ACK 99b9e5f3a9fa29bbc1e45fc958470fbcc207ef23
Tree-SHA512: 4c7ea51cc77ddaa4b3537962ad2ad085f7ef5322982d3b1f5baecb852719eb99dd578436ca63432cb6b0a4fbd8b59fca793caf326c4663a4d6f34301e8146aa2
These methods haven't been used since the chainstate db cache has been
switched from per-tx to per-txout model years ago (PR #10195, commit
d342424301013ec47dc146a4beb49d5c9319d80a).
In Qt 6, registration of `QDataStream` streaming operators is done
automatically. Consequently, `qRegisterMetaTypeStreamOperators()` does
no longer exist. Calls to this method have to be removed when porting
to Qt 6.
See https://doc.qt.io/qt-6/qtcore-changes-qt6.html#the-qmetatype-class
