👍 luke-jr approved a pull request: "doc: Fix incorrect send RPC docs"
(https://github.com/bitcoin/bitcoin/pull/31416#pullrequestreview-2538844451)
tACK fad83e759a4

📝 luke-jr opened a pull request: "tracing: Rename the `MIN` macro to `_TRACEPOINT_TEST_MIN` in log_raw_p2p_msgs"
(https://github.com/bitcoin/bitcoin/pull/31623)
Inspired by: 00c1dbd26ddb816e5541c5724397015a92a3d06b (#31419)

Unless there's a reason we *don't* want the same change here...?

💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2579545560)
Rebased due to the conflict with the merged bitcoin/bitcoin#31542.

💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1908409725)
Tried compiling a `static_assert` with `mib` replaced by `-1`. Got the expected result from the sign bit:
```
/home/hodlinator/bitcoin/src/bitcoind.cpp:164:63: error: static assertion failed
164 | static_assert(std::countl_zero(static_cast<uint64_t>(-1)) >= 21);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
/home/hodlinator/bitcoin/src/bitcoind.cpp:164:63: note: the comparison reduces to ‘(0 >= 21)’
```

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#issuecomment-2579621858)
Here we go!

I generated an alternate mainnet with 2016 blocks, storing timestamps and nonces. The process is explained at the top of the renamed test file: ee1dcc6074323ae361e28cf235ddfc9bf07be1da.

I did leave out some minutia for readability sake, and because things will change over time. E.g. I had to disable the `miner.isTestChain())` guard in `getblocktemplate`, I patched the CPU miner to work with taproot payout addresses https://github.com/pooler/cpuminer/pull/293 and set the coinba
...

👍 dergoegge approved a pull request: "Safegcd-based modular inverses in MuHash3072"
(https://github.com/bitcoin/bitcoin/pull/21590#pullrequestreview-2539393251)
tACK ad67fd2e0bfa6f43f350066596b6cca146391362

Only minor changes since my [last review](https://github.com/bitcoin/bitcoin/pull/21590#pullrequestreview-2104888365).

💬 l0rinc commented on pull request "init,log: Unify block index log line":
(https://github.com/bitcoin/bitcoin/pull/31616#issuecomment-2579651695)
@maflcko do you mean removing these logs in the first place since they're redundant?

💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1908477798)
Did you hard-code the -1 literal? I don't think that holds for a variable `mib` at runtime?

💬 l0rinc commented on pull request "RPC: Fix invalid txid in `gettransaction` example":
(https://github.com/bitcoin/bitcoin/pull/31610#issuecomment-2579657065)
> An old txid will only work with txindex enabled, though

This is also true for the new one - please refer to the example in the PR's description.

@luke-jr, @jonatack, do you have any concerns about the current approach?

💬 theStack commented on pull request "test: raise explicit error if any of the needed release binaries is missing":
(https://github.com/bitcoin/bitcoin/pull/31462#discussion_r1908483101)
Will leave the PR as-is, but happy to review a follow-up PR which refines test/README.md about previous releases and/or adds improvements to the previous release download script itself.

> Last thing is that "At least one release" is not entirely correct, we need the last previous to the current, if you delete the v.28.0 from the releases folder (as specified in the https://github.com/bitcoin/bitcoin/pull/31462#issue-2730936530 of the PR) and you still have v25.0 and/ or v24.0.1, you will get
...

💬 l0rinc commented on pull request "refactor: modernize recent `ByteType` usages and read/write functions":
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2579687987)
> And doing that all over would be a recipe for never-ending churn

Isn't that what we're doing regularly though? Endlessly making the legacy codebase a tiny bit more readable, a tiny bit more modern, a tiny bit more maintainable and consistent?

> I'd rather not be doing random opinionated changes like this.

Previously, I've applied the following changes (as mentioned in the commit message), but I agree that a few of these could be reverted as being indeed opinionated:
- Replaced `unsig
...

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908512682)
Good point, maybe add:

```
The previous `-blockmaxweight` default was `3,996,000`. The block template construction code added an additional `4,000` padding. The new default `-maxcoinbaseweight` value was chosen to maintain that behaviour.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908514307)
Correct.

💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1908520794)
Yes, I hard-coded the literal into the `static_assert` above. What part of integer arithmetic and casting changes when done in a runtime context?

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908522924)
Current `ClampOptions` has a comment that specifically permits `-maxcoinbaseweight` to exceed `-blockmaxweight`. It just means that all blocks will be empty. This seems like an unrealistic scenario anyway.

💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1908525582)
Even if the assert is correct, maybe the fact that this thread exists is justification enough for replacing it with something closer to what it was originally. Like https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1907618229

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908533084)
I agree we should not change the `weight` and `sigops` fields in `getblocktemplate` and `getmininginfo`.

We could either adjust the value in the RPC code, or as @luke-jr suggests restore the original behavior here.

Additionally we should clarify the help text for these RPC methods that they include the coinbase reserved weight.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908538059)
Perhaps `-coinbaseweightreservation` is a better name.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908542626)
Seems reasonable to keep and just make it `DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT};`

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1908547490)
Sorry, I think @luke-jr is right and we shouldn't change this: https://github.com/bitcoin/bitcoin/pull/31384/files#r1908533084