- the Java I2P router and i2pd are the two routers have been heavily tested
with Bitcoin Core and are what people and node software packages use
- i2p-zero (https://github.com/i2p-zero/i2p-zero) hasn't been updated since
July 2021 and its last release was in December 2020
- the other routers in the wikipedia page are niche
The SAM application bridge is not enabled by default in the Java I2P Router,
and the relevant info is somewhat difficult to find in its documentation.
Also, remove a duplicate sentence; the preceding paragraph begins with the same.
daa5a658c0 refactor: rename BCLog::BLOCKSTORE to BLOCKSTORAGE (Jon Atack)
cf622b214b doc: release note re raising on invalid -debug/debugexclude/loglevel (Jon Atack)
6cb1c66041 init: remove config option names from translated -loglevel strings (Jon Atack)
2547829272 test: -loglevel raises on invalid values (Jon Atack)
a9c295888b init: raise on invalid loglevel config option (Jon Atack)
b0c3995393 test: -debug and -debugexclude raise on invalid values (Jon Atack)
4c3c19d943 init: raise on invalid debug/debugexclude config options (Jon Atack)
Pull request description:
and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums.
Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458.
ACKs for top commit:
achow101:
ACK daa5a658c0
ryanofsky:
Code review ACK daa5a658c0. Just translated string template cleanup since last review
pinheadmz:
re-ACK daa5a658c0
Tree-SHA512: 4c107a93d8e8ce4e2ee81d44aec672526ca354ec390b241221067f68204beac8b4ba7a65748bcfa124ff2245c4307fa9243ec4fe0b464d0fa69c787fb322c3cc
5524fa00fa doc: add release note about removal of `deprecatedrpc=walletwarningfield` flag (Sebastian Falbesoner)
5c77db7354 Restorewallet/createwallet help documentation fixups/improvements (Jon Atack)
a00ae31fcc rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet (Sebastian Falbesoner)
Pull request description:
The "warning" string field for wallet creating/loading RPCs (`createwallet`, `loadwallet`, `unloadwallet` and `restorewallet`) has been deprecated with the configuration option `-deprecatedrpc=walletwarningfield` in PR #27279 (released in v25.0). For the next release v26.0, the field and the configuration option can be removed.
ACKs for top commit:
achow101:
ACK 5524fa00fa
jonatack:
ACK 5524fa00fa
Tree-SHA512: 8212f72067d08095304018b8a95d2ebef630004b65123483fbbfb078cc5709c2d825bbc35b16ea5f6b28ae7377347382d7e9afaf7bdbf0575d2c229d970784de
67b7fecacd [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow)
c1061acb9d [functional test] prioritisation is not removed during replacement and expiry (glozow)
0e5874f0b0 [functional test] getprioritisedtransactions RPC (glozow)
99f8046829 [rpc] add getprioritisedtransactions (glozow)
9e9ca36c80 [mempool] add GetPrioritisedTransactions (glozow)
Pull request description:
Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.
Motivation / Background
- `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
- Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen.
- Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart.
- Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them.
- Related: #4818 and #6464.
- There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat.
- Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while.
Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are.
ACKs for top commit:
achow101:
ACK 67b7fecacd
theStack:
Code-review ACK 67b7fecacd
instagibbs:
code review ACK 67b7fecacd
ajtowns:
ACK 67b7fecacd code review only, some nits
Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
65e3abcbf2 doc: document json rpc endpoints (willcl-ark)
Pull request description:
fixes#20246
This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks.
ACKs for top commit:
fanquake:
ACK 65e3abcbf2 - Seems fine. Can be improved if need be.
Tree-SHA512: d557c2caf000a1bdd7b46c9da38afe63dc28446ba4a961526f1af3cec81d994a9da663e02c81ebdc4f609b794585349cfca77a582dc1e788c120de1d3b4c7db6
2cd28e9fef rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky)
95d7de0964 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky)
96233146dd RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky)
702b56d2a8 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky)
Pull request description:
Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects.
This makes it possible to make calls like:
```sh
src/bitcoin-cli -named bumpfee txid fee_rate=10
```
instead of
```sh
src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}'
```
RPC help is also updated to show options as top level named arguments instead of as nested objects.
<details><summary>diff</summary>
<p>
```diff
@@ -15,16 +15,17 @@
Arguments:
1. txid (string, required) The txid to be bumped
-2. options (json object, optional)
+2. options (json object, optional) Options object that can be used to pass named arguments, listed below.
+
+Named Arguments:
- {
- "conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
+conf_target (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
- "fee_rate": amount, (numeric or string, optional, default=not set, fall back to wallet fee estimation)
+fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation)
Specify a fee rate in sat/vB instead of relying on the built-in fee estimator.
Must be at least 1.000 sat/vB higher than the current transaction fee rate.
WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB.
- "replaceable": bool, (boolean, optional, default=true) Whether the new transaction should still be
+replaceable (boolean, optional, default=true) Whether the new transaction should still be
marked bip-125 replaceable. If true, the sequence numbers in the transaction will
be left unchanged from the original. If false, any input sequence numbers in the
original transaction that were less than 0xfffffffe will be increased to 0xfffffffe
@@ -32,11 +33,10 @@
still be replaceable in practice, for example if it has unconfirmed ancestors which
are replaceable).
- "estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
+estimate_mode (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
"unset"
"economical"
"conservative"
- }
Result:
{ (json object)
```
</p>
</details>
**Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation.
ACKs for top commit:
achow101:
ACK 2cd28e9fef
Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436
6fce5ddc17 doc: update getnodeaddresses for CJDNS, I2P and Tor and rm link (Marnix)
Pull request description:
- remove broken link about how to properly configure tor
- generalize getnodeaddresses RPC in doc
ACKs for top commit:
fanquake:
ACK 6fce5ddc17
Tree-SHA512: 3a077a0724c57a5c6182d40fbf34a84d2515bf1bf06ea0ce717174d0a27f5b19b9521c1ed1995adfdf4d43c2ce978a81e2ec9e3c8faf83f5188571fa75ea5314
eefe56967b bugfix: Fix incorrect debug.log config file path (Ryan Ofsky)
3746f00be1 init: Error if ignored bitcoin.conf file is found (Ryan Ofsky)
398c3719b0 lint: Fix lint-format-strings false positives when format specifiers have argument positions (Ryan Ofsky)
Pull request description:
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:
- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.conf` file is ignored with no warning.
- Another way this could happen is if a `-conf=` command line argument points to a configuration file with a `datadir=/path` line and that path contains a `bitcoin.conf` file, which is currently ignored.
This change only adds an error message and doesn't change anything about way settings are applied. It also doesn't trigger errors if there are redundant `-datadir` or `-conf` settings pointing at the same configuration file, only if they are pointing at different files and one file is being ignored.
ACKs for top commit:
pinheadmz:
re-ACK eefe56967b
willcl-ark:
re-ACK eefe56967b
TheCharlatan:
ACK eefe56967b
Tree-SHA512: 939a98a4b271b5263d64a2df3054c56fcde94784edf6f010d78693a371c38aa03138ae9cebb026b6164bbd898d8fd0845a61a454fd996e328fd7bcf51c580c2b
This requires a linux kernel of 3.17.0+, which seems entirely
reasonable. 3.17 went EOL in 2015, and the last supported 3.x kernel
(3.16) went EOL > 4 years ago, in 2020. For reference, the current
oldest maintained kernel is 4.14 (released 2017, EOL Jan 2024).
Support for `getrandom()` (and `getentropy()`) was added to
glibc 2.25, https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00079.html,
and we already require 2.27+.
All that being said, I don't think you would encounter a current day
system, running with kernel headers older than 3.17 (released 2014) but
also having a glibc of 2.27+ (released 2018).
4bfcbbfd4a doc: remove Security section from build-unix.md (fanquake)
Pull request description:
Our compile documentation isn't the right place for generic binary hardening notes, which are neither particularly Bitcoin-Core specific, or as relevant as they might have once been, i.e non-executable stacks are now just the norm.
Just remove the notes for now, if someone has something more interesting/Bitcoin Core specific, it could be added in separate documentation in the future (maybe into the devwiki or similar).
Split from https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196517868.
ACKs for top commit:
jarolrod:
ACK 4bfcbbfd4a
Tree-SHA512: 01b523ba40353d6cafb5dbd6540b514d83a2dc4e462a068fb390ca7de45b78e4a329367f70527f1824006ebe43f8caeea4ad05ec60556d5a5bd0143e27bf6581
fa953f15bf build: Bump minimum supported GCC to g++-9 (MarcoFalke)
fa69955e74 ci: Bump centos:stream8 to centos:stream9 (MarcoFalke)
fa6a755d9f ci: Document the false positive error for g++-9 (MarcoFalke)
Pull request description:
It is a bit frustrating to write valid C++ code only to realize that g++-8 fails to parse it later on.
The only non-EOL operating system still shipping with g++-8 is CentOS Stream 8. I think it is reasonable for users of affected Linux distributions to:
* Upgrade their operating system, or compiler to a supported version.
* Alternatively, stay with a previous release of Bitcoin Core as long as it is supported.
Fixes https://github.com/bitcoin/bitcoin/issues/27537
ACKs for top commit:
hebasto:
ACK fa953f15bf
fanquake:
ACK fa953f15bf
Tree-SHA512: b9cf7e763d3071e1e008c5010de19601d4773afe46d58cf869d3f59285c53240c739a1cd7235a5525ede1bbdf6b6cb6fb091c8fc314864a28d5b27a400bb7632
Our compile documentation isn't the right place for genric binary
hardening notes, which are neither particularly Bitcoin-Core specific,
or as relevant as they might have once been, i.e non-executable stacks
are now just the norm.
Just remove the notes for now, if someone has
something more interesting/Bitcoin Core specific, it could be added in
separate documentation in the future (maybe into the devwiki or
similar).
Split from
https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196517868.
fa29651c3f doc: Rework build-unix.md (MarcoFalke)
Pull request description:
The doc has many issues:
* The fist section contains outdated non-existing and confusing configure flags like `--enable-cxx` and `--disable-shared`, as well as edge-case expert options such as `BDB_PREFIX`. Fix that by removing the section and adding notes elsewhere, if applicable.
* There are links to the depends system before instructions on how to simply build from system packages. Fix that by moving that later.
* Also, remove sections that are duplicate with other depends READMEs.
ACKs for top commit:
fanquake:
ACK fa29651c3f
TheCharlatan:
ACK fa29651c3f
Tree-SHA512: 5348ddf3fa094c630d80b63033ca7b40ec0356427856f9a1075b31244f6bf8ec65cb2a738366e1174ef2fe7e0bf5cc249a62c58f458bbaf50668aceeac954820
7014e08015 doc: remove mention of glibc 2.10+ (fanquake)
Pull request description:
We already require glibc 2.27+, so mentioning a much older version here is redundant.
ACKs for top commit:
TheCharlatan:
ACK 7014e08015
Tree-SHA512: 883a566a80cabe34bfb5d902990f3eca08d0e11438e6c128d311e558f373ec232b0934deb85d12d796baacfeae590af8c73aa1b2faef07f27ffa9011270ffd96
fe49f06c0e doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2f46 Switch hardened derivation marker to h in descriptors (Sjors Provoost)
Pull request description:
This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.
For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.
Both markers can still be parsed.
The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.
See discussion in #15740
ACKs for top commit:
achow101:
ACK fe49f06c0e
darosior:
re-ACK fe49f06c0e
Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
9143b6988b [doc] Add post branch-off note about fuzz input pruning (dergoegge)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 82658faaa31668591853703604edb45ce24ee703b8f4077ab690865f3674e154f76c55c3b523f543a862aab9707d70a46c8bf4d41b51d0002635806413921017
710b83938a rpc: return block hash & height in getbalances, gettransaction & getwalletinfo JSONs (Harris)
Pull request description:
Reopens#18570 and closes#18567.
I have rebased the original PR.
Not sure why the original got closed as it was about to get merged.
ACKs for top commit:
achow101:
ACK 710b83938a
Tree-SHA512: d4478d990be98b1642e9ffb2930987f4a224e8bd64e2e35a5dda927a54c509ec9d712cd0eac35dc2bb89f00a1678e530ce14d7445f1dd93aa3a4cce2bc9b130d
fac395e5eb ci: Bump ci/lint/Dockerfile (MarcoFalke)
fa6eb65167 test: Use python3.8 pow() (MarcoFalke)
88881cf7ac Bump python minimum version to 3.8 (MarcoFalke)
Pull request description:
There is no pressing reason to drop support for 3.7, however there are several maintenance issues:
* There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1484988445)
* Compiling python 3.7 from source is also unsupported on at least macos, according to https://github.com/bitcoin/bitcoin/pull/24017#issuecomment-1107820790
* Recent versions of lief require 3.8, see https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517561645
Fix all maintenance issues by bumping the minimum.
ACKs for top commit:
RandyMcMillan:
ACK fac395e
fjahr:
ACK fac395e5eb
fanquake:
ACK fac395e5eb
Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
0c520679ab doc: add release notes for `abandoned` field in `gettransaction` and `listtransactions` (brunoerg)
a1aaa7f51f rpc, wallet: add `abandoned` field for all categories of transactions in ListTransactions (brunoerg)
Pull request description:
Fixes#25130
ACKs for top commit:
achow101:
re-ACK 0c520679ab
Tree-SHA512: 1864460d76decab7898737c96517d722055eb8f81ca52248fe1035723258c6cd4a93251e06a86ecbbb0b0a80af1466b2c86fb142ace4ccb74cc40d5dc3967d7f
bf77fc9cb4 [test] mempool full in package accept (glozow)
b51ebccc28 [validation] set PackageValidationState when mempool full (glozow)
563a2ee4f5 [policy] disallow transactions under min relay fee, even in packages (glozow)
c4554fe894 [test] package cpfp bumps parents <mempoolminfee but >=minrelaytxfee (glozow)
ac463e87df [test util] mock mempool minimum feerate (glozow)
Pull request description:
Part of package relay, see #27463.
Note that this still allows packages to bump transactions that are below the dynamic mempool minimum feerate, which means this still solves the "mempool is congested and my presigned 1sat/vB tx is screwed" problem for all transactions.
On master, the package policy (only accessible through regtest-only RPC submitpackage) allows 0-fee (or otherwise below min relay feerate) transactions if they are bumped by a child. However, with default package limits, we don't yet have a DoS-resistant way of ensuring these transactions remain bumped throughout their time in the mempool. Primarily, the fee-bumping child may later be replaced by another transaction that doesn't bump the parent(s). The parent(s) could potentially stay bumped by other transactions, but not enough to ever be selected by the `BlockAssembler` (due to `blockmintxfee`).
For example, (tested [here](https://github.com/glozow/bitcoin/commits/26933-motivation)):
- The mempool accepts 24 below-minrelayfeerate transactions ("0-fee parents"), all bumped by a single high-fee transaction ("the fee-bumping child"). The fee-bumping child also spends a confirmed UTXO.
- Two additional children are added to each 0-fee parent. These children each pay a feerate slightly above the minimum relay feerate (e.g. 1.9sat/vB) such that, for each 0-fee parent, the total fees of its two children divided by the total size of the children and parent is above the minimum relay feerate.
- If a block template is built now, all transactions would be selected.
- A transaction replaces the the fee-bumping child, spending only the confirmed UTXO and not any of the outputs from the 0-fee parents.
- The 0-fee parents now each have 2 children. Their descendant feerates are above minrelayfeerate, which means that they remain in the mempool, even if the mempool evicts all below-minrelayfeerate packages.
- If a block template is built now, none of the 0-fee parents or their children would be selected.
- Even more low-feerate descendants can be added to these below-minrelayfeerate packages and they will not be evicted until they expire or the mempool reaches capacity.
Unless we have a DoS-resistant way of ensuring package CPFP-bumped transactions are always bumped, allowing package CPFP to bump below-minrelayfeerate transactions can result in these problematic situations. See #27018 which proposes a partial solution with some limitations, and contains discussion about potential improvements to eviction strategy. While no adequate solution exists, for now, avoid these situations by requiring all transactions to meet min relay feerate.
ACKs for top commit:
ajtowns:
reACK bf77fc9cb4
instagibbs:
re-ACK bf77fc9cb4
Tree-SHA512: 28940f41493a9e280b010284316fb8caf1ed7b2090ba9a4ef8a3b2eafc5933601074b142f4f7d4e3c6c4cce99d3146f5c8e1393d9406c6f2070dd41c817985c9
Show an error on startup if a bitcoin datadir that is being used contains a
`bitcoin.conf` file that is ignored. There are two cases where this could
happen:
- One case reported in
https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043
happens when a bitcoin.conf file in the default datadir (e.g.
$HOME/.bitcoin/bitcoin.conf) has a "datadir=/path" line that sets different
datadir containing a second bitcoin.conf file. Currently the second
bitcoin.conf file is ignored with no warning.
- Another way this could happen is if a -conf= command line argument points
to a configuration file with a "datadir=/path" line and that specified path
contains a bitcoin.conf file, which is currently ignored.
This change only adds an error message and doesn't change anything about way
settings are applied. It also doesn't trigger errors if there are redundant
-datadir or -conf settings pointing at the same configuration file, only if
they are pointing at different files and one file is being ignored.
a2bef805c1 kernel: update m_assumed_* chain params for 25.x (fanquake)
4128e01dba kernel: update chainTxData for 25.x (fanquake)
00b2b114b4 kernel: update nMinimumChainWork & defaultAssumeValid for 25.x (fanquake)
07fcc0a82c doc: update references to kernel/chainparams.cpp (fanquake)
Pull request description:
Update chainparams pre `25.x` branch off.
Co-Author in the commits as a PR (#27223) had previously been opened too-early to do the same.
Note: Remember that some variance is expected in the `m_assumed_*` sizes.
ACKs for top commit:
achow101:
ACK a2bef805c1
josibake:
ACK a2bef805c1
gruve-p:
ACK a2bef805c1
dergoegge:
ACK a2bef805c1 on the new mainnet params
Tree-SHA512: 0b19c2ef15c6b15863d6a560a1053ee223057c7bfb617ffd3400b1734cee8f75bc6fd7f04d8f8e3f5af6220659a1987951a1b36945d6fe17d06972004fd62610
Avoid adding transactions below min relay feerate because, even if they
were bumped through CPFP when entering the mempool, we do not have a
DoS-resistant way of ensuring they always remain bumped. In the future,
this rule can be relaxed (e.g. to allow packages to bump 0-fee
transactions) if we find a way to do so.
7ccdd741fe test: fix importmulti/importdescriptors assertion (Jon Atack)
19d888ce40 rpc: move WALLET_FLAG_CAVEATS to the compilation unit of its caller (Jon Atack)
01df011ca2 doc: release note for wallet RPCs "warning" field deprecation (Jon Atack)
9ea8b3739a test: createwallet "warning" field deprecation test (Jon Atack)
645d7f75ac rpc: deprecate "warning" field in {create,load,unload,restore}wallet (Jon Atack)
2f4a926e95 test: add test coverage for "warnings" field in createwallet (Jon Atack)
4a1e479ca6 rpc: add "warnings" field to RPCs {create,load,unload,restore}wallet (Jon Atack)
079d8cdda8 rpc: extract wallet "warnings" fields to a util helper (Jon Atack)
f73782a903 doc: fix/improve warning helps in {create,load,unload,restore}wallet (Jon Atack)
Pull request description:
Based on discussion and concept ACKed in #27138, add a `warnings` field to RPCs createwallet, loadwallet, unloadwallet, and restorewallet as a JSON array of strings to replace the `warning` string field in these 4 RPCs. The idea is to more gracefully handle multiple warning messages and for consistency with other wallet RPCs. Then, deprecate the latter fields, which represent all the remaining RPC `warning` fields.
The first commit f73782a903 implements https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1474789198 as an alternative to #27138. One of those two could potentially be backported to our currently supported releases.
ACKs for top commit:
achow101:
ACK 7ccdd741fe
1440000bytes:
utACK 7ccdd741fe
vasild:
ACK 7ccdd741fe
pinheadmz:
re-ACK 7ccdd741fe
Tree-SHA512: 314e0a4c41fa383d95e2817bfacf359d449e460529d235c3eb902851e2f4eacbabe646d9a5a4beabc4964cdfabf6397ed8301366a58d344a2f787f83b75e9d64
6b17994ede doc: update OpenBSD build docs for 7.3 (external signer support available) (Sebastian Falbesoner)
Pull request description:
With OpenBSD 7.3, the waitid(2) system call is implemented (see 8112871f19, first mentioned kernel improvement at https://www.openbsd.org/73.html).
This means Boost.Process finally doesn't fail to compile anymore and we can remove the build hint about missing external signer support. Tested on my amd64 machine by reconfiguring / rebuilding master branch and successfully running the functional test wallet_signer.py. ✔️
ACKs for top commit:
fanquake:
ACK 6b17994ede - haven't tested, but looks good to me.
Tree-SHA512: 5bbcecce4ced38d8221f2c906a54667e50317e9ded182554cf73bb7f2fce55a38e53730eca25f813cff1d2d65c94141eb158d40f83228d12dcf859c16a1798b9
With OpenBSD 7.3, the waitid(2) system call is implemented (see
8112871f19).
This means Boost.Process finally doesn't fail to compile anymore and we
can remove the build hint about missing external signer support. Tested
on my amd64 machine by reconfiguring / rebuilding master branch and
successfully running the functional test wallet_signer.py.
a12d9cfa46 doc: correct sqlite & qrencode versions used in depenendencies.md (fanquake)
Pull request description:
Followup to https://github.com/bitcoin/bitcoin/pull/27312 & https://github.com/bitcoin/bitcoin/pull/25378.
ACKs for top commit:
achow101:
ACK a12d9cfa46
hebasto:
ACK a12d9cfa46, I have reviewed the code and it looks OK, I agree it can be merged.
jarolrod:
ACK a12d9cfa46
Tree-SHA512: 29e1fe4c31089fce6acbadb14aa7619fdd55738a882b490f1a0835d7648798a68b4f0d62e213c60d92f8e021ea856a4d1759578da07413265fef2338840da506
499c464394 doc: update DataDirectoryGroupReadable 1 in tor.md (Jesse Barton)
Pull request description:
Updating tor.md doc to include mention of FreeBSD requiring the DataDirectoryGroupReadable be set to 1.
Default per the FreeBSD man page is 0.
DataDirectoryGroupReadable 0|1
If this option is set to 0, don't allow the filesystem groupto
readthe DataDirectory. If the option is setto 1, make the
DataDirectory readable by the default GID. (Default:0)
ACKs for top commit:
vasild:
ACK 499c464394
Tree-SHA512: 8750b49cd04e900435c7991d1a24641fd1171227c1f14ed59afb157f24c1ca60380d30aecfb174ca46fd5b4b99dcdb3a1cfd019aafc343362e8103abf7c17e6a
Move DataDirectoryGroupReadable 1 up a few lines to more clearly
communicate that it is required for the filesystem group to read the
DataDirectory.
Per the Tor documentation
https://2019.www.torproject.org/docs/tor-manual.html.en#DataDirectoryGroupReadable
"If this option is set to 0, don’t allow the filesystem group to read
the DataDirectory. If the option is set to 1, make the DataDirectory
readable by the default GID. (Default: 0)"
This makes it easier to handle descriptor strings manually. E.g. an RPC call that takes an array of descriptors can now use '["desc": ".../0h/..."]'.
Both markers can still be parsed. The default for new descriptors is changed to h. In normalized form h is also used. For private keys the chosen marker is preserved in a round trip.
The hdkeypath field in getaddressinfo is also impacted by this change.
fabb95e7bf doc: add release note for 26899 (brunoerg)
c84c5f6e89 p2p: set `-dnsseed` and `-listen` false if `maxconnections=0` (brunoerg)
Pull request description:
If `maxconnections=0`, it means our possible connections are going to be manual (e.g via `addnode`). For this reason, we can skip DNS seeds and set `listen` false.
ACKs for top commit:
achow101:
ACK fabb95e7bf
vasild:
ACK fabb95e7bf
1440000bytes:
reACK fabb95e7bf
Tree-SHA512: 33919a784723a32450f39ee4f6de3e27cc7c7f4c6ab4b8ce673981d461df334197deaf43e3f882039fa1ac36b2fddc6c6ab4413512d6c393d4a6865302dd05e7
4b7aec2951 Add mempool tracepoints (virtu)
Pull request description:
This PR adds multiple mempool tracepoints.
| tracepoint | description |
| ------------- | ------------- |
| `mempool:added` | Is called when a transaction enters the mempool |
| `mempool:removed` | ... when a transaction is removed from the mempool |
| `mempool:replaced` | ... when a transaction is replaced in the mempool |
| `mempool:rejected` | ... when a transaction is rejected from entering the mempool |
The tracepoints are further documented in `docs/tracing.md`. Usage is demonstrated in the example script `contrib/tracing/mempool_monitor.py`. Interface tests are provided in `test/functional/interface_usdt_mempool.py`.
The rationale for passing the removal reason as a string instead of numerically is that the benefits of not having to maintain a redundant enum-string mapping seem to outweigh the small cost of string generation. The reject reason is passed as string as well, although in this instance the string does not have to be generated but is readily available.
ACKs for top commit:
0xB10C:
ACK 4b7aec2951
achow101:
ACK 4b7aec2951
Tree-SHA512: 6deb3ba2d1a061292fb9b0f885f7a5c4d11b109b838102d8a8f4828cd68f5cd03fa3fc64adc6fdf54a08a1eaccce261b0aa90c2b8c33cd5fd3828c8f74978958
Tracepoints for added, removed, replaced, and rejected transactions.
The removal reason is passed as string instead of a numeric value, since
the benefits of not having to maintain a redundant enum-string mapping
seem to outweigh the small cost of string generation. The reject reason
is passed as string as well, although here the string does not have to
be generated but is readily available.
So far, tracepoint PRs typically included two demo scripts: a naive
bpftrace script to show raw tracepoint data and a bcc script for a more
refined view. However, as some of the ongoing changes to bpftrace
introduce a certain degree of unreliability (running some of the
existing bpftrace scripts was not possible with standard kernels and
bpftrace packages on latest stable Ubuntu, Debian, and NixOS), this PR
includes only a single bcc script that fuses the functionality of former
bpftrace and bcc scripts.
1ff5d61dfd doc: add mempool/contents rest verbose and mempool_sequence args (Andrew Toth)
52a31dccc9 tests: mempool/contents verbose and mempool_sequence query params tests (Andrew Toth)
a518fff0f2 rest: add verbose and mempool_sequence query params for mempool/contents (Andrew Toth)
Pull request description:
The verbose mempool json response can get very large. This adds an option to return the non-verbose response of just the txids. It is identical to the rpc response so the diff here is minimal. This also adds the mempool_sequence parameter for rpc consistency. Verbose defaults to true to remain backwards compatible.
It uses query parameters to be compatible with the efforts in https://github.com/bitcoin/bitcoin/issues/25752.
ACKs for top commit:
achow101:
ACK 1ff5d61dfd
stickies-v:
re-ACK [1ff5d61](1ff5d61dfd)
pablomartin4btc:
tested ACK 1ff5d61dfd.
Tree-SHA512: 1bf08a7ffde2e7db14dc746e421feedf17d84c4b3f1141e79e36feb6014811dfde80e1d8dbc476c15ff705de2d3c967b3081dcd80536d76b7edf888f1a92e9d1
da347de530 doc: update broken links (pablomartin4btc)
Pull request description:
References to `utilstrencodings` and `lint-locale-dependence.sh` where incorrect, updating them accordingly.
Also, adding another reference to util function [`LocaleIndependentAtoi`](https://github.com/bitcoin/bitcoin/blob/master/src/util/strencodings.h#L108-L118), which is related with the updated section of the guide:
```
// LocaleIndependentAtoi is provided for backwards compatibility reasons.
//
// New code should use ToIntegral or the ParseInt* functions
// which provide parse error feedback.
//
// The goal of LocaleIndependentAtoi is to replicate the defined behaviour of
// std::atoi as it behaves under the "C" locale, and remove some undefined
// behavior. If the parsed value is bigger than the integer type's maximum
// value, or smaller than the integer type's minimum value, std::atoi has
// undefined behavior, while this function returns the maximum or minimum
// values, respectively.
```
ACKs for top commit:
MarcoFalke:
lgtm ACK da347de530
Tree-SHA512: c8f4cd9cff1fb3ea367ac9dbe5aa45dc187fc60114f2e2106e02e0e17fea4ee34d6e0c408fe920c2d8765e06b4dc30c231f0454fa35469c4399e0cadbcd341ba
54c4d03578 doc: Show how less noisy clang-tidy output can be achieved (TheCharlatan)
Pull request description:
Adds a paragraph to the clang-tidy section explaining how to de-noise its output. By default clang-tidy will print errors arrising from included headers in leveldb and other dependencies. By passing `--enable-suppress-external-warnings` flag to configure, errors arising from external dependencies are suppressed. Additional errors arrising from internal dependencies such as leveldb are suppressed by passing the `src/.bear-tidy-config` configuration file to bear. This file includes exclusionary rules for leveldb.
ACKs for top commit:
MarcoFalke:
utACK 54c4d03578
Tree-SHA512: c3dd8fb0600157582a38365a587e02e1d249fb246d6b8b4949a800fd05d3473dee49e2a4a556c60e51d6508feff810024e55fe09f5a0875f560fde30f3b6817c
fixes#20246
Document both JSON-RPC endpoints, when they are active and which types
of requests they are able to service.
Adds two example curl requests, one for each endpoint.
b082f28101 rpc, wallet: use the same `next_index` in listdescriptors and importdescriptors (w0xlt)
Pull request description:
Currently `listdescriptors` RPC uses `next` key to represent `WalletDescriptor::next_index` while `importdescriptors` uses `next_index`. This creates two different descriptor formats.
This PR changes `listdescriptors` to use the same key as `importdescriptors`.
ACKs for top commit:
achow101:
ACK b082f28101
aureleoules:
reACK b082f28101
Tree-SHA512: c29ec59051878e614d749ed6dc85e5c14ad00db0e8fcbce3f5066d1aae85ef07ca70f02920299e48d191b7387024fe224b0054c4191a5951cb805106f7b8e37b
2b373fe49d docs: update assumeutxo.md (James O'Beirne)
87a1108c81 test: add snapshot completion unittests (James O'Beirne)
d70919a88f refactor: make MempoolMutex() public (James O'Beirne)
7300ced9de log: add LoadBlockIndex() message for assumedvalid blocks (James O'Beirne)
d96c59cc5c validation: add ChainMan logic for completing UTXO snapshot validation (James O'Beirne)
f2a4f3376f move-only-ish: init: factor out chainstate initialization (James O'Beirne)
637a90b973 add Chainstate::HasCoinsViews() (James O'Beirne)
c29f26b47b validation: add CChainState::m_disabled and ChainMan::isUsable (James O'Beirne)
5ee22cdafd add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}() (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606)
Part two of replacing https://github.com/bitcoin/bitcoin/pull/24232.
---
When a user activates a snapshot, the serialized UTXO set data is used to create an "assumed-valid" chainstate, which becomes active in an attempt to get the node to network tip as quickly as possible. Simultaneously in the background, the already-existing chainstate continues "conventional" IBD to both accumulate full block data and serve as a belt-and-suspenders to validate the assumed-valid chainstate.
Once the background chainstate's tip reaches the base block of the snapshot used, we set `m_stop_use` on that chainstate and immediately take the hash of its UTXO set; we verify that this matches the assumeutxo value in the source code. Note that while we ultimately want to remove this background chainstate, we don't do so until the following initialization process, when we again check the UTXO set hash of the background chainstate, and if it continues to match, we remove the (now unnecessary) background chainstate, and move the (previously) assumed-valid chainstate into its place. We then reinitialize the chainstate in the normal way.
As noted in previous comments, we could do the filesystem operations "inline" immediately when the background validation completes, but that's basically just an optimization that saves disk space until the next restart. It didn't strike me as worth the risk of moving chainstate data around on disk during runtime of the node, though maybe my concerns are overblown.
The final result of this completion process is a fully-validated chain, where the only evidence that the user synced using assumeutxo is the existence of a `base_blockhash` file in the `chainstate` directory.
ACKs for top commit:
achow101:
ACK 2b373fe49d
Tree-SHA512: a204e1d6e6932dd83c799af3606b01a9faf893f04e9ee1a36d63f2f1ccfa9118bdc1c107d86976aa0312814267e6a42074bf3e2bf1dead4b2513efc6d955e13d
7013da07fb Add release note for PR#25943 (David Gumberg)
04f270b435 Add test for unspendable transactions and parameter 'maxburnamount' to sendrawtransaction. (David Gumberg)
Pull request description:
This PR adds a user configurable, zero by default parameter — `maxburnamount` — to `sendrawtransaction`. This PR makes bitcoin core reject transactions that contain unspendable outputs which exceed `maxburnamount`. closes#25899.
As a result of this PR, `sendrawtransaction` will by default block 3 kinds of transactions:
1. Those that begin with `OP_RETURN` - (datacarriers)
2. Those whose lengths exceed the script limit.
3. Those that contain invalid opcodes.
The user is able to configure a `maxburnamount` that will override this check and allow a user to send a potentially unspendable output into the mempool.
I see two legitimate use cases for this override:
1. Users that deliberately use `OP_RETURN` for datacarrier transactions that embed data into the blockchain.
2. Users that refuse to update, or are unable to update their bitcoin core client would be able to make use of new opcodes that their client doesn't know about.
ACKs for top commit:
glozow:
reACK 7013da07fb
achow101:
re-ACK 7013da07fb
Tree-SHA512: f786a796fb71a587d30313c96717fdf47e1106ab4ee0c16d713695e6c31ed6f6732dff6cbc91ca9841d66232166eb058f96028028e75c1507324426309ee4525
0af16e7134 doc: add release note for #25574 (Martin Zumsande)
57ef2a4812 validation: report if pruning prevents completion of verification (Martin Zumsande)
0c7785bb25 init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache (Martin Zumsande)
d6f781f1cf validation: return VerifyDBResult::INTERRUPTED if verification was interrupted (Martin Zumsande)
6360b5302d validation: Change return value of VerifyDB to enum type (Martin Zumsande)
Pull request description:
`VerifyDB()` can fail to complete due to insufficient dbcache at the level 3 checks. This PR improves the error handling in this case in the following ways:
- The rpc `-verifychain` now returns false if the check can't be completed due to insufficient cache
- During init, we only log a warning if the default values for `-checkblocks` and `-checklevel` are taken and the check doesn't complete. However, if the user actively specifies one of these args, we return with an InitError if we can't complete the check.
This PR also changes `-verifychain` RPC to return `false` if the verification didn't finish due to missing block data (pruning) or due to being interrupted by the node being shutdown.
Previously, this PR also included a fix for a possible assert during verification - this was done in #27009 (now merged).
ACKs for top commit:
achow101:
ACK 0af16e7134
ryanofsky:
Code review ACK 0af16e7134. Only small suggested changes since the last review, like renaming some of the enum values. I did leave more suggestions, but they are not very important and could be followups
john-moffett:
ACK 0af16e7134
MarcoFalke:
lgtm re-ACK 0af16e7134 🎚
Tree-SHA512: 84b4f767cf9bfbafef362312757c9bf765b41ae3977f4ece840e40c52a2266b1457832df0cdf70440be0aac2168d9b58fc817238630b0b6812f3836ca950bc0e
and remove m_snapshot_validated. This state can now be inferred by the
number of isUsable chainstates.
m_disabled is used to signal that a chainstate should no longer be used
by validation logic; it is used as a sentinel when background validation
completes or if the snapshot chainstate is found to be invalid.
isUsable is a convenience method that incorporates m_disabled.
Constructs like
```cpp
AssertLockNotHeld(m);
LOCK(m);
```
are equivalent to
```cpp
LOCK(m);
```
for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in
such cases.
d5d4b75840 guix: combine glibc hardening options into hardened-glibc (fanquake)
c49f2b8eb5 guix: remove no-longer needed powerpc workaround (fanquake)
74c9893989 guix: use glibc 2.27 for all Linux builds (fanquake)
Pull request description:
Build against glibc 2.27 for all Linux builds (previously only used for RISC-V), and at the same time, increase our minimum required glibc to 2.27 (2018). This would drop support for Ubuntu Xenial (16.04) & Debian Stretch (9), from the produced release binaries. Compiling from source on those systems may be possible, assuming you can install a recent enough compiler/toolchain etc.
ACKs for top commit:
hebasto:
ACK d5d4b75840, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 910f0ef45b4558f2a45d35a5c1c39aaac97e8aff086dc4fc1eddbb80c0b6e4bd23667d64e21d0fd42e4db37b6f26f447ca5d1150bb861128af7e71fb42835cf8
c572eae989 update the freebsd build doc to reflect recent changes to DB4 install process (Murray Nesbitt)
Pull request description:
This PR introduces documentation changes needed to keep up with #26834.
ACKs for top commit:
fanquake:
ACK c572eae989 - have not tested, but looks ok.
Tree-SHA512: 42a79e7b45834916b1b738db524b51b9ff4fde8348ba66fc331ff6603532dd9fce73ea392eef97d31112326c6d60ec2c5c7c29e66aab33aaf846aab8aea1d1aa
b49e19ccd9 doc: use arch agnostic clang path in fuzzing doc (macOS) (fanquake)
Pull request description:
The current path will only work for clang installed via brew on x86_64 macOS.
ACKs for top commit:
hebasto:
ACK b49e19ccd9, similar to 702836530f.
Tree-SHA512: 8ae4845e1953d5a7178f2b422e2241af1057d8cce1ab79da65df0cd068456dbf85da3489355f81fc4ee09ba602a4b53e989e2dc02476b4abf6c5b3bc3e96473b
75347236f2 docs: document c-style cast prohibition (Pasta)
Pull request description:
In the words of practicalswift:
```
A C-style cast is equivalent to try casting in the following order:
const_cast(...)
static_cast(...)
const_cast(static_cast(...))
reinterpret_cast(...)
const_cast(reinterpret_cast(...))
By using static_cast<T>(...) explicitly we avoid the possibility of an unintentional and
dangerous reinterpret_cast. Furthermore static_cast<T>(...) allows for easier grepping of casts.
For a more thorough discussion, see "ES.49: If you must use a cast, use a named cast"
in the C++ Core Guidelines (Stroustrup & Sutter).
```
Modern tooling, specifically `-Wold-style-cast` can enable us to enforce never using C-style casts. I believe this is especially important due to the number of C-style casts the codebase is currently being used as a reinterpret_cast. reinterpret_casts are especially dangerous, and should never be done via C-style casts.
Update the docs to suggest the use of named cast or functional casts.
Top commit has no ACKs.
Tree-SHA512: 29a98de396f0c78e32d8a1831319162203c4405a670da5add5da956fcc7df200a1cec162ef1cfac4ddfb02714b66406081d40ed435c7f0f28581cfa24d94fac1
44f3c7de21 contrib: remove install_db4.sh (fanquake)
14ce84388f doc: add new NO_* options from #26833 (fanquake)
Pull request description:
Now that we can build a bdb-only depends prefix (#26833), there is no need to
maintain a bdb-building bash script, that does the same thing as
depends, except worse, as it's missing patches and workarounds. i.e #26623.
Someone that wants to compile bdb themselves, but doesn't want to use other depends built libs, can do:
```bash
make -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1
...
to: /path/to/bitcoin/depends/x86_64-pc-linux-gnu
```
which gives them a BDB only prefix, and then compile using:
```bash
export BDB_PREFIX="/path/to/bitcoin/depends/x86_64-pc-linux-gnu"
./autogen.sh
./configure \
BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" \
BDB_CFLAGS="-I${BDB_PREFIX}/include"
```
Wondering if we should extract the build bdb/legacy wallet docs somewhere, to avoid the repetition?
ACKs for top commit:
TheCharlatan:
ACK 44f3c7de21
achow101:
ACK 44f3c7de21
hebasto:
ACK 44f3c7de21
jarolrod:
ACK 44f3c7de21
Tree-SHA512: 50b33ae9df2ab94a1bd114e846cec16f647a61023b72f0d3e547a18db09c01d60bb7b42a04758212f4930314df03016feb6ebc96962dd8a8e26eb8cd4e0d167d
8e85164e7d doc: release note on mempool size in -blocksonly (willcl-ark)
ae797463dc doc: Update blocksonly behaviour in reduce-memory (willcl-ark)
1134686ef9 mempool: Don't share mempool with dbcache in blocksonly (willcl-ark)
Pull request description:
Fixes#9526
When `-blocksonly` has been set reduce default mempool size to avoid surprising resource usage via sharing un-used mempool cache space with dbcache.
In comparison to https://github.com/bitcoin/bitcoin/pull/9569 which either set `maxmempool` size to 0 when `-blocksonly` was set or else errored on startup, this change will permit `maxmempool` options being set.
This preserves the current (surprising?) behaviour of having a functional mempool in `-blocksonly` mode, to permit whitelisted peer transaction relay, whilst reducing average runtime memory usage for blocksonly nodes which either use the default settings or have otherwise configured a `maxmempool` size.
To use the previous old defaults node operators can configure their node with: `-blocksonly -maxmempool=300`.
ACKs for top commit:
ajtowns:
ACK 8e85164e7d
stickies-v:
re-ACK 8e85164e7d
Tree-SHA512: 1c461c24b6f14ba02cfe4e2cde60dc629e47485db5701bca3003b8df79e3aa311c0c967979f6a1dca3ba69f5b1e45fa2db6ff83352fdf2d4349d5f8d120e740d
Changes to the default mempool allocation size now documented.
Provides users with guidance on the mempool implications of -blocksonly
mode, along with instructions on how to re-enable old behaviour.
d96d97ad30 doc: Add release note for shutdownnotify. (klementtan)
0bd73e2c45 util: Add -shutdownnotify option. (klementtan)
Pull request description:
**Description**: Similar to `-startupnotify`, this PR adds a new option to allow users to specify a command to be executed when Bitcoin Core shuts down.
**Note**: The `shutdownnotify` commands will not be executed if bitcoind shut down due to *unexpected* reasons (ie `killall -9 bitcoind`).
### Testing:
**Normal shutdown commands**
```
# start bitcoind with shutdownnotify optioin
./src/bitcoind -signet -shutdownnotify="touch foo.txt"
# shutdown bitcoind
./src/bitcoin-cli -signet stop
# check that foo.txt has been created
```
**Final RPC call**
Commands:
```
$ ./src/bitcoind -signet -nolisten -noconnect -shutdownnotify="./src/bitcoin-cli -signet getblockchaininfo > tmp.txt"
$ ./src/bitcoin-cli stop
$ cat tmp.txt
```
<details>
<summary>Screen Shot</summary>
</details>
ACKs for top commit:
achow101:
ACK d96d97ad30
1440000bytes:
ACK d96d97ad30
theStack:
re-ACK d96d97ad30
Tree-SHA512: 16f7406fd232e8b97aea5e58854c84755b0c35c88cb3ef9ee123b29a1475a376122b1e100da860cc336d4d657e6046a70e915fdb9b70c9fd097c6eef1b028161
Now that we can build a bdb-only depends prefix, there is no need to
maintain a bdb-building bash script, that does the same things as
depends, except worse, as it's missing patches and workarounds. i.e #26623.
fa8fe5b696 scripted-diff: Use new python 3.7 keywords (MarcoFalke)
fa2a23548a Revert "contrib: Fix capture_output in getcoins.py" (MarcoFalke)
dddd462137 Bump minimum python version to 3.7 (MarcoFalke)
Pull request description:
While there is nothing that requires a bump, it may require less maintenance to drop python3.6 support. Python3.7 is available through the package manager on all currently supported operating systems.
ACKs for top commit:
jamesob:
ACK fa8fe5b696
hebasto:
ACK fa8fe5b696
Tree-SHA512: f6e080d8751948bb0e01c87be601363158f345e8037b70ce7e1bc507c611eb61600e4f24f1d2f8a6e7e44877ab09319302869e33ce8118c4c4f71fc89c0a1198
376e01b382 doc: add databases/py-sqlite3 to FreeBSD test suite deps (fanquake)
Pull request description:
Adds missing documentation. See also https://cirrus-ci.com/task/5639240319500288.
ACKs for top commit:
john-moffett:
ACK 376e01b382
Tree-SHA512: a7b8d0bae00c3538934d23eae207b7927a64e748eb228ac6c5754aee0e9b6796c0f39066c7d81cc009bf442c8b1a0c31739adde87d1ebc3445aed54dda47c9ce
cfe5aebc79 rpc: add minconf and maxconf options to sendall (ishaanam)
a07a413466 Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund calls (Juan Pablo Civile)
Pull request description:
This PR adds a "minconf" option to `fundrawtransaction`, `walletcreatefundedpsbt`, and `sendall`.
Alternative implementation of #14641Fixes#14542
Edit: This PR now also adds this option to `send`
ACKs for top commit:
achow101:
ACK cfe5aebc79
Xekyo:
ACK cfe5aebc79
furszy:
diff ACK cfe5aebc, only a non-blocking nit.
Tree-SHA512: 836e610926eec3a62308fba88ddbd6a13d8f4dac37352d0309599f893cde9c1df5e9c298fda6e076493068e4d213e4afa7290a9e3bdb5a95a5d507da3f7b59e8
dc9bad5192 Change dots to an ellipsis and fix capitalization (John Moffett)
9b158ae73f Update to mention restoring wallet via GUI (John Moffett)
Pull request description:
f9783b0f07 Recently added the ability to restore wallets via the GUI, but the current wallet guide says backups must be restored via RPC.
ACKs for top commit:
kouloumos:
ACK dc9bad5192
jarolrod:
re-ACK dc9bad5
hebasto:
re-ACK dc9bad5192
Tree-SHA512: 325a0023ef10c75073b0288f69c99f01b029b0b7b64ae91e7ef72d4ab1fa4da60fe4cd1b4528c1c0d34617122d9aee3cd9cb32aef05a25493fc01e9ec2e6cc10
e6864fa157 contrib: remove builder keys (fanquake)
Pull request description:
This has been superseded by adding a builder-keys/ directory in
guix.sigs, where the presence of keys, and validity of signatures
is checked. Preventing issues like missing keys or invalid signatures.
New (or exisiting) Guix builders can add their key in the next PR
they open adding attestations.
Related to issues like #26566, #26563.
Also follows up with the comment here: https://github.com/bitcoin/bitcoin/pull/26565#issuecomment-1326053939.
ACKs for top commit:
hebasto:
ACK e6864fa157, modulo s/update/remove/ in the PR tittle.
Tree-SHA512: 095b4cf12ed0baeaf0ee7b8edcb3e2647e9c0f812e8fd63915ddb454f81dacc9c2d2b409de2773b7adb5ff643893d614d8aad1bc44c26da648e1bbbe19e11e05
b2aa9e8528 Add release note for MIN_STANDARD_TX_NONWITNESS_SIZE relaxation (Greg Sanders)
8c5b3646b5 Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes (Greg Sanders)
Pull request description:
Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled.
There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical.
Two changes could be accomplished:
1) Anything not 64 bytes could be allowed
2) Anything above 64 bytes could be allowed
In the Great Consensus Cleanup, suggestion (2)
was proposed as a consensus change, and is the simpler of the two suggestions. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5.
The functional test is also modified to test the actual case
we care about: 64 bytes
Related mailing list discussions here:
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/020995.html
And a couple years earlier:
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017883.html
ACKs for top commit:
achow101:
reACK b2aa9e8528
glozow:
reACK b2aa9e8528
pablomartin4btc:
re-ACK b2aa9e8528
jonatack:
ACK b2aa9e8528 with some suggestions
Tree-SHA512: c1ec1af9ddcf31b2272209a4f1ee0c5607399f8172e5a1dfd4604cf98bfb933810dd9369a5917ad122add003327c9fcf6ee26995de3aca41d5c42dba527991ad
This has been superseded by adding a builder-keys/ directory in
guix.sigs, where the presence of keys, and validity of signatures
is checked. Preventing issues like missing keys or invalid signatures.
New (or exisiting) Guix builders can add their key in the next PR
they open adding attestations.
8c3ff7d52a test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky)
d1ca563825 bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky)
6bd1d20b8c rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky)
e2c3b18e67 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky)
Pull request description:
Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones.
Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error.
After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones.
ACKs for top commit:
MarcoFalke:
review ACK 8c3ff7d52a 🗂
kristapsk:
ACK 8c3ff7d52a
stickies-v:
ACK 8c3ff7d52
Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
874c861885 doc: Add I2P bandwidth guidance to i2p.md (willcl-ark)
Pull request description:
Add some general guidance on lowering bandwidth usage when using I2P routers.
ACKs for top commit:
jonatack:
ACK 874c861885
hernanmarino:
ACK 874c861885
pablomartin4btc:
ACK 874c861885.
Tree-SHA512: 3990375b23c01a2ad8e15a51e1b1a0275df8747ecd789ddf1888fbc88c20cde5a3813615982669af5e8d021dc995f6c7b1f080167b33f48574d6f50fc4851498
Specifying same named parameter multiple times is still allowed by bitcoin-cli.
The client implementation overwrites earlier option values with later ones
before sending to server. This is tested by interface_bitcoin_cli.py
Rationale for allowing client parameters to be specified multiple times in
bitcoin-cli is that this behavior has been supported for a long time, and that
when using the command line interactively, it can be convenient to override
earlier option values with new values without having to go back and remove the
old value.
But for the RPC server, there isn't really a good use-case for earlier values
to be discarded if multiple values are specified. JSON keys are generally
supposed to be unique and if they aren't it's probably an indication of some
problem generating the RPC request.
d8b12a75db rpc: Allow named and positional arguments to be used together (Ryan Ofsky)
Pull request description:
It's nice to be able to use named options and positional arguments together.
Most shell tools accept both, and python functions combine options and arguments allowing them to be passed with even more flexibility. This change adds support for python's approach so as a motivating example:
```sh
bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1
```
Can be shortened to:
```sh
bitcoin-cli -named createwallet mywallet load_on_startup=1
```
JSON-RPC standard doesn't have a convention for passing named and positional parameters together, so this implementation makes one up and interprets any unused `"args"` named parameter as a positional parameter array.
This change is backwards compatible. It doesn't change the interpretation of any previously valid calls, just treats some previously invalid calls as valid.
Another use case even if you only occasionally use named arguments is that you can define an alias:
```
alias bcli='bitcoin-cli -named'
```
And now use both named named and unnamed arguments from the same alias without having to manually add `-named` option for named arguments or see annoying error "No '=' in named argument... this needs to be present for every argument (even if it is empty)`" for unnamed arguments
ACKs for top commit:
achow101:
ACK d8b12a75db
stickies-v:
re-ACK d8b12a75d
aureleoules:
re-ACK d8b12a75db
Tree-SHA512: 0cff8b50f584bcbbd376624adccf40536566ed8d1bcd6c88ad565dbc208f19d5e7a48c994efd6329d42b560149340d330397278f08a2912af5f3418d8c8837a9
fa84df1f03 scripted-diff: wallet: rename AvailableCoinsParams members to snake_case (furszy)
61c2265629 wallet: group AvailableCoins filtering parameters in a single struct (furszy)
f0f6a3577b RPC: listunspent, add "include immature coinbase" flag (furszy)
Pull request description:
Simple PR; adds a "include_immature_coinbase" flag to `listunspent` to include the immature coinbase UTXOs on the response. Requested by #25728.
ACKs for top commit:
danielabrozzoni:
reACK fa84df1f03
achow101:
ACK fa84df1f03
aureleoules:
reACK fa84df1f03
kouloumos:
reACK fa84df1f03
theStack:
Code-review ACK fa84df1f03
Tree-SHA512: 0f3544cb8cfd0378a5c74594480f78e9e919c6cfb73a83e0f3112f8a0132a9147cf846f999eab522cea9ef5bd3ffd60690ea2ca367dde457b0554d7f38aec792
It's nice to be able to use named options and positional arguments together.
Most shell tools accept both, and python functions combine options and
arguments allowing them to be passed with even more flexibility. This change
adds support for python's approach so as a motivating example:
bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1
Can be shortened to:
bitcoin-cli -named createwallet mywallet load_on_startup=1
JSON-RPC standard doesn't have a convention for passing named and positional
parameters together, so this implementation makes one up and interprets any
unused "args" named parameter as a positional parameter array.
