💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681668029)
```suggestion
* expecting a `secp256k1_keypair`. This avoids the need to create a `secp256k1_keypair` object and
```

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681648449)
Might make the code more uniform (and maybe the diff simpler, if we'd extract it as suggested above) if instead of not making this a const cast, like the rest, we rather added the const to the method's signature:
```patch
diff --git a/src/secp256k1/include/secp256k1_extrakeys.h b/src/secp256k1/include/secp256k1_extrakeys.h
--- a/src/secp256k1/include/secp256k1_extrakeys.h (revision 21d031e7867ef97c07cee691c115d59b3aebfd19)
+++ b/src/secp256k1/include/secp256k1_extrakeys.h (date 1721245901582
...

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681661927)
```suggestion
* - Otherwise: tweak the internal key with H_TapTweak(pubkey || *merkle_root)
```

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681636478)
maybe it would simplify the diff if we extracted the keypair.data() here
```C++
KeyPair keypair = ComputeKeyPair();
if (!keypair.IsValid()) return false;
const secp256k1_keypair *keypairData = reinterpret_cast<const secp256k1_keypair *>(keypair.data());
```

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681662778)
```suggestion
* (this is used for key path spending with a specific
```

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681669713)
nit: typo in commit message: `Instead of calling ... instead invalidate`

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681672646)
Might be a naive question, but is this a behavior change, does it have any security implications (e.g. could it leave keys in the memory or something)?

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681676666)
this isn't making the trivial linear graph

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681678886)
Why is this benchmark "LIMO" but not `BenchLinearizeNoItersWorstCaseAnc`?

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681682373)
re: `clusterlin: optimize rechunking in LinearizationChunking`

benchmark doesn't seem to show much improvement for this change. I understood this as purely an efficiency improvement which should be reflected in bench results?

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681705464)
I have incorporated something like this into the DepGraphFormatter comment.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681705729)
I have added a comment to explain this briefly.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681706300)
I have moved the connected-component logic, together with its (now optional) usage to #30286.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681706478)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681706944)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681707363)
I don't understand this comment.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681707992)
I have moved this optimization (it is indeed supposed to be one) to #30286.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681708231)
Done!

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2234250901)
I have made a number of changes:
* Addressed a number of typos
* Moved connected-component logic to #30286 (where `MakeConnected` is now optional, and `FindConnectedComponent` has its own fuzz test).
* Moved the chunk-caching logic in `LinearizationChunking` to #30286.
* Added more explanatory comments, especially in the `DepGraphFormatter` format description.
* Made the `DepGraphFormatter` deserializer resilient to EOF (previously it would return an empty graph in this case), plus added a
...

💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-2234439130)
@hebasto it seems there are some [issues](https://gitlab.freedesktop.org/wayland/wayland/-/issues/233) with Wayland on how these actions are being handled ([this one specific](https://bugs.kde.org/show_bug.cgi?id=462574) to KDE but found some old ones in [gnome](https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/730)).

As a workaround (tried other things as well), might be not elegant, but this works fine on both X11/Xorg and Wayland:

```diff --git a/src/qt/transactionview.cpp b/src/qt/tr
...