Commit graph

24714 commits

Author SHA1 Message Date
practicalswift 870f0cd2a0 build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory 2020-06-23 09:09:30 +00:00
fanquake 80fd474e40
Merge #19240: build: macOS toolchain simplification and bump
adf543d714 darwin: pass mlinker-version so that clang enables new features (Cory Fields)
2418f739f7 macos: Bump to xcode 11.3.1 and 10.15 SDK (Cory Fields)
5c2c835433 depends: bump MacOS toolchain (Cory Fields)
85b5e42088 contrib: macdeploy: Remove historical extraction notes (Carl Dong)
351beb5c9a contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx (Carl Dong)
fbcfcf6954 native_cctools: Don't use libc++ from pinned clang (Carl Dong)
3381e4a189 Adapt rest of tooling to new SDK naming scheme (Carl Dong)
b3394ab235 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 adf543d714.
  fanquake:
    ACK adf543d714 - I'll take a look at the linker issue.

Tree-SHA512: 3813b69ebfe9610bee14100f26296fb5438d9bf0dd184ea55e6c38f5ebd94f7c171d98b150fc9e52fde626533f347f7ec51a2b72b79859d946284f578c1084a3
2020-06-23 16:14:49 +08: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 ca5055a5aa 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
f1d21ef1c3 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 f1d21ef1c3 can't hurt to give an advance warning
  Sjors:
    ACK f1d21ef1c3
  laanwj:
    ACK f1d21ef1c3
  theStack:
    ACK f1d21ef1c3

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
20b6e95944 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 20b6e95944

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
ccef5d7bf0 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 ccef5d7bf0 -- more test coverage is better than less test coverage :)
  laanwj:
    ACK ccef5d7bf0
  hebasto:
    ACK ccef5d7bf0, 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
