aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-12-01Merge #20207: Follow-up extra comments on taproot code and testsMarcoFalke
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
2020-12-01Merge #20425: fuzz: Make CAddrMan fuzzing harness deterministicMarcoFalke
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
2020-12-01Merge #20478: Don't set BDB flags when configuring withoutJonas Schnelli
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
2020-12-01Merge #20496: build: Drop unneeded macOS framework dependenciesJonas Schnelli
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
2020-12-01Merge #20512: doc: Add bash as an OpenBSD dependencyJonas Schnelli
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
2020-12-01Merge bitcoin-core/gui#137: refactor: Replace deprecated ↡Jonas Schnelli
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
2020-12-01Merge #20222: refactor: CTxMempool constructor clean upMarcoFalke
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
2020-12-01Merge #20494: refactor: Move node and wallet code out of src/interfacesMarcoFalke
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
2020-12-01Merge #20519: Handle rename failure in DumpMempool(...) by using the ↡MarcoFalke
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
2020-11-30Merge #20499: Remove obsolete NODISCARD ifdef forest. Use [[nodiscard]] (C++17).MarcoFalke
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
2020-11-30Merge #20491: refactor: Drop noop gcc version checksfanquake
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
2020-11-30Merge #20471: build: use C++17 in dependsfanquake
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
2020-11-28Merge #20522: [test] Fix sync issue in disconnect_p2psMarcoFalke
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
2020-11-28Merge bitcoin-core/gui#138: unlock encrypted wallet "OK" button bugfixMarcoFalke
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
2020-11-28Merge #20448: RPC/Wallet: unloadwallet: Allow specifying wallet_name param ↡MarcoFalke
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
2020-11-27[test] Fix wait condition in disconnect_p2psAmiti Uttarwar
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.
2020-11-27qt: unlock wallet "OK" button bugfixMichael Dietz
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.
2020-11-27Add [[nodiscard]] to RenameOver(...)practicalswift
2020-11-27Handle rename failure in DumpMempool(...) by using RenameOver(...) return valuepracticalswift
2020-11-27Merge #20497: [Refactor] Add MAX_STANDARD_SCRIPTSIG_SIZE to policyMarcoFalke
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
2020-11-27refactor: Replace deprecated Qt::SystemLocale{Short,Long}DateHennadii Stepanov
2020-11-26Mention units of MAX_STANDARD_ policy constantsPieter Wuille
2020-11-26Mention in validation that IsWitnessStandard tests for P2TRPieter Wuille
2020-11-26Clean up assets test minimizer LDFLAGSPieter Wuille
2020-11-26Document additional IsWitnessStandard behaviorPieter Wuille
2020-11-26Add comments on CPubKey::IsValidPieter Wuille
2020-11-26Add comments to VerifyTaprootCommitmentPieter Wuille
2020-11-26Document need_vin_vout_mismatch argument to make_spenderPieter Wuille
2020-11-26Fix and improve taproot_construct commentsPieter Wuille
2020-11-26doc: Add bash as an OpenBSD dependencyEmil Engler
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.
2020-11-26Remove NODISCARDpracticalswift
2020-11-26scripted-diff: Use [[nodiscard]] (C++17) instead of NODISCARDpracticalswift
-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-
2020-11-25Add MAX_STANDARD_SCRIPTSIG_SIZE to policysanket1729
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
2020-11-25build: Drop unneeded IOKit framework dependencyHennadii Stepanov
2020-11-25build: Drop unneeded ApplicationServices framework dependencyHennadii Stepanov
2020-11-25Merge #19337: sync: detect double lock from the same threadWladimir J. van der Laan
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
2020-11-25Merge #20489: CI msvc: only build vcpkg dependencies for release (not debug) ↡MarcoFalke
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
2020-11-25Merge #19387: span: update constructors to match c++20 draft spec and add ↡MarcoFalke
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
2020-11-25Drop noop gcc version checksHennadii Stepanov
Since #20413 the minimum required GCC version is 7. Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
2020-11-25Merge #20403: wallet: upgradewallet fixes, improvements, test coverageMarcoFalke
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
2020-11-25This change to the appveyor CI config for msvc builds reverses a change ↡Aaron Clauson
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.
2020-11-25Merge #20469: build: Avoid secp256k1.h include from systemWladimir J. van der Laan
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
2020-11-25Merge #20456: test: Fix intermittent issue in mempool_compatibilityMarcoFalke
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
2020-11-25Merge #20410: wallet: Do not treat default constructed types as None-typeMarcoFalke
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
2020-11-24Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cppRussell Yanofsky
2020-11-24Move ChainImpl from interfaces/chain.cpp to node/interfaces.cppRussell Yanofsky
No changes to ChainImpl or any related classes (review with `git diff --color-moved=dimmed_zebra`)
2020-11-24Move NodeImpl from interfaces/node.cpp to node/interfaces.cppRussell Yanofsky
2020-11-24Don't set BDB flags when configuring withoutJonas Schnelli
2020-11-24Merge #20447: depends: Patch qt_intersect_spans to avoid non-deterministic ↡fanquake
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
2020-11-24Merge #20472: test: Add testing of ParseInt/ParseUInt edge cases with ↡MarcoFalke
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