💬 maflcko commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#discussion_r1784062654)
For reference, according to the iwyu run by CI, as part of the `tidy` task:

```
[13:49:05.974] /ci_container_base/src/common/pcp.cpp should add these lines:
[13:49:05.974] #include <algorithm> // for __equal_fn, equal
[13:49:05.974] #include <compare> // for operator<, strong_ordering
[13:49:05.974] #include <cstring> // for size_t, memcpy
[13:49:05.974] #include <functional> // for function
[13:49:05.974] #include <map> //
...

💬 maflcko commented on pull request "ci: add timestamps to cirrus jobs":
(https://github.com/bitcoin/bitcoin/pull/30981#issuecomment-2387952175)
> It may be a bit annoying when looking at functional test failures, which have a timestamp included and would then get a "wrong" timestamp, but this seems fine .

Another thing that is a bit annoying is the iwyu output in the final full log, see e.g. https://github.com/bitcoin/bitcoin/pull/31011#discussion_r1784062654

But the output would need to be edited anyway, so this seems fine as well.

⚠️ RCasatta opened an issue: "Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/issues/31017)
### Please describe the feature you'd like to see added.

I would like to have the possibility to broadcast a transaction via the REST interface which is currently not possible https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md

### Is your feature related to a problem, if so please describe it.

An indexer like the ones for the electrum protocol should work without having access to the RPC interface, I think the only missing method from the REST interface is the tx broadcast.

...

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2388071828)
> ramdisk

Benchmarking this is a bit more involved and I think requires changes to the CI scripts, so that the ramdisk is (1) mounted into the CI container and (2) picked up as the temp dir. I can take a look in the future, unless someone beats me to it.

💬 fanquake commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723)
> Tried on Intel macOS 15.0 (Xcode 16.0) and 13.7 (Xcode 15.2):

Tried which code? If this was a clean build, according to the log, it's also skipped building Boost for some reason?

This looks odd, because you seem to be hitting code paths for macOS cross-compilation, when building natively. i.e when building on macOS, the tooling should be found by calling `xcrun`:

https://github.com/bitcoin/bitcoin/blob/fc642c33ef28829eda0119a0fe39fd9bc4b84051/depends/builders/darwin.mk#L4-L8

I got
...

💬 fanquake commented on pull request "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0":
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-2388089344)
Not sure about this issue, or making this change. Looks like it needs more investigation. See my comment here: https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723.

💬 antonilol commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-2388103756)
> According to https://en.wikipedia.org/wiki/SipHash key size is 128-bit as standard and that's what is currently supported in Bitcoin Core. Having a smaller key might increase DoS risk to much after all?
> + The key is only stored in one entry in LevelDB.
> + This type of computation using data that is cache-coherent (8 bytes next to the first 8 bytes of the key) should be exceedingly cheap.

You are right, siphash is designed for a 128 bit key, so lets keep it 128 bit then. It is not a cry
...

💬 fanquake commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1784172617)
> This PR isn't in response to any issue or comment. I just noticed it may be useful to have a note about relinking.

We don't really add general things to our build instructions that "may" be useful, especially if they aren't addressing a problem someone has encountered. In any case, in this instance, if your package manager is broken, or incorrectly installing dependencies such that they are broken, that isn't a Bitcoin Core issue, and I don't think we need to add anything here.

💬 dergoegge commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2388163491)
> They are unrelated to the checksum, so if they are failing, it seems like this patch is incomplete and some part is still checking the magic or checksum, no?

I'm guessing that we need to make sure that (if we are fuzzing) `V1Transport::SetMessageToSend` creates valid checksums/magic-bytes according to the new fuzzing mode check.

✅ itornaza closed a pull request: "refactor: Appropriate re-naming of MAX_OPCODE after tapscript"
(https://github.com/bitcoin/bitcoin/pull/30953)

💬 itornaza commented on pull request "refactor: Appropriate re-naming of MAX_OPCODE after tapscript":
(https://github.com/bitcoin/bitcoin/pull/30953#issuecomment-2388190304)
@darosior, highly respecting your opinion and taking note of your suggestions, I am close this pr.

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2388224233)
> Can you explain why the asserts and sanity checks would be failing? They are unrelated to the checksum, so if they are failing, it seems like this patch is incomplete and some part is still checking the magic or checksum, no?

Some previously valid magic or checksum could be now be considered invalid. But I removed the commit that bypasses the asserts and changed the verification to:
```
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
if ((hdr.pchMessageStart[0] & 1) != 0 && hdr.pchM
...

✅ fanquake closed an issue: "ci: Intermittent issue in feature_fee_estimation.py in sanity_check_rbf_estimates (Block sync timed out after 2400s)"
(https://github.com/bitcoin/bitcoin/issues/30990)

✅ fanquake closed an issue: "Intermittent failure in feature_fee_estimation.py in sanity_check_rbf_estimates: est_feerate = node.estimatesmartfee(2)["feerate"] (KeyError: 'feerate')"
(https://github.com/bitcoin/bitcoin/issues/30640)

🚀 fanquake merged a pull request: "test: add missing sync to feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31016)

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1784260961)
I used `std::string` at first too, but then it was inconsistent with the rest of the documentation, which I think is even more unclear. I'll reconsider updating all `std::string` references in this block if I have to force-push again unless you think this is blocking?

💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2388309124)
This discussion might be related: https://discourse.cmake.org/t/is-there-a-downside-for-a-toolchain-to-set-vars-in-cache-instead-of-init/12034.

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388310577)
> It would be cleaner _in the implementation_ to use `util::Result` instead of overwriting the returned value with the error message

I don't think that's a good approach for our codebase, even if there was minimal code churn. All format strings are known at compile-time, so any formatting errors are programming errors. Handling that with `util::Result` would be both a cumbersome interface and bad practice imo. We don't need to let callsites handle formatting errors, they should never occur, b
...

✅ maflcko closed an issue: "Intermittent timeout in tsan feature_init.py"
(https://github.com/bitcoin/bitcoin/issues/30586)

💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2388314370)
Closing for now, but this can be reopened when it happens again the next time.