05f9770c1f doc: Clarify developer notes about constant naming (Russell Yanofsky)
Pull request description:
I'm pretty sure developer notes were intended to say constants should be upper case and variables should be lower case, but right now they are ambiguous about whether to write:
```c++
extern const int SYMBOL;
```
or:
```c++
extern const int g_symbol;
```
First convention above is better than the second convention because it tells you without having to look anything up that the value of `SYMBOL` won't change at runtime. Also I haven't seen other c++ projects using the second convention.
ACKs for top commit:
MarcoFalke:
cr ACK 05f9770c1f
practicalswift:
ACK 05f9770c1f
jarolrod:
ACK 05f9770c1f🥃
Tree-SHA512: 766d0e25d9db818d45df4ad6386987014f2053584cbced4b755ceef8bda6b7e2cfeb34eb8516423bd03b140faaf577614d5e3be2799f7eed0eb439187ab85323
dd49f1b52e doc: doc/README.md IRC updates (Jon Atack)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/22109:
- update remaining link and reference from Freenode to Libera
- remove defunct #bitcoin-dev channel
- other minor fixups
ACKs for top commit:
fanquake:
ACK dd49f1b52e
Tree-SHA512: de20a8f3889028def7139be1cd0fa15daa3351ca987a16090d0caa6b4cd397e370400e2b3033ec776e270aa3a1ab085ada20fbdcc063ac88e2ad3b8351a596e7
957f358427 build: remove check for Boost Process header (fanquake)
df2c933217 build: remove workaround for Boost and std::atomic (fanquake)
2bf211696b build: set minimum required Boost to 1.64.0 (fanquake)
Pull request description:
Setting a newer minimum required Boost means we can remove the awkward header / compile check for Boost Process.
If we don't do this, we should at-least make Boost Process being missing no longer a failure, otherwise anyone building using Boost < 1.64.0 would have to pass `--disable-external-signer` as well.
The only system I can see that is affected here, (doesn't have new enough system packages) is Debian Oldstable. However, anyone compiling there, can use depends. They can also no-longer use the system GCC (6.0), and I'd assume would be using Clang 7, which would be the newest compiler available to them. It's extended, LTS support also end in 1 year from now, so anyone still using it should be considering upgrading.
Debian Buster (Stable) has 1.67+, Ubuntu Bionic has 1.65+, any of the BSDs, recent Fedora, macOS etc all also have well and truly new enough Boost versions available.
I think this is something we should just do for 22.0. If not, definitely for 23.0.
Fixes#22319. Compiling Bitcoin Core should work, as `windows.h` will be included.
Alternative to #22294.
Would also close#22269.
#19128 could be re-opened.
ACKs for top commit:
laanwj:
Tested ACK 957f358427 that this fixes#22269.
MarcoFalke:
review ACK 957f358427
Tree-SHA512: a8ffa7933dce8bf994892ef16664103d7b4e1008e52628e9becb918a7727232dfb51b23100a82dc2b60cd9af5877abc32dc2d3754a7e1b3ac5410a92fdf393f3
bb719a08db style: remove () from assert in rpc_setban.py (Vasil Dimov)
24b10ebda3 doc: fix grammar in doc/files.md (Vasil Dimov)
dd4e957dcd test: ensure banlist can be read from disk after restart (Vasil Dimov)
d197977ae2 banman: save the banlist in a JSON format on disk (Vasil Dimov)
Pull request description:
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
ACKs for top commit:
jonatack:
Code review re-ACK bb719a08db per `git range-diff 6a67366 4b52c72 bb719a0`
achow101:
Code Review ACK bb719a08db
Tree-SHA512: fc135c3a1fe20bcf5d008ce6bea251b4135e56c78bf8f750b4bd8144c095b81ffe165133cdc7e4715875eec7e7c4e13ad9f5d2450b21102af063d7c8abf716b6
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
2ad034a890 doc: update release notes with removal of tor v2 support (Jon Atack)
49938eee9c doc: update tor.md with removal of tor v2 support (Jon Atack)
Pull request description:
Follow-up documentation to #22050 that removed support for Tor version 2 hidden services from Bitcoin Core.
ACKs for top commit:
laanwj:
ACK 2ad034a890
Tree-SHA512: 0f13f9d1db7e11f1e3d9967b6d17b8dc3144b3ab3a258c706464c5e6ac5cbcf2ce2db4ea54be9939f05a82ebd1e7f325f50b435f9822c08b4f21ed4ac58de0af
Since the flag has been removed from the RPC command table in commit
ec6902d0ea (PR #11179), this guideline
is not relevant anymore and can be removed.
2667366aaa tests: check derivation of P2TR (Pieter Wuille)
7cedafc541 Add tr() descriptor (derivation only, no signing) (Pieter Wuille)
90fcac365e Add TaprootBuilder class (Pieter Wuille)
5f6cc8daa8 Add XOnlyPubKey::CreateTapTweak (Pieter Wuille)
2fbfb1becb Make consensus checking of tweaks in pubkey.* Taproot-specific (Pieter Wuille)
a4bf84039c Separate WitnessV1Taproot variant in CTxDestination (Pieter Wuille)
41839bdb89 Avoid dependence on CTxDestination index order (Pieter Wuille)
31df02a070 Change Solver() output for WITNESS_V1_TAPROOT (Pieter Wuille)
4b1cc08f9f Make XOnlyPubKey act like byte container (Pieter Wuille)
Pull request description:
This is a subset of #21365, to aide review.
This adds support `tr(KEY)` or `tr(KEY,SCRIPT)` or `tr(KEY,{{S1,{{S2,S3},...}},...})` descriptors, describing Taproot outputs with specified internal key, and optionally any number of scripts, in nested groups of 2 inside `{`/`}` if there are more than one. While it permits importing `tr(KEY)`, anything beyond that is just laying foundations for more features later.
Missing:
* Signing support (see #21365)
* Support for more interesting scripts inside the tree (only `pk(KEY)` is supported for now). In particular, a multisig policy based on the new `OP_CHECKSIGADD` opcode would be very useful.
* Inferring `tr()` descriptors from outputs (given sufficient information).
* `getaddressinfo` support.
* MuSig support. Standardizing that is still an ongoing effort, and is generally kind of useless without corresponding PSBT support.
* Convenient ways of constructing descriptors without spendable internal key (especially ones that arent't trivially recognizable as such).
ACKs for top commit:
Sjors:
utACK 2667366 (based on https://github.com/bitcoin/bitcoin/pull/21365#issuecomment-846945215 review, plus the new functional test)
achow101:
Code Review ACK 2667366aaa
lsilva01:
Tested ACK 2667366aaa
meshcollider:
utACK 2667366aaa
Tree-SHA512: 61046fef22c561228338cb178422f0b782ef6587ec8208d3ce2bd07afcff29a664b54b35c6b01226eb70b6540b43f6dd245043d09aa6cb6db1381b6042667e75
dafab2b1b3 doc: improve note on choosing posix mingw32 (Jarol Rodriguez)
Pull request description:
The current [windows build doc](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md) can lead someone to believe that the step where you must choose the posix mingw32 g++ compiler option is only for `Ubuntu 18.04`. It is only until you (or just me) go through the cross-compile process and realize that it's not building because you didn't set this option on > `Ubuntu 18.04`. Then you come back and read the footnotes and see: `Starting from Ubuntu Xenial 16.04...`.
This PR improves this portion of the doc by editing the text around this stage to state "this is now the next step". We could add a note saying `Ubuntu 18.04 and up`, but this is redundant as it's unlikely someone will be using < Ubuntu 18.04 since it's not officially supported by our build system. While here, some minor fixups and add some more guidance to completing this step.
**Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md#building-for-64-bit-windows)
**PR:** [render](dafab2b1b3/doc/build-windows.md (building-for-64-bit-windows))
ACKs for top commit:
laanwj:
ACK dafab2b1b3
hebasto:
ACK dafab2b1b3
Tree-SHA512: d8d50fa13c34be753700843183a1a4addad6447c813bfc039865ba11d5fa03c25206ed6f3782331df4c6a44448e3fec4138ae911b995b5cacc5b756bb5d74bb4
13650fe2e5 [policy] detect unsorted packages (glozow)
9ef643e21b [doc] add release note for package testmempoolaccept (glozow)
c4259f4b7e [test] functional test for packages in RPCs (glozow)
9ede34a6f2 [rpc] allow multiple txns in testmempoolaccept (glozow)
ae8e6df709 [policy] limit package sizes (glozow)
c9e1a26d1f [fuzz] add ProcessNewPackage call in tx_pool fuzzer (glozow)
363e3d916c [test] unit tests for ProcessNewPackage (glozow)
cd9a11ac96 [test] make submit optional in CreateValidMempoolTransaction (glozow)
2ef187941d [validation] package validation for test accepts (glozow)
578148ded6 [validation] explicit Success/Failure ctors for MempoolAcceptResult (glozow)
b88d77aec5 [policy] Define packages (glozow)
249f43f3cc [refactor] add option to disable RBF (glozow)
897e348f59 [coins/mempool] extend CCoinsViewMemPool to track temporary coins (glozow)
42cf8b25df [validation] make CheckSequenceLocks context-free (glozow)
Pull request description:
This PR enables validation dry-runs of packages through the `testmempoolaccept` RPC. The expectation is that the results returned from `testmempoolaccept` are what you'd get from test-then-submitting each transaction individually, in that order (this means the package is expected to be sorted in topological order, for now at least). The validation is also atomic: in the case of failure, it immediately halts and may return "unfinished" `MempoolAcceptResult`s for transactions that weren't fully validated. The API for 1 transaction stays the same.
**Motivation:**
- This allows you to test validity for transaction chains (e.g. with multiple spending paths and where you don't want to broadcast yet); closes#18480.
- It's also a first step towards package validation in a minimally invasive way.
- The RPC commit happens to close#21074 by clarifying the "allowed" key.
There are a few added restrictions on the packages, mostly to simplify the logic for areas that aren't critical to main package use cases:
- No package can have conflicts, i.e. none of them can spend the same inputs, even if it would be a valid BIP125 replacement.
- The package cannot conflict with the mempool, i.e. RBF is disabled.
- The total count of the package cannot exceed 25 (the default descendant count limit), and total size cannot exceed 101KvB (the default descendant size limit).
If you're looking for review comments and github isn't loading them, I have a gist compiling some topics of discussion [here](https://gist.github.com/glozow/c3acaf161c95bba491fce31585b2aaf7)
ACKs for top commit:
laanwj:
Code review re-ACK 13650fe2e5
jnewbery:
Code review ACK 13650fe2e5
ariard:
ACK 13650fe
Tree-SHA512: 8c5cbfa91a6c714e1c8710bb281d5ff1c5af36741872a7c5df6b24874d6272b4a09f816cb8a4c7de33ef8e1c2a2c252c0df5105b7802f70bc6ff821ed7cc1a2f
This adds a new descriptor with syntax e.g. tr(KEY,{S1,{{S2,S3},S4})
where KEY is a key expression for the internal key and S_i are
script expression for the leaves. They have to be organized in
nested {A,B} groups, with exactly two elements.
tr() only exists at the top level, and inside the script expressions
only pk() scripts are allowed for now.
fadd98d02f doc: Fix OSS-Fuzz links (MarcoFalke)
Pull request description:
* Add missing link to the coverage report
* Replace unqualified link with qualified one
ACKs for top commit:
practicalswift:
ACK fadd98d02f
jarolrod:
ACK fadd98d02f
Tree-SHA512: 5d5138288ae75d6a9ae536f307a63813ffe60c628a8c9a74b33f0d1be1a34fac2ff5eca9c9fe64be3139f2251736ba088c5f3731f0b3f50c70c5a5f1a89f0ea4
07bc22ef10 docs: improve make with parallel jobs description. (Klement Tan)
Pull request description:
Changed `use -jX here for parallelism` to `use "-j N" for N parallel jobs`
**Rationale**: In my opinion `use -jX here for parallelism` is quite ambiguous as it could be perceived as a single option without any argument. Ie running:
```sh
make -jX
```
Embarrassingly this caused me to be stuck for quite a long time until I opened the help menu for `make` but if I am the only one who faced this issue I would be happy to close this PR.
ACKs for top commit:
jarolrod:
ACK 07bc22ef10
Tree-SHA512: 2d119b6a461668906c63184b865d2cc9fb2f75abeba34e2e44bc1ef3bcb4adec4a49896ddaf3cc6a20c0095ad20d0de0908401b351eaca9443161d24d6b20d0b
11d6459b6e rpc: include_unsafe option for fundrawtransaction (t-bast)
Pull request description:
Allow RPC users to opt-in to unsafe inputs when funding a raw transaction.
Applications that need to manage a complex RBF flow (such as lightning nodes using anchor outputs) are very limited if they can only use safe inputs.
I also added this option to `send` and `walletcreatefundedpsbt` who internally delegate to `fundrawtransaction`.
Fixes#21299
ACKs for top commit:
laanwj:
Code review ACK 11d6459b6e
Tree-SHA512: 5e542a4febcfd6f41cf784678ff02ec9282eae2082c274983f72c5ea87b7ebbe1bd5fdc6a020d7a9d5996157754eb4966b8aeb6c1ceebf0b1519f735579b8bac
47c3ea021e doc: add OSS-Fuzz section to fuzzing.md doc (Adam Jonas)
Pull request description:
This adds documentation about [Bitcoin Core's participation](https://github.com/google/oss-fuzz/pull/5699/files) in Google's OSS-Fuzz program and adds the caveat that the project may not disclose vulnerabilities within the 90-day window described in the [program's disclosure guidelines](https://google.github.io/oss-fuzz/getting-started/bug-disclosure-guidelines/).
ACKs for top commit:
jonatack:
ACK 47c3ea021e
Tree-SHA512: 87bf0146fb74d1e4b3b8839e6c8f3d53046008a6d5b926ffe5b95be3c396a5e47e47967533422f60b04c4446482f49d210ada410b742f69781a7afde623d704d
ebd4be43cc doc: add release notes for 20867 (Antoine Poinsot)
5aa50ab9cc rpc/util: multisig: only check redeemScript size is <= 520 for P2SH (Antoine Poinsot)
063df9e897 test/functional: standardness sanity checks for P2(W)SH multisig (Antoine Poinsot)
ae0429d3af script: allow up to 20 keys in wsh() descriptors (Antoine Poinsot)
9fc68faf35 script: match multisigs with up to MAX_PUBKEYS_PER_MULTISIG keys (Antoine Poinsot)
Pull request description:
As described in https://github.com/bitcoin/bitcoin/issues/20620 multisigs are currently limited to 16 keys in descriptors and RPC helpers, even for P2WSH and P2SH-P2WSH.
This adds support for multisig with up to 20 keys (which are already standard) for Segwit v0 context for descriptors (`wsh()`, `sh(wsh())`) and RPC helpers.
Fixes https://github.com/bitcoin/bitcoin/issues/20620
ACKs for top commit:
meshcollider:
re-utACK ebd4be43cc
instagibbs:
re-ACK ebd4be43cc
Tree-SHA512: 36141f10a8288010d17d5c4fe8d24878bcd4533b88a8aba3a44fa8f74ceb3182d70fee01427e0ab7f53ce7fab46c88c1cd3ac3b18ab8a10bd4a6b8b74ed79e46
Allow RPC users to opt-in to unsafe inputs when funding a raw transaction.
Applications that need to manage a complex RBF flow (such as lightning
nodes using anchor outputs) are very limited if they can only use safe inputs.
Fixes#21299
