…_decode

906d791 fuzz: add missing ECCVerifyHandle to base_encode_decode (Andrew Poelstra)

Pull request description:

  It is possible to trigger a fuzztest failure in the `base_encode_decode` by asking it to decode any PSBT that has HD keypaths in it. For example, this one

  ```
  cHNidP8BAFUCAAAAASeaIyOl37UfxF8iD6WLD8E+HjNCeSqF1+Ns1jM7XLw5AAAAAAD/////AaBa6gsAAAAAGXapFP/pwAYQl8w7Y28ssEYPpPxCfStFiKwAAAAAAAEBIJVe6gsAAAAAF6kUY0UgD2jRieGtwN8cTRbqjxTA2+uHIgIDsTQcy6doO2r08SOM1ul+cWfVafrEfx5I1HVBhENVvUZGMEMCIAQktY7/qqaU4VWepck7v9SokGQiQFXN8HC2dxRpRC0HAh9cjrD+plFtYLisszrWTt5g6Hhb+zqpS5m9+GFR25qaAQEEIgAgdx/RitRZZm3Unz1WTj28QvTIR3TjYK2haBao7UiNVoEBBUdSIQOxNBzLp2g7avTxI4zW6X5xZ9Vp+sR/HkjUdUGEQ1W9RiED3lXR4drIBeP4pYwfv5uUwC89uq/hJ/78pJlfJvggg71SriIGA7E0HMunaDtq9PEjjNbpfnFn1Wn6xH8eSNR1QYRDVb1GELSmumcAAACAAAAAgAQAAIAiBgPeVdHh2sgF4/iljB+/m5TALz26r+En/vykmV8m+CCDvRC0prpnAAAAgAAAAIAFAACAAAA=
  ```

  which I took straight from the PSBT test vectors. The reason is that in src/psbt.h we call `DeserializeHDKeypaths`, which in turn calls `CPubKey::IsFullyValid`, which in turn asserts that a secp context has been created.

  The error appears to be masked on many systems by the definition of `instance_of_eccryptoclosure` in src/script/bitcoinconsensus.cpp, which defines a static object which contains an `ECCVerifyHandle`. If you just comment out that line you can reliably trigger the fuzz test failure, e.g. by creating a file `crash` with the above PSBT, and runnnig

  ```
  ASAN_OPTIONS=symbolize=0:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1 UBSAN_OPTIONS=suppressions=./test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1 FUZZ=base_encode_decode ./src/test/fuzz/fuzz -seed_inputs=crash
  ```

ACKs for top commit:
  practicalswift:
    cr ACK 906d791

Tree-SHA512: b98b60573c21efe28503fe351883c6f0d9ac99d0dd6f100537b16ac53476617b8a3f899faf0c23d893d34a01b3bbe4a784499ec6f9c7000292e850bed449bd85

30aee2d tests: Add test for compact block HB selection (Pieter Wuille)
6efbcec Protect last outbound HB compact block peer (Suhas Daftuar)

Pull request description:

  If all our high-bandwidth compact block serving peers (BIP 152) stall block
  download, then we can be denied a block for (potentially) a long time. As
  inbound connections are much more likely to be adversarial than outbound
  connections, mitigate this risk by never removing our last outbound HB peer if
  it would be replaced by an inbound.

ACKs for top commit:
  achow101:
    ACK 30aee2d
  ariard:
    Code ACK 30aee2d
  jonatack:
    ACK 30aee2d

Tree-SHA512: 5c6c9326e3667b97e0864c371ae2174d2be9054dad479f4366127b9cd3ac60ffa01ec9707b16ef29cac122db6916cf56fd9985733390017134ace483278921d5

4e353cb http: Release work queue after event base finish (João Barbosa)

Pull request description:

  This fixes a race between `http_request_cb` and `StopHTTPServer` where
  the work queue is used after release.

  Fixes bitcoin#18856.

