💬 paplorinc commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1660309760)
Thanks a lot, it's so much better this way!

💬 paplorinc commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#issuecomment-2198760158)
Thanks a lot for your help @tdb3, appreciate your reviews and verifications!

💬 TheCharlatan commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2198761717)
> I picked libbitcoin_util instead of libbitcoin_common to avoid a future dependency of the Stratum v2 Template Provider (https://github.com/bitcoin/bitcoin/pull/29432) on libbitcoin_common. The latter goal requires additional changes and is mostly just nice to have (the important bit is not depending on libbitcoin_node).

So I guess it should ideally only have the kernel as a dependency? It is not quite clear to me yet how this is going to be achieved. The current mining interface requires kn
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198761817)
@paplorinc was trying to implement your suggestion, but it's much too complex I think.
How do we delete the node that points to the sentinel? If we move the sentinel into the node and reassign sentinel to that node, we would still need to erase that node from the `coinsCache` map. We could extract the node handle from the map and have the `CCoinsViewCache` own that, but this seems like far too much code being changed. I don't think there's that much benefit for removing one pointer per entry. W
...

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198762814)
@andrewtoth Just high-level brainstorming, and I really don't want to push this PR into something more complex than necessary.

But would it be possible to have a map from outpoints to entries, with a singly-linked list through the entries, such that *(map[key].m_prev) stores the data for key key, rather than map[key] itself?

👍 kristapsk approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2150199260)
ACK 8ec24bdad89e2a72c394060ba5661a91f374b874

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198767807)
> But would it be possible to have a map from outpoints to entries, with a singly-linked list through the entries, such that *(map[key].m_prev) stores the data for key key, rather than map[key] itself?

@sipa Sorry I'm not sure I follow. What does *(map[key].m_prev) store?

👍 tdb3 approved a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2150201952)
re ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6

Re-ran same tests in
https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2150128035

same results.

💬 fjahr commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1660315938)
```suggestion
conflicting transactions can be seen in the `"mempoolconflicts"` field of
```

💬 fjahr commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1660316415)
Linter fails because the import of `COINBASE_MATURITY` is not needed anymore (line 12).

💬 furszy commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1660318367)
nano nit: would call it "DEFAULT_PRINT_FEERATE" as the "modified" word is internal and might change over time.

🤔 furszy reviewed a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2150204768)
utACK 323ce303086

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198775424)
@andrewtoth

So the data structure we want has (a) an index on COutPoint and (b) a single-linked list that stores an iteration order. The issue (I think, I haven't followed in detail) is that we need the ability to delete entries found through the (a) index, and if we naively do that, the entry that (b) points to the deleted entry will have its prev pointer invalidated.

My suggestion is "shifting" the values of the map by one. `map[key]` stores the UTXO for the entry that is (b)-after `key
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198787407)
@sipa thank you for the suggestion! I think it's missing the fact that we want entries in the map to possibly not belong to the linked list. So we want to clear the linked list after a non-erasing flush, but leave the map intact otherwise. I don't think that's possible with just this data structure. We would need 2 parallel maps, one with unflagged entries and one with dirty entries. I'm not sure if that would be more performant with 2 lookups for every access? I think if we have 2 maps we can a
...

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198788192)
@andrewtoth You can have 1 map, with 2 sentinels, each being the end of one linked list (one for flagged entries, one for unflagged ones)... but yeah, fair point, this perhaps complicates matters even more. And the performance penalty of this idea is probably not insignificant either.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660330315)
This one is problematic though https://github.com/bitcoin/bitcoin/pull/28280/files#diff-3d0856e8b7f136c588b229e0cbd3b2e2c309cd097ade0879521daba4e1bb2a33L609. That would be a big change if we couldn't get the flags.

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

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

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660341870)
Updated comment.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342034)
Mentioned in another comment, we need GetFlags or we will have to change a lot of test code in `coins_tests.cpp`.