💬 Christewart commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1878945351)
This still seems to be an issue on 24.2 . I'm going to try and upgrade my nodes used for test harnesses in to use 25.x and 26.x. From a quick look through release notes, it doesn't seem like there was anything in those releases that would fix this bug.

Note, i am not calling `syncwithvalidationinterface` explicitly after generating blocks on regtest and waiting for that rpc call to complete before continuing with testing logic. This doesn't seem to happen as much on slower machines (x86_64),
...

👍 TheCharlatan approved a pull request: "build: remove `--enable-lto`"
(https://github.com/bitcoin/bitcoin/pull/29185#pullrequestreview-1806367894)
ACK 2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e

Tested just passing in the required flags, seems to work as advertised.

💬 maflcko commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1878956302)
> which should also fix the RISC-V macOS cross build

I think that is currently failing due to a bug in the llvm upstream build system (https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1846869958)

Other than that:

Nice!

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1878961337)
Should we backport 453b4813ebc74859864803e9972b58e4be76a4d6?

💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1878962108)
> I think that is currently failing due to a bug in the llvm upstream build system (https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1846869958)

Right, I was referring to this failure, https://github.com/bitcoin/bitcoin/pull/29078#issuecomment-1863598765, where we can't compile cctools on RISC-V, because it just isn't supported.

💬 maflcko commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1878968120)
> Note, i am not calling `syncwithvalidationinterfacequeue` explicitly

At least one of the following must happen, otherwise this bug will remain, obviously:

* Call `syncwithvalidationinterfacequeue` to produce the compact filter prior to requesting it, to ensure arrival
* Request the compact filter in a loop (busy polling) until it arrives
* Change the code to `syncwithvalidationinterfacequeue` before answering compact filter requests. (This would be my preference, but there was a previo
...

📝 theuni opened a pull request: "RFC: Deprecate libconsensus"
(https://github.com/bitcoin/bitcoin/pull/29189)
This library has existed for nearly 10 years with very little known uptake or impact. It has become a maintenance burden. In several cases it dictates our code/library structure (for example necessitating LIBBITCOIN_CRYPTO_BASE), as well as build-system procedures (building multiple copies of object files especially for the lib).

Several discussions have arisen wrt migrating it to CMake and it has become difficult to justify adding more complexity for a library that is virtually unused anyway
...

💬 glozow commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1878972398)
> The datacarriersize policy option is meant to limit the size of extra data allowed in transactions for relaying and mining.

History of this config option suggests `datacarriersize` is meant to limit the size of data in `OP_RETURN` outputs, so this statement is untrue.

- comments on #3715:
- [comment](https://github.com/bitcoin/bitcoin/pull/3715#issuecomment-35701201) asked about including the usage of bare CHECKSIG and CHECKMULTISIG, arguing it should be in scope for that PR
- [co
...

💬 achow101 commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1443117675)
I think it would be useful to assert that `tx.vout.empty()` somewhere to make it clear that it is not used. Perhaps this function can do `tx.vout.clear()`, and the `FundTransaction` called by it can do the check.

💬 andrewtoth commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-1878979055)
It looks safe to me to replace all uses of `std::string` with `std::string_view` in this PR.

💬 TheCharlatan commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1878984671)
Concept ACK

💬 hebasto commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1878985904)
Expecting that macOS cross-compiling CI job will fail and we need to reapply https://github.com/bitcoin/bitcoin/commit/05aca093819be276ac7d648472c6ed5c7d235cc5.

💬 Christewart commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1878985983)
> > Note, i am not calling `syncwithvalidationinterfacequeue` explicitly

Massive apologies, that was a typo. I _am_ calling `syncwithvalidationinterfacequeue` explicitly. [I _always_ call it in conjunction with `generatetoaddress`](https://github.com/bitcoin-s/bitcoin-s/blob/ff8376ceb664436e9d56da02b1f7598da1dbc459/bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/MiningRpc.scala#L33)

💬 maflcko commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1878988394)
> I can reproduce a slight slowdown here.
> Both libc++ and libstdc++ implement this in terms of the same built-ins we were using before, so I find this surprising. I'd hate to find that the c++ism's have a cost.

I ran the first commit via `./src/bench/bench_bitcoin --filter=BitWidth.*` and I can see a slowdown of the *current* implementation (std is faster). Which is also confusing, given that it is the same code. Doubly confusing, because it is the opposite result of yours?

🤔 hebasto reviewed a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm"
(https://github.com/bitcoin/bitcoin/pull/29188#pullrequestreview-1806416855)
It seems the `*__native_toolchain` variables are no longer needed:
```diff
diff --git a/depends/Makefile b/depends/Makefile
index 319c3498df..8f33b7defa 100644
--- a/depends/Makefile
+++ b/depends/Makefile
@@ -186,7 +186,6 @@ all_packages = $(packages) $(native_packages)
meta_depends = Makefile funcs.mk builders/default.mk hosts/default.mk hosts/$(host_os).mk builders/$(build_os).mk

$(host_arch)_$(host_os)_native_binutils?=$($(host_os)_native_binutils)
-$(host_arch)_$(host_os)_nati
...

💬 hebasto commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1878993368)
Concept ACK.

💬 maflcko commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1878994053)
> I am calling syncwithvalidationinterfacequeue explicitly.

Are you sure, because the debug log you shared does not mention it?

👍 TheCharlatan approved a pull request: "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256"
(https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1806423145)
ACK 86712c3135786b305f27c44dffd0808be0ee7170

Guix builds (aarch64):
```
e00ac9d2dbf9d1b6a0255d7c6669ee3925696109cf4ca549dbf411da235f52bc guix-build-86712c313578/output/aarch64-linux-gnu/SHA256SUMS.part
871f7e66c454ac08d4e04f0d86d23e2b34a6c3a6a3b60aabcb9485c086cdad37 guix-build-86712c313578/output/aarch64-linux-gnu/bitcoin-86712c313578-aarch64-linux-gnu-debug.tar.gz
f0553ca6ebaaf5eb8d00196a681a20c21d4b81cb3ec907cce01ac6a446de02e7 guix-build-86712c313578/output/aarch64-linux-gnu/bitcoin-
...

💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1443133564)
😳 Whoops!

Fixed, thanks :)

👍 TheCharlatan approved a pull request: "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256"
(https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1806424122)
Re-ACK bbf218d06164b7247f5e9df5ba143383022fbf74