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
06c43201a7 cli: use C++17 std::array class template argument deduction (CTAD) (Jon Atack)
edf3167151 addrinfo: raise helpfully on server error or incompatible server version (Jon Atack)
bb85cbc4f7 doc: add cli -addrinfo release note (Jon Atack)
5056a37624 cli: add -addrinfo command (Jon Atack)
db4d2c282a cli: create AddrinfoRequestHandler class (Jon Atack)
Pull request description:
While looking at issue #21351, it turned out that the problem was a lack of tor v3 addresses known to the node. It became clear (e.g. https://github.com/bitcoin/bitcoin/issues/21351#issuecomment-811004779) that a CLI command returning the number of addresses the node knows per network (with a tor v2 / v3 breakdown) would be very helpful. This patch adds that.
`-addrinfo` is useful to see if your node knows enough addresses in a network to use options like `-onlynet=<network>`, or to upgrade to the upcoming tor release that no longer supports tor v2, for which you'll need to be sure your node knows enough tor v3 peers.
```
$ bitcoin-cli --help | grep -A1 addrinfo
-addrinfo
Get the number of addresses known to the node, per network and total.
$ bitcoin-cli -addrinfo
{
"addresses_known": {
"ipv4": 14406,
"ipv6": 2511,
"torv2": 5563,
"torv3": 2842,
"i2p": 8,
"total": 25330
}
}
$ bitcoin-cli -addrinfo 1
error: -addrinfo takes no arguments
```
This can be manually tested, for example, with commands like this:
```
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length <= 22)) | .address' | wc -l
5563
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length > 22)) | .address' | wc -l
2842
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | .address' | wc -l
25330
```
ACKs for top commit:
laanwj:
Tested ACK 06c43201a7
Tree-SHA512: b668b47718a4ce052aff218789f3da629bca730592c18fcce9a51034d95a0a65f8e6da33dd47443cdd8f60c056c02696db175b0fe09a688e4385a76c1d8b7aeb
223b1ba7d9 doc: Use CONFIG_SITE instead of --prefix (Hennadii Stepanov)
Pull request description:
The current examples of `--prefix=...` option usage to point `configure` script to appropriate `depends` directory is not [standard](https://www.gnu.org/prep/standards/html_node/Directory-Variables.html). This causes some [confusion](https://github.com/bitcoin/bitcoin/pull/16691) and a bit of inconvenience.
Consider a CentOS 7 32 bit system. Packages `libdb4-devel`, `libdb4-cxx-devel`, `miniupnpc-devel` and `zeromq-devel` are unavailable from repos. After recommended build with depends:
```
cd depends
make
cd ..
./autogen.sh
./configure --prefix=$PWD/depends/i686-pc-linux-gnu
make
```
a user is unable to `make install` compiled binaries neither locally (to `~/.local`) nor system-wide (to `/usr/local`) as `--prefix` is set already.
Meanwhile, the standard approach with using [`config.site`](https://www.gnu.org/software/automake/manual/html_node/config_002esite.html) files allows both possibilities:
```
cd depends
make
cd ..
./autogen.sh
CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure --prefix ~/.local
make
make install
```
or
```
CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure
make
sudo make install # install to /usr/local
```
Moreover, this approach is used in [Gitian descriptors](https://github.com/bitcoin/bitcoin/tree/master/contrib/gitian-descriptors) already.
ACKs for top commit:
practicalswift:
ACK 223b1ba7d9: patch looks correct
fanquake:
ACK 223b1ba7d9
Tree-SHA512: 46d97924f0fc7e95ee4566737cf7c2ae805ca500e5c49af9aa99ecc3acede4b00329bc727a110aa1b62618dfbf5d1ca2234e736f16fbdf96d6ece5f821712f54
88d4d5ff2f rpc: add help for enumeratesigners and walletdisplayaddress (Sjors Provoost)
b0db187e5b ci: use --enable-external-signer instead of --with-boost-process (Sjors Provoost)
b54b2e7b1a Move external signer out of wallet module (Sjors Provoost)
Pull request description:
In addition, this PR enables external signer testing on CI.
This PR moves the ExternalSigner class and RPC methods out of the wallet module.
The `enumeratesigners` RPC can be used without a wallet since #21417. With additional modifications external signers could be used without a wallet in general, e.g. via `signrawtransaction`.
The `signerdisplayaddress` RPC is ranamed to `walletdisplayaddress` because it requires wallet context. A future `displayaddress` RPC call without wallet context could take a descriptor argument.
This commit fixes a `rpc_help.py` failure when configured with `--disable-wallet`.
ACKs for top commit:
ryanofsky:
Code review ACK 88d4d5ff2f
fanquake:
ACK 88d4d5ff2f
Tree-SHA512: 3242a24e22313aed97eee32a520bfcb1c17495ba32a2b8e06a5e151e2611320e2da5ef35b572d84623af0a49a210d2f9377a2531250868d1a0ccf3e144352a97
d3b0b08b0f doc: release notes for new listbanned fields (Jarol Rodriguez)
60290d3f5e test: increase listbanned unit test coverage (Jon Atack)
3e978d1a5d rpc: add time_remaining field to listbanned (Jarol Rodriguez)
5456b34531 rpc: add ban_duration field to listbanned (Jarol Rodriguez)
c95c61657a doc: improve listbanned help (Jarol Rodriguez)
dd3c8eaa33 rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez)
Pull request description:
This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields
It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](https://github.com/bitcoin/bitcoin/pull/21602#issuecomment-813486134) by jonatack, `time_remaining` is another useful user-centric data point.
Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical.
This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields.
**Master: listbanned**
```
[
{
"address": "1.2.3.4/32",
"banned_until": 1617691101,
"ban_created": 1617604701
},
{
"address": "135.181.41.129/32",
"banned_until": 1649140716,
"ban_created": 1617604716
}
]
```
**PR: listbanned**
```
[
{
"address": "1.2.3.4/32",
"ban_created": 1617775773,
"banned_until": 1617862173,
"ban_duration": 86400,
"time_remaining": 86392
},
{
"address": "3.114.211.172/32",
"ban_created": 1617753165,
"banned_until": 1618357965,
"ban_duration": 604800,
"time_remaining": 582184
}
]
```
ACKs for top commit:
jonatack:
re-ACK d3b0b08b0f
hebasto:
ACK d3b0b08b0f, tested on Linux Mint 20.1 (x86_64).
MarcoFalke:
review ACK d3b0b08b0f🕙
Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5
This commit moves the ExternalSigner class and RPC methods out of the wallet module.
The enumeratesigners RPC can be used without a wallet since #21417.
With additional modifications external signers could be used without a wallet in general, e.g. via signrawtransaction.
The signerdisplayaddress RPC is ranamed to walletdisplayaddress because it requires wallet context.
A future displayaddress RPC call without wallet context could take a descriptor argument.
This commit fixes a rpc_help.py failure when configured with --disable-wallet.
5c446784b1 rpc: improve getnodeaddresses help (Jon Atack)
1b9189866a rpc: simplify/constify getnodeaddresses code (Jon Atack)
3bb6e7b655 rpc: add network field to rpc getnodeaddresses (Jon Atack)
Pull request description:
This patch adds a network field to RPC `getnodeaddresses`, which is useful on its own, particularly with the addition of new networks like I2P and others in the future, and which I also found helpful for adding a new CLI command as a follow-up to this pull that calls `getnodeaddresses` and needs to know the network of each address.
While here, also improve the `getnodeaddresses` code and help.
```
$ bitcoin-cli -signet getnodeaddresses 3
[
{
"time": 1611564659,
"services": 1033,
"address": "2600:1702:3c30:734f:8f2e:744b:2a51:dfa5",
"port": 38333,
"network": "ipv6"
},
{
"time": 1617531931,
"services": 1033,
"address": "153.126.143.201",
"port": 38333,
"network": "ipv4"
},
{
"time": 1617473058,
"services": 1033,
"address": "nsgyo7begau4yecc46ljfecaykyzszcseapxmtu6adrfagfrrzrlngyd.onion",
"port": 38333,
"network": "onion"
}
]
$ bitcoin-cli help getnodeaddresses
getnodeaddresses ( count )
Return known addresses, which can potentially be used to find new nodes in the network.
Arguments:
1. count (numeric, optional, default=1) The maximum number of addresses to return. Specify 0 to return all known addresses.
Result:
[ (json array)
{ (json object)
"time" : xxx, (numeric) The UNIX epoch time when the node was last seen
"services" : n, (numeric) The services offered by the node
"address" : "str", (string) The address of the node
"port" : n, (numeric) The port number of the node
"network" : "str" (string) The network (ipv4, ipv6, onion, i2p) the node connected through
},
...
]
```
Future idea: allow passing `getnodeaddresses` a network (or networks) as an argument to return only addresses in that network.
ACKs for top commit:
laanwj:
Tested ACK 5c446784b1
jarolrod:
re-ACK 5c446784b1
promag:
Code review ACK 5c446784b1.
Tree-SHA512: ab0101f50c76d98c3204133b9f2ab6b7b17193ada31455ef706ad11afbf48f472fa3deb33e96028682369b35710ccd07d81863d2fd55c1485f32432f2b75efa8
