5bad7921d0 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost)
29a21c9061 [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost)
Pull request description:
In https://github.com/bitcoin/bitcoin/pull/13557#pullrequestreview-135905054 I recommended not including bip32 deriviation by default in PSBTs:
> _Bit of a privacy issue_: let's say person A and B are about to spend from a multisig address, sending everything to person A. Person A gives their address to person B, their wallet wallet creates a PSBT, but doesn't sign it. Wallet A then calls `walletprocesspsbt` which signs it and _spontaneously adds the master_fingerprint and bip32 path_. Same issue with `walletcreatefundedpsbt`.
>
> Adding `bip32_derivs` should probably be opt-in.
In practice I find this default quite annoying because I forget it and end up with a confused hardware wallet.
More importantly, in the multisig example I provided, it's actually essential for the other side to know the derivation details (in addition to an xpub). This allows them to check that change is going to an address you can still co-sign for (because the spending policy is unchanged except for an index).
ACKs for top commit:
instagibbs:
utACK 5bad7921d0
jonatack:
ACK 5bad7921d0 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests.
meshcollider:
utACK 5bad7921d0
Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd
63ce882760 doc: link to homebrew's troubleshooting page (Gastón I. Silva)
Pull request description:
A trivial documentation update.
When I was following the build steps for mac, I had some errors installing the dependencies. After searching on the Internet, and correcting the errors, I found that `brew doctor` had all the answers I needed. Could have skipped the Internet searches all together.
ACKs for top commit:
fanquake:
ACK 63ce882760 - a link to the troubleshooting page seems fine. I wouldn't really want our README to have anything more specific than that.
Tree-SHA512: 12c96cd9c9bd39ada21f3f27cbec3ed4bef4b8e74dec7872c892fc6a92a70418a5cc0882ff449883e91d96c01e1ca7104b076590917f397334c82931ec7fda1c
7f1475c711 rpc: update validateaddress RPCExamples to bech32 (Sebastian Falbesoner)
Pull request description:
Another small step to get rid of legacy addresses in the RPC help texts and by that encourage the use of bech32 addresses by default. The (invalid) address is the same as in the `getaddressinfo` RPC (see 2ee0cb3330, kudos to jonatack!), I don't think it adds any value to have a different example address per RPC.
ACKs for top commit:
fanquake:
ACK 7f1475c711
MarcoFalke:
ACK 7f1475c711
Tree-SHA512: 2350f61fa942a9053f9f5c860ea446965dc7209c71c81bdb98a859d03ca23b225ad72c9c506e4a55c8d8988823d9cfbe808c1a452a1eeadb70ab186b146dd4ca
also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
(mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
address suitable for RPCExamples help documentation)
fae9084ac5 build: Skip i686 build by default in guix and gitian (MarcoFalke)
fa55a2554c depends: Remove reference to win32 (MarcoFalke)
Pull request description:
Closes#17504
Now that we no longer provide downloads for i686 on our website (https://bitcoincore.org/en/download/), there is no need to build them by default.
i686 can still be built in depends (tested by ci/travis) and in guix/gitian by setting the appropriate `HOSTS`.
ACKs for top commit:
practicalswift:
ACK fae9084ac5 -- patch looks correct
dongcarl:
ACK fae9084ac5 patch looks correct
laanwj:
Code review ACK fae9084ac5
hebasto:
ACK fae9084ac5, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: b000c19a2cd2a596a52028fa298c4022c24cfdfc1bdb3795a90916d0a00a32e4dd22278db93790b6a11724e08ea8451f4f05c77bc40d1664518e11a8c82d6e29
677fb8e923 test: Add ubsan surpression for crc32c (Wladimir J. van der Laan)
8e68bb1dde build: Disable msvc warning 4722 for leveldb build (Aaron Clauson)
be23949765 build: MSVC changes for leveldb update (Aaron Clauson)
9ebdf04757 build: CRC32C build system integration (Wladimir J. van der Laan)
402252a808 build: Add LCOV exception for crc32c (Wladimir J. van der Laan)
3a037d0067 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan)
84ff1b2076 test: Add crc32c to subtree check linter (Wladimir J. van der Laan)
7cf13a5134 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan)
24d02a9ac0 build: Update build system for new leveldb (Wladimir J. van der Laan)
2e1819311a Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan)
66480821b3 Squashed 'src/leveldb/' changes from f545dfabff4c2e9836efed094dba99a34fbc6b88..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan)
Pull request description:
This updates leveldb to currently newest upstream commit 0c40829872:
- CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future.
- Thread handling uses C++11, instead of platform specific code.
- Native windows environment was added. No need to maintain our own hacky one, anymore.
- Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed.
All changes: a53934a3ae...0c40829872
Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new
There's quite some testing to be done (see below). See https://github.com/bitcoin-core/leveldb/issues/25 and https://github.com/bitcoin-core/leveldb/pull/26 for more history and context.
TODO:
- [x] Subtree `crc32c`
- [x] Make linters happy about crc32 subtree
- [x] Integrate `crc32c` library into build system
- [x] MSVC build system
ACKs for top commit:
sipa:
ACK 677fb8e923
Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
19a354b11f Output a descriptor in createmultisig and addmultisigaddress (Andrew Chow)
Pull request description:
Give a descriptor from `createmultisig` and `addmultisigaddress`.
Extracted from #16528 with `addmultisgaddress` and tests added.
ACKs for top commit:
Sjors:
tACK 19a354b11f
MarcoFalke:
ACK 19a354b11f
promag:
Code review ACK 19a354b11f.
meshcollider:
utACK 19a354b11f
Tree-SHA512: e813125fbbc358ea8d45b1748de16a29a94efd83175b748fb8fa3b0bfc8e783ed36b6c554d84f5d4ead1ba252a83a3e937b6c3f75da7b8d3b4e55f94d6013771
900d8f6f70 util: Disallow network-qualified command line options (Russell Yanofsky)
Pull request description:
Previously these were allowed but ignored.
This change implements one of the settings simplifications listed in #17508. Change includes release notes.
ACKs for top commit:
laanwj:
ACK 900d8f6f70
Tree-SHA512: ab020a16a86c1e8ec709fbf798d533879d32c565eceeb7eb785c33042c49c6b4d1108c5453d8166e4a2abffc2c8802fbb6d3b895e0ddeefa8f274fd647e3c8ad
d3bc184081 doc: update release notes with getaddressinfo label deprecation (Jon Atack)
72af93f364 test: getaddressinfo label deprecation test (Jon Atack)
d48875fa20 rpc: deprecate getaddressinfo label field (Jon Atack)
dc0cabeda4 test: remove getaddressinfo label tests (Jon Atack)
c7654af6f8 doc: address pr17578 review feedback (Jon Atack)
Pull request description:
This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo `label` field. The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=label`.
See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 for more context.
Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=label` flag while verifying the rpc getaddressinfo output and help text.
Next step: add support for multiple labels.
ACKs for top commit:
jnewbery:
ACK d3bc184081
laanwj:
ACK d3bc184081
meshcollider:
utACK d3bc184081
Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
b6c3e84e87 doc: Improve fuzzing docs for macOS users (Fabian Jahr)
Pull request description:
Adds several helpful hints for macOS users trying to get fuzzers to run locally using AFL or libFuzzer. These are partly based on this comment https://github.com/bitcoin/bitcoin/issues/17657#issuecomment-562869600 and discussions in the review club for #17860. See: https://bitcoincore.reviews/17860.html
Based on the doc in the current state I could not compile fuzzers for AFL or libFuzzer. Using these hints, I can
- compile and run fuzzers with AFL
- compile but **not** run fuzzers with libFuzzer
Fuzzers compiled with libFuzzers may be running but don't produce any output. Looking for others to test this to see if it is an issue with my local system. Especially interesting if you have been running libFuzzer fuzzers successfully on macOS before.
Edit: Closes#17914
ACKs for top commit:
MarcoFalke:
ACK b6c3e84e87
Sjors:
ACK b6c3e84
fanquake:
ACK b6c3e84e87 - I think this has been nitpicked enough, and importantly the commands look better now.
Tree-SHA512: fdbacbcf10e9353a4ac3d22edf88663e33185ad2f244b986ff74c513de05f9fa62c4d8b17985d2f9288834c124b352cf52280627b5ff095735b411b12482e2ec
42ec499489 doc: developer notes guideline on RPCExamples addresses (Jon Atack)
Pull request description:
to make explicit the use of invalid addresses for user safety and to encourage
the use of bech32 addresses by default. See https://github.com/bitcoin/bitcoin/pull/17578#discussion_r361752570 and https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362564492.
Fix a typo to appease the linter.
ACKs for top commit:
promag:
ACK 42ec499489, no strong opinion as whether this belongs to developer notes or not but why not.
fjahr:
ACK 42ec499
michaelfolkson:
ACK 42ec499489
Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e
8925df86c4 doc: update release notes (Jon Atack)
8bb405bbad test: getaddressinfo labels purpose deprecation test (Jon Atack)
60aba1f2f1 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack)
7851f14ccf rpc: incorporate review feedback from PR 17283 (Jon Atack)
Pull request description:
This PR builds on #17283 (now merged) and is followed by #17585.
It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs.
before
```
"labels": [
{
"name": "DOUBLE SPEND",
"purpose": "receive"
}
```
after
```
"labels": [
"DOUBLE SPEND"
]
```
The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`.
For context, see:
- https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001
- http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427)
- http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622
Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output.
Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.
ACKs for top commit:
jnewbery:
reACK 8925df8
promag:
Code review ACK 8925df86c4.
meshcollider:
Code review ACK 8925df86c4
Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
to make explicit the use of invalid addresses for user safety and to encourage
the use of bech32 addresses by default.
Fix a typo to appease the linter.
6094222de7 use preferred shebang approach for documentation (hackerrdave)
Pull request description:
Documentation update to use recommended shebang approach mentioned in the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#shebang)
ACKs for top commit:
hebasto:
ACK 6094222de7, I have reviewed the code, and it looks OK, I agree it can be merged.
Tree-SHA512: fc58632f0a6fa82c7abdddfac4897f082110d647426d2b468cba6fabf6b34a015fcad47e5b26be98e629b8b0417b8781e8d89da67189e20da228b97b17f1a532
a5089f62bd fix directory path for secp256k1 subtree in developer-notes (hackerrdave)
Pull request description:
Documentation update to fix the directory path of the `secp256k1` subtree in the developer notes
ACKs for top commit:
laanwj:
ACK a5089f62bd
Tree-SHA512: d0986721d7091af26edaee769db78c9aabac25bbaddb2a1bfa96c7208187226e280e9c38897b5227ee6c9e40d5a1af86bb7c58e72c6a30a94a478c4bf54c086e
fab2f351f2 doc: Update release process with latest changes (MarcoFalke)
Pull request description:
Mainly adding the reminder to bump the flatpak
ACKs for top commit:
laanwj:
ACK fab2f351f2
fanquake:
ACK fab2f351f2
Tree-SHA512: fe279a6cdee881e8dd608cb7d09d992c4b668b01b9d0d2dbfaf92f12f3032b8fcb2c256b20fcee861397451add1338f162b6e5fa7b3c21e76c247cc419315284
5ad4dd1ea1 doc: Changed MiniUPnPc link to https in dependencies.md (Marius Kjærstad)
Pull request description:
doc: Changed MiniUPnPc link to https in dependencies.md
Top commit has no ACKs.
Tree-SHA512: 228ee98c877612468a34d09610999a47257ab1e060f3004a530639f0c29fb473b48e59588ff70297c53a3abeb2bb32bfedbb61e102a7fc10df4bb1b5d0d5893b
27d82b63fb gui: remove macOS start on login code (fanquake)
Pull request description:
The macOS startup item code was disabled for builds targeting macOS >
`10.11` in #15208. Now that we require macOS `10.12` as a minimum (#17550),
we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc,
was removed in macOS `10.12` SDK.
ACKs for top commit:
jonasschnelli:
utACK 27d82b63fb
jonasschnelli:
Tested ACK 27d82b63fb - successfully compiled on 10.15.1
Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409
8f15a31760 doc: add glibc 2.17 requirement to release-notes (fanquake)
16a7be1663 build: Bump minimum versions in symbol checker (Wladimir J. van der Laan)
b77d5ad59f build: Disallow dynamic linking against c++ library (Wladimir J. van der Laan)
Pull request description:
Closes: #17525. Taken over from #17531.
Debian 8 (Jessie) has:
- g++ version 4.9.2
- libc version 2.19
CentOS 7 has:
- g++ version 4.8.5
- libc version 2.17
Ubuntu 16.04.4 (Xenial, oldest supported Ubuntu) has:
- g++ version 5.3.1
- libc version 2.23.0
Taking the minimum of these as our target. According to [GNU ABI document](https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html) this corresponds to:
- GCC 4.8.5: GCC_4.8.0
- (glibc) GLIBC_2_17
This also contains a (long needed) commit to disallow dynamic linking to stdc++, as our releases statically link against that.
ACKs for top commit:
laanwj:
re-ACK 8f15a31760
Tree-SHA512: a3cc92aa1c5de253b1531f4b854d6f5f4a15d614ba6290d9db293542a96994b55c4a8e33e03b601bae16eb65529630b4f94b48b010e0b66b7dc9ff0acf945107
794fe91395 doc: Update and improve Developer Notes (Hennadii Stepanov)
Pull request description:
This PR:
- removes outdated things, e.g., global pointer `pwalletMain` etc
- adds "Sanitizers" to the TOC
- makes filenames, `peer.dat` and `debug.log`, monospaced
- specifies that _compile-time_ constant names are all uppercase
- rewords using `explicit` with constructors
ACKs for top commit:
jamesob:
lazy ACK 794fe91395
practicalswift:
ACK 794fe91395 -- nice improvements!
Tree-SHA512: 2c5f035b1627f5fac8dc2453199d9e46bd101f86771de567cd95698de3c61cc459444ec1a68710e1d280195e1e40b42d9f40906297d12f12bf37749eca58297d
The macOS startup item code was disabled for builds targeting macOS >
10.11 in #15208. Now that we require macOS 10.12 as a minimum, #17550,
we can remove the startup item code entirely, as the API we were using
was removed in macOS 10.12.
Samuel Dobson