🚀 fanquake merged a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174)
(https://github.com/bitcoin/bitcoin/pull/31174)
✅ fanquake closed an issue: "Unit test failures when using multiple jobs and RANDOM_CTX_SEED"
(https://github.com/bitcoin/bitcoin/issues/30696)
(https://github.com/bitcoin/bitcoin/issues/30696)
🚀 fanquake merged a pull request: "doc: Fix grammatical errors in multisig-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31225)
(https://github.com/bitcoin/bitcoin/pull/31225)
💬 laanwj commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2476023531)
> We could internalize the relevant macro parts of systemtap's sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap
This still makes sense to me, it's just a few macros which haven't changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.
Dropping a guix dependency would be good!
But this is just Linux. i don't know how it is for other plat
...
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2476023531)
> We could internalize the relevant macro parts of systemtap's sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap
This still makes sense to me, it's just a few macros which haven't changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.
Dropping a guix dependency would be good!
But this is just Linux. i don't know how it is for other plat
...
🤔 BrandonOdiwuor reviewed a pull request: "cmake: add optional source files to bitcoin_crypto directly"
(https://github.com/bitcoin/bitcoin/pull/31268#pullrequestreview-2435763802)
Approach ACK. Great catch!
Nice simplification! Adding the optional source files directly to `bitcoin_crypto` eliminating the need for intermediate libraries like `bitcoin_crypto_sse41, bitcoin_crypto_avx2, and bitcoin_crypto_x86_shani`
(https://github.com/bitcoin/bitcoin/pull/31268#pullrequestreview-2435763802)
Approach ACK. Great catch!
Nice simplification! Adding the optional source files directly to `bitcoin_crypto` eliminating the need for intermediate libraries like `bitcoin_crypto_sse41, bitcoin_crypto_avx2, and bitcoin_crypto_x86_shani`
💬 laanwj commented on issue "RFC: Adopt C++ Safe Buffers?":
(https://github.com/bitcoin/bitcoin/issues/31272#issuecomment-2476047174)
> However, this requires using the standard library containers or primitives to represent buffers. For example, instead of using a raw C-array, std::array should be preferred. Also, instead of using a raw C-pointer, std::span should be preferred.
i think we've already been going in this direction for quite a while.
(https://github.com/bitcoin/bitcoin/issues/31272#issuecomment-2476047174)
> However, this requires using the standard library containers or primitives to represent buffers. For example, instead of using a raw C-array, std::array should be preferred. Also, instead of using a raw C-pointer, std::span should be preferred.
i think we've already been going in this direction for quite a while.
💬 laanwj commented on issue "guix: Linux and macOS builds are not cross-arch reproducible with powerpc64le build arch":
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2476057139)
> I would be happy to upload my binaries
Yes, please do. i'm happy to try to help find the difference but i don't have access to a ppc64 system.
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2476057139)
> I would be happy to upload my binaries
Yes, please do. i'm happy to try to help find the difference but i don't have access to a ppc64 system.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#issuecomment-2476081352)
> Instead I was worried about evictions (i can't think of other side effects) of add/remove invalid stuff. Doesn't this allow to clear the victims mempool for free?
Just to be clear, in case anyone has lost track: we're specifically talking about a codepath we believe is not possible to trigger, namely one where the `PolicyScriptChecks` pass yet the `ConsensusScriptChecks` fail for some transaction, and someone is able to submit such a transaction as part of a package to our node.
In this
...
(https://github.com/bitcoin/bitcoin/pull/31122#issuecomment-2476081352)
> Instead I was worried about evictions (i can't think of other side effects) of add/remove invalid stuff. Doesn't this allow to clear the victims mempool for free?
Just to be clear, in case anyone has lost track: we're specifically talking about a codepath we believe is not possible to trigger, namely one where the `PolicyScriptChecks` pass yet the `ConsensusScriptChecks` fail for some transaction, and someone is able to submit such a transaction as part of a package to our node.
In this
...
💬 TheCharlatan commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476093725)
Looks like we could could also get rid of the pkgconfig hints in the toolchain file: https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2 (did a guix build with this patch too).
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476093725)
Looks like we could could also get rid of the pkgconfig hints in the toolchain file: https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2 (did a guix build with this patch too).
📝 maflcko opened a pull request: "refactor: Avoid std::string format strings"
(https://github.com/bitcoin/bitcoin/pull/31287)
This changes some unchecked `std::string` format strings to use string literals, which are `consteval` checked at compile-time.
Split out, because it is used in several pull requests.
(https://github.com/bitcoin/bitcoin/pull/31287)
This changes some unchecked `std::string` format strings to use string literals, which are `consteval` checked at compile-time.
Split out, because it is used in several pull requests.
💬 TheCharlatan commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842093313)
Why not keep using clang for this job?
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842093313)
Why not keep using clang for this job?
💬 maflcko commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842104265)
It requires adding a suppression, or some other workaround at some point, see https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2047861599
Given that GCC does not require suppressions, it may be preferable for now.
Though, I am happy to drop the commit, and leave it for a follow-up. I just thought, it would be nice to include here, so that testing is easy for those that want.
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842104265)
It requires adding a suppression, or some other workaround at some point, see https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2047861599
Given that GCC does not require suppressions, it may be preferable for now.
Though, I am happy to drop the commit, and leave it for a follow-up. I just thought, it would be nice to include here, so that testing is easy for those that want.
💬 laanwj commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2476182650)
> @laanwj suggested to post the utxoset when it gets corrupted again, will do that when it does.
It might (or might not) give some insight into the kind of corruption happening, or whether it's a result of a software bug. Mind that leveldb's writing behavior is very predictable: it only ever appends to files, it doesn't do random writes.
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2476182650)
> @laanwj suggested to post the utxoset when it gets corrupted again, will do that when it does.
It might (or might not) give some insight into the kind of corruption happening, or whether it's a result of a software bug. Mind that leveldb's writing behavior is very predictable: it only ever appends to files, it doesn't do random writes.
💬 laanwj commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2476206670)
> The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
Not entirely true, every thread has some memory and overhead, even if it's not doing anything. On 32-bit systems the virtual memory space for the stack space of each thread is also significant.
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2476206670)
> The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
Not entirely true, every thread has some memory and overhead, even if it's not doing anything. On 32-bit systems the virtual memory space for the stack space of each thread is also significant.
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842122891)
Done
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842122891)
Done
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476210165)
I made the test code alternate calls between Chainman's `ProcessNewBlock` and `submitSolution` via the Mining interface.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476210165)
I made the test code alternate calls between Chainman's `ProcessNewBlock` and `submitSolution` via the Mining interface.
💬 TheCharlatan commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842129659)
Mmh, I was more concerned about losing the sanitizer, but I guess there is nothing really gained by having it turned on when running valgrind.
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842129659)
Mmh, I was more concerned about losing the sanitizer, but I guess there is nothing really gained by having it turned on when running valgrind.
💬 maflcko commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842132228)
The only benefit would be the symbolizer, but that's not really needed with valgrind.
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842132228)
The only benefit would be the symbolizer, but that's not really needed with valgrind.
👍 storopoli approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2435944639)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2435944639)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
💬 laanwj commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1842156687)
it seems unnecessary to use strprintf at all just to concatenate two strings and a space 😄
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1842156687)
it seems unnecessary to use strprintf at all just to concatenate two strings and a space 😄