Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 darosior commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3457444677)
Concept ACK.
💬 brunoerg commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2470293205)
Perfect, I wasn't aware of `UpdateModifiedFee`, I just took a look and this is a saturated addition, so it won't have any effect. This can be ignored.
💬 willcl-ark commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3457495113)
No match.

```
❯ uname -m; find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
x86_64
76d395814824929ef63caf98b3f5c0d4be2aa310c91c86cdd4fa7f4835a2462e guix-build-a99bbec14500/output/aarch64-linux-gnu/SHA256SUMS.part
4261545a6177bd944b93e1590d3d5fed5466bcc7aa5cb8f34be48d5aba7f0286 guix-build-a99bbec14500/output/aarch64-linux-gnu/bitcoin-a99bbec14500-aarch64-linux-gnu-debug.tar.gz
50c7bdd8f65ffafc5dbe303705ab3e0b2d80f3834a0
...
🤔 hebasto reviewed a pull request: "refactor: Return uint64_t from GetSerializeSize"
(https://github.com/bitcoin/bitcoin/pull/33724#pullrequestreview-3389834738)
Concept ACK.
💬 maflcko commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-3457573218)
Looks like the new valgrind fuzz task is failing due to a false positive warning, which needs to be disabled like in the other GCC tasks:

```
$ git grep maybe-uninitialized ./ci
ci/test/00_setup_env_arm.sh:export BITCOIN_CONFIG="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"
ci/test/00_setup_env_win64.sh:-DCMAKE_CXX_FLAGS='-Wno-error=maybe-uninitialized'"
📝 hebasto opened a pull request: "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`"
(https://github.com/bitcoin/bitcoin/pull/33725)
This PR continues the work from bitcoin/bitcoin#31308 by extending the treatment of IWYU warnings as errors to the `src/init` and `src/policy` directories.
💬 reardencode commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457597324)
utACK 9c3291f

Given recent developments, it seems that including Bitcoin Knots' maintainer's seed as a trusted bootstrap source for bitcoin core users is no longer advisable.
💬 delta1 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457601403)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Luke considers Bitcoin Core v30 to be "malware", so he probably doesn't want to provide a DNS seed for Core nodes any longer.

<img width="313" height="139" alt="image" src="https://github.com/user-attachments/assets/f3512f65-95ce-47ef-940f-88e763295331" />
💬 maflcko commented on issue "ci: Where to run heavy and high-maintenance CI tasks?":
(https://github.com/bitcoin/bitcoin/issues/33668#issuecomment-3457602512)
> As for fragile, I'm not sure I like the idea of just running fragile things less over running them regularly to get more data on why they are fragile and hopefully fixing that. That said, fixing fragile CI jobs often takes significant development effort and you can often be left wondering if it was worth the investment over disabling the job or accepting a certain amount of flake.

Ah, I think "fragile" may not have been the right word. I was not trying to refer to flaky CI tasks with non-dete
...
💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470424759)
Maybe we should relax this atomic operation so that it doesn't hide any issues by synchronizing them for us.
```suggestion
void operator()() const { m_counter.fetch_add(1, std::memory_order_relaxed); }
```
💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457655364)
I just queried all the DNS seeds and compared responses with BitNodes API, using `jq` to isolate the user agent:

https://gist.github.com/pinheadmz/ff6057f14aa6b9aa642db2c98f331a1a

No real anomolies to speak of, except that `seed.mainnet.achownodes.xyz` returned the **most** knots nodes 🤷
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`":
(https://github.com/bitcoin/bitcoin/pull/33725#issuecomment-3457680787)
Friendly ping @l0rinc @maflcko @ryanofsky @willcl-ark who reviewed https://github.com/bitcoin/bitcoin/pull/31308.
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`":
(https://github.com/bitcoin/bitcoin/pull/33725#discussion_r2470457941)
nit in 30e5b015c973c1ac375e1eae6649238c6c2e2b28: This seems wrong, as there is no boost usage here?

See also my previous comment on this: https://github.com/bitcoin/bitcoin/pull/32668#issuecomment-2934697644
📝 MamunC0der opened a pull request: "chore(ci): bump artifact actions to v5"
(https://github.com/bitcoin/bitcoin/pull/33726)
switch the artifact upload/download steps to the v5 Actions release so we pick up the Node 24 compatibility and latest fixes; proof:[ actions/upload-artifact v5.0.0 release](https://github.com/actions/upload-artifact/releases/tag/v5.0.0)
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`":
(https://github.com/bitcoin/bitcoin/pull/33725#discussion_r2470480649)
It'd also be good if that commit message was more explanatory:
> Additional header changes were required due to visibility adjustments.

Which additional changes? What visibility adjustments?
💬 waketraindev commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457712557)
> update: dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us is not returning v29 or v30 nodes, apparently

I had 9 tries and had the same experience.
fanquake closed a pull request: "chore(ci): bump artifact actions to v5"
(https://github.com/bitcoin/bitcoin/pull/33726)
💬 achow101 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457730865)
Point 0 of the policy states:

> A DNS seed operating organization or person is expected to follow good host security practices, maintain control of applicable infrastructure, and not sell or transfer control of the DNS seed. Any hosting services contracted by the operator are equally expected to uphold these expectations.

Given that Luke was hacked a couple years ago, arguably his DNS seed should have been removed at that time. Furthermore, his personal website still has the following disc
...
💬 fanquake commented on pull request "random: clarify where the environ extern is needed":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2470511528)
I'm not 100% sure that is the only place it's needed, however this has historically been unclear, and the only platform I'm aware of that definitely needs it, is macOS.
💬 m3dwards commented on issue "ci: Where to run heavy and high-maintenance CI tasks?":
(https://github.com/bitcoin/bitcoin/issues/33668#issuecomment-3457750787)
> I was more referring to tasks that are deterministically failing, albeit with false-positive warnings, or otherwise hard-to-understand and hard-to-act-on error messages, blocking pull requests for unrelated reasons

Ah, in that case, sounds good to run them less frequently 👍