💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2387502458)
Pushed to b6368fc285bf00b3033061dcd4e29298b227c6df (rebased on top of fc642c33ef28829eda0119a0fe39fd9bc4b84051)

<details>
<summary>Saw the following CI error for macOS 14 with e6853592361341c27103ed74b25470ac1e098d6d</summary>

This seems to be related to PR #30684

```
node0 2024-10-02T00:15:45.783638Z [init] [src/noui.cpp:57] [noui_InitMessage] init message: Verifying wallet(s)…
Error: node0 2024-10-02T00:15:45.783653Z [init] [src/noui.cpp:31] [noui_ThreadSafeMessageBox] [error] In
...

💬 luisschwab commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2387536123)
Concept ACK

💬 maflcko commented on pull request "test: add missing sync to feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31016#issuecomment-2387683977)
lgtm ACK a1576edab356053c4c736691e4950b58e9a14f76

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2387699942)
> I added a commit to bypass some asserts in `ConnmanTestMsg` that would fail since we're bypassing network magic and checksum validation. Also, I think we will have to change `p2p_transport_bidirectional` target since there are some sanity checks that would fail with the bypasses, especially the assertions about `ReceivedBytes`/`GetReceivedMessage`. Any thoughts?

Can you explain why the asserts and sanity checks would be failing? They are unrelated to the checksum, so if they are failing, it
...

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2387782120)
@ryanofsky that worked!

💬 maflcko commented on issue "ci: failure in win64 unit tests":
(https://github.com/bitcoin/bitcoin/issues/30792#issuecomment-2387848251)
I was able to reproduce locally (in a CI container), but given that there is no useful output, it doesn't help:

```
# while ( LC_ALL=C.UTF-8 DIR_UNIT_TEST_DATA=/ci_container_base/ci/scratch/qa-assets/unit_test_data/ ctest --test-dir /ci_container_base/ci/scratch/build-x86_64-w64-mingw32 -j $( nproc ) ) ; do ( echo 1 >> /tmp/runs ) ; done

...

# cat /tmp/runs |wc -l
48

```

The log:

```
...
1/136 Testing: util_test_runner
1/136 Test: util_test_runner
Command: "/usr/bin/cmake
...

💬 laanwj commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#discussion_r1784030205)
even better, can remove the include of `netif.h` here, it's not actually used in `pcp.cpp`

💬 maflcko commented on issue "ci: failure in win64 unit tests":
(https://github.com/bitcoin/bitcoin/issues/30792#issuecomment-2387925869)
Probably the fastest way to reproduce for now inside the CI container:

```
# while ( LC_ALL=C.UTF-8 BITCOINUTIL=/ci_container_base/ci/scratch/build-x86_64-w64-mingw32/src/bitcoin-util.exe BITCOINTX=/ci_container_base/ci/scratch/build-x86_64-w64-mingw32/src/bitcoin-tx.exe /usr/bin/python3 /ci_container_base/ci/scratch/build-x86_64-w64-mingw32/test/util/test_runner.py ) ; do ( echo '...' && echo 1 >> /tmp/runs ) ; done
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...

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