938312d7a6 docs: clarify RPC credentials security boundary (crStiv)
Pull request description:
Explicitly states that RPC credentials grant full administrative access to the node and filesystem resources accessible by bitcoind. Adds a new section in JSON-RPC-interface.md to address issue https://github.com/bitcoin/bitcoin/issues/32274 by documenting that providing RPC credentials to untrusted clients
reopened#32424
P.S. I've tried to somehow squash all the commits from the previous pr but accidentally closed the pr and had no idea how to return back, therefore created a new pr, I'm really sorry for the inconvenience
ACKs for top commit:
achow101:
ACK 938312d7a6
janb84:
re ACK 938312d7a6
sedited:
ACK 938312d7a6
Tree-SHA512: 54db0651cfe4a92d700d09c822db5cb68f60f17a4634eb8f132939294e7a0ca2aea15ddc4d581610976158f7546e9c4463cfe113de9500162a0f107e168833cd
2104282ddd fuzz: Add tests for CCoinControl methods (Chandra Pratap)
43b09b993d fuzz: Improve oracle for existing CCoinControl tests (Chandra Pratap)
Pull request description:
The `ccoincontrol` fuzzer misses tests for a number of `CCoinControl` operations. Add them.
While at it, improve the oracle for the existing tests.
ACKs for top commit:
l0rinc:
Lightly tested code review ACK 2104282ddd
brunoerg:
reACK 2104282ddd
sedited:
Re-ACK 2104282ddd
Tree-SHA512: bfc8c9a51fca94437332056c476840d841a5b42dd6749cb34105b7ae78215ec9c3eb0f407e1a5f51b3ac20d7abb97cae7c21ad2146d5be9409edbc2cd2c568ee
Add btck_transaction_check() to the libbitcoinkernel C API, exposing
context-free transaction consensus validation (consensus/tx_check.h).
Introduces btck_TxValidationState with introspection and lifecycle
functions. btck_TxValidationResult is exposed for compatibility with
existing validation-state APIs, though btck_transaction_check currently
reaches only UNSET and CONSENSUS.
Includes C++ wrapper and test coverage for btck_transaction_check using
test vectors from tx_valid.json / tx_invalid.json.
1950da94fc test: enable `rpc_bind` on macOS and BSD (Lőrinc)
7236a05503 test: enable `feature_bind_extra` on macOS and BSD (Lőrinc)
Pull request description:
### Problem
Some functional tests are shown as skipped when running on macOS & BSD because `test_framework/netutil.py` only implemented the Linux-specific logic for checking which TCP sockets a node is listening on.
### Fix
Add macOS and BSD implementations in `test/functional/test_framework/netutil.py` so tests can query:
* which TCP sockets a node is listening on (`get_bind_addrs()`, via `lsof`)
* a non-loopback interface address (`all_interfaces()`, via `ifconfig`)
Then enable the previously Linux-only tests by switching to a shared POSIX platform guard.
### Commands
<details>
<summary><code>get_bind_addrs()</code> (<code>lsof</code> + regex)</summary>
> Command used
```bash
lsof -nP -a -p <pid> -iTCP -sTCP:LISTEN -Ftn
```
> Flags
- -D: device cache warnings
- -n: no hostname resolution
- -P: no service/port-name resolution
- -a: AND all conditions
- -p <pid>: filter by process ID
- -iTCP: TCP sockets only
- -sTCP:LISTEN: listening sockets only
- -Ftn: machine-readable output (fields: type `t`, name `n`)
> Regex parser
```regex
t(IPv[46])\nn(\*|\[.+?]|[^:]+):(\d+)
```
> Captured groups
- group 1: IPv4 / IPv6 (used to disambiguate `*`)
- group 2: host (`*`, `[::1]`, `127.0.0.1`, ...)
- group 3: port
</details>
<details>
<summary><code>all_interfaces()</code> (<code>ifconfig</code> + regex)</summary>
> Command used
```bash
ifconfig -au
```
> Regex parsing
Interface blocks:
```regex
(?m)^(?P<iface>\S+):(?P<block>[^\n]*(?:\n[ \t]+[^\n]*)*)
```
IPv4 extraction within each block:
```regex
inet (\S+)
```
</details>
### Notes
The only remaining platform skips on macOS are the USDT/BPF tracing tests (`interface_usdt_*.py`).
ACKs for top commit:
Sjors:
ACK 1950da94fc
achow101:
ACK 1950da94fc
willcl-ark:
tACK 1950da94fc
Tree-SHA512: 4cecc88852623f3fe3a7dccceb0e71932824c1ed7f1d4ab89b953ff6b7991afbd0b016c819c17e966bed53082dd623a832752b8847711861009cd5ffc4677367
1aa78cdab6 clusterlin: adopt STL ranges algorithms (refactor) (Pieter Wuille)
747da25360 feefrac: drop comparison and operator{<<,>>} for sorted wrappers (Pieter Wuille)
Pull request description:
Instead of having an unintuitive but strong implicit sort order on `FeeFrac` (first increasing feerate, then decreasing size), and separate overloaded `operator<<` and `operator>>` that implement a weak ordering that only looks at feerate, replace these with explicit wrapper classes which make the behavior more explicit (`ByRatio` and `ByRatioNegSize`).
This allows for things like `ByRatio{a} <= ByRatio{b}`, instead of the earlier `!(a >> b)`. It also supports usage inside `std::min`/`std::max`/`std::less`, and `std::greater`, so one can use:
* `std::max<ByRatioNegSize<FeeFrac>>(a, b)` to get the highest-feerate `FeeFrac`, tie-breaking by smallest size.
* `std::ranges::sort(v, std::greater<ByRatioNegSize<FeeFrac>>{});` to sort a vector that way.
Suggested in https://github.com/bitcoin/bitcoin/pull/34257#discussion_r2780475893.
ACKs for top commit:
achow101:
ACK 1aa78cdab6
sedited:
ACK 1aa78cdab6
ajtowns:
ACK 1aa78cdab6
Tree-SHA512: d76657b15f6d745e5ca01c67fd5b101fdc418e6301646d14e575b6564bfa2fe0eb40a95a7ff95a4420624ef6b67224d35e4713aa5bbc0d293e08fe44c0cc6db0
This change includes `s/RecursiveMutex/Mutex/` and a pile of
annotations to keep the compiler happy after the type change.
Partially resolves: https://github.com/bitcoin/bitcoin/issues/19303
The only recursive usage of `CConnman::m_nodes_mutex` is from
`PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs()` which uses
nested calls to `CConnman::ForNode()` to trim the size of
`lNodesAnnouncingHeaderAndIDs` to `<= 3`. This need not be nested, so
take it out.
Before:
```
fornode(newnode)
if (size >= 3)
fornode(front) handle removal of front
pop front
push back newnode
```
After:
```
fornode(newnode)
push back newnode
if (size > 3)
fornode(front) handle removal of front
pop front
```
`lNodesAnnouncingHeaderAndIDs` is protected by `cs_main` which is locked
during the entire operation.
08925d5ee7 test: add coverage for loading a wallet in a non-writable directory (furszy)
0218966c0d test: add coverage for wallet creation in non-writable directory (furszy)
bc0090f1d6 wallet: handle non-writable db directories (furszy)
Pull request description:
Make wallet creation and load fail with a clear error when the db directory isn’t writable.
#### 1) For Wallet Creation
Before: creating a wallet would return a generic error:
"SQLiteDatabase: Failed to open database: unable to open database file"
After: creating a wallet returns:
"SQLiteDatabase: Failed to open database in directory <dir_path>: directory is not writable"
#### 2) For Wallet Loading
We currently allow loading wallets located on non-writable directories. This is problematic
because the node crashes on any subsequent write; generating a block is enough to trigger it.
Can be verified just by running the following test on master: 85fa4e2910
Also, to check directory writability, this creates a tmp file rather than relying on the
`permissions()` functions, since perms bits alone may not reliably reflect actual writability
in some systems.
Testing Note:
Pushed the tests in separate commits so they can be cherry-picked on master for comparison.
ACKs for top commit:
rkrux:
re-ACK 08925d5ee7
achow101:
ACK 08925d5ee7
seduless:
Tested ACK 08925d5ee7
Tree-SHA512: e480eab329a1d595fe0b191e83c97956e3ff1d1e335ada8ac6fe72bc4b2bb9b13b0d49db0254d34ad75f816db06d9cd0c21d3063d7d8ee6687a7ea2324c36288
walletcreatefundedpsbt, createpsbt, converttopsbt, and psbtbumpfee take
a psbt_version argument to set the version of the PSBT that they
produce. The default psbt_version is 2.
The global unsigned tx is decomposed into separate fields inside of
PSBT, which mirrors what PSBTv2 will do. However, we still need to get
the global unsigned tx so PSBT::GetUnsignedTx is introduced to do that.
In order to also have a stable unique ID, we also introduce
PSBT::GetUniqueID to replace uses of PSBT.tx.GetHash().
DuplicateMockDatabase is no longer used. Furthermore, as SQLite gets
used more as a database and less as a key value store, this function
gets more complicated and more bug prone. As the benchmarks now run
equivalently quickly with a real database, retaining this duplication
function is no longer necessary.
PSBTInput now has the previous txid and output index, and PSBTOutput has
the amount and script. We no longer need to access the global unsigned
tx for these fields.
Additionally, we can change iterating tx.vin and tx.vout to psbtx.inputs
and psbtx.outputs.
This is in prepration for use with PSBTv2 where the global unsigned tx
will not exist.
When decomposing a transaction into a PSBTv2, the tx version and
locktime need to be stored in their respective global fields. Add those
fields and fill them when constructing.
Instead of allowing PSBTs to be default constructor, force usage of the
deserialization constructor.
CombinePSBTs, DecodeBase64PSBT, and DecodeRawPSBT are all changed to
return std::optional or util::result rather than using an output
parameter to avoid the need for a default constructor.
PSBTInput should be aware of the previous txid, output index, and
sequence numbers for inputs, extracting them from the global
unsigned tx.
PSBTOutput should be aware of the output amount and script, extracting
them from the global unsigned tx.
This prepares for PSBTv2 where these fields are serialized.
Every key has a duplicate key lookup check, and many keys have fixed
size checks. These can be refactored to reduce code duplication.
Co-Authored-By: David Gumberg <davidzgumberg@gmail.com>
