Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 sipa commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035114828)
Or even with `-reindex-chainstate` (which does even less)?
💬 theuni commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2035139205)
Concept ACK.

I think msan is a good proxy for what we want enabled. [From its docs](https://releases.llvm.org/18.1.1/tools/clang/docs/MemorySanitizer.html):

> To get a reasonable performance add -O1 or higher. To get meaningful stack traces in error messages add -fno-omit-frame-pointer. To get perfect stack traces you may need to disable inlining (just use -O1) and tail call elimination (-fno-optimize-sibling-calls).


From [gcc's docs](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Option
...
💬 drkhero commented on issue "rpc method removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/issues/29466#issuecomment-2035144836)
@furszy @glozow Still waiting on this issue to be fixed. Ryan has been waiting for a while now. Thanks!
💬 davidgumberg commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2035153866)
ACK https://github.com/bitcoin/bitcoin/pull/29521/commits/cb4f9fc5847a7e53fe45d54b1fddf504dee5af82

The changes in `bitcoin-cli.cpp` since the last ACK look good to me, and the test coverage is more extensive.

Tested with `make check` and running the functional test suite.

I also verified that passing bad ports to `rpcconnect` and `rpcport` trigger the warning.
💬 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
...