💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356181083)
> CPU-bound (and not IO-bound)
Actually, the long-running fuzz test may be IO-bound so using a ramdisk could speed them up. So this would be another idea to look at.
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356181083)
> CPU-bound (and not IO-bound)
Actually, the long-running fuzz test may be IO-bound so using a ramdisk could speed them up. So this would be another idea to look at.
💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356184027)
> @theStack
>
> Could rebase once more please to refresh the CI?
Yes, done.
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356184027)
> @theStack
>
> Could rebase once more please to refresh the CI?
Yes, done.
👍 hebasto approved a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2310065055)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb, I've backported the listed PRS locally. The only merge conflict I faced was in https://github.com/bitcoin/bitcoin/pull/30807. It was trivial to resolve.
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2310065055)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb, I've backported the listed PRS locally. The only merge conflict I faced was in https://github.com/bitcoin/bitcoin/pull/30807. It was trivial to resolve.
💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2356197180)
> It's clear to see that the header generation is self-contained, and afaik that's the only place that would matter for build performance.
Could rebase on current master, now that the header generation is 30x faster, after 2a0949f0977db2dbb7ac452e8d58d413b9526756 ? This should also change the benchmark in the pull description.
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2356197180)
> It's clear to see that the header generation is self-contained, and afaik that's the only place that would matter for build performance.
Could rebase on current master, now that the header generation is 30x faster, after 2a0949f0977db2dbb7ac452e8d58d413b9526756 ? This should also change the benchmark in the pull description.
💬 maflcko commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356214347)
review ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f
Can't hurt to test this in one task
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356214347)
review ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f
Can't hurt to test this in one task
👍 ryanofsky approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310085862)
Code review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db. Just rebased since last review
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310085862)
Code review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db. Just rebased since last review
👍 TheCharlatan approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310089065)
Re-ACK 7942951e3fcc58f7db0059546d03be9ea243f1db
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310089065)
Re-ACK 7942951e3fcc58f7db0059546d03be9ea243f1db
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2356233258)
Rebased after merge of #30286.
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2356233258)
Rebased after merge of #30286.
💬 0xBEEFCAF3 commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#discussion_r1763450844)
Great suggestion! Added some additional unit tests to script_tests.json testing different combinations of scripts with and without (SCRIPT_VERIFY_DISCOURAGE_OP_CAT, SCRIPT_VERIFY_OP_CAT and SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS)
(https://github.com/bitcoin/bitcoin/pull/29247#discussion_r1763450844)
Great suggestion! Added some additional unit tests to script_tests.json testing different combinations of scripts with and without (SCRIPT_VERIFY_DISCOURAGE_OP_CAT, SCRIPT_VERIFY_OP_CAT and SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS)
💬 jaybny commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2356240428)
ok. is C++20 a requirement? why? it's not fully supported by clang or gcc yet. can get 90% there with libstdc++ , but I guess it's only a matter of time.
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2356240428)
ok. is C++20 a requirement? why? it's not fully supported by clang or gcc yet. can get 90% there with libstdc++ , but I guess it's only a matter of time.
💬 hebasto commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2356249815)
> is C++20 a requirement?
Yes. Please refer to https://github.com/bitcoin/bitcoin/pull/28349.
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2356249815)
> is C++20 a requirement?
Yes. Please refer to https://github.com/bitcoin/bitcoin/pull/28349.
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763467219)
I thought about it a bit and don't see a great non-invasive solution to this: We could add a call to `NotifyHeaderTip` in the calling functions after `cs_main` is released, which would be in `ActivateBestChain()` (plus in the `invalidateblock` / `reconsiderblock` rpcs which seem less important), and I'm unsure if we want to refactor this function to always call `NotifyBestHeader()` just for the special case where a connected block turns out to be invalid.
In any case, since `m_best_header` is
...
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763467219)
I thought about it a bit and don't see a great non-invasive solution to this: We could add a call to `NotifyHeaderTip` in the calling functions after `cs_main` is released, which would be in `ActivateBestChain()` (plus in the `invalidateblock` / `reconsiderblock` rpcs which seem less important), and I'm unsure if we want to refactor this function to always call `NotifyBestHeader()` just for the special case where a connected block turns out to be invalid.
In any case, since `m_best_header` is
...
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763468418)
will change to "/" so it mirrors the "invalidation / reconsideration" in the line before.
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763468418)
will change to "/" so it mirrors the "invalidation / reconsideration" in the line before.
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763474241)
change to `::cs_main`
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763474241)
change to `::cs_main`
💬 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!
(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.
(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.
(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)
(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
(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.
(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.