💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1659967321)
Yeah, I could accept either way. You have shown good persistence with this issue @am-sq!

👍 Christewart approved a pull request: "policy: Add `OP_TRUE <0x4e73>` as a standard output script"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2149824011)
utack 99e9d3c7bc45e1266c6592b2afd47b8999745e04

🤔 paplorinc reviewed a pull request: "Don't empty dbcache on prune flushes: >30% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/28280#pullrequestreview-2149804075)
Added a few more comments, it's a lot more understandable to me now - though I'm still lacking a few important details, I'll do those next week.
Thanks for all your effort!

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659952944)
I don't mind the current one either, but now that you have helper method, maybe we don't even need to expose the internals:
```suggestion
if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
```

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659956700)
this seems like a change in behavior (`=` to `|=`)

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659961465)
nice!

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659956441)
hmmm, shouldn't this be `this->flags |= flags;`? I wonder how the tests passed...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659971494)
so this basically inserts "in the middle" of the linked list all the time, i.e. not at the beginning or the end, right?
I also find that quite confusing, i.e.
```
head <-> newest node <-> second newest <-> third newest <-> oldest node -> nullptr
```

what if we iterated the initialized nodes backwards so that they reflect the order of insertion, something like:
```C++
std::list<CoinsCachePair> CreatePairs(CoinsCachePair& head)
{
std::list<CoinsCachePair> nodes(NUM_NODES);
for
...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659966077)
Could the overall intent be better expressed here, something like `SetCacheStateAndLink`/`ResetCacheStateAndUnlink` or `Sync`/`Reset` or something?
It's obviously doing more after this commit than what it announces...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659957585)
can you please explain why this isn't a change in behavior? (in the commit it was introduced in)

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659968285)
same, it seems to me that it's not the head that's tested, but its next pointer

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659967194)
instead of exposing `GetFlags`, we could check that it's not fresh and not dirty

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660017722)
It isn't because entry is constructed the line above, and we know from default constructor `flags` will be initialized to `0`. So `0 | flags` is same as `flags`.

💬 kevkevinpal commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2198357675)
> > thanks @tdb3 and @alfonsoromanz I added a log in [82c454c](https://github.com/bitcoin/bitcoin/pull/30340/commits/82c454c65658439481b50fe71ed097ecb8d70737)
> > Also @tdb3 I tried doing
> > ```
> > b1hash = index_node.getblockhash(1)
> > index_node.invalidateblock(b1hash)
> >
> > self.log.info("Test obtaining info for a non-existent block hash")
> > assert_raises_rpc_error(-5, "Block not found", index_node.gettxoutsetinfo, hash_type="none", hash_or_height=b1hash, use_index=True)
> >
...

📝 Balcoin-BLC opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30363)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ achow101 closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30363)

📝 achow101 locked a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30363)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 tdb3 commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2198387274)

> > Regarding the hash used for hash_or_height, from what I can tell regtest blocks are allowed a higher target. While using 00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09 (block hash of mainnet block 1000) would be extremely improbably to collide, wouldn't it be better to use something guaranteed not to collide? Something above the max target for regtest?
>
> sounds good that makes sense I just pushed [837af6e](https://github.com/bitcoin/bitcoin/pull/30340/commits/837af6e
...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660083440)
Indeed, same for line 130

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660086532)
Not sure if it helps, just an observation: `m_prev` only uses `.second`, so it might as well be a `CCoinsCacheEntry*`
```patch
Index: src/coins.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/coins.h b/src/coins.h
--- a/src/coins.h (revision c36363f6b24c7ab2afe198d9855f507ddf096e1f)
+++ b/src/coins.h (date 1719730674870)
@@ -123,7 +123,7 @@
* parent cac
...