7a0c224289 Suppress output in test_bitcoin for expected errors (Gert-Jaap Glasbergen)
Pull request description:
Closes#15944
This adds two methods to noui, that allows temporarily suppressing (and then resuming) the output from `noui`. For situations where errors are expected, it's confusing for the test binary to output an error and then conclude with `No errors detected`.
It also uses this supress/reconnect in the tests that currently produce verbose errors when running `test_bitcoin`.
Output of `test_bitcoin` on current master:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_943311758/tempdir/path_does_not_exist" does not exist
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_643733972/tempdir/not_a_directory.dat" is not a directory
Error: Specified -walletdir "wallets" is a relative path
*** No errors detected
```
Output after this code is merged:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
*** No errors detected
```
ACKs for top commit:
l2a5b1:
ACK 7a0c224 - tested and reviewed.
laanwj:
ACK 7a0c224289
Tree-SHA512: c7881f7a431a065329360ffa9937ce4742694c646c90c019d3aff95dfd7fccbdcda9116c5762feb6dfd1108d14f9fb386e203b173c4bde9093afb2b8c977d13d
fac2e6a604 test: Fail early on disconnect in mininode.wait_for_* (MarcoFalke)
Pull request description:
The node might crash or disconnect when our mininode waits for data. Due to the crash, the data is guaranteed to never arrive and we can fail early with an assert
ACKs for top commit:
laanwj:
ACK fac2e6a604
Tree-SHA512: 32ca844eb66bd70ea49103d51c76b953242b1886e0834d96fca8840fc984ff40346d0a799adf8f76b03514a783cb9cec69d45e00bdd328c5192c31b5d8d17af2
b078067b9c gui: Remove unused RPCConsole::tabFocus (João Barbosa)
Pull request description:
Added in #14573 but not used, so begone.
ACKs for top commit:
practicalswift:
utACK b078067b9c
hebasto:
ACK b078067b9c
laanwj:
ACK b078067b9c, there's nothing really to test here
Tree-SHA512: 237276dea4d174b5fca34855447146f79c3faaae7179f4245c70e2070b49282d95f886b1be6d2a33713c81a254f4483a4e4bf850053a8dcb18a3a897bd3da08e
80ba4241a6 extract min & max depth onto coin control (Amiti Uttarwar)
Pull request description:
- Refactor `AvailableCoins` to pull min & max depths from coin control.
- Add `m_max_depth` to coin control to support this.
- Addresses issue https://github.com/bitcoin/bitcoin/issues/15823, see thread for further details.
ACKs for top commit:
laanwj:
ACK 80ba4241a6
Tree-SHA512: 8f7c0aa90b3bc3667baf6741b1da2829f3919e1df92ae097d86c6b239f0c024eb410d7100e6251ea8fc49d022fb5a1214bf79b0f8b0014945b7784b2311647d1
05b56d1c93 [wallet] Remove CMerkleTx serialization logic (John Newbery)
783a76f23b [wallet] Flatten CWalletTx class hierarchy (John Newbery)
b3a9d179f2 [wallet] Move CMerkleTx functions into CWalletTx (John Newbery)
Pull request description:
CMerkleTx is only used as a base class for
CWalletTx. It was previously also used for vtxPrev which
was removed in 93a18a3650.
This PR moves all of the CMerkleTx members and logic
into CWalletTx. The CMerkleTx class is kept for deserialization
and serialization of old wallet files.
This makes the refactor in #15931 cleaner.
ACKs for top commit:
laanwj:
ACK 05b56d1c93. Looks good to me.
Tree-SHA512: 3d3a0069ebb536b12a328f1261e7dc55158a71088d445ae4b4ace4142c432dc296f58c8183b1922e54a60b8cc77e9d17c3dce7478294cd68693594baacf2bab3
0c78e49be3 tests: Switch one of the Travis jobs to an unsigned char environment (-funsigned-char) (practicalswift)
Pull request description:
Switch one of the Travis jobs to an unsigned char environment (`-funsigned-char`).
This will help us catch errors due to code written under the assumption that `char` has the same value range as `signed char`.
The signedness of `char` is implementation-defined.
Example:
```
$ uname -a
Linux […] x86_64 x86_64 x86_64 GNU/Linux
$ cat foo.cpp
#include <iostream>
int main() {
char c;
std::cin >> c;
int i = (unsigned char)c;
std::cout << i << "\n";
}
$ clang++ -o foo foo.cpp
$ echo -e "\xff" | ./foo
255
$ clang++ -fsigned-char -o foo foo.cpp
$ echo -e "\xff" | ./foo
255
$ clang++ -funsigned-char -o foo foo.cpp
$ echo -e "\xff" | ./foo
255
$ cat bar.cpp
#include <iostream>
int main() {
char c;
std::cin >> c;
int i = c;
std::cout << i << "\n";
}
$ clang++ -o bar bar.cpp
$ echo -e "\xff" | ./bar
-1
$ clang++ -fsigned-char -o bar bar.cpp
$ echo -e "\xff" | ./bar
-1
$ clang++ -funsigned-char -o bar bar.cpp
$ echo -e "\xff" | ./bar
255
```
`gcc` chars:
* signed: alpha, hppa, ia64, m68k, mips, sh, sparc, x86
* unsigned: arm, powerpc, s390
About `-funsigned-char`:
> Let the type "char" be unsigned, like "unsigned char".
>
> Each kind of machine has a default for what "char" should be. It is either like "unsigned char" by default or like "signed char" by default.
>
> Ideally, a portable program should always use "signed char" or "unsigned char" when it depends on the signedness of an object. But many programs have been written to use plain "char" and expect it to be signed, or expect it to be unsigned, depending on the machines they were written for.
>
> This option, and its inverse, let you make such a program work with the opposite default. The type "char" is always a distinct type from each of "signed char" or "unsigned char", even though its behavior is always just like one of those two.
ACKs for top commit:
laanwj:
ACK 0c78e49be3
Tree-SHA512: ba04590415c0bb9a0bbd348623e57068f75274f53da7247d5c5ecad82e365a5b45893a4a491d318e82a8feb6a25f019d46e01990afb33162e2c9740d33a343d7
29ee4c417d Specify AM_CPPFLAGS for ZMQ. (Daniel Kraft)
Pull request description:
When building the ZMQ static library, add `AM_CPPFLAGS` to the library `CPPFLAGS`. Otherwise, we may miss important flags that are specified elsewhere. For instance, if `--enable-debug` is passed and
`-DDEBUG_LOCKORDER` set, then that would not apply to the ZMQ library before (causing potential for hard-to-find bugs).
ACKs for top commit:
laanwj:
utACK 29ee4c417d
Tree-SHA512: 64085d71ed3f435a6e4df6dc42bda8b6159a4d292d0547c5b38c09d6ac95e976ad1728cd65278bffdd57363f60a58eb762b1171dafbe055cf94ffcd4f66da877
42a5e912ee [mempool] log correct messages when CPFP fails (John Newbery)
Pull request description:
Fixes a logging issue introduced in #15681
ACKs for top commit:
laanwj:
ACK 42a5e912ee (+utACK from bluematt that isn't registered because it has no commit id)
Tree-SHA512: ff5f423cc4d22838eea00c5b1d39ceda89cd61474c72f256a97c698eb0ec3f2156a97139f537669376132902c1e3943bf84c356a4b98a9a306b4ec57302c2761
4057b7acb7 wallet: Recognize -disablewallet option early (Hennadii Stepanov)
Pull request description:
This PR makes early check for the `-disablewallet` option.
If `-disablewallet=1`, objects `PaymentServer` and `WalletController` are nor created.
ACKs for top commit:
jonasschnelli:
utACK 4057b7acb7
laanwj:
ACK 4057b7acb7
Tree-SHA512: 74633cd1eacd0914c73712e6dff190255b5378595cfee7eaeb91e17671fc9120928034739f4ae1c53b86f46c4b400390877241384376b2fc534de326d3ab0944
59cb722fd0 Update configure to reject unsafe miniUPnPc API ver (Hennadii Stepanov)
ab2190557e doc: Add release notes for 15993 (Hennadii Stepanov)
02709e9560 Align formatting with clang-format (Hennadii Stepanov)
91a1b85083 Use PACKAGE_NAME in UPnP description (Hennadii Stepanov)
9f76e45b9d Drop support of insecure miniUPnPc versions (Hennadii Stepanov)
Pull request description:
1. Minimum supported miniUPnPc API version is set to 10:
- https://packages.ubuntu.com/xenial/libminiupnpc-dev
- https://packages.debian.org/jessie/libminiupnpc-dev
Refs:
- #6583
- #6789
- #10414
2. The hardcoded "Bitcoin" replaced with `PACKAGE_NAME`:
3. Also style-only commit applied.
Pardon: could not reopen my previous PR #15966.
ACKs for top commit:
ryanofsky:
utACK 59cb722fd0. Changes since last review: adding a new commit which updates configure script to fall back to disabling upnp if version is too old, adding a requested comment explaining static_assert condition, and fixing a spelling (jessy/jessie)
Tree-SHA512: 42ed11bc2fb2ec83d5dd58e2383da5444a24fd572707f6cf10b622cb8943e28adfcca4750d06801024c4472625b5ea9279516fbd9d2ccebc9bbaafe1d148e80d
f509e3b8ce doc: remove line numbers from qt package links (fanquake)
1bb1661a40 doc: fix typo in bitcoin_qt.m4 comment (fanquake)
0aeb98ac1f build: remove jpeg lib check from bitcoin_qt.m4 (fanquake)
98a64bd296 build: disable libjpeg in qt (fanquake)
Pull request description:
When gitian building on Windows I'm seeing:
```bash
checking for Qt 5... yes
checking for > Qt 5.7... yes
checking for main in -limm32... yes
checking for main in -lz ... yes
checking for library containing jpeg_create_decompress ... configure: WARNING: libjpeg not found. Assuming qt has it built-in
no
checking for library containing png_error ... -lqtlibpng
checking for library containing pcre2_match_16... -lqtpcre2
checking for library containing hb_ot_tags_from_script ... -lqtharfbuzz
```
We are passing `-qt-libjpeg` to Qt:
https://github.com/bitcoin/bitcoin/blob/e6e99d4f757f2e5052f0cc68951c75e91e4753e3/depends/packages/qt.mk#L66
but I dont think we are doing anything with `jpeg` related regardless?
ACKs for top commit:
laanwj:
ACK f509e3b8ce
promag:
ACK f509e3b8ce.
Tree-SHA512: 61ea20c11df11b9d426644df9a01aac12b76897003121a283fc784a8c30e9b5ad34c9805069fec20926f7aa279e59528e2e13697a944a22760c3acb6366fffbe
07e01d6258 rpc: sendrawtransaction unconditionality/privacy note (Jon Atack)
Pull request description:
In sendrawtransaction RPCHelpMan, mention unconditionality and privacy as per http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-25.html#l-522
before
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submits raw transaction (serialized, hex-encoded) to local node and network.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
after
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submit a raw transaction (serialized, hex-encoded) to local node and network.
Note that the transaction will be sent unconditionally to all peers, so using this
for manual rebroadcast may degrade privacy by leaking the transaction's origin, as
nodes will normally not rebroadcast non-wallet transactions already in their mempool.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
ACKs for top commit:
promag:
ACK 07e01d6258.
laanwj:
ACK 07e01d6258
Tree-SHA512: 427b3ca29384eef271eb496b7b14e883220863543a536ddeb31940aaffd52ea0b607d929d50f2b7958514105ef7823fa05c1ee381d4a432808753c06bd97af58
248e22bbc0 depends: disable unused Qt features (fanquake)
Pull request description:
Related to #16354. Kept separate from #16370, because:
> QT is a monster 😂 - dongcarl in #bitcoin-builds
I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows.
I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in #16354.
ACKs for top commit:
sipsorcery:
tACK 248e22bbc0 (Windows 10 test only)
laanwj:
ACK 248e22bbc0
Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0
50cede3f5a [mempool] Allow one extra single-ancestor transaction per package (Matt Corallo)
Pull request description:
This implements the proposed policy change from [1], which allows
certain classes of contract protocols involving revocation
punishments to use CPFP. Note that some such use-cases may still
want some form of one-deep package relay, though even this alone
may greatly simplify some lightning fee negotiation.
[1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
ACKs for top commit:
ajtowns:
ACK 50cede3f5a -- looked over code again, compared with previous commit, compiles, etc.
sdaftuar:
ACK 50cede3f5a
ryanofsky:
utACK 50cede3f5a. Changes since last review: adding EXTRA_DESCENDANT_TX_SIZE_LIMIT constant, changing max ancestor size from 1,000,000 to nLimitAncestorSize constant (101,000), fixing test comment and getting rid of unused test node.
Tree-SHA512: b052c2a0f384855572b4579310131897b612201214b5abbb225167224e4f550049e300b471dbf320928652571e92ca2d650050b7cf39ac92b3bc1d2bcd386c1c
a52818cc56 net: Make poll in InterruptibleRecv only filter for POLLIN events. poll should block until there is data to be read or the timeout expires. (tecnovert)
Pull request description:
poll should block until there is data to be read or the timeout expires.
Filtering for the POLLOUT event causes poll to return immediately which leads to high CPU usage when trying to connect to non-responding peers through tor.
When USE_POLL is not defined select is used with the writefds parameter set to nullptr.
Removing POLLOUT causes the behavior of poll to match that of select.
Fixes: #16004.
ACKs for top commit:
laanwj:
code review ACK a52818cc56
jonasschnelli:
utACK a52818cc56
Tree-SHA512: 69934cc14e3327c7ff7f6c5942af8761e865220b2540d74ea1e176adad326307a73860417dddfd32d601b5c0e9e2ada1848bd7e3d27b0b7a9b42f11129af8eb1
ae311bc036 Fix autostart filenames on Linux (Hennadii Stepanov)
Pull request description:
Currently, on master the `bitcoin-test.lnk` and `bitcoin-regtest.lnk` files do not work as autostart application `.desktop` files.
This PR fixes it.
Refs:
- #7045
- [Autostart Of Applications During Startup](https://standards.freedesktop.org/autostart-spec/autostart-spec-latest.html)
ACKs for top commit:
promag:
utACK ae311bc, weird why extension `.lnk` was used in #7045.
laanwj:
Code review ACK ae311bc036
Tree-SHA512: 210cc346600d52b0a262c81ed5f258365a3cea2e5522f4b5f4798fd3b54f45ed82aba68eefae59a6b6f1d8e4d00221476c23bdffc038f16f2f45c1acc837f522
a981e749e6 fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa)
Pull request description:
Calling `event_base_loopbreak` before `event_base_dispatch` has no effect. Fix this by calling `event_base_loopbreak` from the event's callback. From the [documentation](http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/event_8h.html#a07a7599e478e4031fa8cf52e26d8aa1e):
> event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement.
This can be tested by running the following with and without this change:
```sh
bitcoind -- -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0
```
Fixes#16376.
ACKs for top commit:
laanwj:
code review ACK a981e749e6
fanquake:
ACK a981e749e6
Tree-SHA512: 328fe71366404d5be8177d7081d5b4868cee73412df631a1865d24fb1c153427210762738109e06b737f037f4c68966812fba041831bb9e8129861f19ce61a63
682a1d0f20 refactoring: remove mapBlockIndex global (James O'Beirne)
55d525ab90 refactoring: make pindexBestInvalid internal to validation.cpp (James O'Beirne)
4ed55dfcd7 refactoring: add block_index_candidates arg to LoadBlockIndex (James O'Beirne)
613c46fe9e refactoring: move block metadata structures into BlockManager (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
Under an assumeutxo model, we have multiple CChainState instances in use at once in order to support background validation. Currently, each CChainState instance has its own mapBlockIndex, a collection of linked block headers, in addition to a few other data structures that are related to maintenance of the block tree but not necessarily to any given chainstate.
In order to avoid duplicating this data across chainstates, this change moves chainstate-agnostic block metadata (and related behavior) into a class, `BlockManager`. Chainstates are parameterized with a reference to a blockmanager instance and in practice they share the same instance.
Most of this change is conceptually move-only, though the diff is somewhat muddled. The first commit can be reviewed slightly more easily with `--color-moved=dimmed_zebra`. Admittedly, that commit is pretty unwieldy; I tried to split it up after the fact with `git add --patch`, but that was difficult because of git's inability to split hunks past a certain point. Some of the moves also ended up being obscured when done over separate commits.
ACKs for top commit:
MarcoFalke:
ACK 682a1d0f20
ryanofsky:
utACK 682a1d0f20, only changes since last review were rebase and fixing conflict on a moved line
ariard:
utACK 682a1d0. Most of the changes are move-only, with main problem being to avoid creating circular dependencies between `BlockManager` and `CChainState`. Tested, comments are mostly nits, feel free to ignore them
Tree-SHA512: 738d8d06539ba53acf4bd2d48ae000473e645bbc4e63d798d55d247a4d5a4f781b73538ed590f6407be9ab402ea9d395570ea20bff0a4b9ce747bcc1600c5108
c7f6ce74d3 docs: Improve netbase comments (Carl Dong)
Pull request description:
Second in a series of PRs documenting the net stack. Contributed with sincere thanks to sipa, laanwj, and gmaxwell for providing much of the history, context, and rationale.
ACKs for top commit:
laanwj:
ACK c7f6ce74d3
Tree-SHA512: ad83054d3b8d0c8c3fb55be011bcf294176e7509513bf61326866afd53e8159644e0d59bb3a2f404717f525cbf736096d4c1990e61cfd89845d51fa6b5394b7c
e263a343d4 test: rpc_users: Make variable names more clear. (Carl Dong)
830dc2dd0f test: rpc_users: Also test rpcauth.py with specified password. (Carl Dong)
c73d871799 test: rpc_users: Add function for testing auth params. (Carl Dong)
604e2a997f test: rpc_users: Add function for auth'd requests. (Carl Dong)
Pull request description:
Fixes#14758
First two commits are tidy-ups which I feel are worthwhile as they are very straightforward, cut down the file by 50%, and made the final diff more minimal. Happy to squash after review.
ACKs for top commit:
laanwj:
ACK e263a343d4
Tree-SHA512: aa75c48570a87060238932d4c68e17234e158077f6195fb4917367e1ecc565e3cd8dd0ae51f9159ddd3d03742739680391bc1246454302db22d4a608c0633e80
751549b52a contrib: guix: Additional clarifications re: substitutes (Carl Dong)
cd3e947f50 contrib: guix: Various improvements. (Carl Dong)
8dff3e48a9 contrib: guix: Clarify SOURCE_DATE_EPOCH. (Carl Dong)
3e80ec3ea9 contrib: Add deterministic Guix builds. (Carl Dong)
Pull request description:
~~**This post is kept updated as this project progresses. Use this [latest update link](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-497303718) to see what's new.**~~
Please read the `README.md`.
-----
### Guix Introduction
This PR enables building bitcoin in Guix containers. [Guix](https://www.gnu.org/software/guix/manual/en/html_node/Features.html) is a transactional package manager much like Nix, but unlike Nix, it has more of a focus on [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html) and [reproducibility](https://www.gnu.org/software/guix/blog/tags/reproducible-builds/) which are attractive for security-sensitive projects like bitcoin.
### Guix Build Walkthrough
Please read the `README.md`.
[Old instructions no. 4](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-497303718)
[Old instructions no. 3](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-493827011)
[Old instructions no. 2](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-471658439)
<details>
<summary>Old instructions no. 1</summary>
In this PR, we define a Guix [manifest](https://www.gnu.org/software/guix/manual/en/html_node/Invoking-guix-package.html#profile_002dmanifest) in `contrib/guix/manifest.scm`, which declares what packages we want in our environment.
We can then invoke
```
guix environment --manifest=contrib/guix/manifest.scm --container --pure --no-grafts --no-substitutes
```
To have Guix:
1. Build an environment containing the packages we defined in our `contrib/guix/manifest.scm` manifest from the Guix bootstrap binaries (see [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html) for more details).
2. Start a container with that environment that has no network access, and no access to the host's filesystem except to the `pwd` that it was started in.
3. Drop you into a shell in that container.
> Note: if you don't want to wait hours for Guix to build the entire world from scratch, you can eliminate the `--no-substitutes` option to have Guix download from available binary sources. Note that this convenience doesn't necessarily compromise your security, as you can check that a package was built correctly after the fact using `guix build --check <packagename>`
Therefore, we can perform a build of bitcoin much like in Gitian by invoking the following:
```
make -C depends -j"$(nproc)" download && \
cat contrib/guix/build.sh | guix environment --manifest=contrib/guix/manifest.scm --container --pure --no-grafts --no-substitutes
```
We don't include `make -C depends -j"$(nproc)" download` inside `contrib/guix/build.sh` because `contrib/guix/build.sh` is run inside the container, which has no network access (which is a good thing).
</details>
### Rationale
I believe that this represents a substantial improvement for the "supply chain security" of bitcoin because:
1. We no longer have to rely on Ubuntu for our build environment for our releases ([oh the horror](https://github.com/bitcoin/bitcoin/blob/72bd4ab867e3be0d8410403d9641c08288d343e3/contrib/gitian-descriptors/gitian-linux.yml#L10)), because Guix builds everything about the container, we can perform this on almost any Linux distro/system.
2. It is now much easier to determine what trusted binaries are in our supply chain, and even make a nice visualization! (see [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html)).
3. There is active effort among Guix folks to minimize the number of trusted binaries even further. OriansJ's [stage0](https://github.com/oriansj/stage0), and janneke's [Mes](https://www.gnu.org/software/mes/) all aim to achieve [reduced binary boostrap](http://joyofsource.com/reduced-binary-seed-bootstrap.html) for Guix. In fact, I believe if OriansJ gets his way, we will end up some day with only a single trusted binary: hex0 (a ~500 byte self-hosting hex assembler).
### Steps to Completion
- [x] Successfully build bitcoin inside the Guix environment
- [x] Make `check-symbols` pass
- [x] Do the above but without nasty hacks
- [x] Solve some of the more innocuous hacks
- [ ] Make it cross-compile (HELP WANTED HERE)
- [x] Linux
- [x] x86_64-linux-gnu
- [x] i686-linux-gnu
- [x] aarch64-linux-gnu
- [x] arm-linux-gnueabihf
- [x] riscv64-linux-gnu
- [ ] OS X
- [ ] x86_64-apple-darwin14
- [ ] Windows
- [ ] x86_64-w64-mingw32
- [ ] Maybe make importer for depends syntax
- [ ] Document build process for future releases
- [ ] Extra: Pin the revision of Guix that we build with with Guix [inferiors](https://www.gnu.org/software/guix/manual/en/html_node/Inferiors.html)
### Help Wanted
[Old content no. 3](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-483318210)
[Old content no. 2](https://github.com/bitcoin/bitcoin/pull/15277#issuecomment-471658439)
<details>
<summary>Old content no. 1</summary>
As of now, the command described above to perform a build of bitcoin a lot like Gitian works, but fails at the `check-symbols` stage. This is because a few dynamic libraries are linked in that shouldn't be.
Here's what `ldd src/bitcoind` looks like when built in a Guix container:
```
linux-vdso.so.1 (0x00007ffcc2d90000)
libdl.so.2 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libdl.so.2 (0x00007fb7eda09000)
librt.so.1 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/librt.so.1 (0x00007fb7ed9ff000)
libstdc++.so.6 => /gnu/store/4sqps8dczv3g7rwbdibfz6rf5jlk7w90-gcc-5.5.0-lib/lib/libstdc++.so.6 (0x00007fb7ed87c000)
libpthread.so.0 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libpthread.so.0 (0x00007fb7ed85b000)
libm.so.6 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libm.so.6 (0x00007fb7ed6da000)
libgcc_s.so.1 => /gnu/store/4sqps8dczv3g7rwbdibfz6rf5jlk7w90-gcc-5.5.0-lib/lib/libgcc_s.so.1 (0x00007fb7ed6bf000)
libc.so.6 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libc.so.6 (0x00007fb7ed506000)
/gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007fb7ee3a0000)
```
And here's what it looks in one of our releases:
```
linux-vdso.so.1 (0x00007ffff52cd000)
libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f87726b4000)
librt.so.1 => /usr/lib/librt.so.1 (0x00007f87726aa000)
libm.so.6 => /usr/lib/libm.so.6 (0x00007f8772525000)
libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f877250b000)
libc.so.6 => /usr/lib/libc.so.6 (0x00007f8772347000)
/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f8773392000)
```
~~I suspect it is because my script does not apply the gitian-input patches [described in the release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#fetch-and-create-inputs-first-time-or-when-dependency-versions-change) but there is no description as to how these patches are applied.~~ It might also be something else entirely.
Edit: It is something else. It appears that the gitian inputs are only used by [`gitian-win-signer.yml`](https://github.com/bitcoin/bitcoin/blob/d6e700e40f861ddd6743f4d13f0d6f6bc19093c2/contrib/gitian-descriptors/gitian-win-signer.yml#L14)
</details>
### How to Help
1. Install Guix on your distro either [from source](https://www.gnu.org/software/guix/manual/en/html_node/Requirements.html) or perform a [binary installation](https://www.gnu.org/software/guix/manual/en/html_node/Binary-Installation.html#Binary-Installation)
2. Try out my branch and the command described above!
ACKs for top commit:
MarcoFalke:
Thanks for the replies. ACK 751549b52a
laanwj:
ACK 751549b52a
Tree-SHA512: 50e6ab58c6bda9a67125b6271daf7eff0ca57d0efa8941ed3cd951e5bf78b31552fc5e537b1e1bcf2d3cc918c63adf19d685aa117a0f851024dc67e697890a8d
93ce4a0b6f Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow)
8f5b81e6ed Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow)
37a79a4fcc Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow)
16f8096e91 Move KeyOriginInfo to its own header file (Andrew Chow)
d9becff4e1 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow)
a913e3f2fb Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow)
c7797ec655 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow)
1b699a5083 Add HaveKey and HaveCScript to SigningProvider (Andrew Chow)
Pull request description:
This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is:
```
SigningProvider -> FillableSigningProvider -> CWallet
```
`SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`).
Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure.
ACKs for top commit:
Sjors:
re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment.
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16227/commits/93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9
Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
bb326add9f Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli)
99aea045d6 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli)
af5d1b5f4a Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli)
Pull request description:
This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network).
Includes: #15519, #15512 (please review those first).
The construct is specified here.
https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite
This aims for being used in v2 peer-to-peer messages.
ACKs for top commit:
laanwj:
code review ACK bb326add9f
Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
0d101a340c test: Add test for maxtxfee option (MarcoFalke)
177550101b wallet: Remove unreachable code in CreateTransaction (MarcoFalke)
5c1b9714cb wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
Pull request description:
Follow up to #16257, this PR makes `bumpfee` aware of `-maxtxfee`.
It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`.
ACKs for top commit:
MarcoFalke:
re-ACK 0d101a340c, only change is small test fixup
Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
84edfc72e5 Update doc and CI config (qmma)
48bcb2ac24 Disable other targets when enable-fuzz is set (qmma)
Pull request description:
This is to fix https://github.com/bitcoin/bitcoin/issues/16094
When the `enable-fuzz` flag is set, disable all other binary targets.
ACKs for top commit:
MarcoFalke:
ACK 84edfc72e5 (only checked that travis compiled this)
Tree-SHA512: f4ac80526388a67709986b22de88b00bf93ab44ae31a20bd4d8923a4982ab97e015a9f13010081d6ecf6c23ae8afeac7ca9d849d198ce6ebe239aa3127151efc
8e7f930828 Add GetNewChangeDestination for getting new change Destinations (Andrew Chow)
33d13edd2b Replace CReserveKey with ReserveDestinatoin (Andrew Chow)
172213be5b Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow)
Pull request description:
The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool.
ACKs for top commit:
instagibbs:
re-utACK https://github.com/bitcoin/bitcoin/pull/16237/commits/8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf
sipa:
ACK 8e7f930828. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK.
laanwj:
ACK 8e7f930828
Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500
64fee48944 qt: Assert QMetaObject::invokeMethod result (João Barbosa)
f27bd96b5f gui: Fix missing qRegisterMetaType(WalletModel*) (João Barbosa)
Pull request description:
Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the `invokeMethod` overload that allows connecting to lambdas or member pointers, which are compile checked.
For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5.
ACKs for top commit:
laanwj:
ACK 64fee48944
Tree-SHA512: d332e5d7eb2c7be5d3fe90e2e4ff20a67800b9664f6637c122a23647a964f7915703d3f086e2de440f695cfe14de268ff581d0092b7736e911952a4f4d248e25
e1a55690e6 Delete error-prone CScript constructor (Gregory Sanders)
Pull request description:
The behavior of this constructor is not the expected behavior compared to the other constructors which directly interpret the vector as a CScript, rather than serialize it into a new CScript. It has only four uses in the entire codebase. Delete this constructor and replace its four uses with the more clear serialization construction.
ACKs for top commit:
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/16128/commits/e1a55690e66ca962179bc8170695b92af8a3caa8
sipa:
Concept and code review ACK e1a55690e6, but I'd like to make sure we have tests covering the FindAndDelete usage.
Tree-SHA512: b6721e343c867ca401a80ec87c25939d7f1fc798f3bf7e5feb0ea6f8280eecb6bd65afc8286912c76ff8119ccea50ad7726b1a4137cae70c9d4fed7d960e10d3
c4606b8432 Add Travis check for single parameter constructors not marked "explicit" (practicalswift)
Pull request description:
Make single parameter constructors `explicit` (C++11).
Rationale from the developer notes:
> - By default, declare single-argument constructors `explicit`.
> - *Rationale*: This is a precaution to avoid unintended conversions that might
> arise when single-argument constructors are used as implicit conversion
> functions.
ACKs for top commit:
laanwj:
ACK c4606b8432
Tree-SHA512: 3e6fd51935fd93b2604b2188664692973d0897469f814cd745b5147d71b99ea5d73c1081cfde9f6393f51f56969e412fcda35d2d54e938a3235b8d40945f31fd
fa90fe6440 refactor: Rename getWallets to getOpenWallets in WalletController (João Barbosa)
224eb9534a gui: Sort wallets in open wallet menu (João Barbosa)
Pull request description:
Ensure wallets are sorted by name in the open wallet menu. This also improves the change done in #15957, since it avoids a second call to `listWalletDir`.
ACKs for top commit:
laanwj:
code review ACK fa90fe6440
Tree-SHA512: 349ea195021e56760dea3551126d1b9dc4821faab44aaf2858229db4fddaf0ce0b5eb0a8fa5025f47c77134b003067a77e8c340f9655a99e1305d430ccecced8
7195fa792f test: Tool wallet test coverage for unexpected writes to wallet (Jon Atack)
3bf2b3a37b test: Split tool_wallet.py test into subtests (Jon Atack)
1eb13f09a9 test: Add log messages to test/functional/tool_wallet.py (Jon Atack)
Pull request description:
This pull request adds test coverage in `test/functional/tool_wallet.py` to reproduce unexpected writes to the wallet as described in https://github.com/bitcoin/bitcoin/issues/15608 and serve as a benchmark for fixing the issue:
- Wallet tool `info` unexpectedly writes to the wallet file if the wallet file permissions are read/write.
- Wallet tool `info` raises with "Error loading . Is wallet being used by another process?" if the wallet file permissions are read-only.
Goals:
1. Reproduce the reported issue, define the current unexpected behavior, and add test coverage to guide a future fix. Add debug-level logging for sanity checking and commented-out assertions to be uncommented when fixing the issue. Add the same coverage to the wallet tool create test and the getwalletinfo test as regression tests while fixing the issue.
2. Add info log messages as there are currently none in the test file.
3. Split the tests out to separate functions as per review feedback.
Thanks to Marco Falke for pointing me in the right direction.
ACKs for top commit:
laanwj:
code review ACK 7195fa792f
Tree-SHA512: 16a41cce989c8f819cf5b02c6cf8ea84653ede2738fb402f6c36cf4dc075b424dff3e2c73a1cfa1ec9c75f614675baecc71e588845a2596db06ba0957db2df7b
e8fabd9253 build: prune dbus from depends (fanquake)
Pull request description:
Since #8210 (59d063d076), we've been passing `-dbus-runtime` when configuring Qt.
```
qtbase-opensource-src-5.9.7 $ ./configure -h | grep -i dbus
-no-dbus ............. Do not build the Qt D-Bus module
-dbus-linked ......... Build Qt D-Bus and link to libdbus-1 [auto]
-dbus-runtime ........ Build Qt D-Bus and dynamically load libdbus-1 [no]
```
This means we don't actually seem to be using the `D-Bus` we build in depends. This was pointed out by theuni at the time, [here](https://github.com/bitcoin/bitcoin/pull/7993#issuecomment-223114395) and [here](https://github.com/bitcoin/bitcoin/pull/8210#issuecomment-226930545), but was never followed up. dongcarl also bought it up as part of #16150.
I've tested building and running `bitcoin-qt` using depends on Debian. Needs further testing.
ACKs for top commit:
laanwj:
code review ACK e8fabd9253
Tree-SHA512: 164e6e52b6f97c04aef42bd185e2a157bc1a42103840f9404c5a795749f45a8c2c35f35873395a3a56398b3cd5955496b90d9c885d929b434c9bc871695abe20
f874e14cd3 [build]: check std::system for -[alert|block|wallet]notify (Sjors Provoost)
cc3ad56ff2 [build] MSVC: set HAVE_SYSTEM for desktop apps (Sjors Provoost)
c1c91bb78d [build] detect std::system or ::wsystem (Sjors Provoost)
Pull request description:
Platforms such as iOs and Universal Windows Platform do not support launching a process through system().
ACKs for top commit:
laanwj:
code review ACK f874e14cd3
Tree-SHA512: 16bb4a8fa1896046ccb22a46c8985e1aa45f5b11ecf5539eb2299e9a58f1a5b085c0c12cb6939c7493d93abce7e84fadcbfc73374c887db63da6d00c08aa476d
fab0c820fa rpc: Clarify that block count means height excl genesis (MarcoFalke)
Pull request description:
There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.
However, it really returns the height, which is `0` for the genesis block.
So clarify that and also remove the misleading "longest blockchain" comment.
Finally, fix the wallet test that incorrectly used this rpc.
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16325/commits/fab0c820fa4c0c3227eec85c64310a3bf938a149
promag:
ACK fab0c82, sorry for the misconception.
Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
8a6810d0d2 Add a 'logpath' field to getrpcinfo (darosior)
Pull request description:
as discussed in #15438
ACKs for top commit:
laanwj:
Tested ACK 8a6810d0d2
Tree-SHA512: 752c7d90f670677c8144efb338c5c97c2264f85f1e65e031fd5a44f04230b6eafbabd0f634db263eb42c25642ecc1c4b1b602d4735e3fab07ec00b566134ddab
0f459d868d fix an undefined behavior in uint::SetHex (Kaz Wesley)
Pull request description:
Decrementing psz beyond the beginning of the string is UB, even though
the out-of-bounds pointer is never dereferenced.
I don't think any clang sanitizer covers this, so I don't see any way a test could catch the original behavior.
ACKs for top commit:
promag:
utACK 0f459d8.
l2a5b1:
utACK 0f459d868d
Tree-SHA512: 388223254ea6e955f643d2ebdf74d15a3d494e9f0597d9f05987ebb708d7a1cc06ce64bd25d447d75b5f5561bdae9630dcf25adb7bd75f7a382298b95d127162
faa1e0fb17 qt: test: Create at most one testing setup (MarcoFalke)
Pull request description:
It is assumed that ideally only one BasicTestingSetup exists at any point in time for each process (due to use of globals).
This assumption is violated in the GUI tests, as a testing setup is created as the first step of the `main` function and then (sometimes) another one for the following test cases.
So, the gui tests create two testing setups:
* `BasicTestingSetup` in `main` (added in fa4a04a5a9)
* a testing setup for individual test cases
Avoid that by destructing the testing setup in main after creation and then move the explicit `ECC_Stop` to the only places where it is needed (before and after `apptests`).
ACKs for top commit:
laanwj:
code review ACK faa1e0fb17
Tree-SHA512: b8edceb7e2a8749e1de3ea80bc20b6fb7d4390bf366bb9817206ada3dc8669a91416f4803c22a0e6c636c514e0c858dcfe04523221f8851b10deaf472f107d82
f53a70ce95 Improve documentation of memory_cleanse() (Tim Ruffing)
cac30a436c Clean up logic in memory_cleanse() for MSVC (Tim Ruffing)
Pull request description:
When working on https://github.com/bitcoin-core/secp256k1/issues/185, I noticed that the logic in memory_cleanse(), which is supposed to clear memory securely, is weird on MSVC. While it's correct, it's at least a code smell because the code clears the memory twice on MSVC. This weirdness was introduced by #11558.
This PR fixes the logic on MSVC and also improves the docs around this function. Best reviewed in individual commits, see the commit messages for more rationale. The second commit touches only comments.
ACKs for top commit:
practicalswift:
utACK f53a70ce95 :-)
laanwj:
code review ACK f53a70ce95
Tree-SHA512: 1c2fd98ae62b34b3e6e59d1178b293af969a9e06cbb7df02a699ce8802f145a336f72edb178c520e3ecec81f7e8083828f90a5ba6367d966a2c7d7c0dd6c0475
3b9bf0eb0e rpc: Allow shutdown while in generateblocks (Patrick Strateman)
Pull request description:
By checking the shutdown flag every loop we can use the entire 32 bit nonce space instead of breaking every 16 bits to check the flag.
This is possible now because the shutdown flag is an atomic where before it was controlled by a condition variable and lock.
ACKs for top commit:
kallewoof:
Re-ACK 3b9bf0e
Tree-SHA512: d0664201a55215130c2e9199a31fb81361daf4102a65cb3418984fd61cb98bfb9136d9ee8d23a85d57e50051f9bb0059bd71fe0488a17f63c38ea5caa6004504
26fe9b9909 Add support for descriptors to utxoupdatepsbt (Pieter Wuille)
3135c1a2d2 Abstract out UpdatePSBTOutput from FillPSBT (Pieter Wuille)
fb90ec3c33 Abstract out EvalDescriptorStringOrObject from scantxoutset (Pieter Wuille)
eaf4f88734 Abstract out IsSegWitOutput from utxoupdatepsbt (Pieter Wuille)
Pull request description:
This adds a descriptors argument to the `utxoupdatepsbt` RPC. This means:
* Input and output scripts and keys will be filled in when known.
* P2SH-witness inputs will be filled in from the UTXO set when a descriptor is provided that shows they're spending segwit outputs.
This also moves some (newly) shared code to separate functions: `UpdatePSBTOutput` (an analogue to `SignPSBTInput`), `IsSegWitOutput`, and `EvalDescriptorStringOrObject` (implementing the string or object notation parsing used in `scantxoutset`).
ACKs for top commit:
jnewbery:
utACK 26fe9b9909
laanwj:
utACK 26fe9b9909 (will hold merging until response to promag's comments)
promag:
ACK 26fe9b9, checked refactors and tests look comprehensive. Still missing a release note but can be added later.
Tree-SHA512: 1d833b7351b59d6c5ded6da399ff371a8a2a6ad04c0a8f90e6e46105dc737fa6f2740b1e5340280d59e01f42896c40b720c042f44417e38dfbee6477b894b245
fa2b083c3f [test] Add test to check mempool consistency in case of reorgs (MarcoFalke)
fabeb1f613 validation: Add missing mempool locks (MarcoFalke)
fa0c9dbf91 txpool: Make nTransactionsUpdated atomic (MarcoFalke)
Pull request description:
Take the mempool read lock during reorgs, so that we don't accidentally read an inconsistent mempool.
ACKs for top commit:
laanwj:
code review ACK fa2b083c3f
ryanofsky:
utACK fa2b083c3f [EDIT: was ~e284e422e75189794e24fe482819d8b1407857c3~, from bad copy and paste]. Changes since last review: rebase after #15976, adding vTxHashes lock annotation, adding new commit dropping mempool lock for nTransactionsUpdated and making it atomic to avoid deadlock between mempool lock and g_best_block_mutex
Tree-SHA512: cfe7777993589087753e000e3736d79d320dca412383fb77b56bef8946a04049722bf888c11b6f722adf677165185c7e58b4a269f7c5fa25e84dda375f6c8a7d
276972cb95 wallet_bumpfee.py: Make sure coin selection produces change (Gregory Sanders)
Pull request description:
I was hitting the case where change-less transactions were being made.
ACKs for top commit:
ryanofsky:
utACK 276972cb95
Tree-SHA512: e2b7a50363daddd3ee749cacfc9d3d685a6c0c7e3e48118bb60131d205bf83ea06cdd66b69dfa3bd4dbb3bbf2b5b673d7225171486ae72fc762e5dabe2c01ef5
d9753383b9 addrdb: Remove temporary files created in SerializeFileDB. Fixes non-determinism in unit tests. (practicalswift)
Pull request description:
Remove temporary files created in `SerializeFileDB` in case of errors.
_Edit: Previously this was hit non-deterministically from the tests: that is no longer the case but the cleanup issue remains :-)_
ACKs for top commit:
laanwj:
code-review ACK d9753383b9
Tree-SHA512: e72b74b8de411f433bd8bb354cacae07ab75a240db6232bc6a37802ccd8086bff5275ce3d196ddde033d8ab9e2794bb8f60eb83554af7ec2e9f91d6186cb4647
806b0052c3 [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)
Pull request description:
`FundTransaction` calls `GetMinimumFee` which, when the fee rate is absurdly high, quietly reduces the fee to `-maxtxfee`.
Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.
Before:
```
bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
{
"psbt": "cHNidP8...gAA=",
"fee": 0.10000000,
"changepos": 1
}
```
After:
```
bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
error code: -25
error message:
Fee exceeds maximum configured by -maxtxfee
```
QT still checks the max fee rate as expected:
<img width="566" alt="Schermafbeelding 2019-06-20 om 19 52 00" src="https://user-images.githubusercontent.com/10217/59888424-a2aa7100-9395-11e9-8ae6-8a3c1f7de585.png">
ACKs for top commit:
laanwj:
Code review ACK 806b0052c3
Tree-SHA512: bee95811711cdab100b614d2347921407af3b400aea613ca156953ed3f60b924ad29a1d335bd0e240c0b7c0fbb360226bab03294d226a5560cdf2a3f21e6d406
fa69c3e6ca util: Explain why the path is cached (MarcoFalke)
Pull request description:
The rationale for caching the datadir is given as
```
// This can be called during exceptions by LogPrintf(), so we cache the
// value so we don't have to do memory allocations after that.
```
Since 8c2d695c4a, the debug log location is actually cached itself in `m_file_path`.
So explain that the caching is now only used to guard against disk access on each call. (See also #16255)
ACKs for top commit:
promag:
ACK fa69c3e6ca.
laanwj:
ACK fa69c3e6ca
ryanofsky:
utACK fa69c3e6ca. Good cleanup. Previous comment was confusing, and definitely not helpful if outdated.
Tree-SHA512: 02108c90026d6d7c02843aaf59a06b4e1fa63d5d4378bb7760f50767efc340dc94c259bf7afb32fa4d47952b48a4e91798d1e0ddc1b051d770405e078636793a
fabc57e07d test: Log to debug.log in all tests (MarcoFalke)
fa4a04a5a9 test: use common setup in gui tests (MarcoFalke)
fad3d2a624 test: Create data dir in BasicTestingSetup (MarcoFalke)
Pull request description:
This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`.
The pull is done in three commits:
* Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away.
* Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did.
* Log to the debug.log in all tests
ACKs for top commit:
laanwj:
ACK fabc57e07d
Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665
8a2656702b torcontrol: Use the default/standard network port for Tor hidden services, even if the internal port is set differently (Luke Dashjr)
Pull request description:
Currently, the hidden service is published on the same port as the public listening port.
But if a non-standard port is configured, this can be used to guess (pretty reliably) that the public IP and the hidden service are the same node.
ACKs for top commit:
practicalswift:
utACK 8a2656702b
naumenkogs:
utACK 8a26567
laanwj:
utACK 8a2656702b
Tree-SHA512: 737c8da4f7c3f0bb22a338647d357987f5808156e3f38864168d0d8c2e2b171160812f7da4de11eef602902b304e357d76052950b72d7b3b83535b0fdd05fadc
86b47fa741 speed up Unserialize_impl for prevector (Akio Nakamura)
Pull request description:
The unserializer for prevector uses `resize()` for reserve the area, but it's prefer to use `reserve()` because `resize()` have overhead to call its constructor many times.
However, `reserve()` does not change the value of `_size` (a private member of prevector).
This PR make the logic of read from stream to callback function, and prevector handles initilizing new values with that call-back and ajust the value of `_size`.
The changes are as follows:
1. prevector.h
Add a public member function named 'append'.
This function has 2 params, number of elemenst to append and call-back function that initilizing new appended values.
2. serialize.h
In the following two function:
- `Unserialize_impl(Stream& is, prevector<N, T>& v, const unsigned char&)`
- `Unserialize_impl(Stream& is, prevector<N, T>& v, const V&)`
Make a callback function from each original logic of reading values from stream, and call prevector's `append()`.
3. test/prevector_tests.cpp
Add a test for `append()`.
## A benchmark result is following:
[Machine]
MacBook Pro (macOS 10.13.3/i7 2.2GHz/mem 16GB/SSD)
[result]
DeserializeAndCheckBlockTest => 22% faster
DeserializeBlockTest => 29% faster
[before PR]
# Benchmark, evals, iterations, total, min, max, median
DeserializeAndCheckBlockTest, 60, 160, 94.4901, 0.0094644, 0.0104715, 0.0098339
DeserializeBlockTest, 60, 130, 65.0964, 0.00800362, 0.00895134, 0.00824187
[After PR]
# Benchmark, evals, iterations, total, min, max, median
DeserializeAndCheckBlockTest, 60, 160, 77.1597, 0.00767013, 0.00858959, 0.00805757
DeserializeBlockTest, 60, 130, 49.9443, 0.00613926, 0.00691187, 0.00635527
ACKs for top commit:
laanwj:
utACK 86b47fa741
Tree-SHA512: 62ea121ccd45a306fefc67485a1b03a853435af762607dae2426a87b15a3033d802c8556e1923727ddd1023a1837d0e5f6720c2c77b38196907e750e15fbb902
- Fetch the ACKs only at sign-off time. This makes sure that any
last-minute ACKs are included (fixes#16200)
- Show a list of ACKs and their author before signing off, and warn if
there are none
Always put the unabbreviated commit IDs in the generated commit messages
and other places. This prevents the developer's `core.abbrev` git setting
from leaking through and is better against ambiguity too.
d7c0542777 doc: update release process with SECURITY.md (Jon Atack)
e4e2b28cf1 doc: clarify support in SECURITY.md (Jon Atack)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/16140:
- Update the release process to maintain SECURITY.md; this looks like the sort of item that can otherwise be easily overlooked during a major release
- Clarify type of support in SECURITY.md
Question: If https://bitcoincore.org/en/lifecycle/#maintenance-period is still current policy, should v0.15 now be unmaintained and v0.16 EOL... seems the schedule on that page could use an update.
ACKs for commit d7c054:
practicalswift:
ACK d7c0542777
fanquake:
ACK https://github.com/bitcoin/bitcoin/pull/16164/commits/d7c054277798cc1975041eb19c679cd07c412d68. This seems to make sense.
Tree-SHA512: ce0f832d9804d7bfd29f2361948d7d6a4e93004a1f57e07a95dfba056caa4d8c4552267c66e6728b689b0309f4688c2d8d59d7b0c26b838c6a30df878a69fceb
53b7de629d Add test for dumping the private key imported from descriptor (MeshCollider)
2857bc4a64 Extend importmulti descriptor tests (MeshCollider)
81a884bbd0 Import private keys from descriptor with importmulti if provided (MeshCollider)
a4d1bd1a29 Add private key derivation functions to descriptors (MeshCollider)
Pull request description:
~This is based on #14491, review the last 3 commits only.~
Currently, descriptors have an Expand() function which returns public keys and scripts for a specific index of a ranged descriptor. But the private key for a specific index is not given. This allows private keys for specific indices to be derived. This also allows those keys to be imported through the `importmulti` RPC rather than having to provide them separately.
ACKs for commit 53b7de:
achow101:
ACK 53b7de629d
Tree-SHA512: c060bc01358a1adc76d3d470fefc2bdd39c837027f452e9bc4bd2e726097e1ece4af9d5627efd942a5f8819271e15ba54f010b169b50a9435a1f0f40fd1cebf3
a352d2ae5 Don't GPG sign intermediate commits with github-merge tool (Steven Roose)
Pull request description:
When you don't have an unencrypted key and you have git configured to always sign all commits, you have to sign three times instead of one.
ACKs for commit a352d2:
MarcoFalke:
utACK a352d2ae5f
Tree-SHA512: f5a1c0868c42de2b5df45126e1e94b4a7d88b041d179244228bb7b02d43d41f565388eb1052a097ab0666fc6003f8453b9e283801fcb33aef47f412b72ba9fb6
8be3f3063 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong)
Pull request description:
```
The original ORCHID prefix was deprecated as of 2014-03, the new
ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not
consider the original ORCHID prefix routable, and I don't see any reason
to consider the new one to be either.
```
Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code.
ACKs for commit 8be3f3:
laanwj:
utACK 8be3f3063
ryanofsky:
utACK 8be3f30633. Only change since last review is rebasing after #15718 merge.
Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
8541cbea2 depends: libX*: --disable-malloc0returnsnull in conf (Carl Dong)
0e752637a depends: libXext: Bump to 1.3.3 to fix _XEatDataWords (Carl Dong)
683b7d7a3 depends: Purge libtool archives (Carl Dong)
14209286d depends: Build secondary deps statically. (Carl Dong)
Pull request description:
```
We use pkg-config where we can, which generally replaces libtool at a
higher level and does not have the same downsides as libtool. These
archives sit in our depends tree with no purpose and pollute the final
bitcoin build with massive overlinking.
```
See [here](https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Handling_Libtool_Archives) for an explanation of the various problems libtool archives can cause.
Unrelated in every way except in spirit: `-D__LIBTOOL_IS_A_FOOL__`!!
-----
This PR is based on #16041, and therefore should be merged after #16041.
ACKs for commit 8541cb:
Tree-SHA512: 76030cf32361f0b1cfe14e3827a0cbec99994e7da00a56194ca40cf6cf7d87f78552f49d03d41ce9cf9b642992b90d993578ed1f0ad6bae15cd3f1c88dfaa4b0
fdd7fa19a docs: create security policy (Neha Narula)
Pull request description:
Github has started supporting SECURITY.md to contain a project's
security policy. Right now, the only place to find this project's
security contact is on bitcoincore.org. Adding this information to the
repository makes it easier to find as SECURITY.md becomes a standard.
This is copied almost exactly from https://bitcoincore.org/en/contact/
and based on conversations with EthanHeilman.
ACKs for commit fdd7fa:
laanwj:
ACK fdd7fa19a9
Tree-SHA512: 9d6b93d10fff6e9c7a5cb6d8c1f0660623cd7a015abac7738f2aa9d141075456e71612b830eb5c707275529e2099fb41a44c531e29d821c9d2857d22241a91c3
58e291cfa Add test for GCC bug 90348 (Pieter Wuille)
Pull request description:
This adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own `sha256d64` test in crypto_tests.cpp, which was failing on some platforms.
This is based on top of #15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it).
ACKs for commit 58e291:
Tree-SHA512: 4dc9084e92dd143a53930e42bb68e33d922a2a2b891406b259d3a0bed4511dcc49e7447a7a8e4eb793a26e3eacb188ca293b71e0e061f9b3230f8e7fcfd29525
7860c98bd wallet: do not encrypt wallets with disabled private keys (whythat)
Pull request description:
Fix for #15635.
Throw an `RPC_WALLET_ENCRYPTION_FAILED` error when attempting to encrypt wallet with disabled private keys. Changed `test/function/wallet_createwallet.py` to test new behavior.
ACKs for commit 7860c9:
achow101:
utACK 7860c98bd5
meshcollider:
utACK https://github.com/bitcoin/bitcoin/pull/16144/commits/7860c98bd59cd8f56e9b2b4ae45265c046e7cfd9
Tree-SHA512: d0cc40efd303a00d0b4d3cb2de59d8d2d7dd35647e7f3fe9d4a8986589499c1f567c5780c83a129e1ab8dbe601279c459c6ebce3b48b1d81d47a28616ef4a369
403e677c9 refactoring: IsInitialBlockDownload -> CChainState (James O'Beirne)
3ccbc376d refactoring: FlushStateToDisk -> CChainState (James O'Beirne)
4d6688603 refactoring: introduce ChainstateActive() (James O'Beirne)
d7c97edee move-only: make the CChainState interface public (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
This changeset starts moving functionality intimately related to CChainState into methods. Parameterizing these functions by a particular CChainState is necessary for the use of multiple chainstates simultaneously (e.g. for asynchronous background validation).
In this change, we
- make the CChainState interface public - since other units will start to invoke its methods directly,
- introduce `::ChainstateActive()`, the CChainState equivalent for `::ChainActive()`,
- and move `IsInitialBlockDownload()` and `FlushStateToDisk()` into methods on CChainState.
Independent of assumeutxo, these changes better encapsulate chainstate behavior and allow easier use from a testing context.
There are more methods that we'll move in the future, but they require other substantial changes (i.e. moving ownership of the `CCoinsView*` hierarchy into CChainState) so we'll save them for future PRs.
---
The first move-only commit is most easily reviewed with `git diff ... --color-moved=dimmed_zebra`.
ACKs for commit 403e67:
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/15976/commits/403e677c9ebbf9744733010e6b0c2d1b182ee850 no need to address my nits herein
Sjors:
utACK 403e677
ryanofsky:
utACK 403e677c9e. Only change since previous review is removing global state comment as suggested.
MarcoFalke:
utACK 403e677c9e, though the diff still seems a bit bloated with some unnecessary changes in the second commit.
promag:
utACK 403e677 and rebased with current [master](c7cfd20a7).
Tree-SHA512: 6fcf260bb2dc201361170c0b4547405366f5f331fcc3a2bac29b24442814b7b244ca1b58aac5af716885f9a130c343b544590dff780da0bf835c7c5b3ccb2257
6e6494b3fb qt: fix opening bitcoin.conf via Preferences on macOS; see #15409 (shannon1916)
Pull request description:
Fix#15409. The QT wallet fail to open the configuration file on Mac, when these is no default application for `*.conf` files.
Here is a feasible way to solve this bug. When `QDesktopServices::openUrl` fails to open `file:///path/bitcoin.conf` with its default application, use `QProcess::startDetached` to run `open -t /path/bitcoin.conf` command instead, so as to open the configuration file with system's default text editor.
ACKs for commit 6e6494:
hebasto:
re-ACK 6e6494b3fb
fanquake:
tACK https://github.com/bitcoin/bitcoin/commit/6e6494b3fb345848025494cb7a79c5bf8f35e417 on macOS 10.14.x
Tree-SHA512: 60e898f4cb77cfd7b8adbc8d33fbebf46bac2a801bdcf40cae15e24b78ad56b1f32358b1879b670623d9f8651dea93961d34269358cea18f4e15b089a8ffcfbf
2d8ad2f997 gui: Enable console line edit on setClientModel (João Barbosa)
Pull request description:
Make console line edit disable by default, and only enable once `RPCConsole::setClientModel` is called.
Fixes#16119.
ACKs for commit 2d8ad2:
fanquake:
tACK https://github.com/bitcoin/bitcoin/pull/16122/commits/2d8ad2f99710a8981e33fe2d6ce834b0076c4e80 on macOS.
Tree-SHA512: 1418ce3c120c08e5ec3e7a7a063572a24402ce0ec541bd4adc21f61d60c4e86b711e82e940ebf5f0445ab861f89c146c2a2e7990fb52bed2c65fc199a1981f71
d2ae6be80f gui: Set progressDialog to nullptr (João Barbosa)
Pull request description:
If a progress notification `> 0` arrives immediately after notification `= 100` then `progressDialog` is a dangling pointer.
Potential fix for #16134.
ACKs for commit d2ae6b:
hebasto:
utACK d2ae6be80f
fanquake:
tACK https://github.com/bitcoin/bitcoin/pull/16135/commits/d2ae6be80f6a0156021bf8c9b9d17cd4966ddffc
Tree-SHA512: 300ddde2f27c494b19a5bd4085400d0f5a1d4980fe8cc3c07bfebb037efc35f777215ff1a095eeb16658407e11f04456137393e88a12fdd767b7aac5f12eab5e
00fade007c contrib: use newer config.guess & config.sub in install_db4.sh (fanquake)
Pull request description:
The `config.guess` and `config.sub` packaged with `db-4.8.30.NC.tar.gz` are fairly old (2009) and fail to identify some system types i.e `ppc64le`. Replace them with more modern versions before configuring `db4`.
Fixes#16064.
ACKs for commit 00fade:
jamesob:
tACK https://github.com/bitcoin/bitcoin/pull/16086/commits/00fade007c8c0aba82a2a8c40129ae0b6e530d33
dongcarl:
utACK 00fade007c
laanwj:
Tested ACK 00fade007c
Tree-SHA512: 54f9f2b0fcf802e6a284214035fcb3833577aa5881beb293921b6036b71d0bcd0872f06d14ddc76a4f8c0eee7ba71461dcc99dc907e7ddb5ae403fc02b245538
54245985fb Squashed 'src/secp256k1/' changes from 0b70241850..b19c000063 (Pieter Wuille)
Pull request description:
It's been 1.5 years since our secp256k1 subtree was updated, while the upstream project has undergone a number of incremental improvements (performance, tests, build system fixes), plus gained the groundwork for batch verification.
As we're early in the 0.19 window, this seems like a good time to get these merged.
ACKs for commit 99df27:
fanquake:
utACK 99df276 the subtree merge, still need to test the actual changes.
laanwj:
utACK 99df276da
Tree-SHA512: 769a699366321635068ebfbd9d3f30f6e72401c4fcdc1fdc84e5b3fd888c3f01437748f6cd23a507ab47cf04c226cd504fd48aee654457c34bb106c9db7e5c09
fa013664ae util: Add type safe GetTime (MarcoFalke)
Pull request description:
There are basically two ways to get the time in Bitcoin Core:
* get the system time (via `GetSystemTimeInSeconds` or `GetTime{Millis,Micros}`)
* get the mockable time (via `GetTime`)
Both return the same type (a plain int). This can lead to (test-only) bugs such as 99464bc38e.
Fix that by deprecating `GetTime` and adding a `GetTime<>` that returns the mockable time in a non-int type. The new util function is currently unused, but new code should it where possible.
ACKs for commit fa0136:
promag:
utACK fa013664.
Tree-SHA512: efab9c463f079fd8fd3030c479637c7b1e8be567a881234bd0f555c8f87e518e3b43ef2466128103db8fc40295aaf24e87ad76d91f338c631246fc703477e95c
c995c870aa depends: add ability to skip building zeromq (fanquake)
Pull request description:
Similar to other depends packages, add the ability to skip building `zeromq` by passing `NO_ZMQ=1`.
Fixes#15918.
ACKs for commit c995c8:
practicalswift:
utACK c995c870aa
jonasschnelli:
utACK c995c870aa
Tree-SHA512: 72269707916d5af0bc8ecdd89f61e49264dba29350f9508fe0a497e8ce8dae66f6a828cf0bf4d97b6f95356b505cb3e6c365e8476219dd56c4535c850df393c9
fadbc5d895 mempool: remove unused magic number from consistency check (Gregory Sanders)
Pull request description:
Unexplained magic numbers are no good. Since the exact number does not matter, opt for a constant that is less peculiar.
Note that this could only possibly affect mempool consistency checks which is not active by default except on regtest.
see discussion: https://github.com/bitcoin/bitcoin/issues/15080
ACKs for commit fadbc5:
practicalswift:
utACK fadbc5d895
Tree-SHA512: 80f95ebc284c5bcc5d825fab0e9f962457a411539946d68ef4c8bdea4b1f2f7f0ead88928fac0eaaa02a1175f01f5ef381613ce53b0f27c3098e90d76ecfe9af
d5dc66e280 doc: fix/improve analyzepsbt in doc/psbt.md (Jon Atack)
Pull request description:
- fix: replace "RPC" with "PSBT"
- output includes the current status of the analyzed psbt's inputs
- apply "if possible" to the fee as well as to the estimated weight and feerate, since the fee is only shown if all utxo slots in the psbt have been filled
- add "final" to the estimated weight and feerate
ACKs for commit d5dc66:
laanwj:
ACK d5dc66e280
fanquake:
utACK d5dc66e
Tree-SHA512: 61ff1ef45ec34182613b300d21cc2b17a28d1e955f70848f5be1a40c82009fe3000db3332d2cfca1833d7c881b61cc4ebc9fc779238f76d38e9e3f706cfb3551
fa2b52af32 Disallow extended encoding for non-witness transactions (take 3) (MarcoFalke)
Pull request description:
(previous title "p2p: Disallow extended encoding for non-witness transactions (take 3)")
Remote peers can send us illegally encoded txs and thus have us write to stderr. Fix that by not writing to stderr.
This is a follow up to the previous (incomplete) attempts at this:
* Disallow extended encoding for non-witness transactions #14039
* Add test for superfluous witness record in deserialization #15893
ACKs for commit fa2b52:
laanwj:
utACK fa2b52af32
ryanofsky:
utACK fa2b52af32. Would change title to something like "Avoid logging transaction decode errors to stderr" instead of "Disallow extended encoding for non-witness transactions." The current title is confusing because this PR isn't really allowing or disallowing anything, just logging the condition differently. "Disallow" also seems to contradict the "Allow exceptions from..." comments in the actual code.
Tree-SHA512: c66990e69b432d00dc1c5510bf976a1188664d0890a32d1e5c6459094e7e27da82a5d227627afcbc203676f5540eec74b7d9b1d71d2c62d3b2069e1781824b4d
3cb9ce85d0 Document strenghtening (Pieter Wuille)
1d207bc46f Add hash strengthening to the RNG (Pieter Wuille)
Pull request description:
This patch improves the built-in RNG using hash strengthening.
At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.
ACKs for commit 3cb9ce:
pstratem:
utACK 3cb9ce85d0
Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
8794a4b3ae QA: add test for HKDF HMAC_SHA256 L32 (Jonas Schnelli)
551d489416 Add HKDF HMAC_SHA256 L=32 implementations (Jonas Schnelli)
3b64f852e4 QA: add test for CKey::Negate() (Jonas Schnelli)
463921bb64 CKey: add method to negate the key (Jonas Schnelli)
Pull request description:
This adds a limited implementation of `HKDF` (defined by rfc5869) that supports only HMAC-SHA256 and length output of 32 bytes (will be required for v2 transport protocol).
This PR also includes a method to negate a private key which is useful to enforce public keys starting with 0x02 (or 0x03) (a requirement for the v2 transport protocol). The new `CKey::Negate()` method is pretty much a wrapper around `secp256k1_ec_privkey_negate()`.
Including tests.
This is a subset of #14032 and a pre-requirement for the v2 transport protocol.
ACKs for commit 8794a4:
Tree-SHA512: 5341929dfa29f5da766ec3612784baec6a3ad69972f08b5a985a8aafdae4dae36f104a2b888d1f5d1f33561456bd111f960d7e32c2cc4fd18e48358468f26c1a
fa8ced32a6 doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de79f test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e7b4 test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7329 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)
Pull request description:
This is de-facto no longer hidden
ACKs for commit fa8ced:
jamesob:
utACK https://github.com/bitcoin/bitcoin/commit/fa8ced32a60dea37ac169241cf9a1f708ef46c4b
Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
662d1171d9 Add option to create an encrypted wallet (Andrew Chow)
Pull request description:
This PR adds a new `passphrase` argument to `createwallet` which will create a wallet that is encrypted with that passphrase.
This is built on #15226 because it needs to first create an empty wallet, then encrypt the empty wallet and generate new keys that have only been stored in an encrypted state.
ACKs for commit 662d11:
laanwj:
utACK 662d1171d9
jnewbery:
Looks great. utACK 662d1171d9
Tree-SHA512: a53fc9a0f341eaec1614eb69abcf2d48eb4394bc89041ab69bfc05a63436ed37c65ad586c07fd37dc258ac7c7d5e4f7f93b4191407f5824bbf063b4c50894c4a
c01c065b9d Do not construct out-of-bound pointers in SHA512/SHA1/RIPEMD160 code (Pieter Wuille)
Pull request description:
This looks like an issue in the current SHA256/512 code, where a pointer outside of the area pointed to may be constructed (this is UB in theory, though in practice every supported platform treats pointers as integers).
I discovered this while investigating #14580. Sadly, it does not fix it.
ACKs for commit c01c06:
practicalswift:
utACK c01c065b9d
Tree-SHA512: 47660e00f164f38c36a1ab46e52dd91cd33cfda6a6048d67541c2f8e73c050d4d9d81b5c149bfad281212d52f204f57bebf5b19879dc7a6a5f48aa823fbc2c02
1b05dff080 Fix portability issue with pthreads (grim-trigger)
Pull request description:
This change resolves the following issue:
https://github.com/bitcoin/bitcoin/issues/15951
Only tested on OpenBSD 6.5/amd64
ACKs for commit 1b05df:
fanquake:
tACK 1b05dff. Tested on OpenBSD6.4 (`vagrant`).
laanwj:
utACK 1b05dff080
Tree-SHA512: af48581af32820d5adc9ae5abb44f8f1b592c323f86fe2484108b81629389f6ef347598f9a087aa6476ac553e59828cd7927bb4ab11dc70e7c9a944a92fc54ae
78e407ad0c GetKeyBirthTimes should return key ids, not destinations (Gregory Sanders)
70946e7fee Replace CScriptID and CKeyID in CTxDestination with dedicated types (Gregory Sanders)
Pull request description:
The current usage seems to be an overloading of meanings. `CScriptID` is used in the wallet as a lookup key, as well as a destination, and `CKeyID` likewise. Instead, have all destinations be dedicated types.
New types:
`CScriptID`->`ScriptHash`
`CKeyID`->`PKHash`
ACKs for commit 78e407:
ryanofsky:
utACK 78e407ad0c. Only changes are removing extra CScriptID()s and fixing the test case.
Sjors:
utACK 78e407a
meshcollider:
utACK https://github.com/bitcoin/bitcoin/pull/15452/commits/78e407ad0c26190a22de1bc8ed900164a44a36c3
Tree-SHA512: 437f59fc3afb83a40540da3351507aef5aed44e3a7f15b01ddad6226854edeee762ff0b0ef336fe3654c4cd99a205cef175211de8b639abe1130c8a6313337b9
beda0dae95 Upgrade gitian image before signing (Hennadii Stepanov)
Pull request description:
The package upgrade in the Ubuntu repositories in the period between the building and the signing causes (particularly, using LXC) an error:
```
Creating package manifest
Could not download some packages, please run gbuild --upgrade
```
For example, the [`busybox-initramfs`](https://packages.ubuntu.com/bionic/busybox-initramfs) package was [upgraded](http://changelogs.ubuntu.com/changelogs/pool/main/b/busybox/busybox_1.27.2-2ubuntu3.2/changelog) from `1:1.27.2-2ubuntu3.1` to `1:1.27.2-2ubuntu3.2` on 2019-03-06.
This PR forces gitian image upgrade for the `--sign` command.
Ref:
[devrandom/gitian-builder/target-bin/grab-packages.sh](https://github.com/devrandom/gitian-builder/blob/04ab7c121842be8c5848b3de5076c8ded7761314/target-bin/grab-packages.sh)
```
#!/bin/sh
# Get an installed package manifest
set -e
cd /var/cache/apt/archives
# make sure all packages with installed versions are downloaded
# (except for held packages, which may not be available for download)
dpkg-query -W -f '${Status}\t${Package}=${Version}\n' | grep -v ^hold | cut -f2- | xargs -n 50 apt-get install -q --reinstall -y -d > /tmp/download.log
grep "cannot be downloaded" /tmp/download.log && { echo Could not download some packages, please run gbuild --upgrade 1>&2 ; exit 1 ; }
sha256sum *.deb | sort --key 2
```
ACKs for commit beda0d:
laanwj:
utACK beda0dae95
fanquake:
utACK beda0da
Tree-SHA512: e2e3b3e3719e098d266ceec39bd69b950344a4eb2f43ae6ad3e696add70f743b363cc83676e339f7caa207d6478029869a8af01fe1f6d5690d2857003f7d8ce8
b6c748f849 doc: Add release notes for 15730 (João Barbosa)
d3e8458365 rpc: Show scanning details in getwalletinfo (João Barbosa)
90e27abe37 wallet: Track current scanning progress (João Barbosa)
2ee811e693 wallet: Track scanning duration (João Barbosa)
Pull request description:
Closes#15724.
ACKs for commit b6c748:
MarcoFalke:
re-utACK b6c748f849 (Only change since my last review is rebase, adding release notes, and returning false instead of null)
laanwj:
utACK b6c748f849
jonatack:
ACK b6c748f849, only changes appear to be rebase for https://github.com/bitcoin/bitcoin/pull/15730#discussion_r280030617 and release notes.
Tree-SHA512: 8ee98f971c15f66ce8138fc92c55e51abc9faf01866a31ac7ce2ad766aa2bb88559eabee3b5815d645c84cdf1c19dc35ec03f31461e39bc5f6040edec0b87116
facfb4111d rpc: Deprecate getunconfirmedbalance and getwalletinfo balances (MarcoFalke)
999931cf8f rpc: Add getbalances RPC (MarcoFalke)
fad13e925e rpcwallet: Make helper methods const on CWallet (MarcoFalke)
fad40ec915 wallet: Use IsValidNumArgs in getwalletinfo rpc (MarcoFalke)
Pull request description:
This exposes the `CWallet::GetBalance()` struct over RPC.
In the future, incorrectly named rpcs such as `getunconfirmedbalance` or rpcs redundant to this such as `getbalance` could be removed.
ACKs for commit facfb4:
jnewbery:
utACK facfb4111d
Tree-SHA512: 1f54fedce55df9a8ea82d2b6265354b39a956072621876ebaee2355aac0e23c7b64340c3279502415598c095858529e18b50789be956250aafda1cd3a8d948a5
00d110463a Install bitcoin-wallet manpage. (Daniel Kraft)
Pull request description:
This change marks the already-existing `bitcoin-wallet.1` manpage file for installation together with the others. Previously, only `bitcoind.1`, `bitcoin-cli.1`, `bitcoin-tx.1` and `bitcoin-qt.1` would be installed.
ACKs for commit 00d110:
laanwj:
utACK 00d110463a
practicalswift:
utACK 00d110463a
Tree-SHA512: ca846e414548f1dc774f460edca2e17d7d619c7e6f0d18db0c58c09e04f9d43c6964fcf2bacb5b1eae94de9c5fdda86abf258ef6b78b0f693715d070dfc10f08
fc929842c2 GUI: Move QRImageWidget to its own file-pair (Luke Dashjr)
77851ab682 GUI: Refactor actual QR code rendering into new QRImageWidget::setQR (Luke Dashjr)
Pull request description:
For at least QR-code based pairing of mobile wallets with nodes, it will be desirable to render QR codes even without wallet support.
Therefore, this prepares by moving the QRImageWidget out of a wallet-specific file into its own `qrencoder` file-pair.
ACKs for commit fc9298:
laanwj:
utACK fc929842c2
jonasschnelli:
utACK fc929842c2
Tree-SHA512: 95529a38c0573a4b3f1253fb5f11ca07a5b3a9840ec24acc7d87270212f3c9f7c5b186d9274d297517a3b80494f38a57574fb9730b1574db01688539b987bd91
