Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301950715)
> It seems like this might also make it more difficult to use `bitcoin-cli` across versions. If `bitcoind` is a newer version with some RPC that has a new string parameter, it may not be possible to pass that new argument by name with a `bitcoin-cli` from an older version if the RPC has an entry in this table.

This is true, and we know a similar issue already happens if a new JSON parameter is added: old clients will incorrectly pass that parameter as a string instead of a JSON value. This PR
...
💬 kevkevinpal commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3225484378)
ACK [2885bd0](https://github.com/bitcoin/bitcoin/pull/33224/commits/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442)

Makes sense to have consistent working with the release note
💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3225542636)
> I think it is not good to have separate tables for string and json parameters.

I would also prefer that.
💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r2302086009)
For a non-P2SH transaction, returning` -1` will result in a `redeemScript` field being added.
I tested this using a P2PKH transaction, and this was the output: `"redeemScript": { "asm": "304402...7981", "type": "nonstandard" }"`
💬 achow101 commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3225666624)
ACK dd7e696762b4863a5f06d150c4fe3fb882ac69dc
🤔 mzumsande reviewed a pull request: "test: Fix CLI_MAX_ARG_SIZE issues"
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3157283355)
> * It claims to be a "limit per arg", but it is a "The maximum length of the arguments": See https://www.man7.org/linux/man-pages/man3/sysconf.3.html, section `ARG_MAX - _SC_ARG_MAX`.

the relevant bottleneck for me on Ubuntu was not `ARG_MAX`, but `MAX_ARG_STRLEN`, which accounts for the maximum, not the sum.
e.g. the folllowing passes
```
arg1=$(printf "%0*d" $((131072 - 1)) 0)
arg2=$(printf "%0*d" $((131072 - 1)) 0)
$(which printf) "%s %s" "$arg1" "$arg2" >/dev/null
```
whereas

...
🤔 mzumsande reviewed a pull request: "[29.x] backport #33212"
(https://github.com/bitcoin/bitcoin/pull/33251#pullrequestreview-3157306080)
Code Review ACK fcac8022d839572f5d8781096eec14ca7ea2e0dd
💬 kristapsk commented on issue "RPC wont bind without an IP address on a non-localhost interface":
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-3225810421)
Noticed this when there is only IPv6 address assigned to eth0 interface (lo has both 127.0.0.1 and ::1). When IPv4 address is also added to eth0, problem disappears.

```
Bitcoin Core version v29.0.0 (release build)
Using the 'arm_shani(1way,2way)' SHA256 implementation
Startup time: 2025-08-26T21:30:01Z
Default data directory /.bitcoin
Using data directory /ssd/bitcoind/mainnet
Config file: /home/bitcoind/mainnet.conf
Config file arg: chain="main"
Config file arg: datadir="/ssd/bitcoind/mainnet
...
💬 achow101 commented on issue "integer sanitizer warning, when running with -natpmp=1 enabled":
(https://github.com/bitcoin/bitcoin/issues/33245#issuecomment-3225829239)
It looks like this is being triggered by a `NLMSG_DONE` message and is something that we could avoid by checking `hdr->nlmsg_type` before doing `NLMSG_DATA` and `RTM_PAYLOAD`. This could be included in #32159 since that's already touching this code.
💬 achow101 commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2302222710)
These could be moved after `if (hdr->nlmsg_type == NLMSG_DONE) {` to avoid the integer sanitizer warning in #33245
📝 polespinasa opened a pull request: "rpc, logging: add backgroundvalidation to getblockchaininfo"
(https://github.com/bitcoin/bitcoin/pull/33259)
`getblockchaininfo` returns `verificationprogress=1` and `initialblockdownload=false` even if there's background validation.
This PR adds information about background validation to rpc `getblockchaininfo` in a similar way to `validationprogress` does.

If assume utxo was used the output of a "sync" node performing background validation:
```
$ ./build/bin/bitcoin-cli getblockchaininfo
...
"mediantime": 1756245019,
"verificationprogress": 1,
"initialblockdownload": false,
"backgr
...
💬 achow101 commented on issue "tracing: test `interface_usdt_net.py` fails due to garbage message type in `net:outbound_message` tracepoint":
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3226064329)
I'm able to reproduce this with a release build, but not a debug build.
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302109933)
we don't actually need the wallet here, right?
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302150941)
we want deterministic randomness for benchmarks, otherwise it's hard to know what we're actually measuring:
```suggestion
FastRandomContext rand{/*fDeterministic=*/true};
```
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302152030)
we could reserve this to avoid the build failure:
```C++
std::vector<CAmount> targets;
targets.reserve(10);
for (size_t i{0}; i < targets.capacity(); ++i) {
targets.push_back(rand_range(rng, 0.1_btc, 1_btc));
}
```
Note that I have reduced the target count to 10 since the benchmark was very slow otherwise
🤔 l0rinc requested changes to a pull request: "bench: Add more realistic Coin Selection Bench"
(https://github.com/bitcoin/bitcoin/pull/33160#pullrequestreview-3157264436)
Concept ACK, the existing test were all quite trivial compared to this new one.
I left a ton of comments, for simplicity here's the final code that I'm suggesting, feel free to pick and choose from it.

<details>
<summary>Details</summary>

```C++
// Copyright (c) 2012-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <bench/bench.h>
#include <consensus/
...
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302156688)
I'd avoid the `Narrow No-Break Space` chars from docs if possible, they're rendered differently on different mediums
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302160133)
Same, seems simpler to just leave it to deterministic instead of hard-coding a confusing seed
```suggestion
FastRandomContext det_rand{/*fDeterministic=*/true};
```

note: can we reuse this for the random source below as well?
💬 l0rinc commented on pull request "bench: Add more realistic Coin Selection Bench":
(https://github.com/bitcoin/bitcoin/pull/33160#discussion_r2302166150)
we could dedup considerable here for better signal-to-noise ratio - it's a lot of work to find the differences between the values:
```C++
const auto txout{wtx->tx->vout.at(0)};
const int input_bytes{CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr)};
const COutput output{(COutPoint{wtx->GetHash(), 0}), txout, /*depth=*/6 * 24, input_bytes, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/0};
if (int y{rng.randrange(100)}; y < 35) {

...