💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2356297541)
[321097a ](https://github.com/bitcoin/bitcoin/commit/321097a685b67f82c32f53652227b7177fe97a95)to [0bd53d9](https://github.com/bitcoin/bitcoin/commit/0bd53d913c1c2ffd2d0779f01bc51c81537b6992): addressed feedback by @jonatack

>Is this ready for review or should we wait until you have addressed this @mzumsande ?

It's ready for review, see https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763467219 - but of course I'm open to suggestions!

💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2356328386)
bdfeb12eec31eb885479eed293fe3677cfac52b2 is a really nice simplification.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763508568)
Did a significant restructure, the tabular tests should be a lot easier to comprehend and modify now.
I still need to get rid of remaining flags in the tests, will do that tomorrow - any early feedback until then is appreciated.

🚀 fanquake merged a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)

🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2310227385)
Rebased b95bb2179610183d9398d50d8c8fd24b1450ad4d -> f9245e4bb880a6d76bbb994a6e1907c6f8c9f205 ([`pr/mine-types.11`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.11) -> [`pr/mine-types.12`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.12), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.11-rebase..pr/mine-types.12)) due to silent conflict with #30440

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1763530005)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345

> [02c6a1e](https://github.com/bitcoin/bitcoin/commit/02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea) `-> (result: BlockTemplate);`

Thanks, should be updated in latest push.

💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763535883)
See https://github.com/bitcoin/bitcoin/blob/6fc4692797121b54de0c54e5b09ee47f322c038a/src/.clang-format#L23, so I'll leave this as-is for now.

💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763536070)
Ok, done

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763544909)
I still think this should be an enum named `EntryState` or something, with all combinations of flags like https://github.com/bitcoin/bitcoin/pull/30906/commits/ba52ebd41673abf63d4495215c84be7244469d30#r1760349038. That's the proper way to do this instead of defining static vars for each value of a char we care about.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763548746)
Absolutely, that's ny plan for tomorrow - but needed to clean up the test before doing that

💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2356432684)
> Agree with ryanofsky that it would be good to document rough compile time impact if any.

Sure added a note that it is less than 1% of both memory and time. The time should be offset by dropping the corresponding stuff from the Python linter, which is slower than a compiler at iterating over strings.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763454509)
Thanks! Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763448694)
Dropped this test for now, and left a todo to add more tests to exercise different methods of mempool submission and ensure we never violate the cluster limits.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763330087)
Removed (it was needed in an intermediate commit).

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1761997599)
Done.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763536059)
Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1761997443)
good point. i wonder if we should just drop the feerate diagram language altogether and leave it at "insufficient fee"?

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763532380)
Thanks! Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763554660)
Thanks, removed.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763546504)
Ah, right. Given that this falls back to the vsize limit being exercised, I think this isn't really worth including? Will leave this for now.