fac63eb5ea doc: Remove -whitelistforcerelay from comment (MarcoFalke)
faabd1514f test: Check that peers with forcerelay permission do not get a feefilter message (MarcoFalke)
fad676b8d2 test: Add connect_nodes method (MarcoFalke)
fac6ef4fb2 test: Add test for no net permission (MarcoFalke)
ffff3fe50a test: Replace self.nodes[0].p2p with conn (MarcoFalke)
faccdc8a31 test: remove redundant generate (MarcoFalke)
fab83b934a 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
cc84460c16 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 cc84460c16 , 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
931dd47608 Make lint-spelling.py happy (Glenn Willen)
11a0ffb29d [gui] Load PSBT from clipboard (Glenn Willen)
a6cb0b0c29 [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen)
5dd0c03ffa FillPSBT: report number of inputs signed (or would sign) (Glenn Willen)
9e7b23b733 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 931dd47608
  jb55:
    ACK 931dd47608
  achow101:
    ACK 931dd47608

Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
2020-06-21 22:57:33 +12:00
Samuel Dobson 47a30ef0c6
Merge #19133: rpc, cli, test: add bitcoin-cli -generate command
22cb303cf0 rpc: add missing space in JSON parsing error message, update test (Jon Atack)
bf53ebef06 test: add multiwallet tests for bitcoin-cli -generate (Jon Atack)
4b859cfff9 cli: add multiwallet capability to GetNewAddress and -generate (Jon Atack)
18f93545a1 test: add tests for bitcoin-cli -generate (Jon Atack)
4818124137 cli: create bitcoin-cli -generate command (Jon Atack)
ff41a36900 cli: extract ParseResult() and ParseError() (Jon Atack)
f4185b26d9 cli: create GenerateToAddressRequestHandler class (Harris)
f7c65a3350 cli: create GetNewAddress() (Jon Atack)
9be7fd35c5 rpc: make generatetoaddress locals const (Jon Atack)
cb00510dba 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 22cb303cf0
  meshcollider:
    utACK 22cb303cf0

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
cc29d1e2c4 [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 cc29d1e2c4 fine with me
  practicalswift:
    ACK cc29d1e2c4

Tree-SHA512: a62474925e71aaff41bdce7960fd5ffd64317da810f694d8084080b054708cf71c2ab2ce3111db5a9260d1c1f9e02d59a2ecb5543b1b6172ce085cb42432160a
2020-06-21 05:58:08 -04:00
Samuel Dobson 02b26ba1c1
Merge #19200: rpc: remove deprecated getaddressinfo fields
bc01f7ae05 doc: release note for rpc getaddressinfo removals (Jon Atack)
90e989390e rpc: getaddressinfo RPCResult fixup (Jon Atack)
a8507c99da rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack)
645a8653c8 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 bc01f7ae05

Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251
2020-06-21 21:07:00 +12:00
Samuel Dobson 6bb5f6d8e3
Merge #16377: [rpc] don't automatically append inputs in walletcreatefundedpsbt
e5327f947c [rpc] fundrawtransaction: add_inputs option to control automatic input adding (Sjors Provoost)
79804fe24b [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 e5327f947c
  meshcollider:
    utACK e5327f947c

Tree-SHA512: d8653b820914396c7c25b0d0a2b7e92de214aa023bc1aa085feb37d3b20fab361ebea90416a7db989f19bdc37e26cf0adfbcb712c80985c87afa67a9bd44fecb
2020-06-21 20:52:34 +12:00
Samuel Dobson bd331bd745
Merge #17938: Disallow automatic conversion between disparate hash types
4d7369125a Disallow automatic conversion between hash types (Ben Woosley)
fa9ef2cdbe Remove an apparently unnecessary conversion (Ben Woosley)
966a22d859 Explicitly support conversion between equivalent hash types (Ben Woosley)
f32c1e07fd Use explicit conversion from WitnessV0KeyHash -> CKeyID (Ben Woosley)
2c54217f91 Use explicit conversion from PKHash -> CKeyID (Ben Woosley)
a9e451f144 Convert CPubKey to WitnessV0KeyHash directly (Ben Woosley)
3fcc468123 Prefer explicit CScriptID construction (Ben Woosley)
0a5ea32ce6 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 4d7369125a
  meshcollider:
    re-utACK 4d7369125a

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
d0a3feea73 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 d0a3feea73

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
951bca61d7 tests: feature_backwards_compatibility.py test 0.16 up/downgrade (Andrew Chow)
3a03a11e8c 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 951bca61d7
  instagibbs:
    re-utACK 951bca61d7
  ryanofsky:
    Code review ACK 951bca61d7. 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
b83cc0fc94 Fix link error with --enable-debug (Hennadii Stepanov)

Pull request description:

  Fixes a link error on master (39bd9ddb87):
  ```
  $ ./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 b83cc0fc94

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
fa05f44893 ci: Upgrade most ci configs to focal (MarcoFalke)
fad6720891 doc: move doc to ci readme (MarcoFalke)
fa880773b4 ci: Have one config run in xenial to test against python3.5 (MarcoFalke)
fa6ddb2fa1 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 fa05f44893, assuming Travis passes
  hebasto:
    ACK fa05f44893

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
MarcoFalke febe5823b4
Merge #19321: ci: Run asan ci config on cirrus
fa2eb3d5d6 ci: Run asan ci config on cirrus (MarcoFalke)
fa93527738 cirrus: Clear dummy task (MarcoFalke)

Pull request description:

  Currently it is not possible to use travis in forked repositories due to the 50 minute limit on builds. A fresh build (uncached) of the address sanitizer config takes more than 50 minutes.

  One approach to fix this could be to throw away tests until the run time is less than 50 minutes. However, the risk of being blind of failures in the thrown away tests is not worth the gain. Also, to detect them, one has to run the asan configuration nightly and failures could only be detected post-merge.

  Another approach would be to ask travis support to raise the limit for a forked repository. This is a tedious and manual one-by-one process, so I'd rather not.

  Finally, a different ci provider can be used, since the config files are designed to be platform-agnostic. This is what I picked.

  I kept all settings identical to the travis machine for now. Both providers run in the google cloud, so this should be a "move-only".

ACKs for top commit:
  hebasto:
    ACK fa2eb3d5d6

Tree-SHA512: 159d7dc6f5b24583e941282cdd40465b15db787f0a658a3e81a7b1a22abdb4cb573709b9b5c4465523e0ba0060b17a68fbdbda7a9ecdeb649f31535d377bbe75
2020-06-19 10:26:37 -04:00
MarcoFalke 5f72ddb7ee
Merge #18863: refactor: Make CScriptVisitor stateless
3351c91ed4 refactor: Make CScriptVisitor stateless (João Barbosa)

Pull request description:

  `CScriptVisitor` was added in 1025440184 (#1357) and the visitor return type was never used. Now `CScriptVisitor` is stateless and `CScript` is the return type.

ACKs for top commit:
  MarcoFalke:
    ACK 3351c91ed4 🏤
  sipa:
    utACK 3351c91ed4

Tree-SHA512: d158ad2ebe8ea4dc8cc090b943dd66fa5421a84f9443e16ab2d661df38e1a85de16ff13cbaa56924489d8d43cba25fa3cd8b6904bbbcbf356b886ffe8ffba19a
2020-06-19 07:52:49 -04:00
MarcoFalke 62948caf44
Merge #18937: refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg
51e9393c1f refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg (Sebastian Falbesoner)

Pull request description:

  Follow-up PR for #18533 -- another small step towards getting rid of the confusing "command" terminology. Also see PR #18610 which tackled the functional tests.

ACKs for top commit:
  MarcoFalke:
    ACK 51e9393c1f

Tree-SHA512: bb6f05a7be6823d5c4eab1d05b31fee944e700946827ad9425d59a3957fd879776c88c606319cbe9832d9451b275baedf913b71429ea3e01e4e82bf2d419e819
2020-06-19 06:54:24 -04:00
Glenn Willen 931dd47608 Make lint-spelling.py happy 2020-06-19 02:20:04 -07:00