63ce882 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 63ce882 - 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

…*UsedDestination

bca8665 scripted-diff: Wallet: Rename incorrectly named *UsedDestination (Luke Dashjr)

Pull request description:

  These functions are used to mark/check if a key of our own has been used to spend (and only for avoid-reuse wallets), which has nothing to do with the destination/address itself.
  Give them more accurate names to avoid confusion.

  -BEGIN VERIFY SCRIPT-
  sed -i -e 's/UsedDestination/SpentKey/g' $(git grep -l 'UsedDestination' ./src)
  -END VERIFY SCRIPT-

ACKs for top commit:
  practicalswift:
    ACK bca8665 -- patch looks correct and rationale makes sense
  instagibbs:
    ACK bitcoin@bca8665, much more meaningful name, thanks
  kallewoof:
    ACK bca8665

Tree-SHA512: ff13d9061ffa748e92eb41ba962c3ec262a43e4b6abd62408b38c6f650395d6ae5851554257d1900fb02767a88d08380d592a27210192ee9abb72d0945976686

…atISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return value

12a2f37 util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t nTime) by checking gmtime_s/gmtime_r return value (practicalswift)

Pull request description:

  Avoid potential uninitialized read in `FormatISO8601DateTime(int64_t)` by checking `gmtime_s`/`gmtime_r` return value.

  Before this patch `FormatISO8601DateTime(67768036191676800)` resulted in:

  ```
  ==5930== Conditional jump or move depends on uninitialised value(s)
  ==5930==    at 0x4F44C0A: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
  ==5930==    by 0x4F511A4: std::ostream& std::ostream::_M_insert<long>(long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
  ==5930==    by 0x4037C3: void tinyformat::formatValue<int>(std::ostream&, char const*, char const*, int, int const&) (tinyformat.h:358)
  ==5930==    by 0x403725: void tinyformat::detail::FormatArg::formatImpl<int>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:543)
  ==5930==    by 0x402E02: tinyformat::detail::FormatArg::format(std::ostream&, char const*, char const*, int) const (tinyformat.h:528)
  ==5930==    by 0x401B16: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:907)
  ==5930==    by 0x4017AE: tinyformat::vformat(std::ostream&, char const*, tinyformat::FormatList const&) (tinyformat.h:1054)
  ==5930==    by 0x401765: void tinyformat::format<int, int, int, int, int, int>(std::ostream&, char const*, int const&, int const&, int const&, int const&, int const&, int const&) (tinyformat.h:1064)
  ==5930==    by 0x401656: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<int, int, int, int, int, int>(char const*, int const&, int const&, int const&, int const&, int const&, int const&) (tinyformat.h:1073)
  ==5930==    by 0x4014CC: FormatISO8601DateTime[abi:cxx11](long) (…)
  ```

  The same goes for other very large positive and negative arguments.

  Fix by simply checking the `gmtime_s`/`gmtime_r` return value :)

ACKs for top commit:
  MarcoFalke:
    ACK 12a2f37
  theStack:
    re-ACK bitcoin@12a2f37
  elichai:
    re ACK 12a2f37

Tree-SHA512: 066142670d9bf0944d41fa3f3c702b1a460b5471b93e76a619b1e818ff9bb9c09fe14c4c37e9536a04c99533f7f21d1b08ac141e1b829ff87ee54c80d0e61d48

…thods

5bad792 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost)
29a21c9 [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost)

Pull request description:

  In bitcoin#13557 (review) 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 bitcoin@5bad792
  jonatack:
    ACK 5bad792 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests.
  meshcollider:
    utACK 5bad792

Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd

…ns are used for type-punning

0653939 Add static_asserts to ser_X_to_Y() methods (Samer Afach)
be94096 Fix a violation of C++ standard rules that unions cannot be switched. (Samer Afach)