ACKs for top commit:
  fjahr:
    Code review ACK 4e353cb
  achow101:
    ACK 4e353cb
  LarryRuane:
    ACK 4e353cb
  hebasto:
    ACK 4e353cb, tested (rebased on top of master 9313c4e) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](bitcoin#19033 (comment)), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct.

Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930

`FromHex` is mostly used for transactions, so we introduce a
shortcut `tx_from_hex` for `FromHex(CTransaction, hex_str)`.

-BEGIN VERIFY SCRIPT-
sed -i 's/\<FromHex\>/from_hex/g' $(git grep -l FromHex)
-END VERIFY SCRIPT-

Co-authored-by: MarcoFalke <falke.marco@gmail.com>

…ative)

Save the banlist in `banlist.json` instead of `banlist.dat`.

This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).

Only read `banlist.dat` if it exists and `banlist.json` does not
exist (first start after an upgrade).

Supersedes bitcoin#20904
Resolves bitcoin#19748

With `banlist.dat` (being written in addrv1 format) if we would try to
write a Tor v3 subnet, it would serialize as a dummy-all-0s IPv6
address and subsequently, when deserialized will not result in the same
subnet.

This problem does not exist with `banlist.json` where the data is saved
in textual, human-readable form.

…time spent waiting

b9e76f1 rpc: Add test for -rpcwaittimeout (Christian Decker)
f76cb10 rpc: Prefix rpcwaittimeout error with details on its nature (Christian Decker)
c490e17 doc: Add release notes for the `-rpcwaittimeout` cli parameter (Christian Decker)
a7fcc8e rpc: Add a `-rpcwaittimeout` parameter to limit time spent waiting (Christian Decker)

Pull request description:

  Adds a new numeric `-rpcwaittimeout` that can be used to limit the
  time we spend waiting on the RPC server to appear. This is used by
  downstream projects to provide a bit of slack when `bitcoind`s RPC
  interface is not available right away.

  This makes the `-rpcwait` argument more useful, since we can now limit
  how long we'll ultimately wait, before potentially giving up and reporting
  an error to the caller. It was discussed in the context of the BTCPayServer
  wanting to have c-lightning wait for the RPC interface to become available
  but still have the option of giving up eventually ([4355]).

  I checked with laanwj whether this is already possible ([comment]), and
  whether this would be a welcome change. Initially I intended to repurpose
  the (optional) argument to `-rpcwait`, however I decided against it since it
  would potentially break existing configurations, using things like `rpcwait=1`,
  or `rpcwait=true` (the former would have an unintended short timeout, when
  old behavior was to wait indefinitely).

  ~Due to its simplicity I didn't implement a test for it yet, but if that's desired I
  can provide one.~ Test was added during reviews.

  [4355]: ElementsProject/lightning#4355
  [comment]: ElementsProject/lightning#4355 (comment)

ACKs for top commit:
  laanwj:
    Code review ACK b9e76f1
  promag:
    ACK b9e76f1.

Tree-SHA512: 3cd6728038ec7ca7c35c2e7ccb213bfbe963f99a49bb48bbc1e511c4dd23d9957c04f9af1f8ec57120e47b26eaf580b46817b099d5fc5083c98da7aa92db8638

…nd check tx vsize

d6d2ab9 test: MiniWallet: fix fee calculation for P2PK and check tx vsize (Sebastian Falbesoner)
ce024b1 test: MiniWallet: force P2PK signature to have fixed size (71 bytes) (Sebastian Falbesoner)

