Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 fanquake commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3324213528)
Ok, then lets close this (and #33293) for now, and reopen either if it starts happening again?
💬 fanquake commented on pull request "Release: 30.0rc2 translations update":
(https://github.com/bitcoin/bitcoin/pull/33452#issuecomment-3324223762)
ACK 33a0d4bb5b475ddec00229366183c7aabe4c3501.
🚀 fanquake merged a pull request: "Release: 30.0rc2 translations update"
(https://github.com/bitcoin/bitcoin/pull/33452)
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372512300)
There are two renames: `FindNode()` -> `AlreadyConnectedToAddressPort()` and `addrName` -> `addr_port`. Which one do you mean, or both? Do you have a suggestion for a better name(s)?

> RPC help for `getpeerinfo` is also wrong

this?

```diff
- {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"},
+ {RPCResult::Type::STR, "addr", "(host:port) The IP address or hostname and port of the peer"},
```
💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3324264411)
> Thanks. Looks like there are some issues with this branch. i.e:

Ah sorry about that! It's fixed on the branch now.

> Want to PR / debug that separately?

Happy to review this and do a separate PR if that's easier. The default changes also require manpage modifications, so it might be better to defer the rc final changes if you'd like a separate PR.
💬 sipa commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372518052)
I was talking about the function name, but I guess the argument rename is equally off.

And, indeed, regarding RPC doc.
💬 fanquake commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3324268844)
Ok, I'll just pull the branch in here (shortly).
💬 moonsettler commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3324276876)
> Can you steelman any legitimate use cases for these options that would justify reversing their deprecation status/future removal?

Idk about a steelman, but I think it was a mistake and correcting a mistake is preferable. There is clearly demand now for the setting whether you think it's misguided or not. There is demand for software that ships with different defaults. I see no value in making the maintenance of such software harder and harder over time, when the alternative has no real cost
...
🚀 fanquake merged a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448)
📝 fanquake opened a pull request: "ci: add libcpp hardening flags to macOS fuzz job"
(https://github.com/bitcoin/bitcoin/pull/33462)
Follows up to https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3323149107.
💬 fanquake commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3324313865)
> Unrelatedly(?), I wonder if it is possible to adjust the existing macos fuzz task to have stdlib hardening enabled.

Opened #33462 to have a look.
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3324325790)
@BitByBitByBitByBit

> I went through all the suggestions for Spanish and made the necessary edits. Thank you for this input! It is a very useful tool even though it requires some fine tuning. It would be helpful to take glossaries into account, as long as they are up-to-date for each language.

@ostruvek

> Hello, sorry for a delay on my side. I have reviewed the Czech suggestions and made the edits.

Thank you for your contributions! Both updated translations have been fetched from T
...
💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3324342116)
> I'm not familiar enough with X to say why it's seemingly not possible to link all the xcb_utils statically, but this change seems sane to me.

Yea, maybe this is the direction we should go. Ideally we can land this change for 30, to remove the awkwardness here. I've updated the commit message and PR description.
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3324395984)
reACK 5ac32aa441bf3ec1f1fa826f689405bc4a7b7354

just a rebase
🤔 vasild reviewed a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448#pullrequestreview-3258413205)
ACK 2738b63e025d240618b3c72c28243c3e4d7d9c79
💬 vasild commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#discussion_r2372603900)
In the `else` branch, why set one of the new variables `stats.m_inv_to_send` to `0` but not the other new variable `stats.m_last_inv_seq`? Both have default value of `0`. I understand that if this code enters the `if` / `true` branch, it will never go to the `else` branch in subsequent calls. That is - `peer->GetTxRelay()` will not return `!= nullptr` once and later return `nullptr`. So, this means that there is no need to zero out stale values in the `else` branch, right? Then `stats.m_inv_to_s
...
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372635323)
What about `AlreadyConnectedTo()` for the function and `peer` or `host_port` for the parameter?
💬 fanquake commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3324447635)
Ah, this probably isn't doing anything, because we are using the oldest supported Apple Clang here. Will push something else.
💬 jb55 commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2372668470)
ACK this ^
👍 ryanofsky approved a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3258531271)
Code review ACK 1bb0f7f8b6d927046fefa2eb29d0132713415cca. Nice code cleanups, though did leave one comment about the NOLINT.

re: https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319

> Could you please take a look at the following IPC-related warning:

Thanks, created https://github.com/capnproto/capnproto/pull/2417 to address it