💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550033829)
nit: this is unnecessary

💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550056609)
nit: documentation please, e.g. number of elements to track

💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550045138)
+1, was going to say this should be removed in 2ef71c73582be554e565ada3f8a6ca77c2cd79f1, or say we are avoiding false warnings instead of preventing tampering of adjusted time

💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550042121)
Or its own rpc commit?

💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035080712)
> Can you also let us know if you're using any particular config options etc.

Beyond the bare minimum (`-connect`, `-datadir`, RPC user/pass etc.) I'm just increasing dbcache and enabling pruning, but neither of those should kick in at this block height. I'll test again using the defaults just to be sure.

> how are you able to perform so many runs in such a short amount of time? Do you have access to a lot of compute?

I'm not doing full IBD, just to block 120,000. As much as I'd like to
...

💬 mzumsande commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035090717)
Does it also appear with `-reindex` (without a second node)?

💬 pablomartin4btc commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1550107069)
It's ok, I got confused at first glance but then read more about how `util::Result` is being used in all our code.

https://github.com/bitcoin-core/gui/blob/0d509bab45d292caeaf34600e57b5928757c6005/src/util/result.h#L19-L33

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