Adds an error output parameter to all GetReservedDestination functions
so that callers can get the actual reason that a change address could
not be fetched. This more closely matches GetNewDestination. This allows
for more granular error messages, such as one that indicates that
bech32m addresses cannot be generated yet.
We don't want the legacy wallet to ever have bech32m addresses so don't
allow importing them. This includes addmultisigaddress as that is a
legacy wallet only RPC
Additionally, bech32m multisigs are not available yet, so disallow them
in createmultisig.
If a transaction as a segwit output, use a bech32m change address if
they are available. If not, fallback to bech32. If bech32 change
addresses are unavailable, fallback to the default address type.
Bech32m addresses need their own OutputType
We are not ready to create DescriptorScriptPubKeyMans which produce
bech32m addresses. So don't allow generating them.
b9e76f1bf0 rpc: Add test for -rpcwaittimeout (Christian Decker)
f76cb10d7d rpc: Prefix rpcwaittimeout error with details on its nature (Christian Decker)
c490e17ef6 doc: Add release notes for the `-rpcwaittimeout` cli parameter (Christian Decker)
a7fcc8eb59 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]: https://github.com/ElementsProject/lightning/issues/4355
[comment]: https://github.com/ElementsProject/lightning/issues/4355#issuecomment-768288261
ACKs for top commit:
laanwj:
Code review ACK b9e76f1bf0
promag:
ACK b9e76f1bf0.
Tree-SHA512: 3cd6728038ec7ca7c35c2e7ccb213bfbe963f99a49bb48bbc1e511c4dd23d9957c04f9af1f8ec57120e47b26eaf580b46817b099d5fc5083c98da7aa92db8638
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 https://github.com/bitcoin/bitcoin/pull/20904
Resolves https://github.com/bitcoin/bitcoin/issues/19748
4e353cb618 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 #18856.
ACKs for top commit:
fjahr:
Code review ACK 4e353cb618
achow101:
ACK 4e353cb618
LarryRuane:
ACK 4e353cb618
hebasto:
ACK 4e353cb618, tested (rebased on top of master 9313c4e6aa) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](https://github.com/bitcoin/bitcoin/pull/19033#issuecomment-640106647), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct.
Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930
30aee2dfe6 tests: Add test for compact block HB selection (Pieter Wuille)
6efbcec4de 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 30aee2dfe6
ariard:
Code ACK 30aee2dfe
jonatack:
ACK 30aee2dfe6
Tree-SHA512: 5c6c9326e3667b97e0864c371ae2174d2be9054dad479f4366127b9cd3ac60ffa01ec9707b16ef29cac122db6916cf56fd9985733390017134ace483278921d5
458a345b05 Add support for SIGHASH_DEFAULT in RPCs, and make it default (Pieter Wuille)
c0f0c8eccb tests: check spending of P2TR (Pieter Wuille)
a2380127e9 Basic Taproot signing logic in script/sign.cpp (Pieter Wuille)
49487bc3b6 Make GetInputUTXO safer: verify non-witness UTXO match (Pieter Wuille)
fd3f6890f3 Construct and use PrecomputedTransactionData in PSBT signing (Pieter Wuille)
5cb6502ac5 Construct and use PrecomputedTransactionData in SignTransaction (Pieter Wuille)
5d2e22437b Don't nuke witness data when signing fails (Pieter Wuille)
ce9353164b Permit full precomputation in PrecomputedTransactionData (Pieter Wuille)
e841fb503d Add precomputed txdata support to MutableTransactionSignatureCreator (Pieter Wuille)
a91d532338 Add CKey::SignSchnorr function for BIP 340/341 signing (Pieter Wuille)
e77a2839b5 Use HandleMissingData also in CheckSchnorrSignature (Pieter Wuille)
dbb0ce9fbf Add TaprootSpendData data structure, equivalent to script map for P2[W]SH (Pieter Wuille)
Pull request description:
Builds on top of #22051, adding signing support after derivation support.
Nothing is changed in descriptor features. Signing works for key path and script path spending, through the normal sending functions, and PSBT-based RPCs. However, PSBT usability is rather low as no extensions have been defined to convey Taproot-specific information, so all script information must be known to the signing wallet.
ACKs for top commit:
achow101:
re-ACK 458a345b05
fjahr:
Code review ACK 458a345b05
Sjors:
ACK 458a345b05
Tree-SHA512: 30ed212cf7754763a4a81624ebc084c51727b8322711ac0b390369213c1a891d367ed8b123882ac08c99595320c11ec57ee42304ff22a69afdc3d1a0d55cc711
9550dffa0c fuzz: Assert roundtrip equality for `CPubKey` (Sebastian Falbesoner)
Pull request description:
This PR is a (quite late) follow-up to #19237 (https://github.com/bitcoin/bitcoin/pull/19237#issuecomment-642203251). Looking at `CPubKey::Serialize` and `CPubKey::Unserialize` I can't think of a scenario where the roundtrip (serialization/deserialization) equality wouldn't hold.
ACKs for top commit:
jamesob:
crACK 9550dffa0c pending CI
Tree-SHA512: 640fb9e777d249769b22ee52c0b15a68ff0645b16c986e1c0bce9742155d14f1be601e591833e1dc8dcffebf271966c6b861b90888a44aae1feae2e0248e2c55
f8866e8c32 Add roundtrip fuzz tests for CAddress serialization (Pieter Wuille)
e2f0548b52 Use addrv2 serialization in anchors.dat (Pieter Wuille)
8cd8f37dfe Introduce well-defined CAddress disk serialization (Pieter Wuille)
Pull request description:
Alternative to #20509.
This makes the `CAddress` disk serialization format well defined, and uses it to enable addrv2 support in anchors.dat (in a way that's compatible with older software). The new format is:
- The first 4 bytes store a format version number. Its low 19 bits are ignored (as those historically stored the `CLIENT_VERSION`), but its high 13 bits specify the actual serialization:
- 0x00000000: LE64 encoding for `nServices`, V1 encoding for `CService` (like pre-BIP155 network serialization).
- 0x20000000: CompactSize encoding for `nServices`, V2 encoding for `CService` (like BIP155 network serialization).
- Any other value triggers an unsupported format error on deserialization, and can be used for future format changes.
- The `ADDRV2_FORMAT` flag in the stream's version does not determine the actual serialization format; it only sets whether or not V2 encoding is permitted.
ACKs for top commit:
achow101:
ACK f8866e8c32
laanwj:
Code review ACK f8866e8c32
vasild:
ACK f8866e8c32
jonatack:
ACK f8866e8c32 tested rebased to master and built/run/restarted with DEBUG_ADDRMAN, peers.dat and anchors ser/deser seems fine
hebasto:
ACK f8866e8c32, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: 3898f8a8c51783a46dd0aae03fa10060521f5dd6e79315fe95ba807689e78f202388ffa28c40bf156c6f7b1fc2ce806b155dcbe56027df73d039a55331723796
faf1af58f8 fuzz: Add Temporary debug assert for oss-fuzz issue (MarcoFalke)
Pull request description:
oss-fuzz is acting weird, so add an earlier assert to help troubleshooting
ACKs for top commit:
practicalswift:
cr ACK faf1af58f8
Tree-SHA512: 85830d7d47cf6b4edfe91a07bd5aa8f7110db0bade8df93868cf276ed04d5dd17e671f769e6a0fb5092012b86aa82bb411fb171411f15746981104ce634c88c1
2f5bdcbc31 gui: misc external signer fixes and translation hints (Sjors Provoost)
d672404466 refactor: make ExternalSigner NetworkArg() and m_chain private (Sjors Provoost)
4455145e26 refactor: reduce #ifdef ENABLE_EXTERNAL_SIGNER usage (Sjors Provoost)
5be90c907e build: enable external signer by default (Sjors Provoost)
7d9453041b refactor: clean up external_signer.h includes (Sjors Provoost)
fc0eca31b3 fuzz: fix fuzz binary linking order (Sjors Provoost)
Pull request description:
This follows the introduction of GUI support in https://github.com/bitcoin-core/gui/pull/4
I don't think we should expect GUI users to self compile. This also enables external signer support by default for RPC users.
In addition this PR reduces the number of `#ifdef ENABLE_EXTERNAL_SIGNER`, which also fixes #21919. When compiled with `--disable-external-signer` such wallets can't be created in RPC or GUI, but they can be loaded. Attempting any action that calls HWI will trigger an error.
Side-note: this PR may or may not (currently) break CI for the GUI repository, as explained here: https://github.com/bitcoin-core/gui/pull/4#issuecomment-769859001
ACKs for top commit:
achow101:
ACK 2f5bdcbc31
hebasto:
re-ACK 2f5bdcbc31
Tree-SHA512: 1b71c5a8bea2be077ee9fa33a01130c957a0cf90951d4b7b04d3d0ef826bb77e474c3963abddfef2e2c1ea99d9c72cd2302d1eb9b5fcb7ba0bd2a625f006aa05
We encountered a linking error when attempting to include external_signer_scriptpubkeyman.cpp when configured with --disable-external-signer.
Everywhere else we have LIBBITCOIN_WALLET, it is always before LIBBITCOIN_COMMON. But if you go up to where FUZZ_SUITE_LD_COMMON is first set, you see that we will end up having LIBBITCOIN_COMMON set before LIBBITCOIN_WALLET which means that the linker will have problems linking things common things that the wallet uses. Because the order is correct for the other targets, we only see a linker error for test/fuzz/fuzz.
In this diff, LIBTEST_UTIL and LIBTEST_FUZZ are moved to the top because they include LIBBITCOIN_SERVER and LIBBITCOIN_COMMON. LIBBITCOIN_SERVER always needs to be the first item in the linker order since it has the most dependencies.
The makefiles for making the fuzz and test binaries should be revisited so that the linking order is made consistent with the rest of the code and to avoid other linker order issues that may crop up in the future.
Co-Authored-By: Andrew Chow <achow101-github@achow101.com>
79c02c88b3 Randomize message processing peer order (Pieter Wuille)
Pull request description:
Right now, the message handling loop iterates the list of nodes always in the same order: the order they were connected in (see the `vNodes` vector). For some parts of the net processing logic, this order matters. Transaction requests are assigned explicitly to peers since #19988, but many other parts of processing work on a "first-served-by-loop-first" basis, such as block downloading. If peers can predict this ordering, it may be exploited to cause delays.
As there isn't anything particularly optimal about the current ordering, just make it unpredictable by randomizing.
Reported by Crypt-iQ.
ACKs for top commit:
jnewbery:
ACK 79c02c88b3
Crypt-iQ:
ACK 79c02c88b3
sdaftuar:
utACK 79c02c88b3
achow101:
Code Review ACK 79c02c88b3
jamesob:
crACK 79c02c88b3
jonatack:
ACK 79c02c88b3
vasild:
ACK 79c02c88b3
theStack:
ACK 79c02c88b3
Tree-SHA512: 9a87c4dcad47c2d61b76c4f37f59674876b78f33f45943089bf159902a23e12de7a5feae1a73b17cbc3f2e37c980ecf0f7fd86af9e6fa3a68099537a3c82c106
e4c916a0ea Bugfix: GUI: Use a different shortcut for "1 d&ay" banning, due to conflict with "&Disconnect" (Luke Dashjr)
94e7cdd7e0 GUI: Add keyboard shortcuts for other context menus (Luke Dashjr)
02b5263cd4 GUI: Restore keyboard shortcuts for context menu entries (Luke Dashjr)
Pull request description:
Various keyboard shortcuts were lost in #263; this restores them, and also adds new ones for other context menus.
Note that with a context menu open, simply the shortcut by itself (no Alt) is used.
ACKs for top commit:
jarolrod:
Code Review ACK e4c916a
hebasto:
ACK e4c916a0ea, tested on Linux Mint 20.1 (Qt 5.12.8).
Tree-SHA512: 949461acf7aac592bc48a1c5abad41b167365830e0cedb3aa11b6a87bd347e16126830ea87936f9c9efc4b7df5b09d3833fae784964d6d119ed45703cfba2ffd
