💬 theuni commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1909384870)
`sha256sum` has never existed by default on macOS afaik. That's actually why this variable exists in the first place. perl or no, shasum is always there.

It's not present for me on Sonoma.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909385059)
I believe it is safe, both with RVO and without (but since C++17 RVO is mandatory, so that suffices).

With RVO, `ret` is constructed directly in the caller's target destination, so this isn't a pointer to local stack space.

Without RVO, the `Ref(Ref&&)` move constructor is invoked, which will update the pointer to the caller's destination.

👍 theuni approved a pull request: "doc: upgrade license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31611#pullrequestreview-2540859694)
ACK b537a2c02a9921235d1ecf8c3c7dc1836ec68131

Just missed it for 28.1, but this needs a backport in case we cut a future v28.

💬 hebasto commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1909390612)
Thanks! This change has been dropped.

💬 l0rinc commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1909395238)
hmmm, it was working for me on Sequoia - it likely comes from `coreutils`, right?

💬 hebasto commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1909397141)
FWIW, on my Sequoia (Intel):
```
% whereis sha256sum
sha256sum: /sbin/sha256sum /usr/share/man/man1/sha256sum.1
% sha256sum --version
sha256sum (Darwin) 1.0
```

👍 TheCharlatan approved a pull request: "depends: Fix spacing issue"
(https://github.com/bitcoin/bitcoin/pull/31627#pullrequestreview-2540881009)
ACK 8a46286da667d19414c30350df48ebf245589e32

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909403690)
> Why is this doing an effective swap?

I think this is quite common, that move-construction is effectively performing a swap.

> I would expect this to call `UnlinkRef` on the moved-from value and reset its `m_graph` and `m_index`

That's possible too, and slightly more efficient I guess.

> Otherwise it wouldn't be unlinked until the moved-from variable goes out of scope, no?

Indeed. I don't think that's a problem.

💬 TheCharlatan commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#discussion_r1909426772)
I meant these checks:
https://github.com/bitcoin/bitcoin/blob/master/src/secp256k1/src/modinv32_impl.h#L414-L419
https://github.com/bitcoin/bitcoin/blob/master/src/secp256k1/src/modinv32_impl.h#L456-L459

But I guess they are not really that important and it is not worth adding the required utilities?

💬 luke-jr commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1909438136)
The manpage says we need `#include <sys/param.h>` too

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/statfs.2.html

📝 hannahz12 opened a pull request: "upstage install"
(https://github.com/bitcoin/bitcoin/pull/31631)
**upstage install**

✅ maflcko closed a pull request: "upstage install"
(https://github.com/bitcoin/bitcoin/pull/31631)

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909458303)
Anyway, done!

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909458391)
Done.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909462275)
It terminates because:
* Every chunk contains at least one element (added an Assume for that)
* In the inner loop, one element from that chunk is Reset() (added an Assume that it indeed resets a bit that was previously set).

I've changed it to a `do {} while(chunk.transactions.Any());` loop in the first commits, though it reverts back to a `while (true) { ... }` loop later, when the loop becomes a bit more complex.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909463152)
Done. I've also added a comment to the `Cluster::ApplyRemovals()` function definition stating that at least one element from the front of `to_remove` must belong to `this` Cluster (which is really why that requirement exists).

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909463444)
Done. The third one disappears in a later commit, though.

📝 fanquake locked a pull request: "upstage install"
(https://github.com/bitcoin/bitcoin/pull/31631)
**upstage install**

🤔 mzumsande reviewed a pull request: "kernel: Move block tree db open to block manager"
(https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2540983303)
Code Review ACK b776e40554c8a315d832f3996d14ff2e3ae7b8cb

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2581309699)
A few changes:
* Added a `TxGraph::DoWork()` functions which performs queued-up computations now, so that future operations are fast.
* Make `Ref&` arguments to `TxGraph::AddDependency`, `TxGraph::RemoveTransaction`, and `TxGraph::SetTransactionFee` const. They do not modify the `Ref` itself; they only modify what the `Ref` points to.
* Address review comments by @ismaelsadeeq here, and @theuni on #31553.