💬 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)

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

No, maflcko proposal returns `verificationprogress=1` when the last verified block is within a 2h offset of our local time.

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2097134143)
> lie to users

Again, this can happen on current master already. Also, it can happen with your suggested diff. The only reliable way to not rely on the exact value of remotely miner-set timestamps would be to not use them, but that is a more involved patch.

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2097152746)
> Also, it can happen with your suggested diff

Oh, you're right if the miner sets the timestamp to a future one. Maybe we could just limit the maximum value to something like `0.999...` until we verified the last block. That's hard coding a bit the values, but as it's only for a greater UX I don't see much problem doing that.

💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2097179087)
It seems Python doesn't believe in `const`, so I ended up just documenting the test and its call site.

💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2097200882)
I dropped the changes to `rpc/mining.cpp` and explained why they're untouched in the commit message. Indeed `checkBlock` is already covered by the unit tests, and the meat and potatoes is in `TestBlockValidity`, which both IPC and RPC call.