💬 maflcko commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3216370455)
> Task `ARM, unit tests, no functional tests`: https://github.com/bitcoin/bitcoin/runs/48709103843
> LLM reason (✨ experimental): The CI failure is caused by a test timeout during the execution of the 'mptest' test.

Unrelated, but this failure looks real? The unit test should normally pass in a few milliseconds, so taking 40 minutes seems odd?

https://cirrus-ci.com/task/5714850606743552?logs=ci#L2822: `[23:07:33.279] 3/148 Test #3: mptest ............................... Passed 0.
...

💬 waketraindev commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2295255521)
Added to changeset

💬 waketraindev commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2295273936)
RPCArg doesn't support '|' in names and will throw a rpc error when it gets called. https://github.com/bitcoin/bitcoin/blob/73220fc0f958f9b65f66cf0cf042af220b312fc6/src/rpc/util.cpp#L923

💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#issuecomment-3216443821)
@achow101
> `PopulateWalletFromDB` inside of `CreateNew` should no longer be necessary.

@pablomartin4btc
> DBErrors WalletBatch::LoadWallet(CWallet* pwallet) is being called during wallet creation I think.

Thanks for catching this, fixed in https://github.com/bitcoin/bitcoin/pull/32636/commits/acc0d3b7d4e8b26e1dc5832497b2b47047b6a453.

> It'd be nice if the load or create intention could be propagated down to the DB level as well. In `SQLiteDatabase::Open`, we pass the flags `SQLITE
...

💬 davidgumberg commented on pull request "wallet, refactor: Remove Legacy check and error":
(https://github.com/bitcoin/bitcoin/pull/33082#issuecomment-3216464378)
crACK https://github.com/bitcoin/bitcoin/pull/33082/commits/d3c5e47391e2f158001e3e199d625852c7f18998

Removes unuseful legacy wallet code, also suggested here: https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2226917021

The suggestion in the PR description won't work without https://github.com/bitcoin/bitcoin/pull/33082/commits/30c6f64eed304560464f9601b80c811c186db20a, if this gets merged first I'll add it to #32636.

💬 maflcko commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3216477238)
review ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763 📌

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 46ca7712cb5f
...

📝 233bjr opened a pull request: "Bot: empty commit"
(https://github.com/bitcoin/bitcoin/pull/33242)

💬 yuvicc commented on issue "Bitcoin Kernel Library Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-3216531189)
> I’d be happy to contribute or test bindings for additional environments such as JavaScript, Kotlin, or Dart.

I think you can start off with javascript language bindings. See the C API header PR -> https://github.com/bitcoin/bitcoin/pull/30595

💬 fjahr commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3216589996)
Code review ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763

💬 TheCharlatan commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3216596839)
> failure looks real? The unit test should normally pass in a few milliseconds, so taking 40 minutes seems odd?

I reproduced this by repeatedly calling `mptest`. Took about 50'000 tries until it hung indefinitely.

📝 maflcko opened a pull request: "2508 test fix cli cmd size"
(https://github.com/bitcoin/bitcoin/pull/33243)
`CLI_MAX_ARG_SIZE` has many edge case issues:

* It seems to be lower on some systems, but it is unknown how to reproduce locally: https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3139957274
* 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`.
* It doesn't account for the additional args added by the `bitcoin` command later on: https://github.com/bitcoin
...

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3216611350)
> > > I wonder if it can be fixed by replacing the hard-coded
> >
> >
> > I can't seem to recreate this locally, so going to follow up.
>
> Same here, I can't reproduce, even on the same machine.

https://github.com/bitcoin/bitcoin/pull/33243 should work around it enough to be able to enable it, hopefully.

⚠️ maflcko opened an issue: "intermittent timeout in mptest unit test"
(https://github.com/bitcoin/bitcoin/issues/33244)
> Task `ARM, unit tests, no functional tests`: https://github.com/bitcoin/bitcoin/runs/48709103843
> LLM reason (✨ experimental): The CI failure is caused by a test timeout during the execution of the 'mptest' test.

this failure looks real? The unit test should normally pass in a few milliseconds, so taking 40 minutes seems odd?

https://cirrus-ci.com/task/5714850606743552?logs=ci#L2822: `[23:07:33.279] 3/148 Test #3: mptest ............................... Passed 0.03 sec`
https:
...

⚠️ maflcko opened an issue: "integer sanitizer warning, when running with -natpmp=1 enabled"
(https://github.com/bitcoin/bitcoin/issues/33245)
I haven't looked here if this is a bug, but it would be good to either suppress this warning (if it is harmless) or change the code (if there is a bug).

To reproduce, compile with `clang` and enable the `integer` sanitizer.

Then, disable ipv6:

```
# sysctl -w net.ipv6.conf.all.disable_ipv6=1
net.ipv6.conf.all.disable_ipv6 = 1
```

Then, run `bitcoind`:

`# UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"
...

🤔 furszy reviewed a pull request: "threading: remove ancient CRITICAL_SECTION macros"
(https://github.com/bitcoin/bitcoin/pull/32592#pullrequestreview-3147579126)
ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763

⚠️ theStack opened an 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)
The functional test `interface_usdt_net.py` currently fails on master (commit 73220fc0f958f9b65f66cf0cf042af220b312fc6) on my aarch64 machine, running Ubuntu 25.04:
```
$ cmake -B build -DWITH_USDT=ON
$ cmake --build build -j12
# ./build/test/functional/interface_usdt_net.py
2025-08-23T10:53:12.137229Z TestFramework (INFO): PRNG seed is: 2587673157417500984
2025-08-23T10:53:12.137976Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_iem787yt
2025-08-23T10:53:12.415409Z Te
...

💬 theStack commented on issue "tracing: issue running `contrib/tracing/log_utxos.bt`":
(https://github.com/bitcoin/bitcoin/issues/33227#issuecomment-3216760599)
Couldn't reproduce this issue on my aarch64 machine (`Linux 6.16.0-21-qcom-x1e #21-Ubuntu SMP PREEMPT_DYNAMIC Sun Aug 10 00:54:48 UTC 2025 aarch64`), running Ubuntu 25.04, systemtap 5.1-4.1 and bpftrace v0.21.2.

🤔 fjahr reviewed a pull request: "index: Don't commit state in BaseIndex::Rewind"
(https://github.com/bitcoin/bitcoin/pull/33212#pullrequestreview-3147906824)
Post merge ACK a602f6fb7bf5f9e57299f4d6e246c82379fad8d2

💬 fjahr commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#discussion_r2295881585)
nit: Would have been nice to give a why (e.g. something like "This ensures they are never ahead of the actual chain on restart since this would be identified as index corruption upon restart") and maybe it could also have mentioned that this is a general issue for all indexes, not coinstats-specific.

💬 sipa commented on pull request "refactor: Use typesafe Wtxid in compact block encodings":
(https://github.com/bitcoin/bitcoin/pull/29752#issuecomment-3216827263)
FWIW, having the wtxids in the vector directly here was intentional, to minimize pointer indirection and cache effects during compact block reconstruction.

It may be worth benchmarking to see if this didn't inadvertently introduced a performance regression.