Age | Commit message (Collapse) | Author |
|
2d8099c713dfd4b546150fd53c2e4f364b9009f4 Mention units of MAX_STANDARD_ policy constants (Pieter Wuille)
84e29c7c0141b52044020ec0c5dfa8a462b7e97f Mention in validation that IsWitnessStandard tests for P2TR (Pieter Wuille)
f867cbcc268a3bfaeef5510a7e40e6d3c0818b6d Clean up assets test minimizer LDFLAGS (Pieter Wuille)
ea0e78677bdbe3313f594118c500cf7784c56970 Document additional IsWitnessStandard behavior (Pieter Wuille)
6040de9a46725826330cd63cdf76e2121a18e728 Add comments on CPubKey::IsValid (Pieter Wuille)
8dbb7de67ce0a71f5fc54289c0ff048ac8dd0acc Add comments to VerifyTaprootCommitment (Pieter Wuille)
cdf900cbf26db05c7edb398ea645f1d23049d810 Document need_vin_vout_mismatch argument to make_spender (Pieter Wuille)
18246ed5f09dd078fa1410b7ec2ba4379cc5e032 Fix and improve taproot_construct comments (Pieter Wuille)
Pull request description:
Addressing some review comments raised here: https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-512238027 and https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-513499921
ACKs for top commit:
jonatack:
ACK 2d8099c per `git range-diff 5009159 4f10965 2d8099c`
ariard:
ACK 2d8099c, only changes are comment light improvements on IsValid/IsWitnessStandard.
Tree-SHA512: c4881546c379ea8efc7ef99a43cbf3b9cd3f9dde5fd97a07ee66f2b593c78aef0bd8784853c5c9c737b66c269241a1048bbbdd6c964a3d872efd8ba0ec410b68
|
|
17a5f172fa9ec509b1c3f950ee8dfb6f025534d2 fuzz: Make addrman fuzzing harness deterministic (practicalswift)
Pull request description:
Make `CAddrMan` fuzzing harness deterministic.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
Crypt-iQ:
utACK 17a5f172fa9ec509b1c3f950ee8dfb6f025534d2
Tree-SHA512: 725f983745233e9b616782247fa18847e483c074ca4336a5beea8a9009128c3a74b4d50a12662d8ca2177c2e1fc5fc121834df6b459ac0af43c931d77ef7c4d8
|
|
982e548a9a78b1b0abad59b54c780b6b06570452 Don't set BDB flags when configuring without (Jonas Schnelli)
Pull request description:
Configuring `--without-bdb` on MacOS leads to a compile error (when BerkeleyDB is not installed). `brew --prefix berkeley-db4` always reports the target directory (even if not installed).
This PR prevents BDB_CFLAGS (et al) from being populated when configuring `--without-bdb`
```
ld: warning: directory not found for option '-L/Users/user/Documents/homebrew/Cellar/berkeley-db@4/4.8.30/lib'
ld: warning: directory not found for option '-L/Users/user/Documents/homebrew/Cellar/berkeley-db@4/4.8.30/lib'
ld: library not found for -ldb_cxx-4.8
ld: library not found for -ldb_cxx-4.8
```
ACKs for top commit:
promag:
Tested ACK 982e548a9a78b1b0abad59b54c780b6b06570452.
hebasto:
ACK 982e548a9a78b1b0abad59b54c780b6b06570452, tested on macOS 11 Big Sur.
Tree-SHA512: f8ca0adca0e18e2de4c0f99d5332cba70d957a9d31a357483b43dcf61c2ed4749d223eabadd45fdbf3ef0781c6b37217770e9aa935b5207eaf7f87c5bdfe9e95
|
|
ec4a46dd9c158e1a0de144e9abb260da941aa702 build: Drop unneeded IOKit framework dependency (Hennadii Stepanov)
65afe4cb69e80278de0f584b6eeb6741f02a31eb build: Drop unneeded ApplicationServices framework dependency (Hennadii Stepanov)
Pull request description:
Bitcoin Core codebase does not contain direct dependencies on the `ApplicationServices` and `IOKit` frameworks.
ACKs for top commit:
jonasschnelli:
utACK ec4a46dd9c158e1a0de144e9abb260da941aa702
practicalswift:
cr ACK ec4a46dd9c158e1a0de144e9abb260da941aa702: patch looks correct!
promag:
Tested ACK ec4a46dd9c158e1a0de144e9abb260da941aa702 (not depends build).
Tree-SHA512: 47b5ad87d761992850133a921f07d485565c70ba2909a3289050f406e6dbd39ad49e1aeeb6cad79c6914385a72ddffd273dfadd69259a35545a13cd17d0e5043
|
|
1d578c078f0ce00cb032d3c6c689fd199b8d2f35 doc: Add bash as an OpenBSD dependency (Emil Engler)
Pull request description:
If we require Python for the test framework, we should also require
bash. It is required for the linters and other scripts and does not
comes in a default OpenBSD installation.
ACKs for top commit:
practicalswift:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
RiccardoMasutti:
ACK 1d578c0
theStack:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
Tree-SHA512: ef14e801983093a99d4c28d134adbc589ca06e5437bf1ff34f8e593968c59793e9d99e8a48f577f847acdfc5185e193276526894b4c632c59d66d87939977910
|
|
Qt::SystemLocale{Short,Long}Date
86b1ab64b1a5b56518787ef16ea54ddbbc97d83e refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date (Hennadii Stepanov)
Pull request description:
As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.
Required for https://github.com/bitcoin/bitcoin/pull/20182.
Details in Qt docs:
- https://doc.qt.io/qt-5/qdatetime.html#toString-1
- https://doc.qt.io/qt-5/qdate.html#toString-1
ACKs for top commit:
jarolrod:
Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1
jonasschnelli:
Tested ACK 86b1ab64b1a5b56518787ef16ea54ddbbc97d83e
Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
|
|
f15e780b9e57554c723bc02aa41150ecf3e3a8c9 refactor: Clean up CTxMemPool initializer list (Elle Mouton)
e3310692d0e9720e960b9785274ce1f0b58b4cd7 refactor: Make CTxMemPool::m_check_ratio a const and a constructor argument (Elle Mouton)
9d4b4b2c2c49774523de740d6492ee5b1ee15e74 refactor: Avoid double to int cast for nCheckFrequency (Elle Mouton)
Pull request description:
This PR cleans up the CTxMemPool interface by including the ratio used to determine when a mempool sanity check should run in the constructor of CTxMempool instead of using nCheckFrequency which required a cast from a double to a uint32_t. Since nCheckFrequency (now called m_check_ratio) is set in the constructor and only every read from there after, it can be turned into a const and no longer needs to be guarded by the 'cs' lock.
Since nCheckFrequency/m_check_ratio no longer needs to lock the 'cs' mutux, mutex lock line in the "CTxMempool::check" function can be moved below where the m_check_ratio variable is checked. Since the variable is 0 by default (meaning that "CTxMempool::check" will most likely not run its logic) this saves us from unnecessarily grabbing the lock.
ACKs for top commit:
jnewbery:
utACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9
MarcoFalke:
ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9 π
glozow:
utACK https://github.com/bitcoin/bitcoin/pull/20222/commits/f15e780b9e57554c723bc02aa41150ecf3e3a8c9
theStack:
Code Review ACK f15e780b9e57554c723bc02aa41150ecf3e3a8c9
Tree-SHA512: d83f3b5311ca128847b621e5e999c7e1bf0f4e6261d4cc090fb13e229a0f7eecd66ad997f654f50a838baf708d1515740aa3bffc244909a001d01fd5ae398b68
|
|
629a9299b2a7241a3fa7d597cb34abcbe1af9255 Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cpp (Russell Yanofsky)
2a26771d8161d30be1853a35acfee588cce03634 Move ChainImpl from interfaces/chain.cpp to node/interfaces.cpp (Russell Yanofsky)
12bd0fc9d70333c54c83ebb08c734272dbd330c2 Move NodeImpl from interfaces/node.cpp to node/interfaces.cpp (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
Move `NodeImpl` from `interfaces/node.cpp` to `node/interfaces.cpp`
Move `ChainImpl` from `interfaces/chain.cpp` to `node/interfaces.cpp`
Move `WalletImpl` from `interfaces/wallet.cpp` to `wallet/interfaces.cpp`
No changes to any classes (can review with `git diff --color-moved=dimmed_zebra`)
Motivation for this change is to move node and wallet code to respective directories where it might fit in better than `src/interfaces/`, but also to remove all unnecessary code from `src/interfaces/` to unblock #19160 review, which has been hung up partially because of code organization. Building on top of this PR, #19160 should now be able to organize interface implementations more understandably in `src/node/` `src/wallet/` `src/ipc/` and `src/init/` directories instead of having so much functionality all in `src/interfaces/`
ACKs for top commit:
promag:
Code review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255.
MarcoFalke:
review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255 πΊ
Tree-SHA512: 87c2b8fd51519bbd4e5ad3539a79debcf88c3bf021eb28c63f3f555186538b62a0c4cc1a3f07cfb4ff13aea8b0b2fdde505d81f22a5e5fd12a6e375b55a92ab8
|
|
RenameOver(...) return value. Add [[nodiscard]] to RenameOver(...).
ce9dd45422e1f4ecce6df68da086b8bfc2100756 Add [[nodiscard]] to RenameOver(...) (practicalswift)
9429a398e291a1b5edcfc657b94fcaf52cd1d8f9 Handle rename failure in DumpMempool(...) by using RenameOver(...) return value (practicalswift)
Pull request description:
Handle rename failure in `DumpMempool(...)` by using the `RenameOver(...)` return value.
Add `[[nodiscard]]` to `RenameOver(...)` to reduce the risk of similar rename issues in the future.
ACKs for top commit:
vasild:
ACK ce9dd454
theStack:
ACK ce9dd45422e1f4ecce6df68da086b8bfc2100756 π·οΈ
Tree-SHA512: 1e63d7f3061e1f6ea2df5750dbc1547a39bd50b6c529812a0c8a0c11d3100c241afdf14094e69b69a38bade7e54a12b2a42888545874398eaf5d02421b57e874
|
|
79bff8e48aca961ec271b0d592aca9278b981e2f Remove NODISCARD (practicalswift)
4848e711076c6ebc5d841feb83baeb6d2bc76c94 scripted-diff: Use [[nodiscard]] (C++17) instead of NODISCARD (practicalswift)
Pull request description:
Remove obsolete `NODISCARD` `ifdef` forest. Use `[[nodiscard]]` (C++17).
ACKs for top commit:
theStack:
ACK 79bff8e48aca961ec271b0d592aca9278b981e2f
fanquake:
ACK 79bff8e48aca961ec271b0d592aca9278b981e2f
Tree-SHA512: 56dbb8e50ed97ecfbce28cdc688a01146108acae49a943e338a8f983f7168914710d36e38632f6a7c200ba6c6ac35b2519e97d6c985e8e7eb23223f13bf985d6
|
|
830ddf413934226d0b6ca99165916790cc52ca18 Drop noop gcc version checks (Hennadii Stepanov)
Pull request description:
Since #20413 the minimum required GCC version is 7.
ACKs for top commit:
fanquake:
ACK 830ddf413934226d0b6ca99165916790cc52ca18
Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964
|
|
2f5dfe4a7ff12b6b57427374142cdf7e266b73bc depends: build qt in c++17 mode (fanquake)
104e859c9755aee5708ea1934454d88b10c266ff builds: don't pass -silent to qt when building in debug mode (fanquake)
e2c500636cb767347ae2b913345788ad3c3e8279 depends: build zeromq with -std=c++17 (fanquake)
2374f2fbef4359476fe3184e2402a2cc741cefad depends: build Boost with -std=c++17 (fanquake)
2dde55702da30ea568cac8a1d1cbddd652d6958e depends: build bdb with -std=c++17 (fanquake)
Pull request description:
In packages where we are passing `-std=c++11` switch to `-std=c++17`, or, `-std=c++1z` in the case of Qt.
This PR also contains a [commit](https://github.com/bitcoin/bitcoin/commit/104e859c9755aee5708ea1934454d88b10c266ff) that improves debug output when building Qt for debugging (`DEBUG=1`).
Now we'll get output like this:
```bash
g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 -fno-exceptions <lots more> ../../corelib/kernel/qcoreapplication.cpp
```
rather than just:
```bash
compiling ../../corelib/kernel/qcoreapplication.cpp
```
Note that when you look at the DEBUG output for these changes when building Qt, you'll see objects being compiled with a mix of C++11 and C++17. The breakdown is roughly:
1. `qmake` built with `-std=c++11`:
```bash
Creating qmake...
make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/qmake'
g++ -c -o project.o -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/qmake/project.cpp
# when qmake, Qt also builds some of it's corelib, such as corelib/global/qmalloc.cpp
g++ -c -o qmalloc.o -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/src/corelib/global/qmalloc.cpp
```
2. `qmake` is run, and passed our build options, including `-c++std`:
```bash
make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase'
<trim>qt/5.9.8-4110fa99945/qtbase/bin/qmake -o Makefile qtbase.pro -- -bindir <trim>/native/bin -c++std c++1z -confirm-license <trim>
```
3. After some cleaning and configuring, we actually start to build Qt, as well as it's tools and internal libs:
```bash
Building qt...
make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src'
# build libpng, zlib etc
gcc -c -m64 -pipe -pipe -O1 <trim> -o .obj/png.o png.c
# build libQt5Bootstrap, using C++11, which again compiles qmalloc.cpp
make[2]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src/tools/bootstrap'
g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 <trim> -o .obj/qmalloc.o ../../corelib/global/qmalloc.cpp
# build a bunch of tools like moc, rcc, uic, qfloat16-tables, qdbuscpp2xml, using C++11
g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/rcc.o rcc.cpp
# from here, Qt is compiled with -std=c++1z, including qmalloc.cpp, for the third and final time:
g++ -c -include .pch/Qt5Core <trim> -g -Og -fPIC -std=c++1z -fvisibility=hidden <trim> -o .obj/qmalloc.o global/qmalloc.cpp
```
4. Finally, build tools like `lrelease`, `lupdate`, etc, but back to using -std=c++11
```bash
make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qttools/src/linguist/lrelease'
g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/translator.o ../shared/translator.cpp
```
If you dump the debug info from the built Qt libs, they should also tell you that they were compiled with `C++17`:
```bash
objdump -g bitcoin/depends/x86_64-pc-linux-gnu/lib/libQt5Core.a
GNU C++17 9.3.0 -m64 -mtune=generic -march=x86-64 -g -O1 -Og -std=c++17 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection
```
ACKs for top commit:
laanwj:
Code review ACK https://github.com/bitcoin/bitcoin/pull/20471/commits/2f5dfe4a7ff12b6b57427374142cdf7e266b73bc
practicalswift:
cr ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc: patch looks correct
fjahr:
Code review ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc
hebasto:
ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: fc5e9d7c7518c68349c8228fb1aead829850373efc960c9b8c079096a83d1dad19c62a9730fce5802322bf07e320960fd47851420d429eda0a87c307f4e8b03a
|
|
3ebde2143aa98af213872b98b474d904e55056f7 [test] Fix wait condition in disconnect_p2ps (Amiti Uttarwar)
Pull request description:
#19315 currently has a [test failure](https://cirrus-ci.com/task/4545582645641216) because of a race. `disconnect_p2ps` is intended to have a `wait_until` clause that prevents this race, but the conditional doesn't match since its comparing two different object types. `MY_SUBVERSION` is defined in messages.py as a byte string, but is compared to the value returned by the RPC. This PR simply converts types to ensure they match, which should prevent the race from occurring.
HUGE PROPS TO jnewbery for discovering the issue π
ACKs for top commit:
jnewbery:
ACK 3ebde2143aa98af213872b98b474d904e55056f7
glozow:
Code review ACK https://github.com/bitcoin/bitcoin/pull/20522/commits/3ebde2143aa98af213872b98b474d904e55056f7
Tree-SHA512: ca096b80a3e4d757a645f38846d6dc89d6a3d35c3435513a72d278e305faddd4aff9e75a767941b51b2abbf59c82679bac1e9a0140d6f285efe3053e51bcc2a8
|
|
8008ef770f3d0b14d03e22371314500373732143 qt: unlock wallet "OK" button bugfix (Michael Dietz)
Pull request description:
When trying to send a transaction from an encrypted wallet, the ask
passphrase dialog would not allow the user to click the "OK" button
and proceed. Therefore it was impossible to send a transaction
through the gui. It was not enabling the "OK" button after the
passphrase was entered by the user, because it was using the same
form validation logic as the "Change passphrase" flow.
I reported this in a comment in https://github.com/bitcoin-core/gui/issues/136. But then I realized this seems to be a flat out bug.
ACKs for top commit:
MarcoFalke:
review ACK 8008ef770f3d0b14d03e22371314500373732143
hebasto:
ACK 8008ef770f3d0b14d03e22371314500373732143, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: cc09b34c7f3aea09729e1c7ccccff05dc11fec56fee2ad369f2d862979572b1edd8b7e738ffe6e91d35d071b819b0c3e0f5d48bf5e27427a80af4a28893f8aaf
|
|
matching RPC endpoint wallet
89bdad5b25ae4ac03a486f729a5b58ae6f21946d RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint (Luke Dashjr)
Pull request description:
Allow specifying the `wallet_name` param to `unloadwallet` on RPC wallet endpoints, so long as it matches the endpoint wallet.
ACKs for top commit:
jonatack:
ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
MarcoFalke:
review ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
|
|
MY_SUBVERSION is defined in messages.py as a byte string, but here we were
comparing this value to the value returned by the RPC. Convert to ensure the
types match.
|
|
When trying to send a transaction from an encrypted wallet, the ask
passphrase dialog would not allow the user to click the "OK" button
and proceed. Therefore it was impossible to send a transaction
through the gui. It was not enabling the "OK" button after the
passphrase was entered by the user, because it was using the same
form validation logic as the "Change passphrase" flow.
|
|
|
|
|
|
e416cfc92bf51f6fd088ab61c2306c5e73877dd0 Add MAX_STANDARD_SCRIPTSIG_SIZE to policy (sanket1729)
Pull request description:
Bitcoin core has a standardness rule for max satisfaction script sig size.
This PR adds to the policy header file so that it is documented along with
along policy rules. The initial reasoning that 1650 is an implicit
limit(would not reach assuming all other policy rules are being
followed) is outdated.
As we now know, bitcoin transactions can have spend conditions are more than
just signatures and there may exist p2sh transactions involving 100 byte
preimages that maybe non-standard because of this rule. Because this
rule is no longer implicit, we should explicitly document it in policy
header file
ACKs for top commit:
sipa:
utACK e416cfc92bf51f6fd088ab61c2306c5e73877dd0
practicalswift:
cr ACK e416cfc92bf51f6fd088ab61c2306c5e73877dd0
theStack:
Code Review ACK e416cfc92bf51f6fd088ab61c2306c5e73877dd0
Tree-SHA512: 1a91ee23dfb6085807e04dd0687d7a443e0f3e0f52d0a995a6599dff28533b0b599afba2724735d93948a64a3e25d0bc016ce3e771c0bd453eef78b22dc2369d
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
If we require Python for the test framework, we should also require
bash. It is required for the linters and other scripts and does not
comes in a default OpenBSD installation.
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i "s/NODISCARD/[[nodiscard]]/g" $(git grep -l "NODISCARD" ":(exclude)src/bench/nanobench.h" ":(exclude)src/attributes.h")
-END VERIFY SCRIPT-
|
|
Bitcoin core has a standardness rule for max satisfaction script sig size.
This PR adds to the policy header file so that it is documented along with
along policy rules. The initial reasoning that 1650 is an implicit
limit(would not reached assuming all other policy rules are being
followed) is outdated.
As we now know, bitcoin transactions can have spend conditions are more than
just signatures and there may exist p2sh transactions involving 100 byte
preimages that maybe non-standard because of this rule. Because this
rule is no longer implicit, we should explicitly document it in policy
header file
|
|
|
|
|
|
95975dd08d8fdaaeaf28e0d06b861ce2748c17b6 sync: detect double lock from the same thread (Vasil Dimov)
4df6567e4cbb4677e8048de2f8008612e1b860b9 sync: make EnterCritical() & push_lock() type safe (Vasil Dimov)
Pull request description:
Double lock of the same (non-recursive) mutex from the same thread would produce an undefined behavior. Detect this from `DEBUG_LOCKORDER` and react similarly to the deadlock detection.
This came up during discussion in another, related PR: https://github.com/bitcoin/bitcoin/pull/19238#discussion_r442394521.
ACKs for top commit:
laanwj:
code review ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
hebasto:
re-ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
Tree-SHA512: 375c62db7819e348bfaecc3bd82a7907fcd8f5af24f7d637ac82f3f16789da9fc127dbd0e37158a08e0dcbba01a55c6635caf1d8e9e827cf5a3747f7690a498e
|
|
to reduce build times
fa18e7cbc5ea6aaba94dca4ebdc850c9db141f89 This change to the appveyor CI config for msvc builds reverses a change introduced in #19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. (Aaron Clauson)
Pull request description:
This change to the appveyor CI config for msvc builds reverses a change introduced in #19960. It re-applies a setting to inform vcpkg to only build release versions of the dependencies rather than the default of debug and release.
It had been expected that the vcpkg manifest mechanism introduced in #19960 would do this automatically but it turns out not to be the case.
ACKs for top commit:
MarcoFalke:
ACK fa18e7cbc5ea6aaba94dca4ebdc850c9db141f89 if green
hebasto:
ACK fa18e7cbc5ea6aaba94dca4ebdc850c9db141f89, AppVeyor build takes 5 minutes less.
Tree-SHA512: 427e7e78190c20e0d85dad9b29beed2b6fa13f99c6bc72bcc1839dfb51237a7cc785ab707b4f851c527c1bb0d3e7ebad9e640969e19d29778584bbaeec75cecf
|
|
lifetimebound attribute
e3e7446305329ce96e9cf5f5161658eb2e1ea888 Add lifetimebound to attributes for general-purpose usage (Cory Fields)
1d58cc7cb040a70f768b632f294db4e0797d3a34 span: add lifetimebound attribute (Cory Fields)
62733fee874bfe7e833e71380eb8efd6a3126fbd span: (almost) match std::span's constructor behavior (Cory Fields)
Pull request description:
Replaces #19382 with a different approach. See [this comment](https://github.com/bitcoin/bitcoin/pull/19382#discussion_r446332852) for the reasoning behind the switch.
--
Description from #19382:
See [here](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0936r0.pdf) for more detail on lifetimebound.
This is implemented using preprocesor macros rather than configure checks in order to keep span.h self-contained.
The ```[[clang::lifetimebound]]``` syntax was chosen over ```__attribute__((lifetimebound))``` because the former is more flexible and works to guard ```this``` as well as function parameters, and also because at least for now, it's available only in clang.
There are currently no violations in our codebase, but this can easily be tested by inserting one like this somewhere and compiling with a modern clang:
```c++
Span<const int> bad(std::vector<int>{1,2,3});
```
The result:
> warning: temporary whose address is used as value of local variable 'bad' will be destroyed at the end of the full-expression [-Wdangling]
Span<const int> bad(std::vector<int>{1,2,3});
```
ACKs for top commit:
sipa:
ACK e3e7446305329ce96e9cf5f5161658eb2e1ea888
ajtowns:
ACK e3e7446305329ce96e9cf5f5161658eb2e1ea888 (drive by; only a quick skim of code and some basic sanity checks)
MarcoFalke:
review ACK e3e7446305329ce96e9cf5f5161658eb2e1ea888 π
jonatack:
ACK e3e7446 change since last review is adding `[[clang::lifetimebound]]` as `LIFETIMEBOUND` to src/attributes.h as suggested in https://github.com/bitcoin/bitcoin/pull/19387#issuecomment-650752959.
Tree-SHA512: 05a3440ee595ef0e8d693a2820b360707695c016a68e15df47c20cd8d053646cc6c8cca8addd7db40e72b3fce208879a41c8102ba7ae9223e4366e5de1175211
|
|
Since #20413 the minimum required GCC version is 7.
Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
|
|
3eb6f8b2e61c24a22ea9396d86672307845f35eb wallet (not for backport): improve upgradewallet error messages (Jon Atack)
ca8cd893bb56bf5d455154b0498b1f58f77d20ed wallet: fix and improve upgradewallet error responses (Jon Atack)
99d56e357159c7154f69f28cb5587c5ca20d6594 wallet: fix and improve upgradewallet result responses (Jon Atack)
2498b04ce88696a3216fc38b7d393906b733e8b1 Don't upgrade to HD split if it is already supported (Andrew Chow)
c46c18b788cb0862aafbb116fd37936cbed6a431 wallet: refactor GetClosestWalletFeature() (Jon Atack)
Pull request description:
This follows up on #18836 and #20282 to fix and improve the as-yet unreleased `upgradewallet` feature and also implement review follow-up in https://github.com/bitcoin/bitcoin/pull/18836#discussion_r519328607.
This PR fixes 4 upgradewallet issues:
- this bug: https://github.com/bitcoin/bitcoin/pull/20403#discussion_r526063920
- it returns nothing in the absence of an RPC error, which isn't reassuring for users
- it returns the same thing both in the case of a successful upgrade and when no upgrade took place
- the error message object is currently dead code
This PR fixes the above and provides:
...user feedback to not silently return without upgrading
```
{
"wallet_name": "disable private keys",
"previous_version": 169900,
"current_version": 169900,
"result": "Already at latest version. Wallet version unchanged."
}
```
...better feedback after successfully upgrading
```
{
"wallet_name": "watch-only",
"previous_version": 159900,
"current_version": 169900,
"result": "Wallet upgraded successfully from version 159900 to version 169900."
}
```
...helpful error responses
```
{
"wallet_name": "blank",
"previous_version": 169900,
"current_version": 169900,
"error": "Cannot downgrade wallet from version 169900 to version 159900. Wallet version unchanged."
}
{
"wallet_name": "blank",
"previous_version": 130000,
"current_version": 130000,
"error": "Cannot upgrade a non HD split wallet from version 130000 to version 169899 without upgrading to support pre-split keypool. Please use version 169900 or no version specified."
}
```
updated help:
```
upgradewallet ( version )
Upgrade the wallet. Upgrades to the latest version if no version number is specified.
New keys may be generated and a new wallet backup will need to be made.
Arguments:
1. version (numeric, optional, default=169900) The version number to upgrade to. Default is the latest wallet version.
Result:
{ (json object)
"wallet_name" : "str", (string) Name of wallet this operation was performed on
"previous_version" : n, (numeric) Version of wallet before this operation
"current_version" : n, (numeric) Version of wallet after this operation
"result" : "str", (string, optional) Description of result, if no error
"error" : "str" (string, optional) Error message (if there is one)
}
```
ACKs for top commit:
achow101:
ACK 3eb6f8b
MarcoFalke:
review ACK 3eb6f8b2e61c24a22ea9396d86672307845f35eb π‘
Tree-SHA512: b767314069e26b5933b123acfea6aa40708507f504bdb22884da020a4ca1332af38a7072b061e36281533af9f4e236d94d3c129daf6fe5b55241127537038eed
|
|
introduced in #19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release.
It had been expected that the vcpkg manifest mechanism introduced in #19960 would do this automatically but it turns out not to be the case.
|
|
e95aaefe2540cb76969818fcc2ff77d33448ed5a build: Avoid secp256k1.h include from system (Niklas GΓΆgge)
Pull request description:
While building i ran into an error because i had a version of `secp256k1.h` under `/usr/local/include` that was incompatible with the secp256k1 code in the repository. This caused a problem because `$(BOOST_CPPFLAGS)` contained `-I/usr/local/include` and the include paths are searched by the compiler in order from left to right, so in the end `$(BITCOIN_INCLUDES)` contained `-I/usr/local/include` before `-I$(srcdir)/secp256k1/include` which caused the compiler to find `secp256k1.h` under `/usr/local/include`.
Looking at git blame i am wondering how this has not happened to anyone else in several years: https://github.com/bitcoin/bitcoin/blame/cb89e18845852c005ba8ab94acc6544d7c876a42/src/Makefile.am#L25
I am on macOS 10.15.
ACKs for top commit:
laanwj:
Code review ACK e95aaefe2540cb76969818fcc2ff77d33448ed5a
hebasto:
ACK e95aaefe2540cb76969818fcc2ff77d33448ed5a, tested on macOS 11 Big Sur by adding `#error` into `/usr/local/include/secp256k1.h`.
Tree-SHA512: 1f0b395725936c179ab60dee3582ec7b21e2f9c0f1895e160d84a487cf0db16d0c7aa47d05800e0aded31685b4362056cac9b9ecca1bb8c308a4c5a810e8dc1d
|
|
fa05d19bd6ba619bb3f9aabc05c439cd18d34544 test: Fix intermittent issue in mempool_compatibility (MarcoFalke)
Pull request description:
Fixes https://cirrus-ci.com/task/5141306890518528?command=ci#L6076
The version is too old to understand getmempoolinfo()[loaded], so it is never called when starting (See https://cirrus-ci.com/task/5141306890518528?command=ci#L5541)
ACKs for top commit:
achow101:
ACK fa05d19bd6ba619bb3f9aabc05c439cd18d34544
Tree-SHA512: e912d5dff6236d2d4ac608f9f3b3e255cc2b611f36d79bfe4e2a940709833a947c682d7703327887e1753eab30b95eb2615d7e2c21ce4bca4f089e717cbb51c4
|
|
fa69c2c78455fd0dc436018fece9ff7fc83a180d wallet: Do not treat default constructed types as None-type (MarcoFalke)
fac4e136fa3d0fab7fde900a6be921313e16e7a6 refactor: Change pointer to reference because it can not be null (MarcoFalke)
Pull request description:
Equating `0==None` and `""==None` is confusing, unneeded and undocumented
ACKs for top commit:
jonatack:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
achow101:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
Sjors:
tACK fa69c2c78455fd0dc436018fece9ff7fc83a180d modulo `unset`
Tree-SHA512: c4c8d0ad80c6697621d356a9545caf28ca2facc82bb2fa8e70eceb52372d25f0685237c73688c4b01da0e75d213c77c0d45011a8bdfe81ea783d85f045786dac
|
|
|
|
No changes to ChainImpl or any related classes (review with `git diff --color-moved=dimmed_zebra`)
|
|
|
|
|
|
behavior in LLVM 8
8f7d1b39efbe65ab2747c593cc3560d4a449a333 Fix QPainter non-determinism on macOS (Andrew Chow)
Pull request description:
Aplies a patch to Qt that fixes the non-determinism by modifying Qt. The source of the non-determinism is how LLVM 8 optimizes qt_intersect_spans when compiling. The particular optimization that seems to be causing the problems is that a temp variable is being added for spans->y. For some reason, when it does this, it chooses different instructions to use when making that variable. We bypass this problem by patching qt_intersect_spans to always make and use this local variable.
Potential alternative to #20436 and #20440
ACKs for top commit:
hebasto:
re-ACK 8f7d1b39efbe65ab2747c593cc3560d4a449a333 ~for merging into the 0.21 branch, but [not into the master](https://github.com/bitcoin/bitcoin/pull/20454) branch.~
fanquake:
ACK 8f7d1b39efbe65ab2747c593cc3560d4a449a333
Tree-SHA512: b0d00a77643554021736524fb64611462ef2ec849a220543c12d99edb0f52f2e8128d2cc61fa82176b7e13b294574774a92d6b649badf8b7630c6d6a7e70ce10
|
|
leading +/-/0:s
05c10953887bd78af2e21ef6d3c07f90dd885572 test: Add testing of ParseInt/ParseUInt edge cases with leading +/-/0:s (practicalswift)
Pull request description:
Add testing of `ParseInt`/`ParseUInt` edge cases with leading `+`/`-`/`0`:s.
Context: While working on #20457 and #20452 I noticed some edge cases which our unit tests are currently not covering.
ACKs for top commit:
MarcoFalke:
review ACK 05c10953887bd78af2e21ef6d3c07f90dd885572
laanwj:
Code review ACK 05c10953887bd78af2e21ef6d3c07f90dd885572
jonatack:
ACK 05c10953887bd78af2e21ef6d3c07f90dd885572
promag:
Code review ACK 05c10953887bd78af2e21ef6d3c07f90dd885572.
Tree-SHA512: bdfb94d8fa0293512dbba89907cb6dd0f8b1418d878267dd6d49c8c397a0e5b9714441345565d41a6a909a1cda052ef7cccece822f355ff604fcf85f2dc8136f
|