Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 Sjors opened a pull request: "Followups for #30409 waitTipChanged()"
(https://github.com/bitcoin/bitcoin/pull/30966)
Based on comments on #30409.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2373386581)
@maflcko #30966
💬 Sjors commented on pull request "Followups for #30409 waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/30966#issuecomment-2373388800)
@theuni also wrote:

> Related: It's a shame that the condvar/mutex leak out of here. Is there a reason not to add getter/waiter/notifier functions?

Not sure how to go about that, but happy to take a patch.
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2373443353)
re-utACK fbed31494af9acf6bd543801143a12399b2c6a1a

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 https://github.com/Sjors/bitcoin/pull/48.
📝 maflcko opened a pull request: " refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967)
`g_genesis_wait_cv` is similar to `m_tip_block_cv` but shuffling everything through a redundant `boost::signals2`.

So remove it, along with some other dead code, as well as minor fixups.
👍 dergoegge approved a pull request: "doc: Adjust links in OSS-Fuzz section"
(https://github.com/bitcoin/bitcoin/pull/30963#pullrequestreview-2327621768)
ACK fa6c1946d235c6ea5b9384d8488d80aa3bcd0ad7
🤔 maflcko reviewed a pull request: "Followups for #30409 waitTipChanged()"
(https://github.com/bitcoin/bitcoin/pull/30966#pullrequestreview-2327623918)
I've opened https://github.com/bitcoin/bitcoin/pull/30967, which includes the follow-ups here, but I solved them in another way.

Feel free to take or leave them. I can drop them from mine, once this one is merged.
💬 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)