24718 Commits

Author SHA1 Message Date
Wladimir J. van der Laan
bd93e32292 refactor: Replace HexStr(o.begin(), o.end()) with HexStr(o)
HexStr can be called with anything that bas `begin()` and `end()` functions,
so clean up the redundant calls.
2020-06-24 18:41:45 +02:00
Wladimir J. van der Laan
205b87d2f6
Merge #19357: doc: add release note for bitcoin-cli -generate
9886c7d98d3386dfdd3ecce1e6189c49ff1c3a65 doc: add release note for bitcoin-cli -generate (Jon Atack)

Pull request description:

  Adds a release note for #19133.

ACKs for top commit:
  laanwj:
    ACK 9886c7d98d3386dfdd3ecce1e6189c49ff1c3a65

Tree-SHA512: 1354e5db0098447788c9ded6f2cd868fd6ea4a1443c99bec8026881b962c92b7257bfea45757769071605faeb989e9db48eaa2fbe9d273513aa2905ee479a8ec
2020-06-24 14:21:34 +02:00
MarcoFalke
dae1bd61b2
Merge bitcoin-core/gui#11: Remove needless headers from qt/walletview.cpp
4f9d9efb4e9afb1b012fca689ce77eb1d8d34f7d qt: Remove needless headers (Hennadii Stepanov)

Pull request description:

  No symbols from the removed headers are used in the `qt/walletview.cpp`.

  This is a small followup of https://github.com/bitcoin/bitcoin/pull/18027.

Top commit has no ACKs.

Tree-SHA512: 986ed5c8f3bac4c0053736ce84d738f8593d3dbf713109af3cb9b7051cd838f23152a39bb3c1e9694a993c4e7accf14e94e5beff5e7881155638cd44fbf7f46f
2020-06-24 08:18:25 -04:00
Hennadii Stepanov
4f9d9efb4e
qt: Remove needless headers 2020-06-24 14:10:01 +03:00
fanquake
80fd474e40
Merge #19240: build: macOS toolchain simplification and bump
adf543d7144a44f8cd28a090a21c4e2a606862da darwin: pass mlinker-version so that clang enables new features (Cory Fields)
2418f739f75824d6689369f326b960cec254cf56 macos: Bump to xcode 11.3.1 and 10.15 SDK (Cory Fields)
5c2c835433a80c204da1335daa51a014670c7324 depends: bump MacOS toolchain (Cory Fields)
85b5e420882b236b81b83acb672b4f5fa4899965 contrib: macdeploy: Remove historical extraction notes (Carl Dong)
351beb5c9a67500bcdb9a6ffe15f30e6aca5aa28 contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx (Carl Dong)
fbcfcf695435c9587e9f9fd2809c4d5350b2558e native_cctools: Don't use libc++ from pinned clang (Carl Dong)
3381e4a1892511d4d555853887c89badf4c940a9 Adapt rest of tooling to new SDK naming scheme (Carl Dong)
b3394ab235b93937321ffd08b8924e57855aac38 contrib: macdeploy: Correctly generate macOS SDK (Carl Dong)

