📝 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.
```

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1905371266)
76a95522b322c1a8ab1594ca8bf5ac99ab379c46: to keep log lines short, maybe say `(ex. coinbase)`. Maybe also move it left: `CreateNewBlock(): block weight (ex. coinbase): ...`

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1905367911)
6cc658075e1be59ef49a060b36921538acf0edb1 nit: current naming convention uses snake case. Maybe just use `max_block_weight`.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1905363548)
354f41f5c76f5f43cbfdddb63660cc77c4a76989: I think this should be `MAX_BLOCK_WEIGHT - coinbase_max_additional_weight`, because later on this fuzzer adds a small coinbase transaction.

(can probably just hardcode something like `- 4000`)

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1905381347)
76a95522b322c1a8ab1594ca8bf5ac99ab379c46 nit: `(excluding coinbase)`, since the description already says "transactions"