💬 maflcko commented on issue "rpc method removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/issues/29466#issuecomment-2035174527)
@drkhero A fix is in https://github.com/bitcoin/bitcoin/pull/29468 , but it is waiting for review and testing. If you want to move it forward, you can help with review and testing.

💬 jonatack commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2035182787)
FWIW, unable to reproduce on arm64 macOS 14.4.1 with Homebrew clang 17.0.6.

💬 maflcko commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#discussion_r1550178421)
I have a hard time following what llvm does each release here. Why is https://libcxx.llvm.org/Hardening.html not available as versioned URL per release?

https://discourse.llvm.org/t/rfc-hardening-in-libc/73925 claims there are no breaking changes in the 18 release. So I wonder if everything can be kept as-is for now, because I suspect no one will be compiling with 19 for now?

💬 fanquake commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#discussion_r1550197827)
> Why is https://libcxx.llvm.org/Hardening.html not available as versioned URL per release?

Looks like it's here: https://releases.llvm.org/18.1.0/projects/libcxx/docs/Hardening.html

> claims there are no breaking changes in the 18 release.

I've also had a hard time following, and I think some of the changes were landed, reverted, and finally re-landed. According too https://releases.llvm.org/18.1.0/projects/libcxx/docs/ReleaseNotes/18.html:

> New hardened modes for the library have
...

💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035211285)
I've set up a benchmark that performs a partial IBD and then runs `-reindex`, however the reindexing takes about ten times as long as the IBD, with CPU and disk usage sitting near zero. Same for `-reindex-chainstate`. Is this normal? Even if it is, is it even worth benchmarking for a difference between 27.0rc1 and 26.0? At first glance it seems about as slow.

💬 petertodd commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/29691#issuecomment-2035243135)
ACK https://github.com/bitcoin/bitcoin/pull/29691/commits/4f273ab4360c9aa72c2feb78787e1811ab58dc16

DNS seems to resolve just fine for me, and returns IP addresses with a fair bit of overlap with the ones my DNS seed has.

💬 maflcko commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2035261425)
It only happens on x86-64, because the asm is in that format.

💬 maflcko commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#discussion_r1550236288)
Ah, I see. The page may have only been written for the 18 release, or not included in the 17 release for some reason.

According to commit 4a825039a509c43ba20b2cd7aab448b3be16bcc3, "From LLVM 17, `_LIBCPP_ENABLE_DEBUG_MODE` can be used instead." C.f. https://web.archive.org/web/20230815180109/https://libcxx.llvm.org/Hardening.html

This makes me wonder if it is worth it to support clang 17 debug mode, if clang 14,15, and 16 isn't supported either.

It seems fine to require clang 18, if som
...

💬 theuni commented on pull request "[WIP] ci: test secp256k1 MSAN asm annotations":
(https://github.com/bitcoin/bitcoin/pull/29742#issuecomment-2035299814)
Ready for bump now that https://github.com/bitcoin-core/secp256k1/pull/1512 is merged.

💬 achow101 commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2035320170)
While this does resolve the compile issue I was having, it does seem to change how gdb is able to debug things, possibly in a meaningful way? This is just an example of a difference that I noticed while stepping through `CWallet::Create` with and without this PR:

On master:

```
Thread 4 "b-httpworker.0" hit Breakpoint 1, wallet::CWallet::Create (context=..., name="funds", database=std::unique_ptr<wallet::WalletDatabase> = {...},
wallet_creation_flags=0, error=..., warnings=std::vect
...

🤔 glozow reviewed a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1966413998)
Sorry for the nits, still need to run the fuzzer but otherwise lgtm

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1550066137)
nit 4f8407cefa6ddb47b166b2c360d03778c0357a00: would prefer to do a restart in the subtest, to minimize the amount of tests that we do on a nondefault settings

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1543093597)
I meant to add docs in the docstring comment :sweat_smile:
also:
- requires `-datacarriersize=100000`
- I think you mean "It will *not* ensure mempools become synced" ?

💬 theStack commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2035347234)
> I think it would be better if the new format could include a version number so that this could be properly reported.

If we decide to do that, I'd suggest to use the chance and go one step further, by also adding a magic number to the start of the file. This way, it can be quickly identified as being / not being an UTXO dump (for better error reporting, but longer-term also potentially for external identifying tools like [file](https://linux.die.net/man/1/file)). Then we could provide more h
...

💬 kristapsk commented on pull request "Change example address from legacy (P2PKH) to bech32m (P2TR)":
(https://github.com/bitcoin-core/gui/pull/808#issuecomment-2035390185)
Concept ACK

👍 TheCharlatan approved a pull request: "guix: Remove another leftover from #29648"
(https://github.com/bitcoin/bitcoin/pull/29797#pullrequestreview-1977787840)
ACK 3cb80febb87696f3b1073469c0cc68a57ba81de9

Guix build (x86_64):
```
8e7bddb8fa49c857bc9a815a7f27f2d60f8a2f8955966eff467ee86c0d0776f6 guix-build-3cb80febb876/output/aarch64-linux-gnu/SHA256SUMS.part
de0c0624b55418b6638b4852cbe4527c5fb4a2ccbb82d442e5dff0febc947a70 guix-build-3cb80febb876/output/aarch64-linux-gnu/bitcoin-3cb80febb876-aarch64-linux-gnu-debug.tar.gz
5f37064b7496125b90f7df1fda7df3e6545149274bf737361579c0e9be94f6f8 guix-build-3cb80febb876/output/aarch64-linux-gnu/bitcoin-3c
...

💬 theuni commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2035434442)
@achow101 Yeah, I see the same. Sadly that's the trade-off. Basically `-O0` is a poor approximation of what real optimized code will look like. But anything above is bound to optimize some things out.

So we have to make a choice: pure debug-ability (including things like inlines which don't actually exist), or more realistic binaries while sacrificing some source code in the debugger. Personally I prefer the latter, but you probably live in gdb more than me :)

💬 theuni commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2035445375)
Note that for local hacking (with clang), you can use:
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 96c4397504..6bc408e843 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2944,7 +2944,7 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons
return MakeDatabase(wallet_path, options, status, error_string);
}

-std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::string& name, std::uni
...

🤔 murchandamus reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-1977655865)
Did a quick pass. I am surprised we are able to get this big of an improvement without p2p changes, seems like a big win. Big Concept ACK, but I must admit this part of the codebase is a bit out of my wheelhouse.

💬 murchandamus commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1550334355)
Nit: Could use [`std::set::contains`](https://en.cppreference.com/w/cpp/container/set/contains) here and below
```suggestion
BOOST_CHECK(expected_parent1_children.contains(child->GetWitnessHash()));
```