f1d21ef1c370898ae10569f2a8569bd98d71a981 doc: add C++17 release note for 0.21.0 (fanquake)
Pull request description:
TLDR: Mention that the codebase is now compatible with C++17, and that the
intention is to require C++17 starting with 0.22.0.
Following some discussion with Cory/Carl, and in #16684, I think this is the next step in the C++17 migration.
While #16684 mentions a gitian/Guix release with C++17, it's not yet clear how that would be done. Are we just going to pass `--enable-c++17` in gitian/Guix?. Are we changing our default in configure.ac?
According to the [last comment](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-643778757) in #16684, we wouldn't be changing anything in depends:
> No, everything (including depends) will stay at C++11.
However I don't think we want to be mixing C++11 built dependencies, with a C++17 built bitcoind, if there is any potential for compatibility issues.
Instead, I'd suggest we build the 0.21.0 release as C++11, and do a complete switch to C++17 for 0.22.0. Also, if we actually wanted to use C++17 in depends for 0.21.0, we couldn't without breaking C++11 compat (Qt). See below.
Here is a potential timeline/TODOs for the migration:
Potential Timeline
* 17 / 6 / 2020 - Today
* Some time prior to split-off:
* Confirm that compiling with C++17 works.
* Confirm that C++11 compatibility has not been broken.
* 1 / 11 / 2020
* [0.21.0 split off happens](https://github.com/bitcoin/bitcoin/issues/18947).
* 2 / 11 / 2020
* Merge an "incompatible with C++11" change into master.
* Switch configure to use C++17 mode by default.
* Update minimum compiler requirements. At least:
* Clang 5: https://clang.llvm.org/cxx_status.html#cxx17
* GCC 7: https://gcc.gnu.org/projects/cxx-status.html#cxx17
* While GCC has some support from 5, it seems a more complete support landed in GCC 7.
* https://gcc.gnu.org/gcc-7/changes.html#cxx
* Switch depends packages to use C++17 where applicable.
* Bump Qt from 5.9.x (no c++17 mode) to, likely, 5.15.x (LTS).
* Drop support for macOS < 10.14.x
* The c++ dylib shipped with macOS [doesn't support c++17, prior to macOS 10.14](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-643722538).
* Building Qt 5.12 or 5.15 in C++17 mode will also require a minimum macOS deployment target of 10.14. https://codereview.qt-project.org/c/qt/qtbase/+/283832.
* Begin merging PRs like #19183 and #19245.
* I've left some comments in #19183 if the macOS runtime issue interests anyone.
* 3 / 12 / 2020
* 0.21.0 released.
* Built as C++11.
* Contains warning in release notes that compiling 0.22.0 will require C++17.
* 3 / 6 / 2021
* 0.22.0 released.
* Full of C++17 code.
One thing worth noting, is that we cannot bump our Qt to a newer LTS for 0.21.0, without breaking C++11 compatibility. Qt 5.12 is not compilable in C++11 mode, as the project has started using C++14 features throughout at least the macOS portions of it's codebase, and seemingly "forgotten" that the release is meant to be C++11 compatible.
Upstream bug here: https://bugreports.qt.io/browse/QTBUG-77310.
> Building Qt requires C+11, at a minimum, but in practice we use later features, usually under a feature define, or with a fallback of some kind. On platforms that support > C11, we've (apparently) not considered the fallback necessary, under the assumption C+14 is always available.
ACKs for top commit:
MarcoFalke:
ACK f1d21ef1c370898ae10569f2a8569bd98d71a981 can't hurt to give an advance warning
Sjors:
ACK f1d21ef1c370898ae10569f2a8569bd98d71a981
laanwj:
ACK f1d21ef1c370898ae10569f2a8569bd98d71a981
theStack:
ACK f1d21ef1c370898ae10569f2a8569bd98d71a981
Tree-SHA512: 706baceb07d9584783ba6e437cdf447531c20f586285b9797edc21f3adb1e9d386059d1c543c70eb298d0f8e555dafb6682a55d35c5836979fc12132e8ba06f5
20b6e959449d0c07639599b99ba917d2cac62493 test: refactor functional tests to use restart_node (Christopher Coverdale)
Pull request description:
fixes#19345
This PR replaces consecutive calls to `stop_node()` and `start_node()` with `restart_node()` where appropriate in the functional tests.
The commit messages are repetitive but focused on each file changed with the intention of squashing if applicable.
ACKs for top commit:
laanwj:
ACK 20b6e959449d0c07639599b99ba917d2cac62493
Tree-SHA512: 1cfa1fb8c5f01a7b00fe44e80dbef072147f21e3891098817acd4275b0c5d91dc1c787594209e117edd418f2fa3a7b2dfcbafdf87efc07f740040938d641f3a9
ccef5d7bf0af8377c6c779295f7b41d5af435c47 test: add two edge case tests for CSubNet (Vasil Dimov)
Pull request description:
This is chopped off from https://github.com/bitcoin/bitcoin/pull/19031. It is needed because later 19031 modifies the related code and the tests ensure that no surprising changes in behavior sneak in.
ACKs for top commit:
practicalswift:
ACK ccef5d7bf0af8377c6c779295f7b41d5af435c47 -- more test coverage is better than less test coverage :)
laanwj:
ACK ccef5d7bf0af8377c6c779295f7b41d5af435c47
hebasto:
ACK ccef5d7bf0af8377c6c779295f7b41d5af435c47, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 6d386672b6598aeddd33dabe3512e816cf548d5c1af56c4c9e6f897d513b62ba4659cde73405811a0df286ffee3a3f084ab7caf8e3a2086fa9ddecd1bdcb3c67
fac63eb5eabcbbc2e51d414b9cf76f0e897dba1a doc: Remove -whitelistforcerelay from comment (MarcoFalke)
faabd1514fecd828451387b025c1cc74a37bc854 test: Check that peers with forcerelay permission do not get a feefilter message (MarcoFalke)
fad676b8d2dfc3a8a62db3d3395d36d3e3076a5b test: Add connect_nodes method (MarcoFalke)
fac6ef4fb2bbe6187a52d716eab734d0b1e9a221 test: Add test for no net permission (MarcoFalke)
ffff3fe50a16bd7dde3d2d206bbe7bc41c483bb8 test: Replace self.nodes[0].p2p with conn (MarcoFalke)
faccdc8a3143c9849e61312a7f438bc6e8232496 test: remove redundant generate (MarcoFalke)
fab83b934abcd1228ff21afdc9f8b30ad09745fa test: pep-8 p2p_feefilter.py (MarcoFalke)
Pull request description:
ACKs for top commit:
jonatack:
re-ACK fac63eb move-only change of two class member functions in test_framework.py and rebases since my review @ faccf0a per `git range-diff 4b5c919 faccf0a fac63eb`. Verified p2p_feefilter and p2p_permissions functional tests are running 🟢 locally.
Tree-SHA512: 30a1c83baee15a4236d127d199c4f264852045372918d5aa5c09ef3d48041762ce3920ff86ef2466d4b2c792ddf56943d12b16c6dce34c6c5aea2a4af2eb4d49
cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a test: move sync_blocks and sync_mempool functions to test_framework.py (Roy Shao)
Pull request description:
This PR moves `sync_blocks` and `sync_mempool` out from `test_framework/util.py` to `test_framework/test_framework.py` so they can take contextual information of test framework into account.
* Change all reference callers to call functions from `test_framework.py`
* Remove `**kwargs` which is not used
* Take into account of `timeout_factor` when respecting timeout in function implementations.
* Pass all tests by running `./test/functional/test_runner.py`
fixes#18930
ACKs for top commit:
MarcoFalke:
ACK cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a , reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space 💫
Tree-SHA512: a79b2a3fa842fc26a7aacb834bb2aea88b3049916c0b754e60002a77ce94bb5954e0ea3b436bf268e9295efb62d721dfef263a09339a55c684ac3fda388c275e
931dd4760855e036c176a23ec2de367c460e4243 Make lint-spelling.py happy (Glenn Willen)
11a0ffb29d1b4dcc55c8826873f340ab4196af21 [gui] Load PSBT from clipboard (Glenn Willen)
a6cb0b0c29d327d01aebb98b0504f317eb19c3dc [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen)
5dd0c03ffa3aeaa69d8a3a716f902f450d5eaaec FillPSBT: report number of inputs signed (or would sign) (Glenn Willen)
9e7b23b73387600d175aff8bd5e6624dd51f86e7 Improve TransactionErrorString messages. (Glenn Willen)
Pull request description:
Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu item, giving options to sign or broadcast the loaded PSBT as appropriate, as well as copying the result to the clipboard or saving it to a file.
This is based on Sjors' #17509, and depends on that PR going in first. (It effectively replaces the small "load PSBT" dialog from that PR with a more feature-rich one.)
Some notes:
* The way I display status information is maybe unusual (a status bar, rather than messageboxes.) I think it's helpful to have the information in it be persistent rather than transitory. But if people dislike it, I would probably move the "current state of the transaction" info to the top line of the main label, and the "what action just happened, and did it succeed" info into a messagebox.
* I don't really know much about the translation/localization stuff. I put tr() in all the places it seemed like it ought to go. I did not attempt to translate the result of TransactionErrorString (which is shared by GUI and non-GUI code); I don't know if that's correct, but it matches the "error messages in logs should be googleable in English" heuristic. I don't know whether there are things I should be doing to reduce translator effort (like minimizing the total number of distinct message strings I use, or something.)
* I don't really know how (if?) automated testing is applied to GUI code. I can make a list of PSBTs exercising all the codepaths for manual testing, if that's the right approach. Input appreciated.
ACKs for top commit:
instagibbs:
tested ACK 931dd47608
Sjors:
re-tACK 931dd4760855e036c176a23ec2de367c460e4243
jb55:
ACK 931dd4760855e036c176a23ec2de367c460e4243
achow101:
ACK 931dd4760855e036c176a23ec2de367c460e4243
Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
22cb303cf099b430d602384bc92706ce01b4f98d rpc: add missing space in JSON parsing error message, update test (Jon Atack)
bf53ebef061a563cfc4c5857f5d6bc93fb136282 test: add multiwallet tests for bitcoin-cli -generate (Jon Atack)
4b859cfff9965eb07044f4d104398cb0e7ab127e cli: add multiwallet capability to GetNewAddress and -generate (Jon Atack)
18f93545a12db00180cea369a4b5cce7f10cd362 test: add tests for bitcoin-cli -generate (Jon Atack)
4818124137732540383a29835afa2be41aa55ca8 cli: create bitcoin-cli -generate command (Jon Atack)
ff41a3690066081772b172f3c31a63f5fe6ea7ed cli: extract ParseResult() and ParseError() (Jon Atack)
f4185b26d9b2ff2e86c99cdfe3ad9be62bb6299a cli: create GenerateToAddressRequestHandler class (Harris)
f7c65a33508c4bb8e9ed896e150a4fa529a243e5 cli: create GetNewAddress() (Jon Atack)
9be7fd35c5d631c2cc34d3b4fa63ae0a9d5a68ef rpc: make generatetoaddress locals const (Jon Atack)
cb00510dbac99b44f3f2cf6e58bb2e4401c5ef28 rpc: create rpc/mining.h, hoist default max tries values to constant (Jon Atack)
Pull request description:
This PR continues and completes the work begun in #17700 working on issue #16000 to create a client-side version of RPC `generate`.
Basically, `bitcoin-cli -generate` wraps calling `generatenewaddress` followed by `generatetoaddress [nblocks] [maxtries]` and prints the following:
```
$ bitcoin-cli -generate
{
"address": "bcrt1qn4aszr2y2xvpa70y675a76wsu70wlkwvdyyln6"
"blocks": [
"01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5",
]
}
$ bitcoin-cli -rpcwallet=wallet-name -generate 3 100
{
"address": "bcrt1q4cunfw0gnsj7g7e6mk0v0uuvvau9mwr09dj45l",
"blocks": [
"7a6650ca5e0c614992ee64fb148a7e5e022af842e4b6003f81abd8baf1e75136",
"01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5",
"3f8795ec40b1ad812b818c177680841be319a3f6753d4e32dc7dfb5bafe5d00e"
]
}
```
Help doc:
```
$ bitcoin-cli -h | grep -A5 "\-generate"
-generate
Generate blocks immediately, equivalent to RPC generatenewaddress
followed by RPC generatetoaddress. Optional positional arguments
are number of blocks to generate (default: 1) and maximum
iterations to try (default: 1000000), equivalent to RPC
generatetoaddress nblocks and maxtries arguments. Example:
bitcoin-cli -generate 4 1000
```
Quite a bit of test coverage turned out to be needed to cover the change and the different cases (arguments, multiwallet mode) and error-handling.
This PR also improves some things that working on these changes brought to light.
Credit to Harris Brakmić for the initial work in #17700.
ACKs for top commit:
adamjonas:
utACK 22cb303cf099b430d602384bc92706ce01b4f98d
meshcollider:
utACK 22cb303cf099b430d602384bc92706ce01b4f98d
Tree-SHA512: 94f67f632fe093d076f614e0ecff09ce7342ac6e424579200d5211a6615260e438d857861767fb788950ec6da0b26ef56dc8268c430012a3b3d4822b24ca6fbf
cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7 [tools] Update clang-format config (John Newbery)
Pull request description:
In some cases, running clang-format has made code _less_ readable by joining declarations and calls for functions with many arguments into very long lines. For example:
```
- size_t getQueueInfo(std::chrono::system_clock::time_point &first,
- std::chrono::system_clock::time_point &last) const;
+ size_t getQueueInfo(std::chrono::system_clock::time_point& first, std::chrono::system_clock::time_point& last) const;
```
(https://github.com/bitcoin/bitcoin/pull/19090#discussion_r431961148)
This change to clang-format would allow arguments/parameters for func declarations/calls to be split over multiple lines, aligned with the opening parens. It does not force args/params to be on new lines (that setting is `BinPackParameters : true`).
ACKs for top commit:
MarcoFalke:
ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7 fine with me
practicalswift:
ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7
Tree-SHA512: a62474925e71aaff41bdce7960fd5ffd64317da810f694d8084080b054708cf71c2ab2ce3111db5a9260d1c1f9e02d59a2ecb5543b1b6172ce085cb42432160a
bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb doc: release note for rpc getaddressinfo removals (Jon Atack)
90e989390ee50633fff0e4f210a1ea23ff00e012 rpc: getaddressinfo RPCResult fixup (Jon Atack)
a8507c99da10791aa69ca277128e06753942e976 rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack)
645a8653c895e4fc7717e9e5ac045612b5deaa60 rpc: remove deprecated getaddressinfo `label` field (Jon Atack)
Pull request description:
These were deprecated in #17578 and #17585, with expected 0.21 removal notified in the 0.20 release notes.
```
- The `getaddressinfo` RPC has had its `label` field deprecated
(re-enable for this release using the configuration parameter
`-deprecatedrpc=label`). The `labels` field is altered from returning
JSON objects to returning a JSON array of label names (re-enable
previous behavior for this release using the configuration parameter
`-deprecatedrpc=labelspurpose`). Backwards compatibility using the
deprecated configuration parameters is expected to be dropped in the
0.21 release. (#17585, #17578)
```
ACKs for top commit:
Sjors:
utACK bc01f7a
adamjonas:
utACK bc01f7a
meshcollider:
utACK bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb
Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251
e5327f947c310849e1ddbb24321e4c9f85564549 [rpc] fundrawtransaction: add_inputs option to control automatic input adding (Sjors Provoost)
79804fe24bd00e183382dfbcab9343960d158aa5 [rpc] walletcreatefundedpsbt: don't automatically append inputs (Sjors Provoost)
Pull request description:
When the user doesn't specificy inputs, it makes sense to automatically select them. But when the user does specify inputs, `walletcreatefundedpsbt` now fails if the amount is insufficient, unless `addInputs` is set to `true`.
Similarly for `fundrawtransaction` if the original transaction already specified inputs, we only add more if `addInputs` is set to `true`.
This protects against fat finger mistakes in the amount or fee rate (see also #16257). The behavior is also more similar to GUI coin selection.
ACKs for top commit:
achow101:
ACK e5327f947c310849e1ddbb24321e4c9f85564549
meshcollider:
utACK e5327f947c310849e1ddbb24321e4c9f85564549
Tree-SHA512: d8653b820914396c7c25b0d0a2b7e92de214aa023bc1aa085feb37d3b20fab361ebea90416a7db989f19bdc37e26cf0adfbcb712c80985c87afa67a9bd44fecb
4d7369125a82214ea42b808a32b71b315a5c3c72 Disallow automatic conversion between hash types (Ben Woosley)
fa9ef2cdbed32438bdb32623af6e06f13ecd35e4 Remove an apparently unnecessary conversion (Ben Woosley)
966a22d859db37b1775e2180e5be032fc4fdf483 Explicitly support conversion between equivalent hash types (Ben Woosley)
f32c1e07fd6c174ff3f6406a619550d2f6c19360 Use explicit conversion from WitnessV0KeyHash -> CKeyID (Ben Woosley)
2c54217f913967703b404747133be67cf2f4feac Use explicit conversion from PKHash -> CKeyID (Ben Woosley)
a9e451f144480d7b170e49087df162989d31cd20 Convert CPubKey to WitnessV0KeyHash directly (Ben Woosley)
3fcc46812334074d2c77a6233e8a961cd0785872 Prefer explicit CScriptID construction (Ben Woosley)
0a5ea32ce605984094c5552877cb99bc81654f2c Prefer explicit uint160 conversion (Ben Woosley)
Pull request description:
This bases the script/standard hash types, TxDestination-related and CScriptID on a base template which does not silently convert the underlying `uintN` type.
Inspired by and built on #17924. Commits are small and focused to ease review.
Note some of these changes may be relative to existing bugs of the same sort as #17924. See particularly "Convert CPubKey to WitnessV0KeyHash directly" and "Remove an apparently unnecessary conversion".
ACKs for top commit:
achow101:
ACK 4d7369125a82214ea42b808a32b71b315a5c3c72
meshcollider:
re-utACK 4d7369125a82214ea42b808a32b71b315a5c3c72
Tree-SHA512: f1b3284ddc6fb6c6e726f2c22668b6d732d45eb5418262ed2b9c728f60be7be43dfb414b6ddd9915025c8dcd7f360dc3b46e997a945a2feb95b0e5c4f05d6b54
d0a3feea73e0751f325ef7f1de20fa668f27332e Change docs for walletcreatefundedpsbt RPC method (Ivan Vershigora)
Pull request description:
`sequence` field in the list of inputs currently marked as "required". Actually it can be omitted and it's value depends on `locktime` and `options.replaceable` fields. Just the same as in `createpsbt` call.
ACKs for top commit:
achow101:
ACK d0a3feea73e0751f325ef7f1de20fa668f27332e
Tree-SHA512: 3f429a2c2eea283a47fb5002a99f7e2a5ed6f67df9fd895c1ab938256c48a6497ed6ac2673d8fe8968dfb67b939f4a84570899d9faf52f3abd6ec90c0703d1bd
951bca61d7376be44fad0775e8abb06ff667e4bf tests: feature_backwards_compatibility.py test 0.16 up/downgrade (Andrew Chow)
3a03a11e8c696e2164b8bb221a4a35a7c3ac4d6d Skip hdKeypath of 'm' (Andrew Chow)
Pull request description:
Previously the seed was stored with keypath 'm' so we need to skip this as well when determining inactive seeds.
Fixes#19051
ACKs for top commit:
Sjors:
ACK 951bca61d7376be44fad0775e8abb06ff667e4bf
instagibbs:
re-utACK 951bca61d7
ryanofsky:
Code review ACK 951bca61d7376be44fad0775e8abb06ff667e4bf. No significant changes since last review, just updated comment and some test tweaks
Tree-SHA512: 930f77e7097c9cf4f1012e540bd2b1a72fd279262517f10c1531b2ad48c632ef95e0dd4edea81bcc3b3db306479d34e5e79e5d6c4ed31dfa4b77a4231436436e
A templated BaseHash does not allow for automatic conversion, thus
conversions much be explicitly allowed / whitelisted, which will
reduce the risk of unintended conversions.
These types are equivalent, in data etc, so they need only their
data cast across.
Note a function is used rather than a casting
operator as CKeyID is defined at a lower level than script/standard
b83cc0fc94df99f0334430e63e8c9fa6ae3790e1 Fix link error with --enable-debug (Hennadii Stepanov)
Pull request description:
Fixes a link error on master (39bd9ddb8783807b9cde6288233e86ad7c85d61f):
```
$ ./configure --enable-debug
$ make
...
bitcoin_wallet-bitcoin-wallet.o:(.data.rel.ro+0x0): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet_tool.a(libbitcoin_wallet_tool_a-wallettool.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-salvage.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-walletdb.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o):(.data.rel.ro+0x8): more undefined references to `InitError(bilingual_str const&)' follow
collect2: error: ld returned 1 exit status
```
See:
- https://github.com/bitcoin/bitcoin/pull/19295#issuecomment-645471771
- https://github.com/bitcoin/bitcoin/pull/19295#issuecomment-645487182
ACKs for top commit:
achow101:
Re-ACK b83cc0fc94df99f0334430e63e8c9fa6ae3790e1
Tree-SHA512: f563d978b6725284049449bb0b3a184d356f32e9b63bcadb0ba71352d3d521af3dbb4a7b4fc0a5a620ed99c357e59f62249c10d0defc0cbe7775f2c06791dabe
fa05f44893d228f672f39436d0cb6b3376f81ac2 ci: Upgrade most ci configs to focal (MarcoFalke)
fad67208914e5a74b64f4cc018368902ef3a2e9b doc: move doc to ci readme (MarcoFalke)
fa880773b425fcd292ed7669d237ee3151a15bc6 ci: Have one config run in xenial to test against python3.5 (MarcoFalke)
fa6ddb2fa167df52f59cb9eaabed48315ddcdb2e travis: Always run multiprocess build (MarcoFalke)
Pull request description:
Generally developers compile with recent compilers, so bumping the ci configs to a recent OS should be uncontroversial. Older OSes (especially with compiler sanitizers) need workarounds that can be dropped by running on a more recent OS.
This pull changes the asan sanitizer and the experimental multiprocess build to use focal.
Also, it runs the no_wallet config on xenial to test against python 3.5, according to `doc/dependencies.md`.
Finally, all configs that mimic gitian (win and mac) will stay at bionic.
ACKs for top commit:
Sjors:
ACK fa05f44893d228f672f39436d0cb6b3376f81ac2, assuming Travis passes
hebasto:
ACK fa05f44893d228f672f39436d0cb6b3376f81ac2
Tree-SHA512: 55ec56c71ba2280d27c1a8856a1e6c310b1fbf469d5a8a1dde228063e3892e1dd1e51408ecff7a3d77ac2ae018daa9e9bbbb60598cdeaab8c32a146b11b3e7c4
fa2eb3d5d6819e42bfcec8a9f02b99438fe718b9 ci: Run asan ci config on cirrus (MarcoFalke)
fa93527738a62ebc13305adcb0fd2b5128073bbc cirrus: Clear dummy task (MarcoFalke)
Pull request description:
Currently it is not possible to use travis in forked repositories due to the 50 minute limit on builds. A fresh build (uncached) of the address sanitizer config takes more than 50 minutes.
One approach to fix this could be to throw away tests until the run time is less than 50 minutes. However, the risk of being blind of failures in the thrown away tests is not worth the gain. Also, to detect them, one has to run the asan configuration nightly and failures could only be detected post-merge.
Another approach would be to ask travis support to raise the limit for a forked repository. This is a tedious and manual one-by-one process, so I'd rather not.
Finally, a different ci provider can be used, since the config files are designed to be platform-agnostic. This is what I picked.
I kept all settings identical to the travis machine for now. Both providers run in the google cloud, so this should be a "move-only".
ACKs for top commit:
hebasto:
ACK fa2eb3d5d6819e42bfcec8a9f02b99438fe718b9
Tree-SHA512: 159d7dc6f5b24583e941282cdd40465b15db787f0a658a3e81a7b1a22abdb4cb573709b9b5c4465523e0ba0060b17a68fbdbda7a9ecdeb649f31535d377bbe75
3351c91ed402895dcb4f803a29d2cac70ccfa8b4 refactor: Make CScriptVisitor stateless (João Barbosa)
Pull request description:
`CScriptVisitor` was added in 1025440184ef100a22d07c7bb543ee45cf169d64 (#1357) and the visitor return type was never used. Now `CScriptVisitor` is stateless and `CScript` is the return type.
ACKs for top commit:
MarcoFalke:
ACK 3351c91ed402895dcb4f803a29d2cac70ccfa8b4 🏤
sipa:
utACK 3351c91ed402895dcb4f803a29d2cac70ccfa8b4
Tree-SHA512: d158ad2ebe8ea4dc8cc090b943dd66fa5421a84f9443e16ab2d661df38e1a85de16ff13cbaa56924489d8d43cba25fa3cd8b6904bbbcbf356b886ffe8ffba19a
51e9393c1f6c9eaac554f821f5327f63bd09c8cf refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg (Sebastian Falbesoner)
Pull request description:
Follow-up PR for #18533 -- another small step towards getting rid of the confusing "command" terminology. Also see PR #18610 which tackled the functional tests.
ACKs for top commit:
MarcoFalke:
ACK 51e9393c1f6c9eaac554f821f5327f63bd09c8cf
Tree-SHA512: bb6f05a7be6823d5c4eab1d05b31fee944e700946827ad9425d59a3957fd879776c88c606319cbe9832d9451b275baedf913b71429ea3e01e4e82bf2d419e819
Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu
item, giving options to sign or broadcast the loaded PSBT as
appropriate, as well as copying the result to the clipboard or saving
it to a file.
fa1904e5f0d164fbcf41398f9ebbaafe82c28419 net: Remove dead logging code (MarcoFalke)
fac12ebf4f3b77b05112d2b00f8d3f4669621a4c net: Avoid redundant and confusing FAILED log (MarcoFalke)
Pull request description:
Remove a redundant and confusing "FAILED" log message and gets rid of the unused return type in `ProcessMessage`
ACKs for top commit:
jnewbery:
utACK fa1904e5f0d164fbcf41398f9ebbaafe82c28419
gzhao408:
utACK fa1904e5f0
troygiorshev:
ACK fa1904e5f0d164fbcf41398f9ebbaafe82c28419
naumenkogs:
utACK fa1904e
Tree-SHA512: bfa553d5efa022727ed17877fb7c08c14849d804fe6d6a7ce172d513857beba35de41ea40b27ff1aedf68b81e2cda7b2a948ac985fcaaf1b6cfb96cce4837c90
f52d403b81e758e9bc33847560b5740b22d95fff [net] split PushInventory() (John Newbery)
Pull request description:
PushInventory() is currently called with a CInv object, which can be a
MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
whether to add the hash to setInventoryTxToSend or
vInventoryBlockToSend.
Since the caller always knows what type of inventory they're pushing,
the CInv is wastefully constructed and thrown away, and tx/block relay
is being split out, we split the function into PushTxInventory() and
PushBlockInventory().
ACKs for top commit:
amitiuttarwar:
utACK f52d403b81. nice cleanup, this has bothered me :)
naumenkogs:
utACK f52d403
sipa:
utACK f52d403b81e758e9bc33847560b5740b22d95fff
Tree-SHA512: 331495199a3b1a2620e6a62beb336e494291b725d8fd64bb44726c02e80807f3974ff4f329bb0f059088e65cd7d41eff276c1065806d2dd6e72c5a9f368e82cd
In FillPSBT, optionally report the number of inputs we successfully
signed, as an out parameter. If "sign" is false, instead report the
number of inputs for which GetSigningProvider does not return nullptr.
(This is a potentially overbroad estimate of inputs we could sign.)
83fd3a6d73eee452dc5141bdf6826da62d7b2dbd init: use std::thread for ThreadImport() (fanquake)
Pull request description:
[Mentioned](https://github.com/bitcoin/bitcoin/pull/19142#issuecomment-638090759) in #19142, which removed the `boost::interruption_point()`
in `ThreadImport()`.
ACKs for top commit:
hebasto:
ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd, I have reviewed the code and it looks OK, I agree it can be merged.
donaloconnor:
ACK 83fd3a6
laanwj:
Code review ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd
MarcoFalke:
ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd
Tree-SHA512: 0644947d669feb61eed3a944012dad1bd3dd75cf994aa2630013043c213a335b162b63e20aa37e0997740d8e3a3ec367b660b5196007a09e13f0ac455b36c821
da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387 Remove WalletDatabase::Create, CreateMock, and CreateDummy (Andrew Chow)
d6045d0ac615b5984b72e83cb25aa8a245a177a0 scripted-diff: Replace WalletDatabase::Create* with CreateWalletDatabase (Andrew Chow)
45c08f8a7b89dda6afb7d7cf9573a8ae8290ac92 Add Create*WalletDatabase functions (Andrew Chow)
Pull request description:
Instead of having `Create`, `CreateMock`, and `CreateDummy` being static functions in `BerkeleyDatabase`, move these to standalone functions in `walletdb.cpp`. This prepares us for having different `WalletDatabase` classes.
Part of #18971. This was originally one commit but has been split into 3 to make it (hopefully) easier to review.
ACKs for top commit:
MarcoFalke:
ACK da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387 🎂
ryanofsky:
Code review ACK da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387. Easy review, nice scripted-diff
Tree-SHA512: 1feb7cb3889168c555154bf3701a49095fd6b8cab911d44b7f7efbf6fcee2280ccb3d4afec8a83755b39a592ecd13b90a318faa655c321f87bdabdf1e2312327
PushInventory() is currently called with a CInv object, which can be a
MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
whether to add the hash to setInventoryTxToSend or
vInventoryBlockToSend.
Since the caller always knows what type of inventory they're pushing,
the CInv is wastefully constructed and thrown away, and tx/block relay
is being split out, we split the function into PushTxInventory() and
PushBlockInventory().
