💬 fanquake commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3324142254)
I guess if this is *working* for now, we can leave it, and re-evaluate in future? Maybe GH cleaned up their images.

💬 fanquake commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3324146609)
Seems like it also ran fine in https://github.com/bitcoin-core/gui/pull/895.

💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3324154229)
I have also been somewhat-reliably informed that GH are testing customisable images internally now, so perhaps they'll also permit a stock Ubuntu image in the near future too (which would work fine for us?).

💬 maflcko commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3324157752)
lgtm ACK b77137a5644e09a08442aed7d8a4a9290fb53526

💬 moonsettler commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3324158646)
Even if I conceded that it is a footgun (which I'm not) to set `datacarriersize` I would still prefer no "gun control". Especially in a case where "self-harm" is the only real concern.

🤔 Eunovo reviewed a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399#pullrequestreview-3258200371)
ACK https://github.com/bitcoin/bitcoin/pull/33399/commits/1ff9e929489e625a603e8755b8efe849feda1f16

I verified that all the `secp256k1_context_sign` that were replaced in this commit were not used (apart from the occasional `VERIFY_CHECK(ctx != null)`). I looked through the secp256k1 code of each function to confirm that they do not use the dynamic context, so it is safe to replace them with `secp256k1_context_static`.

I also confirmed that the changed code in `key.cpp` is covered by tests,
...

🚀 fanquake merged a pull request: "ci: remove Clang build from msan fuzz job"
(https://github.com/bitcoin/bitcoin/pull/33425)

💬 fanquake commented on pull request "Backport Cirrus runners to 29.x":
(https://github.com/bitcoin/bitcoin/pull/33403#issuecomment-3324202757)
Can you also add #33425 here?

💬 willcl-ark commented on pull request "Backport Cirrus runners to 29.x":
(https://github.com/bitcoin/bitcoin/pull/33403#issuecomment-3324205112)
> Can you also add #33425 here?

Sure

💬 Kruwed commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3324205324)
> Even if I conceded that it is a footgun (which I'm not) to set `datacarriersize` I would still prefer no "gun control". Especially in a case where "self-harm" is the only real concern.

The real concern is that the people wanting to set the datacarrier options are under the delusion that they can use it maliciously to censor others' transactions. The self harm is not their main goal, it's just a side effect.

✅ fanquake closed a pull request: "ci: Skip Centos job on GHA runners"
(https://github.com/bitcoin/bitcoin/pull/33365)

💬 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)