Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 jonatack commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2102935031)
Appreciate the excellent doxygen documentation here.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2102983324)
> I think the message you posted in the PR description only shows a single error in a single dependency. I am wondering what happens if there is more than one error in different targets, possibly ones where cmake will never get to print the second error.

[Here](https://api.cirrus-ci.com/v1/task/5614699137466368/logs/ci.log) multiple error are reported.
👋 hebasto's pull request is ready for review: "ci, iwyu: Treat warnings as errors for specific targets"
(https://github.com/bitcoin/bitcoin/pull/31308)
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2901917485)
Rebased.
🚀 fanquake merged a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467)
👍 fanquake approved a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586#pullrequestreview-2861961840)
ACK fa079538e32d20aec6786c93e1117da1e8ea0cab - we can followup
fanquake closed an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode"
(https://github.com/bitcoin/bitcoin/issues/32524)
🚀 fanquake merged a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586)
👋 romanz's pull request is ready for review: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)
💬 fanquake commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2901996108)
cc @instagibbs
💬 jonasschnelli commented on issue "seeds: seed.bitcoin.jonasschnelli.ch not returning results":
(https://github.com/bitcoin/bitcoin/issues/32590#issuecomment-2902032188)
Fixed.
💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2103080376)
The server timer *should* start after receiving the last packet from the client, because its an idle timer not a session timer. So its more correct to have:

> send, server timer starts immediately after (B)

I tried to to nail this in https://github.com/bitcoin/bitcoin/commit/ce9847e284a361fb050f366848cdf1a38b2b933b but that also failed.

And then, if we start the timer too early, we will always get a duration > 1 no matter what the server actually does... maybe its the initial TCP connec
...
🤔 instagibbs reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2862076315)
concept ACK

seem like sensible things to be reporting
💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103091393)
seems odd to add it then modify it immediately next commit, can be one commit?
💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103089215)
to make it more explicit it's not a tx count thing
```Suggestion
LogDebug(BCLog::CMPCTBLOCK, "Initializing PartiallyDownloadedBlock for block %s using a cmpctblock of %lu bytes\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock));
```
💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103092771)
we don't know they were missing them really, just mention they asked for them for the specific block?
🤔 ryanofsky reviewed a pull request: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2859167293)
Great review! I improved a number of things in response to your comments.

Rebased 9ff5bc75659e0314f42c1477fc0c37ac0ad132fa -> 6bf727fb88009f99dab7c06bff7f8fb391147ac1 ([`pr/ipc-cli.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.5) -> [`pr/ipc-cli.6`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-cli.5-rebase..pr/ipc-cli.6)) due to conflict with #28710, also made a number of improvements to -ipcbind docume
...
💬 ryanofsky commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2101199458)
re: https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2070591693

> I notice this `ExecuteHTTPRPC()` bypasses our only authorization barrier for RPC which is an HTTP header, maybe you deal with this in another commit but that should probably be documented in the `ipcbind` help text.

Makes sense, updated ipcbind help text to mention RPC authentication.

> You also left behind the `jreq.authUser` check for RPC whitelist -- which I believe only gets filled in by `HTTPReq_JSONRPC()`.
...
💬 ryanofsky commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2101228151)
re: https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2070587369

> Might want to indicate `status` is an "out" param?

Thanks, expanded comment.
💬 ryanofsky commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2101223433)
re: https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2070617845

> Do we ever use this return value here on on master? It looks like it just gets swallowed inside `HTTPWorkItem`

Nice catch, dropped the unused return value.