💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668526162)
4b1e978b2bafd9da564aa52d2ce64a723cf64036 nit:
```suggestion
// Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and if
```

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1644451932)
maybe mention that caller must ensure inc includes `Ancestors(inc)`

🤔 glozow reviewed a pull request: "cluster mempool: cluster linearization algorithm"
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2118340933)
Looked at the first 4 commits so far, just doing code review and trying the algos out in `MiniMiner`.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1639799588)
In the docs, maybe instead of "best remaining ancestor set", say "highest feerate ancestor set" ?

Side note - I was thinking that this was a replica of the ancestor set algo in `BlockAssembler` but this isn't the minimum of individual and ancestor feerate.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668545670)
304d3cb23ba9f084b98f9f29b47e3dfbb61ca334
may be helpful context
```suggestion
// Never MarkDone the same transaction more than once as this function
// blindly subtracts the transaction's feerate from m_ancestor_set_feerates
Assume(select.IsSubsetOf(m_todo));
```

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668665030)
For each of the `CandidateFinder`s, would it be helpful to expose a `AllDone()` function? Seems slightly inconvenient for the caller to separately keep track of `todo` for a `while(todo.Any())` loop. `FindCandidateSet()` Assumes that todo isn't empty, so we can't use that to query.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668716016)
Note to reviewers: pseudocode for the function when first introduced in 5661fcdb15244bc6d602379c294276c7ec686505 can be found [here](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-21-searching-6)

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668724062)
And I suppose it's not problematic if it is optimal but didn't claim to be so?

📝 theStack opened a pull request: "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results"
(https://github.com/bitcoin/bitcoin/pull/30408)
The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script".

See also the draft BIP "Terminology for Transaction Components" (https://github.
...

💬 maflcko commented on pull request "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results":
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2214477162)
ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b

🚀 ryanofsky merged a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141)

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1668941931)
I updated this to no longer clear flags.

👍 BrandonOdiwuor approved a pull request: "test, assumeutxo: Remove resolved todo comments and add new test"
(https://github.com/bitcoin/bitcoin/pull/30403#pullrequestreview-2163834970)
Code Review ACK 29d19c414c9e138c99f8de84e398528db92ff07e

I will close https://github.com/bitcoin/bitcoin/pull/29681in favor of this

✅ BrandonOdiwuor closed a pull request: "test: loading assumeutxo snapshot start states"
(https://github.com/bitcoin/bitcoin/pull/29681)

💬 BrandonOdiwuor commented on pull request "test: loading assumeutxo snapshot start states":
(https://github.com/bitcoin/bitcoin/pull/29681#issuecomment-2214630023)
Closing PR in favor of https://github.com/bitcoin/bitcoin/pull/30403

💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2214654114)
Oh, sorry @BrandonOdiwuor , I missed https://github.com/bitcoin/bitcoin/pull/29681 😞

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2214655675)
re: https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2210605752

Thanks for the questions.

> I'm not sure what a good approach for reviewing this might be. How can I validate the changes without tests? Should I be running this as part of a parent pull request?

You could test this as part of #10102, and I should probably add some unit test code to [ipc_test.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/test/ipc_test.cpp) to make sure new runtime code in [capnp/chain.cp
...

💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2214720674)
Rebased this, taking advantage of https://github.com/bitcoin/bitcoin/pull/28960. I've also been investigating alternative approaches.

I first tried to move from `fmemopen` toward the more flexible `memfd_create`. It avoided the need for some of the filesystem mocks (which were necessary before because you can't call `fileno` on a `FILE*` created with `fmemopen`). This allowed to drop one commit. Further, this removed the need from creating 128MiB in-memory blk files. This is in turn makes it
...

📝 Sjors opened a pull request: "Introduce waitTipChanged() mining interface and replace RPCNotifyBlockChange"
(https://github.com/bitcoin/bitcoin/pull/30409)
This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients.

In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`.

The second commit adds tests for the methods that are about to be refac
...

💬 fjahr commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214748299)
I think this may be ready for closing since all remaining TODOs have been addressed IMO. There are three remaining PRs that still need to get merged but I think we can expect that nobody will start a new PR based on the TODOs comment anymore.

For reference, the remaining PRs are #30403, #30320, #29996. See also https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2213758446