💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2234167334)
done

🤔 jonatack reviewed a pull request: "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline"
(https://github.com/bitcoin/bitcoin/pull/32279#pullrequestreview-3059739878)
Review ACK d5104cfbaeb82081e4b00a5084516555e446dcdc

💬 jonatack commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2234192567)
I don't mind re-reviewing if you take that approach.

📝 b-l-u-e opened a pull request: "[p2p] Fix signed integer overflow in LocalServiceInfo::nScore"
(https://github.com/bitcoin/bitcoin/pull/33072)
## Problem
The `nScore` field in `LocalServiceInfo` struct was defined as `int` (32-bit signed integer), which could overflow from `INT_MAX` (2,147,483,647) to `INT_MIN` (-2,147,483,648) when incremented by `SeenLocal()` during version handshakes. This is undefined behavior in C++ and could affect address selection logic.

## Solution
Change `nScore` from `int` to `int64_t` (64-bit signed integer) to provide a much larger range and prevent overflow in realistic scenarios.

## Changes
- **
...

💬 l0rinc commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-3124944751)
ACK 943ee6768d9f94da70aaf90d346d4cf2ea1fa39d

Thanks for adding and tweaking the test (checked, it's still passing locally as well)

💬 ajtowns commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-3125312465)
ACK 1c10b7351e194fc788766347f65f4512f61f05e8

💬 gmaxwell commented on pull request "[p2p] Fix signed integer overflow in LocalServiceInfo::nScore":
(https://github.com/bitcoin/bitcoin/pull/33072#issuecomment-3125357027)
This doesn't compile, and why not just saturate? e.g. if(nScore < std::max<int>)nScore++

"No performance impact (64-bit integers are efficient on modern systems)" sounds like LLM blather, the relevant criteria is not the performance of 64-bit integers considering how little it's used. Memory usage might matter if this was a per-peer (or per-block/header) field, but it isn't.

The purpose of it is to track input addresses to see that they were, in fact, reachable. I'm pretty sure that by
...

💬 maflcko commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r2234919472)
I don't think this is accurate. The limit is no longer per output (or possibly per mempool). The limit is now over the total size of all datacarrier output scripts in a single transaction in the mempool.

💬 maflcko commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r2234905470)
nit: Seems a bit odd to use `0` and `nullopt` interchangeably to mean the same (and require call sites to do the conversion manually). It would be easier to just flatten the optional and have a single dimension, directly translating two-way to the `-datacarriersize` option.

Diff:

```diff
diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h
index d57dbb393f..cf873dd48d 100644
--- a/src/kernel/mempool_options.h
+++ b/src/kernel/mempool_options.h
@@ -48,9 +48,8 @@ struct MemPo
...

💬 maflcko commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-3125846012)
> test_framework.authproxy.JSONRPCException: 'rescanblockchain' RPC took longer than 30.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)

You may want to try to increase the test_runner's timeout-factor locally. Increasing the timeout of this specific RPC call is also possible, but leads to a whack-a-mole in the long run.

💬 ajtowns commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r2235004536)
FWIW, that cleanup doesn't seem worthwhile for an option that's already deprecated to me

💬 ajtowns commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r2235006754)
It already uses the plural "outputs" though?

💬 maflcko commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-3125877362)
lgtm ACK 1c10b7351e194fc788766347f65f4512f61f05e8

📝 maflcko converted_to_draft a pull request: "ci: Run unit test parallel with functional tests"
(https://github.com/bitcoin/bitcoin/pull/33000)
Fixes https://github.com/bitcoin/bitcoin/issues/32770

💬 maflcko commented on pull request "ci: Run unit test parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3125926982)
The implementation can be adjusted, if needed. I guess the main questions are:

* Do people want to use this locally, if yes, then there could be a more user friendly way of calling it?
* Does it help with the new CI? With the old CI system planned to be replaced, it makes little sense to fine-tune it this late, so I'll turn this into a draft for now.

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#issuecomment-3126102054)
review ACK d5104cfbaeb82081e4b00a5084516555e446dcdc 🐺

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

💬 w0xlt commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3126116213)
I've created a C++ implementation of the dnssec-prover, available at: [https://github.com/w0xlt/dnssec-validation](https://github.com/w0xlt/dnssec-validation).
This implementation focuses solely on the DNSSEC **validation** component, which is the critical aspect for our use case.

The `dnssec-validation` binary can also be used as an external dependency in this PR. Functionally, it mirrors `dnssec-prover`, with the main difference being its C++ codebase.

If the goal is to eventually integ
...

💬 maflcko commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#issuecomment-3126119690)
> This PR improves mempool_accept_wtxid.py by:
>
> 1. Using a pre-mined chain instead of generating new blocks [522bf76](https://github.com/bitcoin/bitcoin/commit/522bf76d7d8058872a008a721831da264881746d)
>
> 2. Using MiniWallet to avoid RPC calls for signing transactions [4ec5ae9](https://github.com/bitcoin/bitcoin/commit/4ec5ae9fe126ffabcd429277092e3b27f483d430)
>
> 3. Removing child txid variables and using txid.hex directly [361ebd5](https://github.com/bitcoin/bitcoin/co
...

💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/33064#issuecomment-3126161329)
lgtm ACK 8aed477c3322212a636ab69d4923f89e2d9a63a2

💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-3126171593)
> FYI my build fails. (But this may have nothing to do with the migration to codeber.):

Yes, the change here is unrelated to the package in Guix.