💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2235578787)
Yeah agreed, to be left as follow-up if others agree.

📝 maflcko opened a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075)
This spans over several pulls, so add a single note for all of them.

💬 fanquake commented on pull request "doc/zmq: fix unix socket path example":
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126383385)
Backported to 29.x in #33074.

📝 fanquake opened a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33076)
Backports:
* #33001
* #33070

💬 fanquake commented on pull request "doc/zmq: fix unix socket path example":
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126392391)
Backported to `28.x` in #33076.

💬 fanquake commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#issuecomment-3126393675)
Backported to `28.x` in #33076

💬 fanquake commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/33064#issuecomment-3126398686)
> Question: would it make sense to also test "abortrescan" during an active rescan to hit the True path and ensure the scan halts as expected?

Feel free to open a new PR, adding additional test changes.

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719)
> meganit: This newline was logically separating the two commented code blocks and should probably be brought back.
Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)

> nit: It would be good to hyphen-prefix to make it stand out visually (from fields like vsize etc) and also call it "setting" or "argument" rather than "parameter" as those are the terms used in the code.
Fixed as suggested

> nit: I think the end is a bit
...

📝 hebasto opened a pull request: "kernel: create monolithic kernel static library"
(https://github.com/bitcoin/bitcoin/pull/33077)
Currently, consuming `libbitcoinkernel.a` requires all its dependency static libraries to be available. A switch to a monolithic variant, which contains object files from its dependencies, was discussed in the Kernel WG. The necessary preparations in the libsecp256k1 build scripts were completed in https://github.com/bitcoin-core/secp256k1/pull/1678, which are now available in this repository since https://github.com/bitcoin/bitcoin/pull/33036.

The changes in this PR were picked from https://
...

✅ fanquake closed an issue: "test: RPC coverage check doesn't work?"
(https://github.com/bitcoin/bitcoin/issues/27593)

🚀 fanquake merged a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/33064)

👍 rkrux approved a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075#pullrequestreview-3061782410)
ACK fa21a90c3558c8414aafe0e5b68d8b9590cca127

💬 maflcko commented on pull request "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`":
(https://github.com/bitcoin/bitcoin/pull/33065#issuecomment-3126557564)
review ACK 251d02084688c67523e9ec92ec79ee657454ab93 🌨

<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 251d02084688
...

💬 maflcko commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3126594440)
I think the question boils down to: How much UX do we want to provide for data corruption or internal bugs? A valid answer could be: None. In that case you can just use `Assert` to ensure no data corruption or internal bugs happen. (If they do, the program terminates, like with other Asserts that unexpectedly hit in production.) This approach will likely use the least amount of code.

Other valid answers are: Best-effort (use `throw`, and maybe catch it, or not). Full-effort (use `Result`).

👍 rkrux approved a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3061836848)
crACK 251d02084688c67523e9ec92ec79ee657454ab93

Nit: can update the grep command in the PR description.

💬 maflcko commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#issuecomment-3126623052)
lgtm, but I haven't reviewed the hd-split refactor. I'd say it would be good to add a test for this, using a previous release to create an ancient wallet, and then checking that the migration works as expected?

review ACK 60d1042b9a4db8daf9fffdc29053652e99b7126e 🐾

<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 RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5Ak
...

💬 fkorotkov commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126627675)
Hey folks,

Could someone with admin permissions please check that `Allow public repositories` checkbox is set on this page:

https://github.com/organizations/bitcoiin/settings/actions/runner-groups/1

Seems this is the reason runners are not picking jobs at the moment.

💬 fanquake commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126631857)
@fkorotkov. Should be fixed now:

<img width="554" height="111" alt="a" src="https://github.com/user-attachments/assets/9f517086-fd25-4261-b356-8fab387ebf12" />

💬 TheCharlatan commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3126634149)
Tested it out over here: https://github.com/TheCharlatan/rust-bitcoinkernel/tree/monolithic_lib

💬 b-l-u-e commented on pull request "[p2p] Fix signed integer overflow in LocalServiceInfo::nScore":
(https://github.com/bitcoin/bitcoin/pull/33072#issuecomment-3126635012)
@gmaxwell Thank you for the excellent feedback!
The saturation is a much better approach than changing to int64_t.
I've implemented the saturation fix as you suggested using if (nScore < std::numeric_limits<int>::max()) nScore++.
This prevents undefined behavior while maintaining memory efficiency and logical correctness after 2 billion connections, the matter is indeed settled.
I've also added a test case to verify the saturation behavior works correctly.
Thank you for pointing me in t
...