💬 maflcko commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3347684298)
> For me it is just weird to see the script failing here since creating a file or dir shouldn't fail

It actually should fail. The goal of the check is to verify no mainnet datadir access is happening from any of the tests. Maybe the comment could be clarified to mention this?

(Disabling this check would be better by just removing it)

💬 fjahr commented on pull request "rpc: Fix dumptxoutset rollback with competing forks":
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3347787910)
> Couldn't this just be a contrib script or client-side bitcoin-cli feature?

FWIW, the rollback part used to be a contrib script and I integrated it into `dumptxoutset` for better robustness and easier testing in #29553. But the idea of implementing it client-side in bitcoin-cli was never discussed as far as I can remember.

📝 MateuszBratyslawski opened a pull request: "Create code"
(https://github.com/bitcoin/bitcoin/pull/33499)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 MateuszBratyslawski commented on pull request "Create code":
(https://github.com/bitcoin/bitcoin/pull/33499#issuecomment-3347795916)
not sure about this one

💬 Aa777263100 commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2388491359)
fab9ddaf554837624fa8f388e046a30d5bf7626f

🤔 l0rinc reviewed a pull request: "validation: fetch block inputs on parallel threads >10% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-2831320761)
I have re-reviewed the changes again lightly and did quite a few benchmarks on different platforms.
There were a lot of surprises, see my measurements:

<details>
<summary>rpi5-16 IBD from local node or & reindex-chainstate seem is ~27% faster</summary>

```
COMMITS="688c03597afb0b76077f1ffc4608eef19481056e af8a366bd6a08d9362e69a89b0b89b5c94eb63ca"; \
STOP=915961; DBCACHE=450; \
CC=gcc; CXX=g++; \
BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2083465171)
We should let the objects do their job instead of querying their internals and doing it ourselves ("tell, don't ask"):
```suggestion
m_chainman.FetchInputs(CoinsTip(), CoinsDB(), blockConnecting);
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388250227)
what if instead of locking we just iterate every `n`th element (where `n` is the thread index), implicitly dividing the input into `n` buckets without locking. Each thread would work on a distinct set of values - we can pre-filter for existing values on a single thread before forking off. This won't have work stealing, but we can likely assume uniform distribution and the solution would be trivial and lock free.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388213294)
nit: is there anything in the error that we may want to log?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388258759)
nit: the curse of long review queues
```suggestion
// Copyright (c) 2025-present The Bitcoin Core developers
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388224454)
instead of the comment, can we express this in the method name?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388218975)
I haven't reviewed it in detail but was wondering why we need locking here, it should be possible to do most of this lock free (especially if we sort the keys first so that threads are more likely to access different regions). I have started reviewing and testing it in detail, but to have some progress I'm sharing my observations as I go along

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388256954)
could we pre-filter on a single tread and send the results to the fetcher instead? That way we can also decide not to do multi-threaded access for small sets (we can experiment with the values, but we can probably start with set size < nproc should be done on a single thread).

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388236852)
I have tried `std::jthread` in [l0rinc/bitcoin@`6afe2e8` (#40)](https://github.com/l0rinc/bitcoin/pull/40/commits/6afe2e878046703cd71b166a3cf0e53ccd12e478#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dR1363) but it seems the CI's libc++ doesn’t provide it

Q: it's just the second commit and we're already doing the fetching on multiple threads. Can we add a single-threaded input fetcher first and add multithreading only as the very last step?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388273122)
af8a366bd6a08d9362e69a89b0b89b5c94eb63ca
I had something similar in https://github.com/bitcoin/bitcoin/pull/32313/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR254

Can you please explain in the commit message why this change is necessary?

🤔 l0rinc requested changes to a pull request: "validation: fetch block inputs on parallel threads >10% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3280693697)
I have re-reviewed the changes again lightly and did quite a few benchmarks on different platforms.
There were a lot of surprises, see my measurements:

<details>
<summary>rpi5-16 IBD from local node or & reindex-chainstate seem is ~27% faster</summary>

```
COMMITS="688c03597afb0b76077f1ffc4608eef19481056e af8a366bd6a08d9362e69a89b0b89b5c94eb63ca"; \
STOP=915961; DBCACHE=450; \
CC=gcc; CXX=g++; \
BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
...

📝 ishaanam opened a pull request: "p2p: implement sender-initiated package relay"
(https://github.com/bitcoin/bitcoin/pull/33500)
[BIP 331](https://github.com/bitcoin/bips/blob/master/bip-0331.mediawiki) implements receiver-initiated package relay, where receivers can request missing information about packages. However, this requires excessive round trips. If the sender knows that it is giving the receiver a transaction for which it doesn't have the ancestors, it should be able to just send a package (containing the transaction and the unknown ancestor(s)) instead.

This PR could potentially reduce some bandwidth, and c
...

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2388637501)
Is this TODO still relevant? I'm planning to look more in-depth at c14a4bcbb1f84fb4776f43cbb918e7771164029b, but at least on a first rough glance I haven't discovered a scenario where it would be possible to run any of the three methods accessing this map (`FlatSigningProvider::{Set,Get}MuSig2SecNonce` and `FlatSigningProvider::DeleteMuSig2Session`) concurrently for the same `DescriptorScriptPubKeyMan`.

💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348090942)
Thanks! Output from LLDB shows what the problem is and suggests this should be fixed by the patch in #33387, which I will go ahead and make into a PR.

In the stack trace (which I annotated below):

- Thread 1 is the bitcoin node main thread, and it is just waiting for the IPC event loop thread to exit.
- Thread 3 is IPC event loop thread, and it is idle waiting for new I/O events.
- Thread 38 is an IPC worker thread calling `WaitTipChanged`, which is stuck calling `kernel_notifications.m_tip_bl
...

💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348121335)
> I am still unsure why I couldn't reproduce this problem with the python tests I tried to write https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3330765943, and also why the problem did not seem to happen when you called stop RPC instead of using ctrl-c.

FWIW, here's the code I used to trigger this. Like I mentioned previously, it's already outdated since I'm no longer making usage of `waitTipChanged` (and many other parts have also evolved).

https://github.com/plebhash/sv2-bitcoi
...