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