💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2574685525)
Rebased after https://github.com/bitcoin/bitcoin/pull/31581 landed. I also cherry-picked the latest relevant commits from #31583.

@luke-jr I dropped DATUM from the description, since there's still no spec it's hard to understand what it actually does.

The `checkblock` RPC is indeed almost the same as `getblocktemplate` in `proposal` mode. The main difference is that it can check (reduced) proof-of-work. My long term goal for this new method is to make it more powerful by e.g. adding new tr
...

👍 hodlinator approved a pull request: "qa: Improve framework.generate* enforcement (#31403 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/31599#pullrequestreview-2533848399)
re-ACK 1b51616f2e3b58a1c63a19e6dba8e7e9c2aefdeb

```
₿ git range-diff master a3f5573^ 1b51616
...
26: b3100b02c5 = 40: 1b51616f2e test: improve rogue calls in mining functions
```
(Second commit dropped due to typo being fixed in master).

Thanks for adjusting the PR title!

🤔 BrandonOdiwuor reviewed a pull request: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541#pullrequestreview-2533858451)
Concept ACK

💬 laanwj commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2574862196)
> Why? While we are currently relying on the short python repr() algorithm for floats, this has been stable and reliable since it's been introduced in python 3.1 as far as I know

Yes, the thing is, `repr(float)` depends on implementation details and settings of the FPU hardware, isn't guaranteed to be stable across operating systems, C libraries, and CPU architectures. As we see here.

📝 vasild opened a pull request: "test: expect that files may disappear from /proc/PID/fd/"
(https://github.com/bitcoin/bitcoin/pull/31614)
`get_socket_inodes()` calls `os.listdir()` and then iterates on the results using `os.readlink()`. However a file may disappear from the directory after `os.listdir()` and before `os.readlink()` resulting in a `FileNotFoundError` exception.

It is expected that this may happen for `bitcoind` which is running and could open or close files or sockets at any time. Thus ignore the `FileNotFoundError` exception.

<!--
*** Please remove the following help text before submitting: ***

Pull reque
...

👍 dergoegge approved a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2534002155)
Code review ACK a96b84cb1b76e65a639e62f0224f534f89858c18

💬 vasild commented on pull request "test: expect that files may disappear from /proc/PID/fd/":
(https://github.com/bitcoin/bitcoin/pull/31614#issuecomment-2574971127)
A failure like this observed in https://github.com/bitcoin/bitcoin/pull/31349 in CI job https://github.com/bitcoin/bitcoin/actions/runs/12633511389/job/35241370470?pr=31349:

```
2025-01-07T09:20:07.629000Z TestFramework (INFO): Bind test for ['[::1]']
2025-01-07T09:20:08.155000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/home/runner/work/_temp/test/functional/test_framework/test_framework.py", line 135, in main
self.r
...

💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2575001138)
The weak block test was broken, because regtest uses the almost highest possible target. Using a multiplier would overflow it. I dropped 180ce81182e840f01e6e9534872ebf92647c2e39 and adjusted the tests. I also dropped b61f8cca13320088a83aadef5f6af66375342fae and b74e145bc46e6dbf7d1496f078ff22cf80d8c92a I don't need them for the test.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2575019631)
The current CI failure is unrelated to this PR and is fixed separately in https://github.com/bitcoin/bitcoin/pull/31614. It will probably pass if the CI task is restarted because it is a race.

> 14:47:32.023429 IP6 1111:1111::4.33262 > 1111:1111::1.53: 9493+ A? debuginfod.fedoraproject.org. (46)

@fanquake so some piece of the command:

```
DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest --stop-on-failure "${MAKEJOB
...

👍 hodlinator approved a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2533921553)
ACK 815467f46f47df6ff52686b0144430c14a31f4a8

Amazed by how frictionless it was to switch from `shared_ptr` -> `unique_ptr`. `unique_ptr` still is an extra level of unnecessary indirection compared to my 80fca845b5f28677207a8fea4a173baaef23036f, but the latter is a much more disruptive change. **Should probably fix remaining mention of `shared_ptr`, see inline comment.**

Thanks for deleting the copy-ctor & assignment operator [as I (imprecisely) suggested](https://github.com/bitcoin/bitcoin
...

💬 hodlinator commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1905297433)
Still mentions `shared_ptr`.

💬 hodlinator commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1905219580)
nit: Could be more helpful?
```suggestion
// Delete copy constructor and assignment operator, use Clone() instead
```

🤔 brunoerg reviewed a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2534104622)
crACK a96b84cb1b76e65a639e62f0224f534f89858c18

👍 hodlinator approved a pull request: "kernel: Move kernel-related cache constants to kernel cache"
(https://github.com/bitcoin/bitcoin/pull/31483#pullrequestreview-2534107645)
crACK 54bdaaefbc683a61f3051dd90bbaf69de5d6cac5

Thanks again for incorporating my suggestions. Preserving previous behavior for negative `-dbcache` values seems good.

3 nits remaining:

54bdaaefbc683a61f3051dd90bbaf69de5d6cac5 still has "not"-typo in commit message:
> Add a release not for the change in behaviour on 32-bit systems.

💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1905322706)
nit: If you re-touch, slightly more correct:
```suggestion
Assert(std::countl_zero(static_cast<uint64_t>(mib)) >= 21); // Ensure sign bit is unset + enough zeros to shift.
```

💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1905324826)
nit: `<limits>` should be added in same commit as `<bit>` and `MiBToBytes()`.

📝 Eunovo opened a pull request: "Ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615)
assumevalid is not always used during reindex because the chainwork of the best header might be less than the minimumchainwork if the previous IBD was interrupted before it could connect blocks up to minimumchainwork.
See Issue https://github.com/bitcoin/bitcoin/issues/31494

This does not occur during normal IBD because the node does not connect blocks before minchainwork headers.

💬 luke-jr commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2575161578)
What would use the "weak block" check that doesn't need the ability to do it independently from proposals anyway?

While the existing GBT BIPs are Final, the protocol was designed to allow for extensions. If there's a use case for this feature, doing it that way seems fine

🤔 Sjors reviewed a pull request: "mining: bugfix: Fix duplicate coinbase tx weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384#pullrequestreview-2533761935)
Code reviewed ededdd13f3263df08f7878aa2514d3796436cbfb, almost there.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1905127347)
ededdd13f3263df08f7878aa2514d3796436cbfb: maybe replace this line with:

```md
This specifies how many weight units to reserve for
the coinbase transaction.

Upgrade note: the default of `-maxcoinbaseweight`
ensures backward compatibility for users who did
not override `-blockmaxweight`. If you previously
configured `-blockmaxweight=4000000` then you can
safely set `-maxcoinbaseweight=4000` to maintain
existing behavior.
```