💬 achow101 commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3549269412)
> we can either add that in a follow-up

I'm not merging something that doesn't compile on my machine, it will break merging of other things.

💬 achow101 commented on issue "29.x depends: fallback server missing capnp downloads":
(https://github.com/bitcoin/bitcoin/issues/33901#issuecomment-3549276978)
Backport #33580?

We can also upload the tarball.

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3549356286)
> Agree that deduplicating this as a follow-up is a good idea. Maybe even something like a fancy ForkGenerator class would make sense

I think it would also be good to have optional args that make non-empty forks to test mix-and-match cases of things re-entering the mempool / already in.

For now I think this is RFM.

💬 achow101 commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3549362236)
I think it would be better to have the `prepareTransaction` have a `sign` parameter that is set if "Create Unsigned" is selected. Having PSBT silently drop `scriptSigs` (and `scriptWitness`es too) feels like there is the possibility for misuse/unexpected behavior. `PartiallySignedTransaction` probably needs some changes to more strongly enforce its invariants.

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3549372484)
> I've been playing around with the fuzz test and wanted to update here, even if it's still a bit inconclusive.
>
> I'm running into non-reproducible timeouts when using `fork` with libFuzzer. I'm overusing my cores by a lot (`fork=25` plus the 3 workers each), and I'm not yet sure if these timeouts would happen eventually when using less cores.
>
> I have "fixed" this by changing `notify_one` in `Submit` to `notify_all`, although I can't say I'm exactly sure why this works. It almost seem
...

💬 achow101 commented on issue "depends: fallback server missing Qt downloads":
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3549403050)
The file is located at https://bitcoincore.org/depends-sources/CMakeLists.txt-6.7.3

Not sure how to have download figure that out.

🤔 maflcko reviewed a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3479655338)
review ACK a1f76230209629c5141984aaac1cc4ba7b4f761a 💨

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK a1f76230209629c514198
...

💬 jlopp commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-3549483423)
For the record, I do believe there is a strong use case for this feature. In particular, due to the travel rule. We have clients who get requests to prove the ownership of their wallet and since they can't use the legacy message signing option with their multisig wallet, the exchanges and other service providers are requiring them to send dust-level amounts of bitcoin to prove control. This, of course, is not really a viable long-term strategy.

💬 achow101 commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3549508955)
> To compute the short channel Id we simply call `getrawtransaction` which gives us the block hash for the block that confirmed the tx, then load that block to get the tx position in that block.

So you need txindex and make 3 RPC calls? ISTM storing the position in this new index would make it much more efficient and more useful to other people.

✅ ryanofsky closed an issue: "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage"
(https://github.com/bitcoin/bitcoin/issues/33554)

💬 ryanofsky commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3549515533)
> [@ryanofsky](https://github.com/ryanofsky) should we close this?

Thanks will close. The original issue where then node crashes when a client requests to multiple IPC calls to execute simultaneously on the same worker thread should be fixed by https://github.com/bitcoin-core/libmultiprocess/pull/214 and https://github.com/bitcoin/bitcoin/pull/33519 which replaces the crashes with "thread busy" errors.

The crash reported later where killing the rust code in the middle of a waitNext call causes
...

💬 achow101 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2539683665)
nit: This is an unnecessary style change.

💬 achow101 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2539683746)
nit: This is an unnecessary style change.

💬 ryanofsky commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3549542846)
Rereading the issue, I noticed I missed the references to docker, and just posted about alternatives to TCP generally.

As far as I know, there should be no issue sharing the IPC socket between the docker containers or between the host and a container on linux. The socket should look like normal file on the filesystem (`node.sock`) that can be shared with docker's `--volume` / `-v` option. On macos I don't think it's possible to share the socket between a host and container, but it should be pos
...

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2539707339)
Done.

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2539707584)
Done as suggested.

👍 hebasto approved a pull request: "ci: Consistenly only cache on the default branch"
(https://github.com/bitcoin/bitcoin/pull/33905#pullrequestreview-3479795946)
ACK fa411f938e4796cf3806f234c9787b7c79492cc2.

💬 ryanofsky commented on issue "rfc: virtio-vsock for RPC and IPC":
(https://github.com/bitcoin/bitcoin/issues/33897#issuecomment-3549565261)
It should be pretty easy to support vsock if we want that. I think it would only require a change to the [`ipc::ParseAddress`](https://github.com/bitcoin/bitcoin/blob/2444488f6ad32dcbbed51a73cd4f59ff3a239e32/src/ipc/process.cpp#L71-L98) function and updates to documentation and tests.

TCP support could also be added by modifying the `ParseAddress` function, but I'd be wary of doing that since it would not really be safe without authentication (https://github.com/bitcoin/bitcoin/issues/32802#iss
...

🤔 achow101 reviewed a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-3479788805)
ACK f53dbbc5057b6f676db4be9bc720898149f293fc

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2539711136)
In f53dbbc5057b6f676db4be9bc720898149f293fc "test: Add functional tests for named argument parsing"

nit: Logging at the end of the test case is unnecessary