Pull request description:

  Type punning in C++ is not like C. As per the C++ standard, one cannot use unions to convert the bit type. A discussion about this can be found [here](https://stackoverflow.com/questions/25664848/unions-and-type-punning). In C++, a union is supposed to only hold one type at a time. It's intended to be used only as `std::variant`. Switching types is undefined behavior.

  In fact, C++20 has a special casting function, called [`bit_cast`](https://en.cppreference.com/w/cpp/numeric/bit_cast) that solved this problem.

  Why has it been working so far? Because some compilers tolerate using unions and switching types, like gcc. More information [here](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning).

  One important thing to mention is that performance is generally not affected by that memcpy. Compilers are smart enough to convert that to a memory cast when possible. But we have to do it the right way, otherwise, it's jut undefined behavior that depends on the compiler.

ACKs for top commit:
  practicalswift:
    ACK 0653939
  elichai:
    ACK 0653939
  laanwj:
    Code review ACK 0653939
  kristapsk:
    ACK 0653939

Tree-SHA512: f6e89de39fc964750429139bab6b5a1346f7060334b7afa020e315bdad8f8c195bce2b8a9e343f06e7fff175e2dfb1cdabfcb6fe405bea0febe4962f0cc62557

…ackages.py

b902bd6 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner)

Pull request description:

  This is a follow-up PR to bitcoin#17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~
  1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~
  2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~

  ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions:
  - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~
  - all txs in node1 mempool are a subset of txs in node0 mempool
  - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool
  - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool

ACKs for top commit:
  JeremyRubin:
    Excellent. utACK b902bd6

Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf

…ests

fa45d60 test: Reduce unneeded whitelist permissions in tests (MarcoFalke)

Pull request description:

  It makes the tests confusing and fragile when overwriting default command line values that are not needed to be overwritten.

ACKs for top commit:
  fanquake:
    ACK fa45d60
  laanwj:
    ACK fa45d60

Tree-SHA512: 8ae5ad8c6be156b1a983adccbca8d868ef841e00605ea88e24227f1b7493987c50b3e62e68dd7dc785ad73d6e14279eb13d7a151cb0a976426fe2fd63ce5cbcd

…v_activation.py

5ffaf88 test: eliminiated magic numbers in feature_csv_activation.py (Sebastian Falbesoner)
09f706a test: check for OP_CSV empty stack fail reject reason in feature_csv_activation.py (Sebastian Falbesoner)
cbd345a test: test OP_CSV empty stack fail in feature_csv_activation.py (Sebastian Falbesoner)

Pull request description:

  Adds an empty stack failure check for OP_CSV (BIP112) to the functional test `feature_csv_activation.py` by prepending a valid scriptSig with `OP_CHECKSEQUENCEVERIFY`.
  If BIP112 is inactive, the operator just behaves as a NOP (for both tx versions 1 and 2) and the transaction remains valid -- if it is active, the tx is invalid due to an empty stack (for both tx versions 1 and 2, as well).

Top commit has no ACKs.

Tree-SHA512: 81102aaead5be11e02b894867fa9a9cc17358ec0eb2f21ce2d3db845b87691d305e6ed7c525f9c7e5bcb3c5c609eb28deca0fbaa3d5e9ff928cecd3b91ff129a

aaaae4d test: Add p2p test for forcerelay permission (MarcoFalke)
fa6b57b test: Fix whitespace in p2p_permissions.py (MarcoFalke)
faf4081 test: Make msg_tx a witness tx (MarcoFalke)

Pull request description:

  The commit `test: Make msg_tx a witness tx` is needed so that the python mininode does not strip the witness from transactions before sending them over p2p. The commit should also be done to keep symmetry with msg_block. See:

  *  tests: Make msg_block a witness block bitcoin#15982

ACKs for top commit:
  laanwj:
    ACK aaaae4d

Tree-SHA512: b4b546c88f7f0576cb512f0872bc6bef9d4df65783803f226986e56175937f418aa1ed906417ac909f27f1fd521d64629621fda83250fa925c46ef9513db0e4c

facb715 net: Remove forcerelay of rejected txs (MarcoFalke)

Pull request description:

  This removes the code that supposedly handled the forced relay of txs from a permissioned peer that were rejected from our mempool. The removal should be fine, because it is dead code for the following reasons:

  * While `RelayTransaction` enqueues the inv for all peers, the inv is never processed because it can not be found in the mempool. See https://github.com/bitcoin/bitcoin/blob/4a072330763b3ff2d1b5c5b8d30a9517873ac6de/src/net_processing.cpp#L3862-L3866

  * Even if the peers we intended to send the inv to can somehow reply with a getdata to the never-received inv, they won't receive the tx as a reply because it was never added to the "relay memory" (`mapRelay`)

  The dead code is (obviously) untested: https://marcofalke.github.io/btc_cov/total.coverage/src/net_processing.cpp.gcov.html#2574

  This feature was (intentionally or accidentally) removed in 4d8993b, which was released in Bitcoin Core 0.13.0. So all currently supported versions of Bitcoin Core ship without this feature. I am not aware of any complaints about this feature or actual documented use-cases. So instead of reviving an unneeded feature, just remove the dead code.

ACKs for top commit:
  hebasto:
    ACK facb715, locally running the unit and functional tests.

Tree-SHA512: bfceae6f2983c1510fa0649a9a63c343cbbc1c4ab3a3698039cccf454c81e58c8f5114b147ed42a1bc867da74c43a5b53764ab14f942e191b6f59079044108b5