💬 fjahr commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411433522)
Might be good to duplicate for the avoidance of doubt.

💬 fjahr commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#issuecomment-3377941576)
Looks good to me, this matches what I run to get experimental secp branches. I think @furszy 's comment should be addressed then this should be good to go.

🤔 Ystel2001 reviewed a pull request: "[29.x] finalise v29.1"
(https://github.com/bitcoin/bitcoin/pull/33271#pullrequestreview-3311286244)
SSH

💬 epiccurious commented on issue "30.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/33369#issuecomment-3377969964)
Nit feedback -

> Before compiling, make sure that your system has all the correct [dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) installed (note the new Cap'n proto (optional) build dependency)

`(optional)` should be either removed or changed to `(required by default)`, since capnp is now required unless disabled with the `DENABLE_IPC=OFF` cmake option.

For example:

> Before compiling, make sure that your system has all the correct [dependencies](https://g
...

💬 janb84 commented on issue "30.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/33369#issuecomment-3377982088)
> Nit feedback -
>
> > Before compiling, make sure that your system has all the correct [dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) installed (note the new Cap'n proto (optional) build dependency)
>
> `(optional)` should be either removed or changed to `(required by default)`, since capnp is now required unless disabled with the `DENABLE_IPC=OFF` cmake option.
>
> For example:
>
> > Before compiling, make sure that your system has all the correct [depen
...

🤔 stickies-v reviewed a pull request: "validation: Improve warnings in case of chain corruption"
(https://github.com/bitcoin/bitcoin/pull/33553#pullrequestreview-3309605995)
Approach ACK, agreed that something non-invasive like logging is the way to go here.

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2410252917)
nit: trivial docstring, would prefer removing (or elaborating)

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2411490892)
nit to respect line-length, clang-format and grammatically make things clearer that the peer in question did not send the previous version of the header:

<details>
<summary>git diff on d55830cd50</summary>

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 073a434335..4574c6cf19 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2957,8 +2957,10 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer,
if (!processed) {

...

💬 Sjors commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411501268)
Done

💬 vasild commented on issue "build: depends sqlite compile failure for FreeBSD Clang cross":
(https://github.com/bitcoin/bitcoin/issues/33560#issuecomment-3378059234)
When I run this on Ubuntu 24.04.3 I get:

```sh
$ make -C depends/ HOST=aarch64-unknown-freebsd CC=clang CXX=clang++ CFLAGS="--sysroot=/path/to/sysroot/ " CXXFLAGS="--sysroot=/path/to/sysroot/ -stdlib=libc++" LDFLAGS="-fuse-ld=lld" AR=llvm-ar STRIP=llvm-strip NM=llvm-nm RANLIB=llvm-ranlib NO_QT=1
...
Configuring boost...
-- The CXX compiler identification is Clang 18.1.3
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - failed
-- Check for working CXX compiler: /usr/bin/cla
...

💬 stringintech commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2411570436)
Yes. I am compiling using `afl-clang-fast++` (it seems that other options are either unavailable or obsolete on macOS). However, I just added a `#undef __AFL_LOOP` at the top of `fuzz.cpp` to disable persistent mode (I made sure I no longer see the `[+] Persistent mode binary detected` log when running) and reverted the above change to `rand_path()`, but the same crash still occurs.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3378291849)
Compared it against master on a Raspberry Pi 5 synchronizing from real peers for realism, ran it twice for good measure until 917000 blocks with dbcache 450:

This isn't the [latest version](https://github.com/bitcoin/bitcoin/compare/a8f9a806751b5755bdec5b096186f70c0bfddcfa..cf209da104d483aa064aa3bec621f1adc9574749) of the PR, but should likely be representative anyway.

<details>
<summary>First run: 19% faster, finished IBD in 13h:11m | IBD | 917000 blocks | dbcache 450 | rpi5-16-2 | aarch
...

🤔 glozow reviewed a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3182210966)
code review ACK 5ac32aa441b

I focused on checking that all the dynamic memory is accounted for, places where `m_cluster_usage` is updated, and correctness of the `SingletonCluster` implementation. Feel free to ignore my nits.

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2403086164)
Note to self/reviewers: `Depgraph::Compact()` also calls `shrink_to_fit()`, meaning positions do not change and the mapping is not invalidated.

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2320034473)
Note to self/reviewers: since we've done `ApplyDependencies`, `ClusterSet.m_group_data`, `m_deps_to_add`, and `m_to_remove` are all empty so we don't need to add their dynamic memory usage. We also aren't counting `m_removed` since that'd be at staging level or an `m_unlinked` entry.

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411544659)
nit: In the `SingletonClusterImpl` case, there is only 1 Entry
```suggestion
```

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411563912)
nit: perhaps worth a comment that the special "singleton at the end of the cluster" `LinearizationIndex(-1)` is used here

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411624776)
As far as I can tell, this function should never be called?

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411594625)
Hm, should we be checking `m_quality` before adding to chunk index? Singletons are always "linearized" optimally of course, but `QualityLevel::OVERSIZED_SINGLETON` is also possible here (probably only theoretically).

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2408429695)
If the cluster is empty, then L718 will call `front()` on an empty vector. Should we `assert` instead of `Assume` that it's not empty?

Alternatively, `if (!Assume(!m_linearization.empty()) return -1;`?

💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2410975604)
`m_depgraph` is a `DepGraph<SetType>`