Bitcoin Core Github
42 subscribers
126K links
Download Telegram
📝 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.
💬 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.