💬 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.

💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1808023902)
To avoid timeouts in this target, I will leave it as draft until #28815 (or other solution) gets merged.

💬 maflcko commented on pull request "doc: rewrite explanation for `-par=`":
(https://github.com/bitcoin/bitcoin/pull/28858#issuecomment-1808030339)
lgtm ACK d799ea26edfd63434b6d1cf55500de184dc67fac

💬 Sjors commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1391015144)
5dd066d4301342859e124abcd61141fbb8b5a003: can you clarify in the commit message or patch file why we're disabling this? Since the comment suggests it's needed.

💬 Sjors commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1391028980)
e65a429b6a895d183aa2131b2001dfee0ec78240 delete `native_clang.mk`?

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1391032235)
IIRC this was because LLVM ranlib didn't support this option, so qt would fail to build.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1391032353)
It was, seems it returned in a rebase. Will drop it on the next push.

🚀 fanquake merged a pull request: "multiprocess compatibility updates"
(https://github.com/bitcoin/bitcoin/pull/28721)

🤔 Sjors reviewed a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1727230446)
A few suggestions after testing and reviewing 3cb390f3c59d3ed0475f1be0c886d46a044fa898.

💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391059644)
On macOS 13.6 this silently exists with `-1` when a node is _not_ pruned (`pruneheight` is not set).

💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391061396)
If I abort here with `ctrl + c` it tries to cleanup, so maybe move the `trap` calls down a bit?

💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391067752)
Note that once `invalidateblock` has been called, this won't stop it. It will go all the way back and then forward again.

💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#issuecomment-1808097606)
By the way, at some point when a bash script gets very complicated, it may be better to convert it to Python.

💬 willcl-ark commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1808117267)
> I think it's human readable? For machines we could just give the raw hex script and not bother with an asm format.

This has always been my interpretation too.

> Anyway, how about going for something more left field...

Square brackets is something I could get behind; to me it is clear from your example that the brackets are indicating _something_, and just by looking at them I can see in most cases that they're hex values. To use the previous (less common?) example formatted with the b
...