💬 fanquake commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#issuecomment-2140064712)
> then it could just be moved into Common,

Have done this.

👍 hebasto approved a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2088736908)
ACK ecb278bb19c53b007380e262fa86d809255eeb49, I have reviewed the code and it looks OK.

It seems worth considering to add a couple of edge cases to the `src/test/fuzz/vecdeque.cpp` like self-assignment and self-swap.

💬 theuni commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1621023146)
Grr, right.

👍 TheCharlatan approved a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198#pullrequestreview-2088766567)
ACK 0a3631fc352eda849290db940844e5ef723436df

Guix build (aarch64)
```
1efa3e0205032d6cfe5517ce36ab63379afa22268dcbb0b92150719baa030682 guix-build-0a3631fc352e/output/aarch64-linux-gnu/SHA256SUMS.part
e3db302484148c00cab7150e3451d2c95dae14062cb4165c8e5a28d71d77e630 guix-build-0a3631fc352e/output/aarch64-linux-gnu/bitcoin-0a3631fc352e-aarch64-linux-gnu-debug.tar.gz
f9792f8db8635bb19f6a2d7c73f1b0780e5776734c12912c5e7172f357f7fa1f guix-build-0a3631fc352e/output/aarch64-linux-gnu/bitcoin-0a
...

👍 theuni approved a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198#pullrequestreview-2088769385)
utACK 0a3631fc352eda849290db940844e5ef723436df

👍 theuni approved a pull request: "depends: consolidate dependency docs"
(https://github.com/bitcoin/bitcoin/pull/30204#pullrequestreview-2088770597)
ACK 01cd425f083b2d38bced8bda8163d87359ea1f59

🤔 theuni reviewed a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201#pullrequestreview-2088776148)
Concept ACK

💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#discussion_r1621045177)
Yea. Depending on when this lands, we could update this to recommend LLVM 18, as that's likely what is going to be used in the CI, if we just `apt install clang` on Ubuntu `24.04`. At a minimum, given we use `-fixup_chains`, we require at least LLVM 16 or later.

💬 maflcko commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621052228)
Might as well put in `build-essential` when this includes Bitcoin Core dependencies such as `bsdmainutils`?

Ref: https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#dependency-build-instructions

💬 fanquake commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621057895)
Right, we could, but https://packages.ubuntu.com/mantic/build-essential is basically gcc, g++ and make. Should we then remove make from this list too, and g++ from the macOS list?

I think I'd actually rather remove `bsdmainutils`, as I really wanted the dependencies listed here to just be what is required to actually build depends, rather than everything needed for Bitcoin Core.

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2140176994)
This seems very useful. I'll try to use it for the (currently very brittle) `Sv2Transport` tests in #29432, and review it along the way.

💬 fanquake commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621059049)
Happy to also update build-unix at the same time, to add anything that is missing.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1621060075)
I got a bit confused about these two new PR's, but I guess they are orthogonal.

💬 maflcko commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621061352)
I agree (see my previous ACK), but probably it doesn't matter much. Should be fine to merge either.

💬 laanwj commented on issue "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk":
(https://github.com/bitcoin/bitcoin/issues/30191#issuecomment-2140183410)
> maybe it saves tons of logs in /var/ log ??

Bitcoin core itself doesn't write anything to `/var` or anywhere but the data directory. Of course, it may be that some other process is doing that. You'd want to figure out what files use the disk space. i don't know what's a good tool for that in windows but in Linux you'd typically use `du -h`.

💬 Sjors commented on pull request "Introduce Miner interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2140187315)
Let me know if I should expand the interface to cover more of `rpc/mining.cpp` or stick to this for now.

💬 maflcko commented on issue "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk":
(https://github.com/bitcoin/bitcoin/issues/30191#issuecomment-2140191219)
On Windows you can just right click and the Properties dialogue should spit out the folder size, IIRC? (Haven't used it in a decade). But yeah, without more details, there is nothing that can be done here.

💬 Sjors commented on pull request "Introduce Miner interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1621066944)
f04cf9f75054573c204245f7bb1e6813cce1fed8: I guess I should change the `BlockAssembler` constructor

💬 Sjors commented on pull request "Introduce Miner interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1621067334)
f04cf9f75054573c204245f7bb1e6813cce1fed8: I guess I should change the `BlockAssembler` constructor

💬 maflcko commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#discussion_r1621067469)
Probably best to move everything (CI + guix) at the same time, in the same pull?