Pull request description:

  This PR is a follow-up to bitcoin#21945. It aims to both fix the fee calculation for P2PK mode transactions and enable its vsize check. Currently, the latter assumes a fixed tx length, which is fine for anyone-can-spend txs but doesn't apply to P2PK output spends due to varying DER signature size; the vsize check is therefore disabled for P2PK mode on master branch.

  Creating one million DER signatures with MiniWallet shows the following distribution of sizes (smart people with better math skills probably could deduce the ratios without trying, but hey):

  | DER signature size [bytes]  | #occurences (ratio) |
  | ------------- | ------------- |
  | 71  | 498893 (49.89%) |
  | 70 | 497244 (49.72%) |
  | 69 | 3837 (0.38%) |
  | 68 | 22 (0.0022%) |

  Note that even smaller signatures are possible (for smaller R and S values with leading zero bytes), it's just that the probability decreases exponentially.     Instead of choosing a large vsize check range and hoping that smaller signatures are never created (potentially leading to flaky tests), the proposed solution is ~~to limit the signature size to the two most common sizes 71 and 70 (>99.6% probability) and then accordingly only check for two vsize values; the value to be used for fee calculation is a decimal right between the two possible sizes (167.5 vbytes) and for the vsize check it's rounded down/up integer values are used.~~ to simply grind the signature to a fixed size of 71 bytes (49.89% probability, i.e. on average each call to `sign_tx()`, on average two ECC signing operations are needed).

  ~~The idea of grinding signatures to a fixed size (similar to bitcoin#13666 which grinds to low-R values) would be counter-productive, as the signature creation in the test suite is quite expensive and this would significantly slow down tests that calculate hundreds of signatures (like e.g. feature_csv_activation.py).~~

  For more about transaction sizes on different input/output types, see the following interesting article: https://medium.com/coinmonks/on-bitcoin-transaction-sizes-97e31bc9d816

ACKs for top commit:
  MarcoFalke:
    Concept ACK d6d2ab9

Tree-SHA512: 011c70ee0e4adf9ba12902e4b6c411db9ae96bdd8bc810bf1d67713367998e28ea328394503371fc1f5087a819547ddaea56c073b28db893ae1c0031d7927f32

…yments

fa5658e Use DeploymentEnabled to hide VB deployments (MarcoFalke)
fa11fec doc: Move buried deployment doc to the enum that enumerates them (MarcoFalke)

Pull request description:

  Plus a doc commit.

ACKs for top commit:
  jnewbery:
    utACK fa5658e
  ajtowns:
    utACK fa5658e

Tree-SHA512: 2aeceee0674feb603d76656eff40695b7d7305de309f837bbb6a8c1dbb1d0b962b741f06ab7b9a8b1dbd1964c9c0c9aa5dc9588fd8e6d896e620b69e08eedbaa

ceb7b35 refactor: move UpdateTip into CChainState (James O'Beirne)
4abf077 refactor: no mempool arg to GetCoinsCacheSizeState (James O'Beirne)
46e3efd refactor: move UpdateMempoolForReorg into CChainState (James O'Beirne)
6176617 validation: make CChainState::m_mempool optional (James O'Beirne)

Pull request description:

  Make `CChainState::m_mempool` optional by making it a pointer instead of a reference. This will allow a simplification to assumeutxo semantics (see bitcoin#15606 (review)) and help facilitate the `-nomempool` option.

ACKs for top commit:
  jnewbery:
    ACK ceb7b35
  naumenkogs:
    ACK ceb7b35
  ryanofsky:
    Code review ACK ceb7b35 (just minor style and test tweaks since last review)
  lsilva01:
    Code review ACK and tested on Signet ACK bitcoin@ceb7b35
  MarcoFalke:
    review ACK ceb7b35 😌

Tree-SHA512: cc445ad33439d5918cacf80a6354eea8f3d33bb7719573ed5b970fad1a0dab410bcd70be44c862b8aba1b71263b82d79876688c553e339362653dfb3d8ec81e6

…ctEvictionCandidatesByRatio()

b1d905c p2p: earlier continuation when no remaining eviction candidates (Vasil Dimov)
c9e8d8f p2p: process more candidates per protection iteration (Jon Atack)
02e411e p2p: iterate eviction protection only on networks having candidates (Jon Atack)
5adb064 bench: add peer eviction protection benchmarks (Jon Atack)
566357f refactor: move GetRandomNodeEvictionCandidates() to test utilities (Jon Atack)

Pull request description:

  This follow-up to bitcoin#21261 improves `ProtectEvictionCandidatesByRatio()` for better performance.

  Benchmarks are added; the performance improvement is between 2x and 5x for the benchmarked cases (CPU 2.50GHz, Turbo off, performance mode, Debian Clang 11 non-debug build).

  ```
  $ ./src/bench/bench_bitcoin -filter="EvictionProtection*.*"
  ```

  The refactored code is well-covered by existing unit tests and also a fuzzer.

  - `$ ./src/test/test_bitcoin -t net_peer_eviction_tests`
  - `$ FUZZ=node_eviction ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/node_eviction`

ACKs for top commit:
  klementtan:
    Tested and code review ACK b1d905c.
  vasild:
    ACK b1d905c
  jarolrod:
    ACK b1d905c

Tree-SHA512: a3a6607b9ea2fec138da9780c03f63e177b6712091c5a3ddc3804b896a7585216446310280791f5e20cc023d02d2f03a4139237e12b5c1d7f2a1fa1011610e96

a16378e doc: Remove unnecessary steps from translations update process (Wladimir J. van der Laan)
2584929 doc: Add steps for transifex to release process (Wladimir J. van der Laan)

Pull request description:

  Document how to update settings on and for the transifex website before and after branch-off of a new release.

  (This is bitcoin#21440, updated with the review feedback.)

ACKs for top commit:
  laanwj:
    ACK a16378e
  hebasto:
    ACK a16378e

Tree-SHA512: 9669cc3bc7ba056f913ee77c2ef9d9ee2313da947fc07f8cd955807c34c5d39e3af73587adfe734274ab2412a7dfb1e2dfe7ee4904ca28cfef9bf8061d26a573

fa84cae doc: added info to bitcoin.conf doc (Brian Liotti)

Pull request description:

  Should probably be explicitly stated to not make modifications to the conf file while daemon is running. ref bitcoin#11586

  For example, if rpc credentials are modified while bitcoind is running, `bitcoin-cli stop` is unable to stop bitcoind until the original credentials are restored in `bitcoin.conf`

ACKs for top commit:
  rajarshimaitra:
    ACK bitcoin@fa84cae
  Zero-1729:
    ACK fa84cae
  theStack:
    ACK fa84cae 🗄️

Tree-SHA512: f6ddffc25563c0b01e661b6abe43a7909938ad8eca38d6d0e2d4a2ce9fb850e51b54d950ef3118b74b6e340c64fe3f37205861720a2de2933db29782234869bb

…by us)

7593b06 test: ensure I2P addresses are relayed (Vasil Dimov)
e746813 test: make CAddress in functional tests comparable (Vasil Dimov)
33e211d test: implement ser/unser of I2P addresses in functional tests (Vasil Dimov)
8674281 test: use NODE_* constants instead of magic numbers (Vasil Dimov)
ba45f02 net: relay I2P addresses even if not reachable (by us) (Vasil Dimov)

Pull request description:

  Nodes that can reach the I2P network (have set `-i2psam=`) will relay
  I2P addresses even without this patch. However, nodes that can't reach
  the I2P network will not. This was done as a precaution in
  bitcoin#20119 before anybody could
  connect to I2P because then, for sure, it would have been useless.

  Now, however, we have I2P support and a bunch of I2P nodes, so get all
  nodes on the network to relay I2P addresses to help with propagation,
  similarly to what we do with Tor addresses.

ACKs for top commit:
  jonatack:
    ACK 7593b06
  naumenkogs:
    ACK 7593b06.
  laanwj:
    Code review ACK 7593b06
  kristapsk:
    ACK 7593b06. Code looks correct, tested that functional test suite passes and also that `test/functional/p2p_addrv2_replay.py` fails if I undo changes in `IsRelayable()`.

Tree-SHA512: c9feec4a9546cc06bc2fec6d74f999a3c0abd3d15b7c421c21fcf2d610eb94611489e33d61bdcd5a4f42041a6d84aa892f7ae293b0d4f755309a8560b113b735

If a ScriptPubKeyMan does not implement Upgrade, then using upgraewallet
will fail unexpectedly. By changing the default to return true, then
this error can be avoided. This is still correct because a successful
upgrade can be that nothing happened.

…r_eviction.cpp

e49d50c bench: fix 32-bit narrowing warning in bench/peer_eviction.cpp (Jon Atack)

Pull request description:

  Closes bitcoin#22459.

ACKs for top commit:
  hebasto:
    ACK e49d50c, tested on Debian 10.10 (i386):

Tree-SHA512: 8ca366fc296c633dbc8b8e0e7d80f4f6a64d02fb3da86d199881364f027d34b816a3c964b3fea2c1cc0b3ad51dd02d93c8bb14b5ebbd99fb4073cd1031766332

…flag

3c4c8e7 build: Add -Werror=implicit-fallthrough compile flag (Hennadii Stepanov)
014110c Use C++17 [[fallthrough]] attribute, and drop -Wno-implicit-fallthrough (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  fanquake:
    ACK 3c4c8e7 - looks ok to me now. Checked that warnings occur in our code & leveldb by removing a `[[fallthrough]]` or `FALLTHROUGH_INTENDED`.
  jarolrod:
    ACK 3c4c8e7
  theStack:
    ACK 3c4c8e7

Tree-SHA512: 4dce91f0f26b8a3de09bd92bb3d7e1995e078e3a8b3ff861c4fbf6c0b32b2327d063633b07b89c4aa94a1141d7f78d46d9d43ab8df865273e342693ad30645b6

fb7be92 Mark print-% target as phony. (Dmitry Goncharov)

Pull request description:

  .PHONY does not take patterns (such as print-%) as prerequisites.
  Have print-% depend on force and mark force as phony.

  This change ensures print-% rule works even when there is a file that matches the target.

  ```
  $ # on master
  $ make print-host
  host=x86_64-pc-linux-gnu
  $ touch print-host
  $ make print-host
  make: 'print-host' is up to date.
  $
  $ git co mark_print_as_phony
  Switched to branch 'mark_print_as_phony'
  $ make print-host
  host=x86_64-pc-linux-gnu
  $ touch force
  $ make print-host
  host=x86_64-pc-linux-gnu
  ```

ACKs for top commit:
  hebasto:
    ACK fb7be92, tested on Linux Mint 20.2 (x86_64).

Tree-SHA512: b89ae66aa8c7aa6a7ab5f0956f9eb3b3ef9d56994b60dc2a97d498d4c1bba537845c190723e8a10310280b1b35df2cd935cc30aeb76735cac2dc621ad7823772

1155978 build, qt: Do not install *.prl files (Hennadii Stepanov)
763793b build, qt: Fix wrong cross-compiling detection on macOS (Hennadii Stepanov)
3098272 build, qt: Force bootstrap while building linguist tools (Hennadii Stepanov)
689320e build, qt: Drop translations.pro hack (Hennadii Stepanov)
6a1f98f build, qt: Drop lrelease dependency patch (Hennadii Stepanov)
39e561e build, qt: Add linguist_tools list (Hennadii Stepanov)
27d3def build: Use Qt top-level build facilities (Hennadii Stepanov)

Pull request description:

  This PR:
  - uses Qt top-level build facilities without the need to download all-in-one archive
  - is based on **BlockMechanic**'s [idea](bitcoin#20600), and is an alternative to bitcoin#20600
  - makes it easy to integrate [new modules](bitcoin#16883) into static builds
  - has the minimal diff
  - makes the qt package build process streamlined by dropping some patches and hacks (an alternative to  bitcoin#21420 and bitcoin#20642)

  Fixes bitcoin#18536 (a non-intrusive alternative to bitcoin#21589 and bitcoin#19785).

  Fixes bitcoin#14648.

  Fixes bitcoin#21588 (a non-intrusive alternative to bitcoin#21591).

  Required for adding [Wayland support](bitcoin#19950) on Linux.

  ---

  **Note for reviewers**: With 9046de8 from bitcoin#21995 it is easy to verify that there are no changes in the resulted `qt` package archive on the per commit basis. For example, for `HOST=i686-pc-linux-gnu` no commit in this PR introduces any changes.

ACKs for top commit:
  fanquake:
    ACK 1155978

Tree-SHA512: 667b06b72cb7ff26d68b9b88e8dddb51084783ca9e3d80b3392710794c1dc7fd77bbcc3ccf4ccece9919d33b9bf8fce13c5059502bd228021dc7c93fdb87ca7a

1edddf5 Avoid GCC 7.1 ABI change warning in guix build (Pieter Wuille)

Pull request description:

  The arm-linux-gnueabihf guix build output is littered with warnings like:

  ```
  /gnu/store/7a96hdqdb2qi8a39f09n84xjy2hr23rs-gcc-cross-arm-linux-gnueabihf-8.4.0/include/c++/bits/stl_vector.h:1085:4: note:
                   parameter passing for argument of type '__gnu_cxx::__normal_iterator<CRecipient*, std::vector<CRecipient> >' changed in GCC 7.1
  ```

  These are irrelevant for us. Disable them using `-Wno-psabi`.

ACKs for top commit:
  laanwj:
    ACK 1edddf5
  hebasto:
    ACK 1edddf5, after thorough reading related materials, I agree this change can be merged. As I mentioned above, I have been compiling my arm-32bit binaries with `-Wno-psabi` flag for two years, and no related flaws were observed.

Tree-SHA512: 485c7500547ac5da567ad23847341c18ff832607f5a1002676404cc647e437cf3445b6894ecff5b52929ca52bea946c06bd90eace1997c895e56204e787065e4

…helpers from util.h to util.cpp

a2aca20 Move implementations of non-template fuzz helpers (Sriram)

Pull request description:

  There are 78 cpp files that include `util.h` (`grep -iIr "#include <test/fuzz/util.h>" src/test/fuzz | wc -l`). Modifying the implementation of a fuzz helper in `src/test/fuzz/util.h` will cause all fuzz tests to be recompiled. Keeping the declarations of these non-template fuzz helpers in `util.h` and moving their implementations to `util.cpp` will skip the redundant recompilation of all the fuzz tests, and builds these helpers only once in `util.cpp`.

  Functions moved from `util.h` to `util.cpp`:
  - `ConsumeTxMemPoolEntry`
  - `ContainsSpentInput`
  - `ConsumeNetAddr`
  - Methods of `FuzzedFileProvider::(open, read, write, seek, close)`

ACKs for top commit:
  MarcoFalke:
    review ACK a2aca20 🍂

Tree-SHA512: e7037ebb86d0fc56048e4f3d8733eefc21da11683b09d2b22926bda410719628d89c52ddd9b4c18aa243607a66fdb4d13a63e62ca010e66b3ec9174fd18107f0

8465978 Make IsSegWitOutput return true for taproot outputs (Pieter Wuille)

Pull request description:

  This fixes a bug: currently `utxoupdatepsbt` will not fill in UTXO data for PSBTs spending taproot outputs.

ACKs for top commit:
  achow101:
    Code Review ACK 8465978
  jonatack:
    ACK 8465978
  meshcollider:
    utACK 8465978

Tree-SHA512: 2f8f873450bef4b5a4ce5962a231297b386c6b1445e69ce5f36ab28eca7343be3a11bc09c38534b0f75e6f99ba15d78d3ba5d484f6c63e5a9775e1f3f55a74e0

The purpose of this commit is to reduce the amount of work make does.
This simplifies make -d output and improves performance (this
performance gain is hardly noticeable).

By default make remakes all makefiles.
This build system does not have rules to remake makefiles.
This commit adds an explicit rule for each makefile.

Each rule serves 2 purposes.
1. Each rule prevents make from searching for an implicit rule.
2. Once found, the rule prevents make from remaking the makefile.
That's why the rule is double colon. Make won't use a double colon rule to
remake a makefile. See
https://www.gnu.org/software/make/manual/make.html#Remaking-Makefiles.

$ # on this branch.
$ make -d print-host |grep expat
Reading makefile 'packages/expat.mk' (search path) (no ~ expansion)...
Makefile 'packages/expat.mk' might loop; not remaking it.
$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
$ make -d print-host |grep expat |wc
    367    1916   19918
$

Make prints 367 lines per makefile.
With this change make prints 2 lines per makefile.

…tcoin into avoid_rebuilding_makefiles

* 'avoid_rebuilding_makefiles' of github.com:dgoncharov/bitcoin:
  Avoid remaking makefiles.