💬 hebasto commented on issue "CMake `CheckPIESupported` doesn't always work":
(https://github.com/bitcoin/bitcoin/issues/30771#issuecomment-2501164856)
The upstream issue has been [fixed](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/10034) for CMake 3.32.

💬 jonatack commented on issue "Prioritize processing of peers based on their CPU usage":
(https://github.com/bitcoin/bitcoin/issues/31033#issuecomment-2501239124)
Concept ACK on potentially adjusting peers based on their resource usage, though the devil may be in the details. If I'm not misremembering, this has been brought up as an idea worth exploring since several years.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858826260)
Yep, I'm planning to drop it

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858826625)
I'm planning to simulate both cases and report back

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858828041)
@naumenkogs can this be resolved?

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858829100)
Can this be resolved?

🤔 darosior reviewed a pull request: "Testnet4 including PoW difficulty adjustment fix"
(https://github.com/bitcoin/bitcoin/pull/29775#pullrequestreview-2462021059)
One comment on the timewarp fix. It's OK in this specific case to have the check in `ContextualCheckBlockHeader()` since this is a new network and a previous version could not conceivably have let in an invalid header. However for a patch to an existing network, we should consider something akin to what Matt did in #15482 for the 64 bytes txs check. See also this comment in `ConnectBlock()`:
https://github.com/bitcoin/bitcoin/blob/f7144b24be09e7db2173a0b15d73f99a08b98118/src/validation.cpp#L243
...

💬 hebasto commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2501271136)
The upstream [fix](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/10034) has been mentioned in the comment.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858866659)
That commit is from an old revision

📝 furszy opened a pull request: "wallet: fix crash during watch-only wallet migration"
(https://github.com/bitcoin/bitcoin/pull/31374)
The crash occurs because we assume the cached scripts structure will not be empty,
but it can be empty for watch-only wallets that start blank.

Testing Notes:
This can be verified by cherry-picking and running the test commit on master.
It will crash on there but pass on this branch.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2501448936)
@achow101 ready for merge?

💬 yancyribbens commented on pull request "Add coin-grinder example test":
(https://github.com/bitcoin/bitcoin/pull/31352#issuecomment-2501552639)
> If you simply are reproducing the example code, so I tend to concept NACK on adding this to the test, the example is only an example.

That's fair. I wanted a way to follow the example code to trace values during various assignments and conditions. I figured since I wrote the test it might be useful to add, although if not, I can still use it and we can close the PR.

📝 ryanofsky opened a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375)
Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don't cause confusion.

Idea and implementation of this were discussed in https://github.com/bitcoin/bitcoin/issues/30983

💬 achow101 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1859024481)
`blockhash` should be omitted too.

📝 darosior opened a pull request: "Miner: never create a template which exploits the timewarp bug"
(https://github.com/bitcoin/bitcoin/pull/31376)
This check was introduced in #30681 but only enabled for testnet4. To avoid potentially creating an invalid block template if a soft fork to fix the timewarp attack were to activate in the future, we should have this check on all networks. It also seems wise for our miner to not support it whether or not a soft fork activates to fix it at the consensus level.

💬 achow101 commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2501632607)
ACK b031b7910d62819443813b91705211c466933a53

💬 TheCharlatan commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2501642097)
Concept ACK

🚀 achow101 merged a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)

💬 achow101 commented on pull request "interpreter: Use the same type for SignatureHash in the definition":
(https://github.com/bitcoin/bitcoin/pull/31365#issuecomment-2501660248)
ACK c288c790cd9abe91e53164aba5d975ef1e26ee3f

💬 achow101 commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1859064957)
In c32104f5449bbea89a3fde5b99c97f7de6914829 "gen-manpages: Update default build path"

The documentation states that this script can work from any directory for in-tree builds, but this change would break that (although we do not currently do in-tree builds). It further states that `BUILDDIR` should be set for out-of-tree builds. Either the documentation should be updated, or this default should not be changed. I prefer the latter.