💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1390665847)
Dropped

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#issuecomment-1807555704)
Pushed some cleanups.

💬 Sjors commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1807573920)
Getting the same hashes as @hebasto. Briefly tested that the binary still works on Intel macOS 13.6

💬 maflcko commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1390771582)
Could add a comment why this is needed?

💬 maflcko commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1390822510)
Not sure why that'd be difficult. In C++, it is possible to forward declare a struct:

```diff
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 7c905ca4f4..199fe948a3 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -22,6 +22,9 @@
#include <stdint.h>

class CBlockIndex;
+class CTxMemPoolEntry;
+
+using CTxMemPoolEntryRef = std::reference_wrapper<const CTxMemPoolEntry>;

struct LockPoints {
// Will be set to the blockc
...

💬 fanquake commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1807787620)
> Sent a patch upstream, https://lists.gnu.org/archive/html/guix-patches/2023-11/msg00362.html, to see if we can get some unversioned pointers to stable/longterm added.

No traction on this yet, going to move forward here.

> Yet boost doesn't make the same compatibility guarantees, and needs to be checked.

I haven't seen anything that would cause issues, let me know / open an issue if you see otherwise.

💬 dergoegge commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1390843395)
I think this runs into the same timeouts as the `mocked_descriptor_parse` target. We should probably wait for #28832 to be merged so `HasDeepDerivPath` can be used here as well.

🚀 fanquake merged a pull request: "guix: switch to 6.1 kernel headers over 5.15"
(https://github.com/bitcoin/bitcoin/pull/28786)

👋 fanquake's pull request is ready for review: "guix: update time-machine"
(https://github.com/bitcoin/bitcoin/pull/28580)

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1807801984)
A fresh one: [clusterfuzz-testcase-load_external_block_file-5183508842414080.bin.txt](https://github.com/bitcoin/bitcoin/files/13333454/clusterfuzz-testcase-load_external_block_file-5183508842414080.bin.txt)

💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1807805694)
LLVM 16 & 17 are now available. This is ready for review. This bump also contains the recently released [Mes 0.25](https://www.mail-archive.com/info-gnu@gnu.org/msg03232.html), which should improve the bootstrapping experience for RISC-V. cc @laanwj who upstreamed a bunch of the changes there.

💬 fanquake commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1807828903)
See also the CMake implementation of this check: https://github.com/hebasto/bitcoin/pull/50.

🚀 fanquake merged a pull request: "doc: update docs for `CHECK_ATOMIC` macro"
(https://github.com/bitcoin/bitcoin/pull/28777)

📝 TheCharlatan opened a pull request: "test, refactor: Magic bytes array followup"
(https://github.com/bitcoin/bitcoin/pull/28857)
This is a followup-PR for #28423

* Initialize magic bytes in constructor
* Add a small unit test for serializing arrays.

🚀 fanquake merged a pull request: "test: Avoid intermittent failures in feature_init"
(https://github.com/bitcoin/bitcoin/pull/28831)

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1390901971)
Will push this if there are further things that come up.

🤔 dergoegge requested changes to a pull request: "fuzz: wallet, add target for Spend"
(https://github.com/bitcoin/bitcoin/pull/28236#pullrequestreview-1726964487)
Coverage looks decent: https://dergoegge.github.io/bitcoin-coverage/pr28236/fuzz.coverage/index.html

💬 dergoegge commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1390888668)
```suggestion
const CWallet& GetWallet()

```

It's otherwise hard to tell if the global wallet instance is mutated. Same for `GetCoins()`.

If the global instance is mutated, the target is likely to be non-deterministic which hurts fuzzer efficiency. The target should behave exactly the same when given the same input, but that might not be the case if global state is used and modified.

(The suggested change might not compile if `wallet` or `available_coins` are used as non-const/m
...

💬 maflcko commented on pull request "test, refactor: Magic bytes array followup":
(https://github.com/bitcoin/bitcoin/pull/28857#discussion_r1390907969)
```suggestion
CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn)
: pchMessageStart{pchMessageStartIn}
```

nit: on the next line, according to clang-format?

👍 dergoegge approved a pull request: "fuzz: Minor improvements to tx_package_eval target"
(https://github.com/bitcoin/bitcoin/pull/28825#pullrequestreview-1726994935)
https://dergoegge.github.io/bitcoin-coverage/pr28825/fuzz.coverage/src/validation.cpp.gcov.html

Coverage looks good to me ACK 6a917918b76eef154c6757fe9ecf7713d526c3dd