📝 hebasto opened a pull request: "ci, iwyu: Drop backported mappings"
(https://github.com/bitcoin/bitcoin/pull/29186)
See https://github.com/include-what-you-use/include-what-you-use/pull/1026.
Split from https://github.com/bitcoin/bitcoin/pull/27710 as a non-controversial change.
(https://github.com/bitcoin/bitcoin/pull/29186)
See https://github.com/include-what-you-use/include-what-you-use/pull/1026.
Split from https://github.com/bitcoin/bitcoin/pull/27710 as a non-controversial change.
💬 maflcko commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1878901826)
> * Building for mac, on Linux, using our downloaded LLVM (Clang + lld + tools)
>
> * Building for mac, in Guix, using Guix's Clang + our downloaded lld + tools??
>
> * Or shift further into using all Guix linker + tools, or, try drop Guix Clang?
I think the benefit of using guix (over downloading a random binary blob from someone else's computer) is that cross compilation to macOS works on more architectures than the uploads of the llvm binaries. (For example riscv64)
So simply
...
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1878901826)
> * Building for mac, on Linux, using our downloaded LLVM (Clang + lld + tools)
>
> * Building for mac, in Guix, using Guix's Clang + our downloaded lld + tools??
>
> * Or shift further into using all Guix linker + tools, or, try drop Guix Clang?
I think the benefit of using guix (over downloading a random binary blob from someone else's computer) is that cross compilation to macOS works on more architectures than the uploads of the llvm binaries. (For example riscv64)
So simply
...
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1878913852)
> and nuking clang from depends,
After discussion today with @theuni @hebasto @TheCharlatan, I'm actually about to open a PR to do exactly this.
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1878913852)
> and nuking clang from depends,
After discussion today with @theuni @hebasto @TheCharlatan, I'm actually about to open a PR to do exactly this.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1878918062)
> an interesting behaviour which i realised made sense was to keep retrying with v2 when latency issue failures happen on tor.
Yes! The connection must have been established on the TCP/IP level for a reconnection attempt to happen. It's the same reason why we don't try to reconnect with v1 if the peer is just offline.
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1878918062)
> an interesting behaviour which i realised made sense was to keep retrying with v2 when latency issue failures happen on tor.
Yes! The connection must have been established on the TCP/IP level for a reconnection attempt to happen. It's the same reason why we don't try to reconnect with v1 if the peer is just offline.
💬 fjahr commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1878920263)
> Any reason not to do `Rotl` in sha3 and `ROTL` in siphash as well?
I don't see any, apparently I just need to learn to grep patterns better...
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1878920263)
> Any reason not to do `Rotl` in sha3 and `ROTL` in siphash as well?
I don't see any, apparently I just need to learn to grep patterns better...
⚠️ 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