💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251021307)
FWIW it is now working for GUI startup according to https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2829171034 in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a

💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251022089)
MacOS corrected to macOS in according to #31453 (comment)

💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251024649)
I've avoided explicit recommendation of a FS on purpose here; as far as I know exFAT is the only known-problematic FS on macOS?

Can be convinced otherwise, but leaving as-is for now

💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3149949745)
While I was re-touching this for the outstanding comments I took the chance to inline the check for a smaller diff and generally a little cleaner/better code (IMO).

💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2251119754)
Removed in dc1613169db

💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3150096441)
Pushed another run with caches saving to re-test cache hit/restore later.

👍 stickies-v approved a pull request: "kernel: create monolithic kernel static library"
(https://github.com/bitcoin/bitcoin/pull/33077#pullrequestreview-3083677951)
tACK fdbade6f8ded63519b637c8fa6f39914a400ab5e

I rebased https://github.com/TheCharlatan/bitcoin/commits/kernelApi_Cpp_Internal_Headers/ onto fdbade6f8ded63519b637c8fa6f39914a400ab5e (resulting in https://github.com/stickies-v/bitcoin/tree/kernel-cpp-monolith), and I was able to statically link my `pbk_ext` py-bitcoinkernel nanobind extension to the monolithic libbitcoinkernel.

I reviewed the code changes and could not see any issues, but I am also a cmake novice. The monolithic approach ma
...

💬 maflcko commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3150231458)
Would be nice to see a test run with additional logging added to `ChainstateManager::LoadExternalBlockFile`, and a log showing the size of the blk dat file.

💬 cedwies commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#discussion_r2251205963)
When I first wrote the test, I had repeated issues on MacOS. the wallet’s scanning flag sometimes didn’t turn true quickly enough when I fired a full rescan. Too quickly, I convinced myself that the issue is with the tail length (I honestly never questioned it anymore until your question).

Upon inspection: Yes, I will fix it (which simplifies the code a lot).

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2251210391)
Definitely - should be fixed now:
```diff
$ git diff 784459ac79 104b40c8c9
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index c8d824bb6c..05ec73697c 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -1080,8 +1080,6 @@ bool BlockManager::ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos
return true;
}

-static const uint32_t HEADER_SIZE = ::GetSerializeSize(CBlockHeader{});
-
bool BlockManager::ReadTxFromBlock(CTransac
...

💬 cedwies commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#issuecomment-3150239992)
Also, I will block the test on t.join(). (no timeout), so the
background rescan thread is guaranteed to exit before the test returns (which I think causes the CI
cleanup step from failing with “OSError: directory not empty”).

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2251213333)
Oops, sorry -> should be fixed now:
```diff
$ git diff b2a22ce33d 4441827ef4
diff --git a/doc/REST-interface.md b/doc/REST-interface.md
index ffb3c09af1..8f8431c29a 100644
--- a/doc/REST-interface.md
+++ b/doc/REST-interface.md
@@ -42,7 +42,7 @@ Given a block hash and transaction offset within it: returns a transaction in bi
Responds with 404 if the transaction doesn't exist.

By default, this endpoint will also deserialize the leading transactions, before reading and returning the
...

💬 hebasto commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2251227924)
> transifex updated

I've pulled the most recent translations.

💬 hebasto commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2251239067)
> > Are we going to pull in the updates, or leave this as-is?
>
> I've also created a preliminary review for all languages: https://github.com/maflcko/b-c-gui-translations-review/tree/main/reviews
>
> There is some obvious erroneous translations, like https://github.com/maflcko/b-c-gui-translations-review/blob/05100fec918fc44787d43004ea3b02749f5a16ee/reviews/te.md#L160-L166, but I am not familiar with those languages, so I can't evaluate if the LLM generated translation is correct.

Than
...

💬 janb84 commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3150314952)
Concept NACK

What is the usecase for this ? forks would also fork the GUIX build script and if this is intended as multi version build script than move it to an independent repo.

💬 hebasto commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3150347633)
> ... showing the size of the blk dat file.

You mean `blkdat.GetPos()`?

👍 Sjors approved a pull request: "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key"
(https://github.com/bitcoin/bitcoin/pull/32471#pullrequestreview-3083871225)
ACK 45e06e05e7f8d638ecc91a1ba62e1b38a5133a2b

I find that the separation of 23431cde206bd6e18f274773f77bd914553bf2cc makes it easier to follow. The separation of https://github.com/bitcoin/bitcoin/commit/d2b84af58073630f87df32a7b785971fe32cb9be from https://github.com/bitcoin/bitcoin/commit/49f41141c674f83d0b66262a5b21aad623704264 is less useful, but fine by me.

💬 Sjors commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2251304473)
In 49f41141c674f83d0b66262a5b21aad623704264 _descriptor: ToPrivateString() pass if at least 1 priv key exists_: maybe for good measure:

```cpp
*has_priv_key = false;
```

In both places it seems better to check that `priv_key` exists, even if that's always the case in this fuzzer. IIUC in that case you can leave out `tmp{false}` and just pass `nullptr` below.

💬 Sjors commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2251274240)
In 64213213b3691c9d93de95f880f820a640cd3c35 _descriptors: add IsWatchOnly()_: nit: you don't need `output_type.has_value()`

💬 maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2251381743)
> As for this PR, the last commit only serves only to demonstrate that the modified build system handles translation files properly. So I don't think that any translation-specific issue should block it.

Agree. Discussion can be closed.