💬 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

💬 mzumsande commented on pull request "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates":
(https://github.com/bitcoin/bitcoin/pull/30479#issuecomment-2892651335)
Looks like there is a intermittent failure in the added test - I'll look into it soon.

💬 luke-jr commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2892721133)
>In general it doesn't make sense to mark virtual methods const, because parent classes can't know how subclasses will implement the virtual methods and what state they may access. The point of pure virtual methods is usually to declare abstract interfaces without leaking information about implementations or making assumptions about them.

The constness of methods is a property of the interface. It is a way to say calling it will not change the object, and thereby allows using const references
...

✅ maflcko closed a pull request: "doc: Fix gen-manpages to check build options"
(https://github.com/bitcoin/bitcoin/pull/29457)

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#issuecomment-2892971961)
thx, I pushed a commit similar to yours. (Generally, when it comes to the dev notes, I think it is clear that no one is reading them, because of all the stale references in it. We should probably work on reducing them to only contain the important stuff that is not already checked by the automated linters or not already covered by other docs)