Pull request description:

  This PR achieves 3 main things:
  1. It simplifies the macOS SDK generation by putting the logic inside a (semi-)portable python3 script `gen-sdk`
  2. It transitions us to using `libc++` headers extracted from the `Xcode.app`, which is more correct as those headers better match the `.tbd` library stubs we use from the `MacOSX.sdk` (located under the same `Xcode.app`). Previously, we used `libc++` headers copied from our downloaded, pinned clang (see `native_cctools.mk`).
  3. It bumps the macOS toolchain in a way that fulfills all of the following constraints:
      1. The new SDK should support compiling with C++17 (our current one doesn't)
      2. The new toolchain should not change our minimum supported macOS version (`-mmacosx-version-min`)
      3. The new toolchain should expect to use a version of `cctools` that is supported by https://github.com/tpoechtrager/cctools-port

  For the constraints in (3), you can reference [this chart](https://en.wikipedia.org/wiki/Xcode#Xcode_7.0_-_11.x_(since_Free_On-Device_Development)) to see that the newest toolchain we can use with our `cctools-port` is `11.3.1`, and the rest of the constraints were tested with local builds.

  #### But [the other Wikipedia chart](https://en.wikipedia.org/wiki/Xcode#Xcode_11.x_(since_SwiftUI_framework)) says that the "min macOS to run" for Xcode 11.3.1 is 10.14.4, doesn't that violate constraint (ii)?

  This confused me at first too, but the "min macOS to run" is for the Xcode.app App itself. The SDK still supports 10.12, as evident in a few plist files and as proven through local builds.

  #### Why bundle all of this together in a single PR?

  We need (1) and (2) together, because if we don't, manually adding the `libc++` headers and writing that out in a `README.md` is going to result in a lot of user error, so it's great to have these together to be more correct and also make it easier on the user at the same time.

  We need (3) together with everything else because bumping (or in the case of (1), renaming) the SDK requires some human coordination and may break some builds. And since it's not that complicated a change, it makes sense to do it together with the rest.

ACKs for top commit:
  theuni:
    ACK adf543d7144a44f8cd28a090a21c4e2a606862da.
  fanquake:
    ACK adf543d7144a44f8cd28a090a21c4e2a606862da - I'll take a look at the linker issue.

Tree-SHA512: 3813b69ebfe9610bee14100f26296fb5438d9bf0dd184ea55e6c38f5ebd94f7c171d98b150fc9e52fde626533f347f7ec51a2b72b79859d946284f578c1084a3
2020-06-23 16:14:49 +08:00
Jon Atack
9886c7d98d
doc: add release note for bitcoin-cli -generate 2020-06-23 07:09:27 +02:00
Cory Fields
adf543d714
darwin: pass mlinker-version so that clang enables new features
Without this clang fails to add any newly-added linker features.

Removing this in ca5055a5aa07aba81a87cf12f6f0526a63c423b5 was likely a
regression.

See https://github.com/bitcoin/bitcoin/pull/19240#issuecomment-647764049
for more discussion.
2020-06-22 17:00:45 -04:00
Wladimir J. van der Laan
e3fa3c7d67
Merge #19305: doc: add C++17 release note for 0.21.0
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
2020-06-22 19:13:22 +02:00
Wladimir J. van der Laan
a6aac20019
Merge #19350: test: Refactor tests using restart_node
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
2020-06-22 18:27:28 +02:00
Wladimir J. van der Laan
f591a1a184
Merge #19351: test: add two edge case tests for CSubNet
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
2020-06-22 18:01:02 +02:00
Cory Fields
2418f739f7
macos: Bump to xcode 11.3.1 and 10.15 SDK
This gets us a newer SDK with c++17 support and retains 10.12
back-compat.

Co-authored-by: Carl Dong <contact@carldong.me>
2020-06-22 10:14:33 -04:00
Cory Fields
5c2c835433
depends: bump MacOS toolchain
clang   6.0.1  -> 8.0.0
cctools 921    -> 949.0.1
ld64    409.12 -> 530
2020-06-22 10:14:05 -04:00
Carl Dong
85b5e42088
contrib: macdeploy: Remove historical extraction notes 2020-06-22 10:14:03 -04:00
Carl Dong
351beb5c9a
contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx 2020-06-22 10:14:02 -04:00
Carl Dong
fbcfcf6954
native_cctools: Don't use libc++ from pinned clang
Now that we include the macOS SDK libc++ headers in our macOS SDK
tarball, we no longer need this hack to use the libc++ from our pinned
clang.
2020-06-22 10:14:01 -04:00
Carl Dong
3381e4a189
Adapt rest of tooling to new SDK naming scheme 2020-06-22 10:13:55 -04:00
Vasil Dimov
ccef5d7bf0
test: add two edge case tests for CSubNet 2020-06-22 15:30:31 +02:00
Christopher Coverdale
20b6e95944
test: refactor functional tests to use restart_node 2020-06-22 12:58:14 +01:00
MarcoFalke
8ef15e8a86
Merge #19198: test: Check that peers with forcerelay permission are not asked to feefilter
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
2020-06-21 13:21:00 -04:00
MarcoFalke
fac63eb5ea
doc: Remove -whitelistforcerelay from comment
Instead, permission flags should be used. For example
-whitelist=forcerelay@127.0.0.1
2020-06-21 12:18:10 -04:00
MarcoFalke
faabd1514f
test: Check that peers with forcerelay permission do not get a feefilter message 2020-06-21 12:17:55 -04:00
MarcoFalke
fad676b8d2
test: Add connect_nodes method 2020-06-21 11:36:07 -04:00
MarcoFalke
fac6ef4fb2
test: Add test for no net permission 2020-06-21 11:35:46 -04:00
MarcoFalke
ffff3fe50a
test: Replace self.nodes[0].p2p with conn 2020-06-21 11:35:44 -04:00
MarcoFalke
faccdc8a31
test: remove redundant generate
setup_nodes takes care of getting out of ibd
2020-06-21 11:35:35 -04:00
MarcoFalke
fab83b934a
test: pep-8 p2p_feefilter.py 2020-06-21 11:35:25 -04:00
MarcoFalke
4b5c9191e3
Merge #19208: test: move sync_blocks and sync_mempool functions to test_framework.py
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
2020-06-21 09:17:39 -04:00
Samuel Dobson
c27330897d
Merge #18027: "PSBT Operations" dialog
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
2020-06-21 22:57:33 +12:00
Samuel Dobson
47a30ef0c6
Merge #19133: rpc, cli, test: add bitcoin-cli -generate command
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
2020-06-21 22:24:07 +12:00
MarcoFalke
e6f807f87c
Merge #19095: [tools] Update clang-format config for multi-line function declarations and calls
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
2020-06-21 05:58:08 -04:00
Samuel Dobson
02b26ba1c1
Merge #19200: rpc: remove deprecated getaddressinfo fields
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
2020-06-21 21:07:00 +12:00
Samuel Dobson
6bb5f6d8e3
Merge #16377: [rpc] don't automatically append inputs in walletcreatefundedpsbt
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
2020-06-21 20:52:34 +12:00
Samuel Dobson
bd331bd745
Merge #17938: Disallow automatic conversion between disparate hash types
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
2020-06-21 20:26:59 +12:00
MarcoFalke
879acc681a
Merge #19018: docs: fixing description of the field sequence in walletcreatefundedpsbt RPC method
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
2020-06-20 07:30:24 -04:00
MarcoFalke
d4f9ae0025
Merge #19054: wallet: Skip hdKeypath of 'm' when determining inactive hd seeds
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
2020-06-19 16:14:47 -04:00
Ben Woosley
4d7369125a
Disallow automatic conversion between hash types
A templated BaseHash does not allow for automatic conversion, thus
conversions much be explicitly allowed / whitelisted, which will
reduce the risk of unintended conversions.
2020-06-19 12:14:08 -07:00
Ben Woosley
fa9ef2cdbe
Remove an apparently unnecessary conversion
CScript -> CScriptID -> ScriptHash is unnecessary because
ScriptHash and CScriptID do the same thing.
2020-06-19 12:14:08 -07:00
Ben Woosley
966a22d859
Explicitly support conversion between equivalent hash types
ScriptHash <-> CScriptID
CKeyID -> PKHash
PKHash -> WitnessV0KeyHash
2020-06-19 12:14:08 -07:00
Ben Woosley
f32c1e07fd
Use explicit conversion from WitnessV0KeyHash -> CKeyID
These types are equivalent, in data etc, so they need only their
data cast across.
2020-06-19 12:14:08 -07:00
Ben Woosley
2c54217f91
Use explicit conversion from PKHash -> CKeyID
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
2020-06-19 12:14:07 -07:00
Ben Woosley
a9e451f144
Convert CPubKey to WitnessV0KeyHash directly
The round-tripping through PKHash has no effect, and is
potentially misleading as such.
2020-06-19 12:14:07 -07:00
Ben Woosley
3fcc468123
Prefer explicit CScriptID construction 2020-06-19 12:14:07 -07:00
Ben Woosley
0a5ea32ce6
Prefer explicit uint160 conversion 2020-06-19 12:14:06 -07:00
MarcoFalke
f3d776b593
Merge #19309: refactor: Fix link error with --enable-debug
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
2020-06-19 14:38:13 -04:00
Carl Dong
b3394ab235
contrib: macdeploy: Correctly generate macOS SDK
Previously, we did not include the macOS SDK libc++ headers in our SDK
creation process and instead used whichever libc++ headers shipped with
the clang package we downloaded in depends.

This change adds a script (which works on both GNU/Linux and macOS) to
correctly generate the macOS SDK including the libc++ headers. This can
be thought of as a simplified rewrite of tpoechtrager's script:

d3392f4eae/tools/gen_sdk_package.sh

The location within the SDK where we place the libc++ headers is chosen
such that clang's search path detection logic for sysroots would pick up
the headers properly.

We also document this change.
2020-06-19 14:35:48 -04:00
MarcoFalke
6dc1b45087
Merge #19267: ci: Upgrade most ci configs to focal
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
2020-06-19 12:53:23 -04:00
MarcoFalke
fa05f44893
ci: Upgrade most ci configs to focal 2020-06-19 10:44:11 -04:00
MarcoFalke
fad6720891
doc: move doc to ci readme 2020-06-19 10:44:00 -04:00
MarcoFalke
fa880773b4
ci: Have one config run in xenial to test against python3.5
Also, bump the travis env to bionic. This shouldn't matter at all
because all ci configs run inside a docker, but it does seem to fix a
bug. See
https://github.com/bitcoin/bitcoin/pull/19267#issuecomment-643630309
2020-06-19 10:43:21 -04:00
MarcoFalke
fa6ddb2fa1
travis: Always run multiprocess build 2020-06-19 10:31:49 -04:00