Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 maflcko commented on pull request "Followups for #30409 waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/30966#discussion_r1774844673)
Seems overly complicated to turn a reference into a pointer, then turn it into std::any, then recover it from any, then assert the pointer isn't null.

It would be better to remove all of this and just pass the reference.

So it would be good to drop this commit or replace it with fadd531a57710809df0e4b027ce30b5524e5f40f, which does the above.
💬 maflcko commented on pull request "Followups for #30409 waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/30966#discussion_r1774841423)
Seems verbose to lock this every time. Also, the comment above is still wrong. It would be good to drop this commit, or replace it with fa7dd1d072dc36997e2d5d702e2da529a093c90f, which also fixes up the outdated comment.
💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2373566011)
rebased after conflict with https://github.com/bitcoin/bitcoin/pull/30409
💬 TheCharlatan commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2373601290)
Nice, Concept ACK
💬 maflcko commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-2373608939)
> The UX for downloading two verification files sucks IMO. I think we should just go for the clearsign option, even if it's exclusive.

See also: https://www.gnupg.org/documentation/manuals/gnupg/Operational-GPG-Commands.html :

> Note: When verifying a cleartext signature, gpg verifies only what makes up the cleartext signed data and not any extra data outside of the cleartext signature or the header lines directly following the dash marker line. The option --output may be used to write out
...
🚀 fanquake merged a pull request: "doc: Adjust links in OSS-Fuzz section"
(https://github.com/bitcoin/bitcoin/pull/30963)
💬 fjahr commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#issuecomment-2373628404)
ACK e52ccc0203aefdebccd5a2f7146e58a816539467
💬 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.