Age | Commit message (Collapse) | Author |
|
architecture-independent
fafd121026c4f1e25d498983e4f88c119516552b refactor: Make CFeeRate constructor architecture-independent (MarcoFalke)
Pull request description:
Currently the constructor is architecture dependent. This is confusing for several reasons:
* It is impossible to create a transaction larger than the max value of `uint32_t`, so a 64-bit `size_t` is not needed
* Policy (and consensus) code should be arch-independent
* The current code will print spurious compile errors when compiled on 32-bit systems:
```
policy/feerate.cpp:23:22: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
```
Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes.
ACKs for top commit:
theStack:
re-ACK fafd121026c4f1e25d498983e4f88c119516552b
promag:
Code review ACK fafd121026c4f1e25d498983e4f88c119516552b.
Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833
|
|
aca0e5dcdb174ef7e88b76910d6fffd633688235 Remove `GetDataDir(bool fNetSpecific = true)` function (Kiminuo)
b3e67f20a02bcf43f20873029ce30617c95eb9da scripted-diff: Replace `GetDataDir(true)` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
4c3a5dcbfc3010965332ad568c3a70618c930ef3 scripted-diff: Replace `GetDataDir()` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
13bd8bb0536f008118b1de921654925ed9ce1da7 Make `ArgsManager.GetDataDirPath` private and drop needless suffix (Kiminuo)
4d8189f62076d47be182e709857774bc3921b334 scripted-diff: Change `ArgsManager.GetDataDirPath()` to `ArgsManager.GetDataDirBase()` in tests (Kiminuo)
0f53df47d5b8319fc71f3b87c15a115ff797fb50 Add `ArgsManager.GetDataDirBase()` and `ArgsManager.GetDataDirNet()` as an intended replacement for `ArgsManager.GetDataDirPath(net_identifier)` (Kiminuo)
716de29dd8672d28bfe0a08424e9958fe61054d2 Make `m_cached_blocks_path` mutable. Make `ArgsManager::GetBlocksDirPath()` const. (Kiminuo)
Pull request description:
This PR is a follow up PR to #21244. The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by removing `GetDataDir(net_specific)` and replacing it by `gArgs.GetDataDir(net_specific)` calls.
The approach of this PR attempts to be similar to the one chosen in "De-globalize ChainstateManager" (#20158). The goal is to pass `ArgsManager` to functions (or ideally to have `ArgsManager` as a member of a class where needed; inspiration from here: #21789) instead of having it as a global variable (i.e. `gArgs`).
**Notes:**
* First commit makes `m_cached_blocks_path` `mutable` as was suggested [here](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615274095) but not fully applied in #21244. (`m_cached_datadir_path` and `m_cached_network_datadir_path` were marked as `mutable` in #21244) This commit can be in a separate PR too.
* Other commits deal with removing of `GetDataDir(net_specific)` function.
* This was originally part of #21244 but it was [left]((https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-633779754)) for a follow up PR.
* I think that the proposed changes show nicely where there is reliance on `gArgs` which is IMO a good thing.
If you know about a better approach how to do this, please share it here.
ACKs for top commit:
hebasto:
ACK aca0e5dcdb174ef7e88b76910d6fffd633688235
MarcoFalke:
re-ACK aca0e5dcdb174ef7e88b76910d6fffd633688235 👃
Tree-SHA512: deec4d88edb32d7f4c818c3a74ffbb64709685819b88242dcf5dbaa1fb611f3ce2b29d2576ddb9e0dc5e75288e43538968224008c0a80e7149fc81c309f7c9da
|
|
|
|
calls
-BEGIN VERIFY SCRIPT-
git ls-files -- 'src' ':(exclude)src/util/system.h' ':(exclude)src/util/system.cpp' | xargs sed -i 's/GetDataDir(true)/gArgs.GetDataDirNet()/g';
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
git ls-files -- 'src' ':(exclude)src/util/system.h' ':(exclude)src/util/system.cpp' | xargs sed -i 's/GetDataDir()/gArgs.GetDataDirNet()/g';
-END VERIFY SCRIPT-
|
|
|
|
`ArgsManager.GetDataDirBase()` in tests
-BEGIN VERIFY SCRIPT-
git ls-files src/test/*_tests.cpp src/test/util/setup_common.cpp | xargs sed -i 's/.GetDataDirPath()/.GetDataDirBase()/g';
-END VERIFY SCRIPT-
|
|
intended replacement for `ArgsManager.GetDataDirPath(net_identifier)`
|
|
4bea30169218e2f21e0c93a059966b41c8edd205 test: use P2PK-MiniWallet for feature_csv_activation.py (Sebastian Falbesoner)
dc7eb64e83f5b8e63f12729d5f77b1c920b136e4 test: MiniWallet: add P2PK support (Sebastian Falbesoner)
Pull request description:
This PR adds support for creating and spending transactions with raw pubkey (P2PK) outputs to MiniWallet, [as suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/21900#discussion_r629524841). Using that mode in the test `feature_csv_activation.py`, all txs submitted to the mempool follow the standard policy, i.e. `-acceptnonstdtxn=1` can be removed.
Possible follow-ups:
* Improve MiniWallet constructor Interface; an enum-like parameter instead of two booleans would probably be better
* Look at other tests that could benefit from P2PK (e.g. feature_cltv.py?)
* Check vsize also for P2PK txs (vsize varies due to signature, i.e. a range has to be asserted)
ACKs for top commit:
laanwj:
Code review ACK 4bea30169218e2f21e0c93a059966b41c8edd205
Tree-SHA512: 9b428e6b7cfde59a8c7955d5096cea88af1384a5f49723f00052e9884d819d952d20a5ab39bb02f9d8b6073769c44462aa265d84a33e33da33c2d21670c488a6
|
|
-noincludeconf=0
fad0867d6ab9430070aa7d60bf7617a6508e0586 Cleanup -includeconf error message (MarcoFalke)
fa9f711c3746ca3962f15224285a453744cd45b3 Fix crash when parsing command line with -noincludeconf=0 (MarcoFalke)
Pull request description:
The error message has several issues:
* It may crash instead of cleanly shutting down, when `-noincludeconf=0` is passed
* It doesn't quote the value
* It includes an erroneous trailing `\n`
* It is redundantly mentioning `"-includeconf cannot be used from commandline;"` several times, when once should be more than sufficient
Fix all issues by:
* Replacing `get_str()` with `write()` to fix the crash and quoting issue
* Remove the `\n` and only print the first value to fix the other issues
Before:
```
$ ./src/bitcoind -noincludeconf=0
terminate called after throwing an instance of 'std::runtime_error'
what(): JSON value is not a string as expected
Aborted (core dumped)
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=a b
-includeconf cannot be used from commandline; -includeconf=c
```
After:
```
$ ./src/bitcoind -noincludeconf=0
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="a b"
```
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34493
Testcase: https://github.com/bitcoin/bitcoin/files/6515429/clusterfuzz-testcase-minimized-system-6328535926046720.log
```
FUZZ=system ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-system-6328535926046720.log
```
See https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md
ACKs for top commit:
sipa:
utACK fad0867d6ab9430070aa7d60bf7617a6508e0586
Tree-SHA512: b44af93be6bf71b43669058c1449c4c6999f03b5b01b429851b149b12d77733408cb207e9a3edc6f0bffd6030c4c52165e8e23a1c2718ff5082a6ba254cc94a4
|
|
const.
|
|
fa91994b1b9b6da3bb2a2f722d9251d7d0f2167e fuzz: Add utxo_snapshot target (MarcoFalke)
Pull request description:
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/21953/commits/fa91994b1b9b6da3bb2a2f722d9251d7d0f2167e
Tree-SHA512: a00f077102a4e4e321bd1464c3fa11e7a5b9e04324b9be87aa28cfdc77630db7fc772d3a3768dc6ec36bbdd2d67b7e0719f0cf3fd87b4a1087365b934e137b5c
|
|
Remove the erroneous trailing newline '\n'. Also, print only the first
value to remove needless redundancy in the error message.
|
|
|
|
bbbb51877a96c3473aeea5914f751eec7835b5c4 fuzz: Speed up transaction fuzz target (MarcoFalke)
Pull request description:
`hashBlock` and `include_addresses` are orthogonal, so no need to do an exhaustive "search".
Might fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34491
ACKs for top commit:
practicalswift:
cr ACK bbbb51877a96c3473aeea5914f751eec7835b5c4: patch looks correct, and `TxToUniv` surprisingly wide in the `transaction_fuzz_target` flame graph! Putting it on a diet makes sense.
Tree-SHA512: 1e7c30c7fecf96364a9a1597c0a22139389fdeb67db59f3c2c6fc088196e3332877b2865991a957980d542f99a2f48cc066dd7cc16c695a5113190fe06205089
|
|
harness tries to create a TCP socket (belt and suspenders)
393992b049d3bcf0b2a3439be128d13d6567f0b1 fuzz: Terminate immediately if a fuzzing harness ever tries to create a TCP socket (belt and suspenders) (practicalswift)
Pull request description:
Terminate immediately if a fuzzing harness ever to create a TCP socket (belt and suspenders).
Obviously this _should_ never happen, but if it _does_ happen we want immediate termination instead of a TCP socket :)
ACKs for top commit:
MarcoFalke:
ACK 393992b049d3bcf0b2a3439be128d13d6567f0b1
Tree-SHA512: 5bbff1f7e9a58b3eae24f742b7daf3fc870424c985f29bed5931e47a708d9c0984bfd8762f43658cffa9c69d32f86d56deb48bc7e43821e3398052174b6a160e
|
|
793b2682841b0bdd7eb93163e34728765cfe52b2 txmempool: add thread safety annotations (Anthony Towns)
Pull request description:
Add missing thread safety guards to CTxMempool members.
ACKs for top commit:
MarcoFalke:
cr ACK 793b2682841b0bdd7eb93163e34728765cfe52b2
hebasto:
re-ACK 793b2682841b0bdd7eb93163e34728765cfe52b2, only suggested changes since my [previous](https://github.com/bitcoin/bitcoin/pull/22003#pullrequestreview-664529633) review.
Tree-SHA512: c5eb197c63375c80c325a276f322177e84e0181c94a124720b1a364e964ac223fc6fdfd89bd0e152b76959fb6b97bfbf82dd36ec105ed6e2dc045ede717df4ae
|
|
f152c1ac66bfe5c77d947d5c99ce5da3a88797fa build: libevent 2.1.12-stable (fanquake)
Pull request description:
Start using [libevent 2.1.12-stable](https://github.com/libevent/libevent/releases/tag/release-2.1.12-stable) in depends. Changelog is available here: https://raw.githubusercontent.com/libevent/libevent/release-2.1.12-stable/ChangeLog. This release basically only contains bug fixes.
Upgrading will also prevent some oss-fuzz reports which are coming from libevent code, such as https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34355 & https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34412.
Guix builds:
```bash
aa365feab851d2055c4c3cd787fc1db972a8f2efe1e7c75bc62b34594846a021 guix-build-f152c1ac66bf/output/aarch64-linux-gnu/bitcoin-f152c1ac66bf-aarch64-linux-gnu-debug.tar.gz
80cf96c4618279f4420b3f15ce3d3331937506e7996d8c325ce741f4da431fad guix-build-f152c1ac66bf/output/aarch64-linux-gnu/bitcoin-f152c1ac66bf-aarch64-linux-gnu.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/aarch64-linux-gnu/inputs.SHA256SUMS
b6261b19ed8f55e655122d515a4f76a984e1f74f932630ce40ca34bb76fadfc6 guix-build-f152c1ac66bf/output/arm-linux-gnueabihf/bitcoin-f152c1ac66bf-arm-linux-gnueabihf-debug.tar.gz
be1018237e5e739a742f777efdbdd28ff794cfc2faa39930d5da6dfcfa620da3 guix-build-f152c1ac66bf/output/arm-linux-gnueabihf/bitcoin-f152c1ac66bf-arm-linux-gnueabihf.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/arm-linux-gnueabihf/inputs.SHA256SUMS
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 guix-build-f152c1ac66bf/output/dist-archive/SKIPATTEST.TAG
52fa25961965b53e73319f5f8ba5a62f789b56ff3a07286ed5fe88771c2674bf guix-build-f152c1ac66bf/output/dist-archive/bitcoin-f152c1ac66bf.tar.gz
069dfc72b83527722552d12db86f3f7046be9a85d0ccc5168c71af0d38dbc9a9 guix-build-f152c1ac66bf/output/powerpc64-linux-gnu/bitcoin-f152c1ac66bf-powerpc64-linux-gnu-debug.tar.gz
dba47d56dccdd4183762f888ab2e64bf8c074a6873dcf6dac663f88633932926 guix-build-f152c1ac66bf/output/powerpc64-linux-gnu/bitcoin-f152c1ac66bf-powerpc64-linux-gnu.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/powerpc64-linux-gnu/inputs.SHA256SUMS
e43e2c631ed0dd587a52705b36e5e69e9e622d493eb6741d438976197a1416f2 guix-build-f152c1ac66bf/output/powerpc64le-linux-gnu/bitcoin-f152c1ac66bf-powerpc64le-linux-gnu-debug.tar.gz
f47f006c68262e3cd30acc355478d4ea056bb1c055bc51c5adab56305c260725 guix-build-f152c1ac66bf/output/powerpc64le-linux-gnu/bitcoin-f152c1ac66bf-powerpc64le-linux-gnu.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/powerpc64le-linux-gnu/inputs.SHA256SUMS
f62805fd8652e28d935da14b0328f62a583be24b5517a5c7f189970bdd3c6eb0 guix-build-f152c1ac66bf/output/riscv64-linux-gnu/bitcoin-f152c1ac66bf-riscv64-linux-gnu-debug.tar.gz
db16cbddfc790798af8617213cc726002545f05cfb90ec00e462ba5d3aca10b1 guix-build-f152c1ac66bf/output/riscv64-linux-gnu/bitcoin-f152c1ac66bf-riscv64-linux-gnu.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/riscv64-linux-gnu/inputs.SHA256SUMS
2c1afbc6ea44e5947ffcbdf031233f03f7a63b93ca08ddfec2c45c661c1666bc guix-build-f152c1ac66bf/output/x86_64-apple-darwin18/bitcoin-f152c1ac66bf-osx-unsigned.dmg
65cff5a1bc7eb411b1bb26eb407c2a944e5bf24b08cfcd7a9eeea47924106ac8 guix-build-f152c1ac66bf/output/x86_64-apple-darwin18/bitcoin-f152c1ac66bf-osx-unsigned.tar.gz
98a5c002aed8a35912853cca5e5ffd7afc91eb839b81ca8bc73ddf010839e3ac guix-build-f152c1ac66bf/output/x86_64-apple-darwin18/bitcoin-f152c1ac66bf-osx64.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/x86_64-apple-darwin18/inputs.SHA256SUMS
84e3188d369aae01a459b477e1935a9466338bb9ea6d4beb93675275a5ac62fd guix-build-f152c1ac66bf/output/x86_64-linux-gnu/bitcoin-f152c1ac66bf-x86_64-linux-gnu-debug.tar.gz
ea3f4d928f82ea3d1079be3d4e020349e9d412c621149656ea409a61de906c4c guix-build-f152c1ac66bf/output/x86_64-linux-gnu/bitcoin-f152c1ac66bf-x86_64-linux-gnu.tar.gz
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/x86_64-linux-gnu/inputs.SHA256SUMS
6dc14c4e1d0b353b865a9f807dafe7ec56e829a8bc104a5ac6ef331bdc9bc726 guix-build-f152c1ac66bf/output/x86_64-w64-mingw32/bitcoin-f152c1ac66bf-win-unsigned.tar.gz
bd382152587c2dc5816d615d71e36df80f54eac5b4913569148e44117b80517c guix-build-f152c1ac66bf/output/x86_64-w64-mingw32/bitcoin-f152c1ac66bf-win64-debug.zip
d2f38b52ea54e913ccadbda6c264a258ca9a55b40a8af96ab8123e8311170aef guix-build-f152c1ac66bf/output/x86_64-w64-mingw32/bitcoin-f152c1ac66bf-win64-setup-unsigned.exe
8846b6edcdb0b3c29bdbab08b3582ee13b4877724bf3811246360b14b73ae420 guix-build-f152c1ac66bf/output/x86_64-w64-mingw32/bitcoin-f152c1ac66bf-win64.zip
53db9e6b78fa13e0753cab4a35da5e66156c54485020c6c49b1cd1ab406e978b guix-build-f152c1ac66bf/output/x86_64-w64-mingw32/inputs.SHA256SUMS
```
Gitian builds:
```bash
# macOS:
19401d11d0eda5cdcbab8c1353f7110963288e5223fd95d8cc3380107b66a7e4 bitcoin-f152c1ac66bf-osx-unsigned.dmg
3eb2383c5af734a86eaf1895f261c1ba5fddc130d9682857eff08fa7d0599e93 bitcoin-f152c1ac66bf-osx-unsigned.tar.gz
c2572b6f17613a380cf95417ed6a533d8f75fb1418428e234cb90cc0d3d5a655 bitcoin-f152c1ac66bf-osx64.tar.gz
52fa25961965b53e73319f5f8ba5a62f789b56ff3a07286ed5fe88771c2674bf src/bitcoin-f152c1ac66bf.tar.gz
4bcf84076345c61736c63f33e6f9db57b626030c417c2a1b668752d5f87dab22 bitcoin-core-osx-22-res.yml
# Windows:
5857fda233fc9fdb04391bd1718522b774978d093912650d4dc6e49b70dde40b bitcoin-f152c1ac66bf-win-unsigned.tar.gz
beff745cde9f19d3d619c3c360a9eb37dbe94169cbc968ac7958e80521584787 bitcoin-f152c1ac66bf-win64-debug.zip
e92010f76d8b286e291c4420349018029e72013ae8bf5b98eccfbf9f856a1490 bitcoin-f152c1ac66bf-win64-setup-unsigned.exe
46c38498fb16e5353de24caffd11d00b23162ec92f126de57888b4f92f375d79 bitcoin-f152c1ac66bf-win64.zip
52fa25961965b53e73319f5f8ba5a62f789b56ff3a07286ed5fe88771c2674bf src/bitcoin-f152c1ac66bf.tar.gz
851ebc1f3028ec421d733a15acf7514474a44837b75e2f94c1f80ab383b1a364 bitcoin-core-win-22-res.yml
# Linux:
4270014b016a309b6ccff39cf87b90f3b5a36a10eab5d2c41697b12b9eb5e63e bitcoin-f152c1ac66bf-aarch64-linux-gnu-debug.tar.gz
f226aaeb9cf7786fa5f4dc379611c8b1b3b580df503f79905158d0e2271c3862 bitcoin-f152c1ac66bf-aarch64-linux-gnu.tar.gz
4cd6208dc30c15b71d80d9cd59d367aba676934609d59581c8f418f9e864994d bitcoin-f152c1ac66bf-arm-linux-gnueabihf-debug.tar.gz
21746b2bbf4fd6778ea0357271f569975029667dea2ad0000abb6751eeba8071 bitcoin-f152c1ac66bf-arm-linux-gnueabihf.tar.gz
d73e6b2a779061037eaa3174615a7e5bb6082056ee4dc13de4026cc3ad21f3c5 bitcoin-f152c1ac66bf-powerpc64-linux-gnu-debug.tar.gz
e6c5876dcdd30b8936b957afc8adb9341c64a1201ba8c4285f5f76bc37689c7c bitcoin-f152c1ac66bf-powerpc64-linux-gnu.tar.gz
fc0270f7c220519e91ecd27c37df7ec130e16db8508e51b52eb88eae1f0d05d2 bitcoin-f152c1ac66bf-powerpc64le-linux-gnu-debug.tar.gz
3729a36717f704a6e30a0368a68d2356ec9af240b706a7a8c117f7ebcc5ebbca bitcoin-f152c1ac66bf-powerpc64le-linux-gnu.tar.gz
b661fc47dbf4a56cecfe02abfff78644524cd6d94aac51a38d83a4ba96a21b6e bitcoin-f152c1ac66bf-riscv64-linux-gnu-debug.tar.gz
8934ae4727e7908f626d2392877cad153696c6d8e832a9dbc0960e090f633ab4 bitcoin-f152c1ac66bf-riscv64-linux-gnu.tar.gz
cc8511b71c89575f15cfbb360c18c0af298869fd4e8c4a27ef2c04b6bc0f0703 bitcoin-f152c1ac66bf-x86_64-linux-gnu-debug.tar.gz
1cfb8cf750ca7af47cbab00d8eaee336fb11c22756a62b26b5c9a1263b5b0cd4 bitcoin-f152c1ac66bf-x86_64-linux-gnu.tar.gz
52fa25961965b53e73319f5f8ba5a62f789b56ff3a07286ed5fe88771c2674bf src/bitcoin-f152c1ac66bf.tar.gz
3b5053c7392a1a6264f3d192136e50fc9db969834d9d3643d860a3f0e2f79535 bitcoin-core-linux-22-res.yml
```
ACKs for top commit:
practicalswift:
~~cr ACK f152c1ac66bfe5c77d947d5c99ce5da3a88797fa: patch looks correct and this should resolve the OSS-Fuzz issues~~ ACK temporarily redacted while awaiting non-determinism fix
MarcoFalke:
Approach ACK f152c1ac66bfe5c77d947d5c99ce5da3a88797fa
hebasto:
~ACK f152c1ac66bfe5c77d947d5c99ce5da3a88797fa~ [retracted due to non-reproducible builds](https://github.com/bitcoin/bitcoin/pull/21991#issuecomment-844005749)
Tree-SHA512: 43253131f6d7fadfcda541e00c13d5e2fd395555563bcc42218ba4657d8f94b09760cbad6b8f0a17e426eed817624b745464d2733f7d5ac00fbb92a6ffadab84
|
|
|
|
7eea659fc908e5edfc90c185a6958ed07ecf5cd4 qt, test: use qsignalspy instead of qeventloop (Jarol Rodriguez)
Pull request description:
This PR refactors our GUI `apptests` to use [QSignalSpy](https://doc.qt.io/qt-5/qsignalspy.html) instead of [QEventLoop](https://doc.qt.io/qt-5/qeventloop.html).
`QSignalSpy` is more appropriate for our GUI test's as it is purpose-built for testing emission of signals and sets up its own `QEventLoop` when the `wait` function is called.
ACKs for top commit:
hebasto:
ACK 7eea659fc908e5edfc90c185a6958ed07ecf5cd4, tested on Linux Mint 20.1 (Qt 5.12.8).
promag:
Code review ACK 7eea659fc908e5edfc90c185a6958ed07ecf5cd4.
Tree-SHA512: 3adddbcc5efd726302b606980c9923025c44bb8ee16cb8a183e633e423179c0822db66de9ccba20dc5124fff34af4151a379c9cd18130625c60789ce809ee6fd
|
|
2a45134b5694c12546d77cdff541612881f7e3e7 qt: Add shortcuts for console font resize buttons (Hennadii Stepanov)
a2e122f0fe72d695762db2b83905e246f451300c qt: Add GUIUtil::AddButtonShortcut (Hennadii Stepanov)
4ee9ee72363d46c5ba0c71b8d8283d9c5621e3ed qt: Use native presentation of shortcut (Hennadii Stepanov)
Pull request description:
On `master` the only way to resize the console font is to manually move your mouse and click the resize buttons. This PR introduces convenient keyboard shortcuts to resize the console font.
The common resize shortcuts for applications are `Ctrl+=`/`Ctrl++` and `Ctrl+-`/`Ctrl+_`. This means that the resize QPushButtons need two shortcuts each, but you cannot assign multiple shortcuts to a QPushButton. See: https://doc.qt.io/qt-5/qabstractbutton.html#shortcut-prop
To get around this, we introduce a new function in `guiutil`, which connects a supplied `QKeySequence` shortcut to a `QAbstractButton`. This function can be reused in other situations where more than one shortcut is needed for a button.
| PR on macOS | PR on Linux |
| ---------------- | ------------ |
| ![mac-resize-shortcuts](https://user-images.githubusercontent.com/23396902/114750132-a2752580-9d21-11eb-9542-15716f2c257d.gif) | ![linux-resize-shortcuts](https://user-images.githubusercontent.com/23396902/114750165-aacd6080-9d21-11eb-8abc-5388690dcf0b.gif) |
ACKs for top commit:
hebasto:
re-ACK 2a45134b5694c12546d77cdff541612881f7e3e7
Talkless:
tACK 2a45134b5694c12546d77cdff541612881f7e3e7, tested on Debian Sid with Qt 5.15.2, shortcuts still work.
Tree-SHA512: e894ccb7e5c695ba83998c21a474d6c587c9c849f12ced665c5e0034feb6b143e41b32ba135cab6cfab22cbf153d5a52b1083b2a278e6dfca3f5ad14c0f6c573
|
|
socket (belt and suspenders)
|
|
getnodeaddresses by network
ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d doc: release note for getnodeaddresses by network (Jon Atack)
3f89c0e9902338ad8a507a938dceeeb3191eece6 test: improve getnodeaddresses coverage, test by network (Jon Atack)
6c98c099918bd20e2d3aa123643d6e3594e080e4 rpc: enable filtering getnodeaddresses by network (Jon Atack)
80ba294854e5025bcada58f1403858e6ea1d4380 p2p: allow CConnman::GetAddresses() by network, add doxygen (Jon Atack)
a49f3ddbbabfb971a537f0a6c7affb24e20ff192 p2p: allow CAddrMan::GetAddr() by network, add doxygen (Jon Atack)
c38981e748f438d972ba12ba998c8a8a597e01c1 p2p: pull time call out of loop in CAddrMan::GetAddr_() (João Barbosa)
d35ddca91ebbcf8d8b790c3b9f8cf218fafb7a53 p2p: enable CAddrMan::GetAddr_() by network, add doxygen (Jon Atack)
Pull request description:
This patch allows passing a network argument to CAddrMan::GetAddr(), CConnman::GetAddresses(), and rpc getnodeaddresses to return only addresses of that network.
It also contains a performance optimisation by promag.
ACKs for top commit:
laanwj:
Code review and lightly tested ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
vasild:
ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
Tree-SHA512: 40e700d97091248429c73cbc0639a1f03ab7288e636a7b9026ad253e9708253c6b2ec98e7d9fb2d56136c0f762313dd648915ac98d723ee330d713813a43f99d
|
|
|
|
6e2eb0d63b42288c11a65d585d487108643888d0 rpc/wallet: use OMITTED_NAMED_ARG instead of Default(VNULL) (Karl-Johan Alm)
4983f4cba44c4ffaa4972fdede7cf6fcf8caec00 rpc/createwallet: omitted named arguments (Karl-Johan Alm)
dc4db23b30b4bc7884bb28630b2b24edd81c1799 rpc: address:amount dictionaries are OBJ_USER_KEYS (Karl-Johan Alm)
c8cf0a3d513b8c892f1ae16b8c0cda184064a07b rpc/getpeerinfo: bytesrecv_per_msg is a dynamic dictionary (Karl-Johan Alm)
eb4fb7e507b583bd4ae8d1e3747f41616c782ded rpc/gettxoutsetinfo: hash_or_height is a named argument (Karl-Johan Alm)
Pull request description:
This is a follow-up to #21897, and I believe covers the remaining cases, at least that I could find.
Edited to remove unrelated information about a side project.
ACKs for top commit:
laanwj:
Documentation diff ACK 6e2eb0d63b42288c11a65d585d487108643888d0
promag:
Code review ACK 6e2eb0d63b42288c11a65d585d487108643888d0.
Tree-SHA512: d26f6e074e13d64bbca2a114a0adc7f905d47d238c4e9bc49f70ca0b775afbebf9879fc3794ab29dc316a6dbd00ba8cbeb01197e236ee4ab2e9854db25f23f04
|
|
CheckTxInputs
fae4ee545a652cc2934773b0e1fdb9004b0c5ba6 fuzz: Add missing CheckTransaction before CheckTxInputs (MarcoFalke)
faacb7eadb04a8af666e7bb59bcd79915fe3a80a fuzz: Sanity check result of CheckTransaction (MarcoFalke)
Pull request description:
This bug was introduced by myself in commit eeee8f5be1d4ccfb7e237248be5c6bef45b0fbb8 (https://github.com/bitcoin/bitcoin/pull/21553)
Reproducer: https://github.com/bitcoin/bitcoin/files/6492249/clusterfuzz-testcase-minimized-coins_view-6109460079706112.log
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34301
ACKs for top commit:
practicalswift:
cr ACK fae4ee545a652cc2934773b0e1fdb9004b0c5ba6: patch looks correct :)
Tree-SHA512: 9ece7a5c4bfa60f5e5ffeba3f0ee52a07944c9bd6102588dd7ff7405695e6b32449945b7c41bd25baf38814df5a2436521e655ceff87223ad03c69ed39053023
|
|
functions by const ref
39393479c514f271c42750ffcd0adc6bc1db2e2f p2p: pass strings to NetPermissions::TryParse functions by const ref (Jon Atack)
Pull request description:
instead of by value, as these are "in" params that are not cheap to copy.
Reference: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f16-for-in-parameters-pass-cheaply-copied-types-by-value-and-others-by-reference-to-const
ACKs for top commit:
MarcoFalke:
cr ACK 39393479c514f271c42750ffcd0adc6bc1db2e2f
Tree-SHA512: 294fe0f2d900293b4447d4e1f0ccc60c1ed27b3bdbd0f5d71d3dbf71de86879638b1b813fadfb44c58b4acff4e7d75b7ed6a4f9cc5fcf627108224e6a21b524c
|
|
|
|
489ebb7b34c403a3ce78ff6fb271f8e6ecb47304 wallet: make chain optional for CWallet::Create (Ivan Metlushko)
d73ae939649f3b30e52b5a2cccd7fafd1ab36766 CWallet::Create move chain init message up into calling code (Ivan Metlushko)
44c430ffac940e1d1dd7f5939a495470bc694489 refactor: Add CWallet:::AttachChain method (Russell Yanofsky)
e2a47ce08528dfb39c0340145c6977f6afd587f6 refactor: move first run detection to client code (Ivan Metlushko)
Pull request description:
This is a followup for https://github.com/bitcoin/bitcoin/pull/20365#discussion_r522265003
First part of a refactoring with overall goal to simplify `CWallet` and de-duplicate code with `wallettool`
**Rationale**: split `CWallet::Create` and create `CWallet::AttachChain`.
`CWallet::AttachChain` takes chain as first parameter on purpose. In future I suggest we can remove `chain` from `CWallet` constructor.
The second commit is based on be164f9cf89b123f03b926aa980996919924ee64 from #15719 (thanks ryanofsky)
cc ryanofsky achow101
ACKs for top commit:
ryanofsky:
Code review ACK 489ebb7b34c403a3ce78ff6fb271f8e6ecb47304. Only changes since last review were adding a const variable declaration, and implementing suggestion not to move feerate option checks to AttachChain. Thanks for updates and fast responses!
Tree-SHA512: 00235abfe1b00874c56c449adcab8a36582424abb9ba27440bf750af8f3f217b68c11ca74eb30f78a2109ad1d9009315480effc78345e16a3074a1b5d8128721
|
|
e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a net: flag relevant Sock methods with [[nodiscard]] (Vasil Dimov)
Pull request description:
Flag relevant Sock methods with `[[nodiscard]]` to avoid issues like the one fixed in https://github.com/bitcoin/bitcoin/pull/21631.
ACKs for top commit:
practicalswift:
cr ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a: the only changes made are additions of `[[nodiscard]]` and `(void)` where appropriate
laanwj:
Code review ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a
Tree-SHA512: addc361968d24912bb625b42f4db557791556bf0ffad818252a89a32d76ac22758ec70f8282dcfbfd77eebec20a8e6bb7557c8ed08d50a58de95378c34955973
|
|
9938d610b043bf018e0b34d92e3daaffe1f17fcc wallet: refactor: dedup sqlite PRAGMA assignments (Sebastian Falbesoner)
dca8ef586c57075587e6cd44db88eaa5c4b707c7 wallet: refactor: dedup sqlite PRAGMA integer reads (Sebastian Falbesoner)
Pull request description:
This refactoring PR deduplicates repeated SQLite access to PRAGMA settings. Two functions `ReadPragmaInteger(...)` (reads a single integer value via statement `PRAGMA key`) and `SetPragma(...)` (sets a key to specified value via statement `PRAGMA key = value`) are introduced for this purpose.
This should be more readable and less error-prone, e.g. in case other PRAGMA settings need to be read/set in the future or the error handling has to be adapted.
ACKs for top commit:
achow101:
Code Review ACK 9938d610b043bf018e0b34d92e3daaffe1f17fcc
laanwj:
Looks good to me now, code review ACK 9938d610b043bf018e0b34d92e3daaffe1f17fcc
Tree-SHA512: 5332788ead6d8d652e28cb0cef1bf0be2b22d6744f8d02dd9e04a4a68e32e14d4a21f94d9b940c37a0d815be3f0091d956c9f6e269b0a6819b62b40482d3bbd2
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
class
7075f604e8d0b21b2255fa57e20cd365dc10a288 scripted-diff: update noban documentation in net_processing.cpp (Jon Atack)
a95540cf435029f06e56749802d71315ca76b0dd scripted-diff: rename NetPermissionFlags enumerators (Jon Atack)
810d0929c1626bba141af3f779a3c9cd6ece7e75 p2p, refactor: make NetPermissionFlags a uint32 enum class (Jon Atack)
7b55a9449778c5ac89799ce4c607c8c8d797ddfb p2p: NetPermissions::HasFlag() pass flags param by value (Jon Atack)
91f6e6e6d1720e1154ad3f70a5098e9028efa84a scripted-diff: add NetPermissionFlags scopes where not already present (Jon Atack)
Pull request description:
While reviewing #20196, I noticed the `NetPermissionFlags` enums are frequently called as if they were scoped, yet are still global. This patch upgrades `NetPermissionFlags` to a scoped class enum and updates the enumerator naming, similarly to #19771. See https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum-enumerations for more info.
This change would eliminate the class of bugs like https://github.com/bitcoin/bitcoin/pull/20196#discussion_r610770148 and #21644, as only defined operations on the flags would compile.
ACKs for top commit:
laanwj:
Code review ACK 7075f604e8d0b21b2255fa57e20cd365dc10a288
vasild:
ACK 7075f604e8d0b21b2255fa57e20cd365dc10a288
Tree-SHA512: 7fcea66ee499f059efc78c934b5f729b3c8573fe304dee2c27c837c2f662b89324790568246d75b2a574cf9f059b42d3551d928996862f4358055eb43521e6f4
|
|
74bf850ac47735f2ef4306059d3e664d40cac85e faster HexStr => 13% faster blockToJSON (Martin Ankerl)
Pull request description:
`std::string`'s push_back is rather slow because it needs to check & update the string size. For
`HexStr` the output string size is already easily know, so we can initially create the string with
the correct size and then just assign the data.
`HexStr` is heavily usd in `blockToJSON`, so this change is a noticeable benefit. Benchmark on an i7-8700 @3.2GHz:
* 71,315,461.00 ns/op master
* 62,842,490.00 ns/op this commit
So this little change makes `blockToJSON` about ~13% faster.
ACKs for top commit:
laanwj:
Code review ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
theStack:
re-ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
Tree-SHA512: fc99105123edc11f4e40ed77aea80cf7f32e49c53369aa364b38395dcb48575e15040b0489ed30d0fe857c032a04e225c33e9d95cdfa109a3cb5a6ec9a972415
|
|
`CNetAddr::ToStringIP()`
6c280adcd865ae3da4df53d630c9bf737283a56f net: Return IPv6 scope id in `CNetAddr::ToStringIP()` (W. J. van der Laan)
Pull request description:
If a scope id is provided, return it back in the string representation. Also bring back the test (now in platform independent fashion). Closes #21982. Includes #21961 (apart from the MacOS remark).
ACKs for top commit:
practicalswift:
cr ACK 6c280adcd865ae3da4df53d630c9bf737283a56f
Tree-SHA512: 77792c35679b6c3545fd3a8d3d74c4f515ac2ee9f02d983251aeaaac715d55c122bbb0141abbeac272011f15520b439bd2db4ec8541a58df9b366921d212ca5f
|
|
|
|
|
|
This commit does not change behavior, it just moves code from
CWallet::CreateWalletFromFile to CWallet:::AttachChain so it can be updated in
the next commit.
This commit is most easily reviewed with
"git diff -w --color-moved=dimmed_zebra" or by diffing CWallet:::AttachChain
against the previous code with an external diff tool.
|
|
|
|
33b0b26a03a401bd39b88931b69d162c3c538d31 doc: note that brew installed qt is not supported (Raul Siles)
Pull request description:
picking up #21791, the author has stated they [cannot squash](https://github.com/bitcoin/bitcoin/pull/21791#issuecomment-828770283).
This is a useful note to prevent any issues from being opened up about this. The reason that both cannot co-exist and build bitcoin is stated [here](https://github.com/bitcoin/bitcoin/pull/21791#issuecomment-837278123):
> ... the reason is sharing /usr/local/include/ and /usr/local/lib/ directories by both qt5 and qt6 installations.
Changes from original PR:
- slightly move the note up in this section, this placement seems more appropriate to me
- drop "Note:"
[PR Render](https://github.com/jarolrod/bitcoin/blob/33b0b26a03a401bd39b88931b69d162c3c538d31/doc/build-osx.md#qt)
ACKs for top commit:
laanwj:
LGTM ACK 33b0b26a03a401bd39b88931b69d162c3c538d31
hebasto:
ACK 33b0b26a03a401bd39b88931b69d162c3c538d31
Tree-SHA512: f9efac1921a7a33b5791a9f9f4bada4b5369d358fc42e9884c077bfb4dc3f273fdd4432ce012006a8009dfafb87e13bddd56c6336fe84b6133f4b22f849c289a
|
|
namespace/class" warnings
88bdc4d33eaeefb5cfe41bb4aa565c0cb361a982 build: Silent lupdate "unknown namespace/class" warnings (Hennadii Stepanov)
Pull request description:
This PR removes multiple _"Qualifying with unknown namespace/class"_ warnings in `make -C src translate` output.
Also all of the `lupdate` options are moved before input files (as documented).
The remaining warnings are fixed in Qt 5.12.2 (see [QTBUG-42736](https://bugreports.qt.io/browse/QTBUG-42736)).
ACKs for top commit:
laanwj:
Tested ACK 88bdc4d33eaeefb5cfe41bb4aa565c0cb361a982
Tree-SHA512: 9c12ff5425a84758aaf8073554891ffe7eb2d75650a12be14ece364b6b7a4461626654885e35c5543462f0941abb020cc36dc64e656c85c8d6d2da36efc7ea2c
|
|
If a scope id is provided, return it back in the string representation.
Also bring back the test. Closes #21982.
Co-authored-by: Jon Atack <jon@atack.com>
|
|
This change removes multiple "Qualifying with unknown namespace/class"
warnings.
Also all options are moved before input files (as documented).
|
|
requirement
fa25ce45e9d54a2f0d5f2c0de9254d102a855a76 build: improve macro for testing -latomic requirement (MarcoFalke)
Pull request description:
This fixes the issue where `-latomic` is incorrectly omitted from the linker flags.
Steps to reproduce on vanilla Ubuntu Focal:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang llvm g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y && ( cd depends && make DEBUG=1 HOST=i686-pc-linux-gnu NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/i686-pc-linux-gnu/share/config.site" ./configure CC='clang -m32' CXX='clang++ -m32' --enable-fuzz --with-sanitizers=fuzzer && make -j $(nproc)
```
Before:
```
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1000000ll> >, std::memory_order)':
net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::load(std::memory_order) const':
net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1ll> >, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
After:
Clean
ACKs for top commit:
jarolrod:
ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76
hebasto:
ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76, tested on Ubuntu 20.04.2 LTS.
fanquake:
ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76
Tree-SHA512: 7b0e76876c28a4ef8d8cde1ea26c18aeec51da3d54e4da75829276b9b2a259f9d0feab215be0d39e651c6bd89071859bc29aa48131e627e745a828d9f8d4a98e
|