💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556526561)
Done in 04d1dc78599362623bdd4b55c7be0c7b0f9f2174

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556529044)
Resolving since commit message changed so no longer relevant.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556535563)
Done in the latest push to 039c3aab3b79e61e8ffe193bffee36b826ca2984

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556547584)
Done in f07ef4149b89d2c1d71af2cafd1bf29f6661b35a which introduces `rand_path` using strong randomness, and `-fuzzcopydatadir` and uses both in the same commit.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556550709)
Resolving as the commit has been removed.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2556552245)
Resolving as the commit has been removed.

💬 maflcko commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3571111965)
Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?

Ref: https://codereview.qt-project.org/c/qt/qtwayland/+/418094

lgtm, but I haven't tested this.

review ACK 0672e727bf1db5995562e9656d18b699aeba5fe0 🎩

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_an
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556578753)
Ok, if a transaction that we are trying to broadcast gets double spent then this will be handled by the `else` branch below:

https://github.com/bitcoin/bitcoin/blob/73ea6405da30725358cd2973a8898d9b0ba719cd/src/net_processing.cpp#L1676-L1693

🤔 hodlinator reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3499037428)
Reviewed 78d6402458d7d14533444d893c989f0534a3896f

Main issue blocking A-C-K is in doc/REST-interface.md (see inline comment).

Left minor comment/question in related presentation: https://docs.google.com/presentation/d/1Zez-6DApKRu59kke4i_g9jwxQlaFKKRpOPdYFYsFXfA/edit?disco=AAABwl3JY0U
(I was re-reading it to remind myself why switching from a tx hash to a block hash would be more efficient, re-realized that mapping block height=>block hash is quite natural).

Experimented with further optimiza
...

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2555214226)
Incorrect names:
```suggestion
- `GET /rest/blockpart/<BLOCK-HASH>.<bin|hex>?offset=X&size=Y`
```

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2555229245)
nit: Consistency
```suggestion
* - `/blockpart/` via `rest_block_part()` (doesn't support JSON response, so `tx_verbosity` is unset)
```

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2555223914)
nit: I think it's worth including the parameters to make the reason behind adding this more understandable.
```suggestion
- A new REST API endpoint (`/rest/blockpart/BLOCKHASH?offset=X&size=Y`) has been introduced
```

💬 plebhash commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3571209737)
I ran two sessions with https://github.com/bitcoin/bitcoin/pull/33936 at a3a6861e131120eabf6e2f7ecd15e5ea805c66b2, where the client was our Sv2 Rust code

both of them had at least one chain tip update, which should trigger templates being flushed from memory (so I'd expect to see a sharp decline in RAM consumption)

in the first one, we're not calling `destroy`:

<img width="640" height="480" alt="Image" src="https://github.com/user-attachments/assets/3e3cb655-8069-4107-8532-bed5455189dd" />

i
...

💬 ryanofsky commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3571213239)
re: https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428

> do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?

Following up on this, it's good practice to call `destroy` methods on objects which have them, to guarantee the objects are destroyed right away instead of asynchronously. This is most important if objects have nontrivial destructors, or if it matters what order objects get destroyed.

Block templates usi
...

👍 fanquake approved a pull request: "ci: Use latest Xcode that the minimum macOS version allows"
(https://github.com/bitcoin/bitcoin/pull/33932#pullrequestreview-3500865738)
ACK https://github.com/bitcoin/bitcoin/pull/33932/commits/fa9537cde10120b12c96061cbc3f79a7680f9d64 - seems fine.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556652874)
Hmm, is this a general frowning against `size_t`?

Atomic is not necessary for `num_broadcasted`.

My thinking is that CPUs work best with word sized integers, so use that when the values will be small and there is no concern about overflow. 64 bit integers on 32 bit CPUs will bloat the program unnecessary and 32 bit integers on 64 bit CPUs may end up not saving space or be slower because of alignment.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556657318)
It is correct either way. Performance is irrelevant here. Leaving it as it is for now unless somebody asks for a change. Lets move on to more important things.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3571252838)
`73ea6405da...7826148b12`: do https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2553247864

💬 furszy commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2556684462)
As the index stores the tx position in disk, how is this correct?

👍 darosior approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3500924127)
reACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e