💬 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

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2580192438)
Updated 182945e946ac6a92daad012d6579e5441b9641cc -> 384c73d4fcda4af7c2b4bfb945a248cb93dba47d ([blockmanDB_5](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_5) -> [blockmanDB_6](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_5..blockmanDB_6))

* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1906052767), added back interrupt check.
* Addressed @ryanofsky's [com
...

📝 hebasto opened a pull request: "depends: Fix spacing issue"
(https://github.com/bitcoin/bitcoin/pull/31627)
This PR resolves an issue where a missing space caused the value of the `build_AR` variable to be concatenated with the "NM=" string. This resulted in subsequent calls to `${AR}` and `${NM}` failing.

Here is a diff for the `make -C depends print-build_id DEBUG=1` output:
```diff
@@ -110,50 +110,18 @@
CXX_STANDARD=c++20
END CXX
BEGIN AR
-ar: invalid option -- '='
-Usage: ar [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV] [--plugin <name>] [member-name] [count] archive-file file
...

💬 maflcko commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908825505)
Are you sure? rand_uniform_delay should cast the range to the duration type of the timepoint, which is the system clocks type.

See also the unit test:

```cpp
src/test/random_tests.cpp: BOOST_CHECK_EQUAL(26443, ctx.rand_uniform_delay(time_point, 9h).time_since_epoch().count());

💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1908827904)
It should be
```suggestion
AR='$(build_AR)' NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' \
```

Fixed in https://github.com/bitcoin/bitcoin/pull/31627.

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1908833307)
Ah I see what you mean, yes!

💬 fanquake commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2580228775)
What is the status of this

👍 theStack approved a pull request: "test: raise an error in `_bulk_tx_` when `target_vsize` is too low"
(https://github.com/bitcoin/bitcoin/pull/30322#pullrequestreview-2539977783)
ACK 92787dd52cd4ddc378cf1bcb7e92a649916a0f42

Tested the new error raising code paths manually by introducing `create_self_transfer` calls (in an arbitrary functional test), one with too-high-fee and another one with too-low-target-vsize:

```
wallet.create_self_transfer(fee=Decimal("50.00000001"))
...
=> RuntimeError: UTXO value 50.00000000 is too small to cover fees 50.00000001


wallet.create_self_transfer(utxo_to_spend=utxo, target_vsize=100)
...
=> RuntimeError: target_vsize 100
...

💬 theStack commented on pull request "test: raise an error in `_bulk_tx_` when `target_vsize` is too low":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1908841262)
nit: could deduplicate the expression for calculating the final fee in sats

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908874529)
I've checked
```patch
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp (revision 1ac02221219a2ae6a8605dfddff4390a60712d56)
+++ b/src/validation.cpp (date 1736431426006)
@@ -2869,6 +2869,10 @@

if (should_write || m_next_write == NodeClock::time_point::max()) {
constexpr auto range{DATABASE_WRITE_INTERVAL_MAX - DATABASE_WRITE_INTERVAL_MIN + 1min};
+ for (int i = 0; i < 10; ++i) {
+ auto random_delay = FastRand
...

💬 pablomartin4btc 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_r1908883714)
Oh, I see... yeah, ok... in the case of `WalletMigrationTest` I can see the required versions defined in the setup. Thank you!

🤔 pablomartin4btc reviewed a pull request: "test: raise explicit error if any of the needed release binaries is missing"
(https://github.com/bitcoin/bitcoin/pull/31462#pullrequestreview-2540045704)
tACK 1ea7e45a1f445d32a2b690d52befb2e63418653b

💬 kehiy commented on pull request "doc: upgrade license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31611#issuecomment-2580352679)
> lgtm ACK [b537a2c](https://github.com/bitcoin/bitcoin/commit/b537a2c02a9921235d1ecf8c3c7dc1836ec68131)
>
> Looks similar to what was done in commit [1f8450f](https://github.com/bitcoin/bitcoin/commit/1f8450f066724dfbb5c5bc4060843e2f3340ed88) last year.

@maflcko thanks. yes, every year we had these changes on this repo.