Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 m3dwards commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2624979670)
I was able to compile with Clang 19.1.7 by using LLVM's linker `lld`.

```shell
brew install lld
cmake --preset=libfuzzer -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" -DAPPEND_LDFLAGS=-Wl,-no_warn_duplicate_libraries -DCMAKE_CXX_FLAGS="-fuse-ld=lld"
```

```shell
[100%] Linking CXX executable fuzz
[100%] Built target fuzz
```

A note that clang 19.1.7's sanitizers required a minimum OSX target of 14.7.
💬 ryanofsky commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2625046815)
I can't speak for anyone else but I am using "experimental" to mean that the feature is new and has not had as much testing as other features because it is new. Therefore, it is not recommended for widespread use, but is recommended for use by people who want to experiment and help improve it.

> I feel like in the past we have fairly regularly had "experimental" features, in the sense that we didn't want to commit to this feature being a stable interface

Right this is not really what what I am
...
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2625046923)
@fanquake @pinheadmz Can one of you please do a build and make detached sigs for this PR for testing?
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2625064735)
@ryanofsky getting a 403 again this time on a different job: https://cirrus-ci.com/task/6262892556713984?logs=ci#L1459

This might just be an availability issue?
💬 davidgumberg commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2625065868)
utreACK https://github.com/bitcoin/bitcoin/commit/aeb3977db51792d8ad22b0a5ff8fc5ff20d69a00
💬 sr-gi commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#discussion_r1935967583)
I thought about this, but some of these are only used in this test.

I just realized that some of them were even duplicated amongst themselves. For instance, `INBOUND_PEER_TX_DELAY` and `NONPREF_PEER_TX_DELAY` refer to the same concept.

I'll do a second pass to further trim this
📝 danielabrozzoni opened a pull request: "rpc: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767)
Previously, -nodebug cleared all prior -debug configurations in the command line while allowing subsequent debug options to be applied.
However, -debug=0 and -debug=none completely disabled debugging, even for categories specified afterward.

This commit ensures consistency by making -debug=0 and -debug=none behave like -nodebug: they now clear previously set debug configurations but do not disable debugging for categories specified later.

See https://github.com/bitcoin/bitcoin/pull/30
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r1935978404)
> I'm not familiar with the snapshot code.

It seems to me that it may make more sense to move the warning to `CCoinsViewDB::BatchWrite` instead, which could already have `m_dirty` available through the `CoinsViewCacheCursor`, and since it's called from `Flush` and `Sync`, an AssumeUTXO load-and-dump would also trigger the warning (not just an IBD).

I also don't mind doing it myself in a follow-up, if you think it's a good idea.
💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2625086314)
> @ryanofsky getting a 403 again this time on a different job: https://cirrus-ci.com/task/6262892556713984?logs=ci#L1459

Thank you and good find. The error actual error there is `make: *** [funcs.mk:320: /ci_container_base/depends/sources/download-stamps/.stamp_fetched-native_libmultiprocess-.hash] Error 22` which indicates a bug in the depends makefile in the base PR.

It looks like make is trying to download the libmultiprocess library when it should be using a subtree shouldn't be trying
...
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1935982283)
> > without this PR's change, we may be doing the re-evaluation work up to num_inbound times erroneously?
>
> yes - I suggested this because it seemed a bit wasteful in the honest case, but it is probably no big deal, because the actual full validation will only be done once and checking inputs repeatedly shouldn't be too much additional work.

Hm. What do you mean by "the actual full validation will only be done once" btw?
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r1935983684)
Yes, thank you.
The only remaining nit is that in [other cases we set it dirty first](https://github.com/bitcoin/bitcoin/blob/586f2a6b1b53cb57342fa7c1135f1ea1e9c63705/src/coins.cpp#L103-L104), before incrementing (on a single thread this likely won't matter, though).
💬 danielabrozzoni commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1935984413)
Thank you to both of you! I opened https://github.com/bitcoin/bitcoin/pull/31767 :)

> FWIW, when I gave this suggestion to chatgpt it suggested:

I ended up using a for loop instead of ranges, but I kept the rest; I put you as the commit co-author, let me know if you'd rather not be a co-author though!
💬 sr-gi commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#issuecomment-2625102131)
@tdb3 I de-duplicated even more, plus renamed some of the remaining ones to match the actual names in `txdownloadman.h` (and updated the comment referring to `net_processing`, given the constants do not live there anymore).

As mentioned in-line, notice how `INBOUND_PEER_TX_DELAY` and `NONPREF_PEER_TX_DELAY` refer to the same concept. The latter was introduced in #20171 instead of remaining (or re-using) the former.
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r1935992350)
> I don't understand why you're looking at snapshot size

> not a good measure of data written

> and not use snapshot size as a proxy

I understand that the dirty entries in the snapshot are not a perfect representation (except for the AssumeUTXO tests I was doing, which likely skewed my recommendations), but I though it's what we're already doing here when we're calculating "approx_disk_usage" from "coins_dirty_count".

> None of that seems a good fit for this PR, though

I agree, if
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r1935997972)
Sure, though this won't warn us if it actually becomes unused.
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r1935999555)
Alternatively, we could collect a few hundred stale blocks (found 45 raw blocks so far via https://github.com/bitcoin-data/stale-blocks) and create a stale-block-proxy node for testing reorgs with real data:
* A freshly built test node would connect only to this proxy, which would simulate the original chain forks.
* It serves headers & blocks exactly as they were originally seen (without storing them, requesting non-stale ones from the network).
* The proxy pretends a stale block is the tip,
...
💬 victorvianna commented on pull request "[WIP] leveldb: pull upstream C++23 changes":
(https://github.com/bitcoin/bitcoin/pull/31766#issuecomment-2625180290)
FYI the upstream PR landed
💬 yancyribbens commented on pull request "test: Add expected result assertions":
(https://github.com/bitcoin/bitcoin/pull/31656#issuecomment-2625214697)
@theuni maybe you could review this? In short, the test does not test what it purports to without also asserting the expected result.
💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2625235833)
> The macOS 14 native no depends job fails with [chaincodelabs/libmultiprocess#138](https://github.com/chaincodelabs/libmultiprocess/issues/138).

Seems to be passing now so I guess the attempted fix in f3f4685bc4f8d42c6ef6dbd223de25da235155ee is working. Will go ahead and merge that and add it to the base PR.

I also have a fix for the 403 download / "Checksum missing or mismatched for native_libmultiprocess" errors.

<details><summary>diff</summary>
<p>

```diff
--- a/depends/funcs.m
...