💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004261310)
if you decide to keep this test (which was meant to demonstrate the safety of the change), please remove the logging part

💬 ryanofsky commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2738079385)
Updated ecd8e80fdef8675991d5bb3f9effd83a4ac97d2a -> 26c2136a2bc7fec9697f61eadf422c439e0735a2 ([`pr/ipc.215`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc.215) -> [`pr/ipc.216`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc.216), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc.215..pr/ipc.216)) to CI fix https://cirrus-ci.com/task/5047408398172160?logs=ci#L2206 caused by wallet_encryption.py not checking wallet process log file

💬 mabu44 commented on pull request "net: Block v2->v1 transport downgrade if !fNetworkActive":
(https://github.com/bitcoin/bitcoin/pull/32073#issuecomment-2738096709)
ACK 6869fb417096b43ba7f74bf767ca3e41b9894899
Reviewed the code and tested it with the command provided in https://github.com/bitcoin/bitcoin/pull/32073#issuecomment-2731477125. I was able to reproduce the problem on master only once and never on this branch.

💬 hodlinator commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-2738114741)
> https://bitcoin.stackexchange.com/questions/122646/libxcb-xinerama0-library-required-by-bitcoin-qt

(This seems to have led to #30061 with the same title).

💬 hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2003066433)
Thread https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001211668:

Now that the dependencies for bitcoin-qt double from 12->24 it almost feels like they should be in their own list, so we don't accidentally introduce dependencies on them from the CLI binaries?

I can work on a patch for that if there is interest.

💬 hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2003451280)
Should ninja-build and CRB be mentioned in build-unix.md or dependencies.md?

👍 hodlinator approved a pull request: "build: Switch to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2697929054)
ACK 94967c353ed8041115625b6b3c9acba7961e1f20

- Beginner at CMake/Qt, but couldn't spot anything malicious-looking in CMake changes nor Qt 6 patches.
- Have not tested MacOS builds. Only Linux, Windows Native, Windows Cross, see [Concept review](https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2572359973).

<details><summary>sha256sum-detour</summary>

> hebasto:
> [My Guix build:](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2736277550)

Surprisingly to me
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2738142996)
@hodlinator
> Building on `x86_64`. Maybe just a dirty tree leaking into guix somehow, but surprised that one of the builds matched. :\
> Also built aarch64-linux-gnu, arm-linux-gnueabihf, powerpc64-linux-gnu, riscv64-linux-gnu and x86_64-linux-gnu.

... and hashes do _not_ match https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2736277550, right?

💬 hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2738151903)
> @hodlinator
>
> > Building on `x86_64`. Maybe just a dirty tree leaking into guix somehow, but surprised that one of the builds matched. :\
> > Also built aarch64-linux-gnu, arm-linux-gnueabihf, powerpc64-linux-gnu, riscv64-linux-gnu and x86_64-linux-gnu.
>
> ... and hashes do _not_ match [#30997 (comment)](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2736277550), right?

Yes, they do *not* match:

<details><summary>hashes</summary>

```
₿ find guix-build-$(git rev-
...

💬 mabu44 commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2738161185)
In reference to my first comment https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2731126667, I note that the imported descriptor and the one returned in the error message are not equal. I imagine that the one in the error message represents the public key for the provided private key. This could reduce the clarity of the error message that no longer enables a direct comparison with the imported descriptors to understand which one generated the error. On the other end, we do not return
...

💬 willcl-ark commented on issue "test: No unit test covers BIP342 tapscript signatures":
(https://github.com/bitcoin/bitcoin/issues/32012#issuecomment-2738172847)
I also cannot reproduce OP:

```bash
will@ubuntu in …/src/core/bitcoin on  master [$!?] via △ v3.31.6 : 🐍 (core)
$ git diff -p | cat
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index a35306b6935..d2904fb3f14 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1480,7 +1480,7 @@ bool SignatureHashSchnorr(uint256& hash_out, ScriptExecutionData& execdata, cons
uint8_t ext_flag, key_version;
switch (sigversion) {
case SigVersion::TAPR
...

🤔 sipa reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2700118245)
Big change: `std::vector<ClusterSet>` is gone, replaced by `m_main_clusterset` and `m_staging_clusterset`.
* New helper functions `GetTopLevel()`, `GetSpecifiedLevel()`, `GetClusterSet()` made this fairly easy to do.
* Simplifications resulted in `ClearLocator`, `GetConflicts`, `PullIn`, `MakeTransactionsMissing`, `StartStaging`, `CommitStaging`, `ExtractCluster`.
* `CopyTo` was renamed to `CopyToStaging`.
* `LevelDown` was renamed to `MoveToMain`.
* `MakeTransactionsMissing` was renamed to
...

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004315650)
`NEEDS_SPLIT_OPTIMAL` is gone now after further discussion. Marking resolved.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004306838)
@ajtowns That comment was put in the wrong commit. I've fixed it.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004287728)
Late update (discussed elsewhere): this property actually did not hold. Added comments, and partially rewritten.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004304219)
Done.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004308947)
I added the following comment:

```
// This is a big simulation test for TxGraph, which performs a fuzz-derived sequence of valid
// operations on a TxGraph instance, as well as on a simpler (mostly) reimplementation (see
// SimTxGraph above), comparing the outcome of functions that return a result, and finally
// performing a full comparison between the two.
```

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004286166)
See my comment here: https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2734449766 about how `Assume()` gives you compiler-guaranteed side-effect-equivalence, largely without runtime cost.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004312349)
Together with a lot of changes (getting rid of the `ClusterSet` vector), done.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2004316851)
Done.