Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1838152072)
re: https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834348410

> Nit: Shouldn't this be called `txid`?

Nice catch, fixed! Might be a good idea to make sure these names are correct, even though they are just documentation and don't affect behavior.
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1838357818)
re: https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834362068

> How does this `write` argument here map to the `action` argument in the c++ interface? They seem to have different types.

This is a bug, and it should have been caught by the libmultiprocess library. https://github.com/chaincodelabs/libmultiprocess/pull/120 adds a static_assert to ensure that it would result in a compile error if a specified capnproto type is ever not compatible with a c++ enum type.
🤔 mzumsande reviewed a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2430152067)
Code Review ACK 9c5775c331e02dab06c78ecbb58488542d16dda7
💬 theuni commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2471040172)
> > Which I think can be removed now, no?
>
> No, we can't. Only for Windows builds `*.exe` and `*.dll` files reside in a single `bin` subdirectory. On other platforms, shared libraries still reside in their own `lib` subdirectory, which requires the continued use of RPATH.
>
> > Edit: Also, I think this obsoletes the `CMAKE_SKIP_BUILD_RPATH` comment in `CMakeLists.txt`.
>
> I believe this comment is still relevant.

Yes, you're right on both counts. I forgot about the different lib p
...
maflcko closed a pull request: "ci: Bump valgrind tasks to clang-18"
(https://github.com/bitcoin/bitcoin/pull/31273)
💬 maflcko commented on pull request "ci: Bump valgrind tasks to clang-18":
(https://github.com/bitcoin/bitcoin/pull/31273#issuecomment-2471093188)
Closing as duplicate of https://github.com/bitcoin/bitcoin/issues/29635. Lets keep the discussion in one place.
Sjors closed an issue: "v27.2 guix build fails with hash mismatch"
(https://github.com/bitcoin/bitcoin/issues/31266)
💬 Sjors commented on issue "v27.2 guix build fails with hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/31266#issuecomment-2471100700)
I was able to do a complete guix build for v27.2: https://github.com/bitcoin-core/guix.sigs/pull/1433
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1838474496)
Good call, done.
👍 danielabrozzoni approved a pull request: "doc: mention `descriptorprocesspsbt` in psbt.md"
(https://github.com/bitcoin/bitcoin/pull/31277#pullrequestreview-2430251087)
ACK ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e
💬 maflcko commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1838505227)
> Yeah, looking at it again, this doesn't make sense because the test may fail if someone actually scales the wait time down. I changed it back and will leave the mocktime approach for a follow-up.


Source: https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172239
🚀 glozow merged a pull request: "test: enhance p2p_orphan_handling"
(https://github.com/bitcoin/bitcoin/pull/31037)
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2471185762)
reACK 5c2e291060c via range-diff. Nothing but a rebase and removing the conflict.
💬 fanquake commented on issue "valgrind: Conditional jump or move depends on uninitialised value(s)":
(https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2471199603)
I think we could move to Ocular (gives us Valgrind 3.23.0, and we remember to switch to 25 before June), Clang 18 (31273) + some suppressions. Was running some tests of #31273 + this diff:
```bash
diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
index d3c95af99e..f0e8e92889 100755
--- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
+++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
@@ -6,7 +6,7 @@

export LC_ALL
...
💬 fanquake commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>{8}` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2471201204)
> Wouldn't that require a cmake generation step from binary to header which would basically produce the exact same lines as what we have now?

Yes. See `bench/data/block413567.raw` & `bench/data/block413567.raw.h`, where at build time a header file of ~`125'000` lines is produced.

> Would it help if I simply extracted it to a separate header file instead?

I don't think so. The point is more to not add 100'000s of lines of "data" to this repo, which doesn't scale across many benchmarks, c
...
💬 instagibbs commented on pull request "test: enhance p2p_orphan_handling":
(https://github.com/bitcoin/bitcoin/pull/31037#issuecomment-2471202429)
nice cleanup :+1:
💬 maflcko commented on issue "valgrind: Conditional jump or move depends on uninitialised value(s)":
(https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2471212773)
> I think we could move to Ocular (gives us Valgrind 3.23.0

Isn't valgrind 3.22 enough, so we could just stay with the LTS Ubuntu? Also, the suppression won't work for libc++, likely. Not sure if this is worth it to support.
💬 fanquake commented on issue "valgrind: Conditional jump or move depends on uninitialised value(s)":
(https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2471216093)
> Also, the suppression won't work for libc++

We aren't using libc++ though?
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2471226445)
fwiw I ran a full memtest for 4 hours and had no memory issues, so at this point I have no idea. it seems to be running stable for now.
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2471227779)
@laanwj suggested to post the utxoset when it gets corrupted again, will do that when it does.