💬 maflcko commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#discussion_r1773719129)
> However I got timeout's intermittently after running this PR with `self.noban_tx_relay = True` commented out

tx relay may take time, if trickle is enabled. Especially if many nodes are hopped. You'll have to increase the timeout-factor as explained in https://github.com/bitcoin/bitcoin/pull/30948#issuecomment-2367507024

🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2325866285)
Rebased d59f5a31a1b888051eb47e2f8a5fb8d901de1d10 -> 3499c2ca4ba5ae70cebe84614498cc105e208f3d ([`pr/mine-types.14`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.14) -> [`pr/mine-types.15`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.14-rebase..pr/mine-types.15)) to fix silent conflict with #30409. Also made some improvements in response to recent comments.

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

Updated now to use concepts from serialiize.h instead of defining new ones

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

Rewrote the comment so hopefully it is less confusing now.

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

Added a comment to mining.capnp about getting rid of BlockValidationState here and returning something simpler from testBlockValidity

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

I rewrote the comment and tweaked implementation of this to avoid use of non-standard Span class, so this could be dropped and moved to libmultiprocess in the future. But leaving that for a followup since it will require other changes to libmultiprocess.

💬 brunoerg commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#issuecomment-2371885788)
What is the state of this?

🤔 mzumsande reviewed a pull request: "test: Add missing sync_mempools() to fill_mempool()"
(https://github.com/bitcoin/bitcoin/pull/30948#pullrequestreview-2325964327)
Tested ACK faf801515f8fcd11a3454105cab66c38f6f240fe

📝 mzumsande opened a pull request: "validation: Disable CheckForkWarningConditions for background chainstate"
(https://github.com/bitcoin/bitcoin/pull/30962)
The comparison of `m_best_invalid` with the tip of the chainstate makes no sense for the background chainstate and can lead to incorrect log messages.

Fixes #30958

💬 mzumsande commented on issue "`invalidateblock` during background validation":
(https://github.com/bitcoin/bitcoin/issues/30958#issuecomment-2372000480)
I agree that these messages make no sense and think that the check should just be skipped for the background chainstate, see #30962.

💬 ismaelsadeeq commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#discussion_r1773865111)
Thanks, by the way this was Tested ACK faf801515f8fcd11a3454105cab66c38f6f240fe

🤔 ismaelsadeeq reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2326163992)
Concept ACK

👍 TheCharlatan approved a pull request: "validation: Disable CheckForkWarningConditions for background chainstate"
(https://github.com/bitcoin/bitcoin/pull/30962#pullrequestreview-2326305786)
ACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee

💬 theuni commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1774001261)
This needs a `GUARDED_BY(m_tip_block_mutex)`.

Adding it causes new warnings in `validation_chainstate_tests.cpp`. For ex:

>
> src/test/validation_chainstate_tests.cpp:73:46: warning: reading variable 'm_tip_block' requires holding mutex 'm_node.notifications->m_tip_block_mutex' [-Wthread-safety-analysis]

I assume those particular accesses are harmless (maybe actually racy in the right conditions?), but future users may not be.

Related: It's a shame that the condvar/mutex leak out o
...

💬 fjahr commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2372255365)
utACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774007908)
Gave it another go in the latest push (071342d4c7627200323d8f22cc37ea022b2e9d69) and your direction seems to be working fine. Happy to see the linter changes go!

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774010008)
(Latest push 071342d4c7627200323d8f22cc37ea022b2e9d69 doesn't touch linter - resolving).

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774014280)
`constexpr` functions are not guaranteed to be able to evaluate at compile time for all possible parameters, so exercising that path in the test still provides some value IMO.

📝 maflcko opened a pull request: "doc: Adjust links in OSS-Fuzz section"
(https://github.com/bitcoin/bitcoin/pull/30963)
Adjust the links after the google issue tracker migration and replace the remaining paragraph with a link to https://bitcoincore.org/en/security-advisories/

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#issuecomment-2372298030)
(Just added co-authorship of maflcko in latest push from 071342d4c7627200323d8f22cc37ea022b2e9d69 -> a911efc6e868ab1a23d05ee8ab3e94f7ecb89e2b).