Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 maflcko commented on issue "Unit test failures when using multiple jobs and RANDOM_CTX_SEED":
(https://github.com/bitcoin/bitcoin/issues/30696#issuecomment-2475616303)
This was fixed in ad9c2cceda9cd893c0f754e49f7fca6e417ee95f by using the test name as seed.

At least I can no longer reproduce, even when pinning the time to a constant:


```diff
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 46a6daa88c..185adf3457 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -138,7 +138,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)

const std::stri
...
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841697846)
Probably a taste thing. I think `auto` should be used only when it's obvious what type it is.

Here there are two non-obvious occasions:
- `auto x = bool + bool` resulting in x=2.
- casting 2 to bool for the result.

I won't insist but i think verbose types are better here. Feel free to close.
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1841700796)
Have you left a comment on my branch? I can't see it for some reason.
💬 naumenkogs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#issuecomment-2475640512)
@sdaftuar

>EDIT: I hacked up the validation logic to trigger this currently unreachable block of code -- ie to make the ConsensusScriptChecks() fail in a package validation context -- and verified that logic to remove the newly added transactions seems to work. Would be great for others to verify this as well!

I had a little doubt that it will remove the transactions... Instead I was worried about evictions (i can't think of other side effects) of add/remove invalid stuff. Doesn't this a
...
👍 rkrux approved a pull request: "test: enable running independent functional test sub-tests"
(https://github.com/bitcoin/bitcoin/pull/30991#pullrequestreview-2435413648)
reACK 409d0d629378c3e23388ed31516376ad1ae536b5
💬 rkrux commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1841802108)
If I'm not missing something, why not just remove it?
💬 rkrux commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1841810405)
> if no binaries are found in /build

This could also be because the user might not have built the binaries yet even in the case of default build path.

A more verbose message could be:
```
'No binaries found in {builddir}. Please ensure the binaries are present in {builddir}, or set another build path using the BUILDDIR env variable.'
```
💬 naumenkogs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1841755721)
7dc8a0ba1be86352fa51860af504ed09fba9307e

What does `valid` mean in this context? Can't this word be dropped?
💬 naumenkogs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1841816179)
f306a940d2ba470a13120853ace49509a8e496a6

looks odd, perhaps a pasting issue.
💬 naumenkogs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1841774691)
7020d29e7fb276af9877b6767b4bacf1edfe050c

wanna also check for non-standard if add one more?
💬 maflcko commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#issuecomment-2475784402)
review ACK 409d0d629378c3e23388ed31516376ad1ae536b5
💬 naumenkogs commented on pull request "validation: Remove RECENT_CONSENSUS_CHANGE validation result":
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2475789631)
ACK https://github.com/bitcoin/bitcoin/commit/e80e4c6ff91e27d7d40f099a2d7942c29085234c
🚀 fanquake merged a pull request: "validation: Remove RECENT_CONSENSUS_CHANGE validation result"
(https://github.com/bitcoin/bitcoin/pull/31269)
💬 TheCharlatan commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2475883027)
Guix build (aarch64):
```
a9e1f628dd64913209e6f55f183453b046c91e44fb959ea61e0c40c0a8457e98 guix-build-2833befb7822/output/aarch64-linux-gnu/SHA256SUMS.part
d082fbc96abb72a4bb0495833b6361d1ab61f8eefc3423571589920ed6c89678 guix-build-2833befb7822/output/aarch64-linux-gnu/bitcoin-2833befb7822-aarch64-linux-gnu-debug.tar.gz
6d94622d87e86ceef908e2b05435ac7f0e4b7f3d224454f619155e44c36128b9 guix-build-2833befb7822/output/aarch64-linux-gnu/bitcoin-2833befb7822-aarch64-linux-gnu.tar.gz
ca288057fc
...
💬 fanquake commented on pull request "guix: remove `util-linux`":
(https://github.com/bitcoin/bitcoin/pull/31285#issuecomment-2475890313)
> Of those, hexdump was required

Thanks, that is what I'd been looking for, but missed in the list 😅.
🚀 fanquake merged a pull request: "tinyformat: Add compile-time checking for literal format strings"
(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)
🚀 fanquake merged a pull request: "doc: Fix grammatical errors in multisig-tutorial.md"
(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
...
🤔 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`
💬 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.