💬 darosior commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2096457284)
I adapted to the surrounding code. For `-dbcache` we trunk so i did the same. For `-maxmempool` we return an error so i did the same.

💬 darosior commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2096457403)
I disagree this is a more descriptive bitness check. The size of pointers is what we actually care about here. Shouldn't matter in practice but i don't even think it's strictly guaranteed that `SIZE_MAX == sizeof(void*)`. Finally, this is the exact same check we do elsewhere in this codebase to check for 32-bit systems. For these reasons, i kept it as-is.

Made `constexpr` as requested, though.

💬 mzumsande commented on pull request "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates":
(https://github.com/bitcoin/bitcoin/pull/30479#discussion_r2096471698)
Good idea - done!

👍 ryanofsky approved a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2840680200)
Code review ACK c759ec1256a5cdc02b6eef1f4c0935c693bb8087. Since last review just rebased, applied some suggested changes to use BlockValidationState, and split the new python test into functions.

💬 ryanofsky commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2089235016)
In commit "test: more template verification tests" (c759ec1256a5cdc02b6eef1f4c0935c693bb8087)

Maybe add comment `# solve block_2` if that would clarify dependency between pow_test and later tests.

(Alternately could change `pow_test` to return a different variable like `block_2_solved`)

💬 ryanofsky commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2096473337)
In commit "Add checkBlock to Mining interface" (116a9d2daced606e5f35896372da39fd914d3da1)

Just a thought, but it doesn't seem to me that the changes to rpc/mining.cpp here and below in getblocktemplate are really improvements. They are making code more verbose and changing behavior in ways that don't necessarily seem better (producing an error message with a trailing " - " if debug string is empty here, adding a new LogDebug statement below with only the debug string, not the reason string).
...

🤔 ismaelsadeeq reviewed a pull request: "multiprocess: add bitcoin-mine test program"
(https://github.com/bitcoin/bitcoin/pull/30437#pullrequestreview-2851969416)
Concept ACK

This can serve as a usage example for the interfaces.

I've rebased this on `master`: https://github.com/ismaelsadeeq/bitcoin/tree/05-2025-bitcoin-mine-rebase and did a few tests.

1 - Ran `bitcoin-mine` without a `bitcoin-node` process — it executed correctly and showed an appropriate error message.
<details>
<summary>logs</summary>

```terminal
2025-05-19T21:00:18Z Default data directory /Users/abubakarismail/Library/Application Support/Bitcoin
2025-05-19T21:00:1
...

💬 ismaelsadeeq commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2096479030)
In " multiprocess: Add bitcoin-mine test program " 8e31982d5a940f04a815ec235cb55c0dc2f870d7

I think it could be helpful to make the name a bit more generic, since the program demonstrates IPC usage rather than actual mining. Maybe something like bitcoin-interfaces-client or interfaces-client?

💬 ismaelsadeeq commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2096468681)
In " multiprocess: Add bitcoin-mine test program " 8e31982d5a940f04a815ec235cb55c0dc2f870d7

I think this commit should be split into two, the first that introduces `src/init/bitcoin-mine.cpp` and then next commit should introduce `src/bitcoin-mine.cpp`

💬 fjahr commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#issuecomment-2892383304)
utACK 33332829333b589420f8038541d04ec6970f051d

I think I found two leftover mentions of the `ParseInt` in documentation: https://github.com/fjahr/bitcoin/commit/1f2f1a432b0fdb3d84eecc9dc3a2db34d88dc892 I can option a follow-up if you don't want to address it here.

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2096549655)
Mmm, this is kinda awkward either way. It'd be better imo to pass the actual `CCheckQueueControl&` as an argument to `CheckInputScripts`, but that makes this much more complicated for a bunch of reasons.

I pushed an additional commit to try to make this a little easier to read. Please let me know what you think. If it's over-complicating what's otherwise a pretty trivial PR, I can remove it.

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2096550581)
Done. I wasn't sure if `__LINE__` would work correctly when called from another macro, but it seems to do the right thing :)

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2892405627)
Updated to resolve @ryanofsky's comments. I added a new comment to (hopefully) clarify the `CheckInputScripts` calling.

💬 jasonribble commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2096561613)
What's the benefit of doing it this way? /curi

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2096569684)
> What's the benefit of doing it this way? /curi

By checking if the last block verified is the one matching the last best header known, we avoid previous blocks inside the 2h offset return `verificationprogress=1`.

💬 1440000bytes commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2892444152)
Configured 2 tor proxies for ipv4 and ipv6 and they are working as expected:

```
$ curl --socks5-hostname 127.0.0.1:9050 http://ipv4.icanhazip.com
45.90.185.109

$ curl --socks5-hostname [::1]:9053 http://ipv6.icanhazip.com
2605:6400:30:f174:1:1:1:1
```

Used these proxies to run `bitcoind`:

```
bitcoind -signet -proxy=127.0.0.1:9050=ipv4 -proxy=[::1]:9053=ipv6
```

I see the proxies in `getnetworkinfo` and some outbound peers in `getpeerinfo`:

```
{
"name": "ipv4
...

🤔 1440000bytes reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2852127113)
ACK https://github.com/bitcoin/bitcoin/pull/32425/commits/e98c51fcce9ae3f441a416cab32a5c85756c6c64

<details>
<summary>Signature</summary>

<pre>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK e98c51fcce9ae3f441a416cab32a5c85756c6c64
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaCuzGQAKCRAtIwUgISpZ
AVt3AP98Co6T1/S4RqaIWypoL4NI1B1PhTPdRdFdvPP0sHWxIgD/SlBxLKNBIehf
tvLSnyIV9cvCBRefydIJCTf+dS06tAw=
=D/ae
-----END PGP SIGNATURE-----
</pre>
</detai
...

💬 jasonribble commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2096614639)
Good moves. I like ternary.

💬 jasonribble commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2096615053)
But doesn't the current code already solve the problem?

💬 jasonribble commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2892521248)
cACK

💬 achow101 commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#discussion_r2096675994)
Done as suggested