💬 fanquake commented on pull request "doc: fix broken link to release":
(https://github.com/bitcoin/bitcoin/pull/30945#issuecomment-2366733545)
Thanks. However we generally don't modify historical release notes.

✅ fanquake closed a pull request: "doc: fix broken link to release"
(https://github.com/bitcoin/bitcoin/pull/30945)

💬 l0rinc commented on pull request "doc: fix broken link to release":
(https://github.com/bitcoin/bitcoin/pull/30945#discussion_r1770527410)
https://bitcoin.org/bin/insecure/bitcoin-core-0.10.0 contains everything that https://bitcoin.org/bin/bitcoin-core-0.10.0 contains - and the rest of the <0.10.0 release links are also wrong and should be fixed.

Please squash the commits (we need a clean history) and fix all of them via a single [scripted-diff](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#scripted-diffs) commit.

Approach NACK

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2366787167)
Rebased to benchmark `reindex-chainstate` with #28358 and a cache size holding the entire utxo set.

> the full pre-assumevalid IBD consistently takes ~45 minutes (8%) longer after the change

I got a hold of an HDD, and surprisingly my benchmarks show the opposite. I kept the blocksdir on the SSD and only had the datadir on the HDD to isolate the effects of this change. When changing from writing at the end to writing every hour, writing every hour was ~25 minutes (6%) faster.

| Command
...

💬 Sjors commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2366813947)
```
x86_64

753aee81b01ff7a560a6c2c9ad64e2f3d3a535d0f5484364ea6b0f7ac7d8d595 guix-build-953aa067c565/output/aarch64-linux-gnu/SHA256SUMS.part
2eec13b0a0a28d16b9ec2b09908d6785666b49cd2076fd7b677259be0f30d624 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu-debug.tar.gz
732591be3acdc241d8d7a649bad755561dafe97839e00105682d962844acbe54 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu.tar.gz
279a5e977129a58cd743ac4
...

🤔 furszy reviewed a pull request: "wallet: Write best block to disk before backup"
(https://github.com/bitcoin/bitcoin/pull/30678#pullrequestreview-2320866299)
ACK bd8fee512301059cc03673099422c19b39f2ed65

💬 furszy commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1770567400)
nit: `w` would throw an error if used prior to loading the wallet. Should first load the wallet and secondly call `get_wallet_rpc`.

💬 furszy commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1770567603)
nit: could call `address_to_scriptpubkey()` only once outside the for loop.

💬 Sjors commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2366814938)
On the demo branch:

```
MULTIPROCESS=1 ./contrib/guix/guix-build

... # in progress
```

🤔 furszy reviewed a pull request: "test: Introduce ensure helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2320867678)
looking good, left few nits.

💬 furszy commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1770570342)
maybe rename "delay" to "check_interval" or another similar term?

💬 furszy commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1770568972)
tiny nit: if `duration < delay`, then `delay` should be set to duration?

💬 furszy commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1770575892)
nit: As far as I can see, no test case in this file uses the `WalletTestingSetup` wallet. So we could save some initialization time on each test case by changing this to `TestingSetup`. And if any future test requires a wallet, it could be specified in-place.

💬 andrewtoth commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2366842936)
Researching this more, I think I can answer my own question. The pool allocator introduced in https://github.com/bitcoin/bitcoin/pull/25325 cannot shrink in memory. It can only allocate more chunks, and freed memory just adds some nodes in the chunks to a freelist. But, we don't discount the available nodes in the freelist. So, even though we clear all elements, the memory usage reported by the map stays the same. This works great if we just fill up the map and then reallocate it, but it doesn't
...

💬 andrewtoth commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#issuecomment-2366845182)
Could we also track available memory in the freelist and expose it publicly? Right now when the dbcache fills up, our only option is to clear it and reallocate. We don't have any insight into how much memory is available in the freelist. We can't just evict a bunch of least recently added clean entries during a periodic flush, because the memory usage reported by the map will be the same. Having the amount of freed memory can let us make more granular decisions on when to actually reallocate the
...

💬 beage666 commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2366846364)
Ok

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1770588271)
Ah yes I see it was moved up with `AddFlags` to private. Maybe better to keep it on `SetDirty`.

> Adding a flag also requires a self reference to the pair that contains this entry in the CCoinsCache map

There's no context in the method signature that it is an entry in the `CCoinsCache` map.

> and a reference to the sentinel of the flagged pair linked list

There's no context in the method signature about a flagged linked list.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1770589885)
```suggestion
constexpr CoinEntry(CAmount v, State s) : value(v), state(s) {}
```
then all `const static MaybeCoin` declarations below can be made `constexpr`.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1770592376)
> There's no context in the method signature that it is an entry in the CCoinsCache map

isn't that what `CoinsCachePair` means?

> method signature about a flagged linked list

The sentinel hints at it.

But, again, I kept the methods for AddFlags - and don't want to duplicate it for SetFresh and SetDirty.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1770595924)
Hm, just doing that would have an unexpected effect, I think. With the code unchanged if the user would today pass the parameters with delay=duration, then I think the predicate would only be checked once in the beginning and not at the end because at the second conditional check the time passed should be the runtime of predicate + the sleep of duration, so the loop wouldn't run a second time. That's not what a user expects when they pass a high delay probably. So I have done the following: If d
...