💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-2373637158)
@maflcko You're right, I didn't put much effort on it. Do you think the issue is still relevant?

💬 hodlinator commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2373644670)
https://github.com/bitcoin/bitcoin/actions/runs/11030752196 gives me:

actions/upload-artifact@v4 is not allowed to be used in bitcoin/bitcoin. Actions in this workflow must be: within a repository owned by bitcoin or matching the following: actions/cache@*, actions/cache/restore@*, actions/cache/save@*, actions/checkout@*, ilammy/msvc-dev-cmd@*.

Maybe one could (ab)use *actions/cache/save* but I'll pause here for more input.

💬 l0rinc commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#issuecomment-2373646709)
ACK e52ccc0203aefdebccd5a2f7146e58a816539467

💬 maflcko commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2373652165)
Maybe you can leave it in your repo and just trigger the task every 3 hours for two weeks to get the dump eventually?

💬 Sjors commented on pull request "Followups for #30409 waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/30966#issuecomment-2373702089)
Let's go with #30967.

✅ Sjors closed a pull request: "Followups for #30409 waitTipChanged()"
(https://github.com/bitcoin/bitcoin/pull/30966)

👍 hebasto approved a pull request: "[28.x] backports and finalize (or rc3)"
(https://github.com/bitcoin/bitcoin/pull/30959#pullrequestreview-2327894614)
ACK 1f6c3caf30849a6e53957aa15c6ac27b97004dc5.

A new backport only affects the test code, so I agree to skip the rc3 phase.

👍 Sjors approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2327893660)
utACK fa964f82c52a7d0fd40c5133cf5fc18ef13819e5

Dropping `g_genesis_wait_cv` is a nice simplification.

💬 Sjors commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1775007062)
fa964f82c52a7d0fd40c5133cf5fc18ef13819e5 nit: seems everything below here could be its own commit

👍 hebasto approved a pull request: "doc: correct the zmq automatic build info"
(https://github.com/bitcoin/bitcoin/pull/30946#pullrequestreview-2327923109)
ACK e52ccc0203aefdebccd5a2f7146e58a816539467.

💬 hebasto commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#discussion_r1775024220)
pedantic nit: The technically correct term for the first `cmake` invocation is ["generating a project buildsystem"](https://cmake.org/cmake/help/latest/manual/cmake.1.html#generate-a-project-buildsystem). The actual "building" is performed by the generated build system during the following step. Elsewhere in our documentation, we use phrases such as "configuring Bitcoin Core" or "configuring the build system".

👍 hebasto approved a pull request: "doc: correct the zmq automatic build info"
(https://github.com/bitcoin/bitcoin/pull/30946#pullrequestreview-2327933232)
re-ACK 06e7e83632985bd8b648d24f9a51591d3a3bbec3.

💬 tdb3 commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#discussion_r1775030740)
Yeah, you're right. I was on the fence about naming (since the actual build is done with `cmake --build build -jN`. Better to fix it now, so I pushed a change.

💬 l0rinc commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#issuecomment-2373794712)
ACK commit/06e7e83632985bd8b648d24f9a51591d3a3bbec3

🚀 fanquake merged a pull request: "doc: correct the zmq automatic build info"
(https://github.com/bitcoin/bitcoin/pull/30946)

💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2373847587)
Additionally, if we remove settings of non-toolchain variables, such as `BUILD_GUI` or `ENABLE_WALLET` from the toolchain file, it allows the reuse of this toolchain file to build depends themselves, instead of relying on this code:https://github.com/bitcoin/bitcoin/blob/39219fe145e5e6e6f079b591e3f4b5fea8e71804/depends/funcs.mk#L173-L199

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2373862834)
Thanks for the review!

> I didn't check if the new `CustomBuildField` and `CustomReadField` to serialize `std::chrono` is identical to that in #30437. If not, then I'll test it next time I rebase [Sjors#48](https://github.com/Sjors/bitcoin/pull/48).

They should be the same except the builder now has static asserts to make sure the capnp number type has enough range to represent the std::chrono duration type (that capnp Float64 type can represent MillisecondsDouble in this case).

💬 maflcko commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#discussion_r1775104055)
> by the way this was Tested ACK

Reviews in resolved conversations are unlikely to be seen. My recommendation would be to put them in a normal comment that isn't hidden.

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

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