💬 dayer3 commented on issue "Error: Cannot resolve -bind address: 'bitcoind:8334=onion'":
(https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-3385337855)
Hi all,
Sorry, so what is the recommended workaround for creating a Bitcoin Core onion service if Tor service is running on a different host or container, without following the _Manually create a Bitcoin Core onion service_ way and bind=a.b.c.d:8334=onion?
Best regards

💬 maflcko commented on issue "29.x: using a local libmultiprocess install will no-longer work":
(https://github.com/bitcoin/bitcoin/issues/33576#issuecomment-3385348769)
> This does seem like something good to document. Maybe the 29.x [dependencies.md](https://github.com/bitcoin/bitcoin/blob/29.x/doc/dependencies.md) file could document the version of multiprocess known to work with v29, consistent with the version used in depends

Yeah, makes sense to document this. (Looks like it isn't documented at all right now). Just documenting the exact commit should be good enough and I think tagging isn't needed, but no strong opinion.

💬 sipa commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385401468)
@maflcko My understanding is that from the Python side, GIL or no GIL is largely unobservable, apart from a few minor things. How do you think it would affect our test framework?

💬 fanquake commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385402711)
> I'd expect there is a bunch of our own test code which relies on the GIL. I guess there is no way to find such "unsafe" Python code other than to try to run with the GIL disabled and wait for an intermittent issue to pop up at some point in time?

Possibly; however this was the only test I've seen fail so far. I'm just assuming devs/people will start using the free-threaded pythons (maybe in the near future) on their machines, and if they do, this test will fail.

💬 ryanofsky commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385442350)
Not sure about the larger issue, but it seems practically speaking it might be nice if test framework just ignored or suppressed the GIL RuntimeWarning for interface_ipc.py if the test otherwise passes.

🤔 janb84 reviewed a pull request: "depends: Use $(package)_file_name when downloading from the fallback"
(https://github.com/bitcoin/bitcoin/pull/33580#pullrequestreview-3318653692)
ut ACK 671b774d1b58c491b53f2b2f6ee42fb6b65a0e71

PR changes the third argument of `fetch_file_inner()` in the fallback scenario. This change ensures that, in the fallback scenario, the "friendly filename" is used to download the file instead of the "filename."

The file contains only the download logic, and I haven’t been able to verify whether the "friendly filename" is constructed the same way for the upload to the fallback server.


<details><summary>code analysis</summary>

The code
...

💬 l0rinc commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416657216)
Updated the PR description, thanks.

💬 l0rinc commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416662941)
> a more minimal one-line temporary(?) workaround

I don't like throwing primitives anyway, so I don't think the current change is a workaround only.

💬 l0rinc commented on pull request "doc: bump the template macOS version since 14 is now the minimum supported version":
(https://github.com/bitcoin/bitcoin/pull/33573#discussion_r2416668248)
+1 (except the double space in the suggestion)

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2416674361)
I have extracted it on purpose to signal that both branches are doing something similar and have the same dependencies

💬 maflcko commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416744379)
https://clang.llvm.org/extra/clang-tidy/checks/misc/throw-by-value-catch-by-reference.html says "the usage of string literals is idiomatic." But no strong opinion.

💬 ryanofsky commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2416741361)
re: https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2414400956

Not important, but FWIW sjors suggestion makes a little more sense to me to I don't see sending unknown log messages to the trace stream, which is effectively a black hole, as being more conservative. IMO ideal thing to do would be to return `std::optional` and `nullopt` and then log unknown messages at debug level with a prefix like "[unrecognized flags "0x%08x]".

💬 ryanofsky commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2416732939)
re: https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410969953

Yeah just a tradeoff between having extra variables and having an extra level a nesting. I think having more variables is worse, but no strong opinion and you should follow your preference

👍 ryanofsky approved a pull request: "multiprocess: Fix high overhead from message logging"
(https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3318880294)
Code review ACK dcaf4b736f9ddfc6a2695ce19221a7f383e43e20. Just rebased onto #33518 and changed assert(false) to log trace and since last review

re: https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3313376282

> The former seems too chatty, the latter too quiet.

Good observations, created https://github.com/bitcoin-core/libmultiprocess/issues/227 to track this

🤔 brunoerg reviewed a pull request: "test: add functional test for `TestShell` (matching doc example)"
(https://github.com/bitcoin/bitcoin/pull/33546#pullrequestreview-3318966739)
Concept ACK

💬 jonbourne commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3385887546)
Please do not release this version now. Reschedule and rethink. It has many people who disagree with choices made, and they have not been discussed enough to release the code. This is a very important turning point, and the implications are very difficult, if not impossible, to take back. Specifically, remove any code that touches OP_RETURN. Please do not release this version.

🤔 cedwies reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3318992246)
Tested ACK 44a7261
Unit/Functional tests pass.
Checked `git grep RelayTransaction`

💬 cedwies commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2416807372)
Nit: Maybe we should change this log message.
Something like
`LogPrint(BCLog::NET, "Force scheduling tx %s for broadcast\n", tx.GetHash().ToString());`
That wording would better match the new method name and clarifies that we are queuing it, not sending instantly.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416836217)
There are two reasons for this. One is that if you define a struct, the user has to instantiate that struct and copy the data to it instead of providing a pointer directly. This also introduces some questions around the lifetime of the data. I still think this function is straight forward enough to use. This might look clunky in C, but we don't expect many developers to use the C header directly. Language bindings can provide nicer ways to use it.

💬 aimtiaz11 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3385946895)
Please let common sense prevail and avoid making this regrettable mistake of increasing OP_RETURN. It’s not too late.