💬 maflcko commented on pull request "doc: upgrade license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31611#issuecomment-2580086626)
lgtm ACK b537a2c02a9921235d1ecf8c3c7dc1836ec68131

Looks similar to what was done in commit 1f8450f066724dfbb5c5bc4060843e2f3340ed88 last year.

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2580086993)
Updated 941a194c944826049f823858cb15c41963e4619a -> 82706e217f34d1f09cbd30dde6b8ae5ac0385f0a ([kernel_cache_sizes_10](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_10) -> [kernel_cache_sizes_11](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_11), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_10..kernel_cache_sizes_11))

* Fix UBSan failure by avoiding undefined behaviour by casting to `size_t`.

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1908758174)
Are there places where this would be re-used? I'd be happy to add something more generic if there is demand for it.

💬 hebasto commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1908759779)
No, it doesn't, unfortunately.

NetBSD's [`sha256`](https://man.netbsd.org/sha256.1) is not quite compatible with our depends build syytem.

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760035)
Done

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760127)
Added it to the assert removal commits, thanks!

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760232)
Done

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760480)
I've updated the scripted diff to use consistent Read/Write terminology - dropping the `[To/From]Disk` part which also just seems like noise to me. Thanks for the proposals.

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760615)
Done, also added TODOs to the commit (+ message) to obviate that they're temporary

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760710)
I usually went in the other direction to make sure we know the sizes exactly.
Since `size_t` doesn't really serialize (e.g. in `fileout << GetParams().MessageStart() << blockundo_size` -> `call to 'Serialize' is ambiguous`) - but I made it into a `unsigned int` (I don't particularly like it, but it's not that important).

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760793)
Done, went with `unsigned int`

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908761002)
I've reverted the comments that were used to group functionality (write header ... write [undo[block), but I've deleted the ones that were just repeating code. Especially after extracting the constant to names values, they don't add any value.

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#issuecomment-2580109368)
Thanks a lot for the reviews, I've pushed [these changes](https://github.com/bitcoin/bitcoin/compare/2ff0ea366c61b2fcb80ad1f711480915c7a7aa2e..e25f0292117bce2e9f82240a4bfe86cc91e2f6c2):
* changed title and commit messages to reflect the new purpose of refactoring and code cleanup;
* removed repetitive mention of follow-up #31539;
* split benchmark renaming into a separate diff to ease review;
* replaced all temporary asserts to static_asserts (with todos to help the reviewers);
* added asse
...

💬 maflcko commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908767141)
My guess was that for serialization fixed-size ints are required and this was the motivation for the change in the first place, so I am surprised to see it reverted.

Not that it matter in this codebase in practise, given that int is assumed to be 32-bit, but I wanted to mention it.

💬 l0rinc commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#issuecomment-2580131442)
utACK f0b70fd7a1b1520502f99c516f8e1e3d83368d2c

💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908785810)
> for serialization fixed-size ints are required

Yes, it's the same argument as in https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1908760710

🤔 sipa reviewed a pull request: "validation: write chainstate to disk every hour"
(https://github.com/bitcoin/bitcoin/pull/30611#pullrequestreview-2539909469)
utACK b37452ca437856ed5d8effce2cef737d9df479c1

💬 sipa commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908793472)
This will result in a delay between 50 and 70 minutes, but it will always be a multiple of 1 minute, which seems like an unnecessary restriction.

Making the type of range `std::chrono::milliseconds` (and dropping the `+ 1min`) should fix that, I think.

💬 maflcko commented on pull request "qa: Improve framework.generate* enforcement (#31403 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31599#issuecomment-2580158800)
lgtm ACK 1b51616f2e3b58a1c63a19e6dba8e7e9c2aefdeb

💬 maflcko commented on pull request "init,log: Unify block index log line":
(https://github.com/bitcoin/bitcoin/pull/31616#issuecomment-2580163829)
lgtm ACK e04be3731f4921cd51d25b1d6210eace7600fea4