💬 dergoegge commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288512730)
> Up to you, but there is a small benefit of going with the fuzz tests first (if possible)

It looks like the tests here mostly operate on the split out modules after the refactoring, as opposed to a implementation agnostic test through the p2p interface (which would be a giant pita to write). So splitting the fuzz test out would be fine as they don't directly assert that no behavior changes are made. It would of course be sad if we split them out and then only the refactoring lands (which has
...

💬 fanquake commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2288526278)
> The test target is generated by CMake. Such targets [cannot](https://cmake.org/cmake/help/latest/command/add_dependencies.html) be subject to top-level target dependencies.

Ok. So what about `deploy`? If it's a custom target of ours, it should know to build `bitcoin-qt` first?

🤔 marcofleon reviewed a pull request: "fuzz: Faster utxo_snapshot fuzz target"
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2237934699)
Tested ACK fa198d1a196482bf363ba57a3f4d6d2d860b8ce5

🤔 stickies-v reviewed a pull request: "refactor: Replace ParseHex with consteval ArrayFromHex"
(https://github.com/bitcoin/bitcoin/pull/30377#pullrequestreview-2237840417)
Concept ACK for more compile time validation.

Strangely enough, 09458eadc9a4484ba37a70d1b378ed3f3c9e31d0 doesn't compile for me even though CI seems fine.

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716732991)
nit: while touching, might be worth updating to

```suggestion
assert(bytes.size() == m_keydata.size());
```

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716752262)
I'm unable to compile 09458eadc9a4484ba37a70d1b378ed3f3c9e31d0 using:

```
% clang --version
Homebrew clang version 17.0.6
Target: arm64-apple-darwin23.3.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
```


```
git clean -xdf && git checkout 09458eadc9a4484ba37a70d1b378ed3f3c9e31d0 && ./autogen.sh && ./configure --without-gui && make -j 7

./uint256.h:131:19: error: call to consteval function 'ConstevalHexDigit' is not a constant expression
auto lo = Const
...

📝 hebasto opened a pull request: "guix: Drop unused module from manifest"
(https://github.com/bitcoin/bitcoin/pull/30653)
My Guix build:
```
x86_64
170df52c2238510bd166f3fb1c4c3c11d2c1480a2e468fd532cb4d0435ac11cf guix-build-c7fb80a08f98/output/aarch64-linux-gnu/SHA256SUMS.part
54e71ef5135464f58e3db4a3b893fa2f26a2c9cfb465699a363bb59a0d1bd94f guix-build-c7fb80a08f98/output/aarch64-linux-gnu/bitcoin-c7fb80a08f98-aarch64-linux-gnu-debug.tar.gz
806d6042151e0af026748379b9bbbfea53d4c91555b2f0d05ed11faf83f429bb guix-build-c7fb80a08f98/output/aarch64-linux-gnu/bitcoin-c7fb80a08f98-aarch64-linux-gnu.tar.gz
96f111f81
...

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288577940)
> > to you, but there is a small benefit of going with the fuzz tests first (if possible)
>
> It looks like the tests here mostly operate on the split out modules after the refactoring, as opposed to a implementation agnostic test through the p2p interface (which would be a giant pita to write).

Indeed, all of these tests require the refactors (on master we'd have to create a chain to fire validation events + construct actual transactions with each type of invalidity).

> So splitting th
...

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716833566)
What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, so probably up for debate.

The CI passes, because it is using Xcode 15, see d742be3d3f5d5063d7160f72422bce2fec953f38

💬 paplorinc commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716839513)
It was working for me with:
```
% clang --version
Homebrew clang version 18.1.8
Target: arm64-apple-darwin23.5.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin

% xcodebuild -version
Xcode 15.4
Build version 15F31d
```

👍 vasild approved a pull request: "log: expand BCLog::LogFlags (categories) to 64 bits"
(https://github.com/bitcoin/bitcoin/pull/26619#pullrequestreview-2238035999)
ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288632385)
Thanks for the review! Obviously this is fine to merge, but I am still unhappy about the average performance increase, which isn't 100x, but closer to the worst case performance increase (1.35x). I'll push once more, sorry.

🤔 BrandonOdiwuor reviewed a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636#pullrequestreview-2238068310)
Concept ACK

💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288658870)
Sounds good, I'll review again when it's improved then. Looked to be about 2x when I just tested it.

🤔 furszy reviewed a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2238083184)
ACK 8f2522d

💬 epiccurious commented on pull request "feat(build): improve dependency error reporting in autogen.sh":
(https://github.com/bitcoin/bitcoin/pull/30646#issuecomment-2288687022)
Thanks for reviewing. I'll close the PR as won't do.

✅ epiccurious closed a pull request: "feat(build): improve dependency error reporting in autogen.sh"
(https://github.com/bitcoin/bitcoin/pull/30646)

✅ maflcko closed an issue: "contrib: Automation for Bitcoin Full Node Deployment"
(https://github.com/bitcoin/bitcoin/issues/30638)

💬 maflcko commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2288699195)
@willcl-ark Maybe leave a new comment in the packaging issue 55 about your dockerfile?

I agree closing this for now. It may be best to continue discussion in https://github.com/bitcoin-core/packaging/issues/55 (yes it is closed, but discussion can continue).

And given that the discussion here seems about a *user* dockerfile, not a *dev* dockerfile, the packaging repo seems more appropriate.

🚀 hebasto merged a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824)

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716909297)
> What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, nor tested, so probably up for debate.

Thanks, I'm on macOS 14.3.1 and just bumped XCode from 14.3.1.0.1.1683849156 to 15.3.0.0.1.1708646388 which resolves the issue 👍