⚠️ luke-jr opened an issue: "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)"
(https://github.com/bitcoin/bitcoin/issues/29187)
The `datacarriersize` policy option is meant to limit the size of extra data allowed in transactions for relaying and mining. Since the end of 2022, however, attackers have found a way to bypass this limit by obfuscating their spam inside OP_FALSE OP_IF patterns instead of using the standardized OP_RETURN. This remains under active exploitation to a degree very harmful to Bitcoin even today.
A straightforward way to address this is to simply fix the bug (#28408), but that was (inappropriately
...
(https://github.com/bitcoin/bitcoin/issues/29187)
The `datacarriersize` policy option is meant to limit the size of extra data allowed in transactions for relaying and mining. Since the end of 2022, however, attackers have found a way to bypass this limit by obfuscating their spam inside OP_FALSE OP_IF patterns instead of using the standardized OP_RETURN. This remains under active exploitation to a degree very harmful to Bitcoin even today.
A straightforward way to address this is to simply fix the bug (#28408), but that was (inappropriately
...
📝 fanquake opened a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm"
(https://github.com/bitcoin/bitcoin/pull/29188)
Always use the system Clang when compiling for macOS.
Removes the native_llvm package.
Simplifies #21778 (which should also fix the RISC-V macOS cross build).
(https://github.com/bitcoin/bitcoin/pull/29188)
Always use the system Clang when compiling for macOS.
Removes the native_llvm package.
Simplifies #21778 (which should also fix the RISC-V macOS cross build).
💬 hebasto commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1878933847)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1878933847)
Concept ACK.
⚠️ Christewart reopened an issue: "Failing to fetch `cfheader` corresponding to block header in `headers` message"
(https://github.com/bitcoin/bitcoin/issues/27085)
<!-- Describe the issue -->
Occasionally when I receive a `headers` message on the p2p network, and attempt to fetch the `cfheader` that corresponds to a block header inside of a `headers`, i get this error message inside of my `debug.log`
>2023-02-07T16:47:15Z [net] Failed to find block filter hashes in index: filter_type=basic, start_height=150, stop_hash=3ca9030aeb6c2721cfbab0116b8d96e2d3c7e00738238010e0bc622566dc2aed
**Expected behavior**
I should be able to fetch a `cfheader`
...
(https://github.com/bitcoin/bitcoin/issues/27085)
<!-- Describe the issue -->
Occasionally when I receive a `headers` message on the p2p network, and attempt to fetch the `cfheader` that corresponds to a block header inside of a `headers`, i get this error message inside of my `debug.log`
>2023-02-07T16:47:15Z [net] Failed to find block filter hashes in index: filter_type=basic, start_height=150, stop_hash=3ca9030aeb6c2721cfbab0116b8d96e2d3c7e00738238010e0bc622566dc2aed
**Expected behavior**
I should be able to fetch a `cfheader`
...
💬 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),
...
(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.
(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!
(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?
(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.
(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
...
(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
...
(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
...
(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.
(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.
(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
(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.
(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)
(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?
(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
...
(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.
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1878993368)
Concept ACK.