💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2295237186)
Thanks for pointing this out, I've fixed this issue in https://github.com/bitcoin/bitcoin/pull/32636/commits/2826ccd38a56ef6f6a6dde0830c5e0ad3bd40ebb

💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2295238189)
Thanks for the suggestion, fixed in https://github.com/bitcoin/bitcoin/pull/32636/commits/b5de137824d266357dd34953a4dc506289acf83c

💬 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