💬 maflcko commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1807876054)
Unrelated: I've been trying to run `guix pull` on riscv64. However, it failed. Let me try again and come back with a result next week.

👍 dergoegge approved a pull request: "fuzz: call lookup functions before calling `Ban`"
(https://github.com/bitcoin/bitcoin/pull/27935#pullrequestreview-1727011437)
ACK fca0a8938e34cb4f6c400e1d1d0be02f027d80c5

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1807893266)
Rebased on #28580 (LLVM 17 in Guix) and #28783, which should be the last blockers for this.

👍 maflcko approved a pull request: "fuzz: Minor improvements to tx_package_eval target"
(https://github.com/bitcoin/bitcoin/pull/28825#pullrequestreview-1727032774)
Nice

💬 maflcko commented on pull request "fuzz: Minor improvements to tx_package_eval target":
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1390931631)
```suggestion
static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TRUE_STACK{{uint8_t{OP_TRUE}}, {}};
static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TWO_STACK{{uint8_t{OP_2}}, {}};
```

nit: Use the safe cast for new code, where possible, over the narrowing one.

Also, could use `EMPTY` instead of `{}`. But anything is fine here.

🚀 fanquake merged a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391)

✅ fanquake closed an issue: "fuzz: banman, Assertion `banmap == banmap_read' failed"
(https://github.com/bitcoin/bitcoin/issues/27924)

🚀 fanquake merged a pull request: "fuzz: call lookup functions before calling `Ban`"
(https://github.com/bitcoin/bitcoin/pull/27935)

💬 fanquake commented on pull request "depends: Build `capnp` with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1807936546)
Any reason to not change `native_capnp` at the same time?

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1390973314)
It's not. Replaced with a TODO comment to add wallet test for a pruned node.

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1390973698)
Also dropping this import since the wallet is no longer watch-only.

💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1807988802)
Now we can actually almost do #21778, and avoid having to do this patching.

📝 fanquake opened a pull request: "doc: rewrite explanation for `-par=`"
(https://github.com/bitcoin/bitcoin/pull/28858)
The negative bound for script threads comes from the machine which generates the man pages, so may only be correct for that machine. Any other placeholder value will also be wrong for some machines. Fix this be removing the value. This also fixes help2man incorrectly bolding the value, as if it were a paramater.

Closes #28850.

💬 Sjors commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1808004406)
On Intel macOS 13.6 before this PR I get the warning 6 times:

```
./configure --with-gui --enable-wallet --disable-fuzz-binary --disable-bench --disable-tests
...
checking whether the linker accepts -Wl,-bind_at_load... no
...
make
...
CXXLD bitcoin-wallet
ld: warning: -single_module is obsolete
CXXLD bitcoin-util
ld: warning: -bind_at_load is deprecated on macOS
ld: warning: -bind_at_load is deprecated on macOS
ld: warning: -bind_at_load is deprecated on macOS
ld: warn
...

💬 stickies-v commented on pull request "Improve peformance of CTransaction::HasWitness (28107 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/28766#discussion_r1390988379)
nit: Using default initializer makes more sense here I think:

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

```diff
diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp
index 77a363f7b6..a20cf25f53 100644
--- a/src/primitives/transaction.cpp
+++ b/src/primitives/transaction.cpp
@@ -93,8 +93,10 @@ Wtxid CTransaction::ComputeWitnessHash() const
return Wtxid::FromUint256((CHashWriter{0} << *this).GetHash());
}

-CTransaction::CTransaction(const C
...

👍 stickies-v approved a pull request: "Improve peformance of CTransaction::HasWitness (28107 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/28766#pullrequestreview-1727118277)
ACK d51fb9caa622add96c6b594e162da5584eb927fc

I think the PR and commit description would benefit from describing that this is not a strict improvement, as we slightly increase memory usage (which I think is an acceptable trade-off).

💬 hebasto commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1808019528)
> Rebased on #28580 (LLVM 17 in Guix) and #28783, which should be the last blockers for this.

Why #28783 is considered as a blocker for this PR?

💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1391018631)
I agree. I will draft this to wait for #28832.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1808021422)
> Why https://github.com/bitcoin/bitcoin/pull/28783 is considered as a blocker for this PR?

LLD will output spurious warnings about not implementing bind_at_load, which we can avoid by merging #28783.

📝 brunoerg converted_to_draft a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)
This PR adds fuzz target for `DescriptorScriptPubKeyMan`. Also, moves `MockedDescriptorConverter` to `fuzz/util/descriptor` to be used here and in `descriptor` target.