💬 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
...

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1770595947)
renamed to `check_interval`

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2366877891)
Addressed feedback from @furszy and added some better documentation.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1770595622)
Can we just hardcode value3 for write? It's the same for every case here.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1770597044)
Wouldn't that be inconsistent?
In every other case, the constant contained both values.

💬 fjahr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1770597724)
done

💬 fjahr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1770597731)
done

💬 fjahr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#issuecomment-2366881046)
Added improvements suggested by @furszy , thanks!