Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 fanquake commented on pull request "doc: use new block_to_connect parameter name":
(https://github.com/bitcoin/bitcoin/pull/33237#issuecomment-3213559439)
cc @stringintech
💬 stickies-v commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#issuecomment-3213648051)
re-ACK a602f6fb7bf5f9e57299f4d6e246c82379fad8d2

CI [failure](https://github.com/bitcoin/bitcoin/actions/runs/17131871632/job/48598055509?pr=33212) (_"The hosted runner lost communication with the server"_) looks unrelated, would be good to restart?
💬 hodlinator commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3213700908)
Agreed that different names depending on originating platform might be a bit confusing.

Would be slightly more straightforward to switch to *bitcoin-macos-app.zip* as output for all, or allow `-zip` without a value in *contrib/macdeploy/macdeployqtplus* and just produce something like "macdeploy-output.zip".
🤔 janb84 reviewed a pull request: "doc: use new block_to_connect parameter name"
(https://github.com/bitcoin/bitcoin/pull/33237#pullrequestreview-3143812329)
ACK 1c3db0ed8e6f51d66facf4820a9225f6c617ac53
🚀 fanquake merged a pull request: "depends: remove xinerama extension from libxcb"
(https://github.com/bitcoin/bitcoin/pull/33217)
🚀 fanquake merged a pull request: "doc: Remove wrong and redundant doxygen tag"
(https://github.com/bitcoin/bitcoin/pull/33236)
💬 fanquake commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#issuecomment-3213895412)
The dependency on `libxcb-xinerama0` has been removed in #33217 (which will also be backported to the `29.x` branch) so I think pretty much all of the text being added here, is no-longer needed?
💬 dergoegge commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3213905094)
> Not everyone knows what "33106" is

"33106" refers to the pull request with that number: #33106, which is also linked in the PR description (only 12 times). It's common practice to link to a related PR like this (e.g. for a followup) in the PR title. Hope this helps.
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2293413758)
I'm happy to put this in draft and wait for #32579 to be merged first... This PR is smaller, and probably easier to rebase. What do you think?
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2293426298)
I would still keep the reformatting as `clang-format` doesn't like the one on master, and reformats it as I previously did (with the member initializers pushed to the right). At least this reformat is nice and `clang-format` doesn't try to modify it.

I hadn't considered `git blame` though, I don't love the idea of polluting it...
💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3214068903)
@pinheadmz it's on my review list, will do soon(tm)
💬 fanquake commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3214071991)
cc @hebasto; given this would change translations after translation string freeze.
💬 fanquake commented on pull request "doc: follow-ups to "Add bitcoin-{node,gui} to release binaries for IPC"":
(https://github.com/bitcoin/bitcoin/pull/33233#issuecomment-3214086173)
If there are more changes, there can be more PRs. We can fixup the docs now, and open more PRs when needed.
🚀 fanquake merged a pull request: "doc: follow-ups to "Add bitcoin-{node,gui} to release binaries for IPC""
(https://github.com/bitcoin/bitcoin/pull/33233)
📝 fanquake opened a pull request: "[29.x] depends: remove xinerama extension from libxcb"
(https://github.com/bitcoin/bitcoin/pull/33238)
Backports #33217 to the `29.x` branch.
Same rationale as #33217. `xinerama` was obseleted well before it's removal in Qt6.
💬 fanquake commented on pull request "depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33217#issuecomment-3214089111)
Backported to 29.x in #33238.
💬 luke-jr commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2293514007)
Might be a good idea to actually test this is correct?
🤔 luke-jr reviewed a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3144205566)
I guess this basically works for the existing params, since a block hash is never valid JSON (which forbids leading zeros), but could be dangerous to use for new parameters that are ambiguous.

With that in mind, it might be worth instead having a new `RPCResult::Type::BLOCK_HEIGHT_OR_HASH` type to sanity check this. Or at least a warning comment (but I don't know where you'd put it to ensure it's seen).
💬 luke-jr commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2293533369)
```suggestion
{"peer_id|peer_ids", RPCArg::Type::ARR, RPCArg::Optional::OMITTED,
```