Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 fanquake commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2315732101)
```bash
du -csh testnet3/chainstate/ testnet3/blocks/
16G testnet3/chainstate/
194G testnet3/blocks/
210G total
```
Can increase to 240.
💬 fanquake commented on issue "ci: tidy job is producing output":
(https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3244852511)
> So if the output is a problem we could turn it off globally, or apply the patch there which suppresses it locally with a NOLINT annotation

I don't think we should have spurious/undocumented output in the CI, so it'd be good to fix this.
📝 stickies-v opened a pull request: "cmake: make missing Python interpreter behaviour more explicit"
(https://github.com/bitcoin/bitcoin/pull/33278)
We require Python 3.10 for multiple targets, and currently raise a general warning if it is missing, leading to:
1. functional test suite: runtime failure if python missing (as e.g. described in #31476)
2. maintenance targets: targets skipped if python missing
3. macos deploy target: target created, but build fails if python missing

This PR:
- adds a `BUILD_FUNCTIONAL_TESTS` option (default `BUILD_TESTS`, i.e. `ON`) and raises `FATAL_ERROR` if Python is missing and we're building the func
...
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r2315840032)
Good point! I do not know why I chose lowercase since it also violates the coding style. Maybe it was like that before and I left it as it is even though I touched those lines and the callers.

Append the following as a new commit or amend it into the last one `fuzz: make it possible to mock (fuzz) CThreadInterrupt`?

<details>
<summary>[patch] Uppercase methods of CThreadInterrupt</summary>

```diff
commit 486db14b9ce13a90b69f69dafb59d3d8932498f7 (HEAD -> fuzz_connman)
Parent: 0802398e
...
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-3244991538)
@brunoerg, this has 2 ACKs and a stale ACK from you, maybe you would like to take a look and re-ACK it?
⚠️ fanquake opened an issue: "build: clang-16 build broken on Debian Bookworm"
(https://github.com/bitcoin/bitcoin/issues/33279)
Our minimum required Clang is 16. Building with Clang-16 on Debian Bookworm is broken due to issue with capnp. See more details here: https://github.com/bitcoin-core/libmultiprocess/issues/199.

```bash
# clang-16 --version
Debian clang version 16.0.6 (15~deb12u1)

cmake --build build
...
In file included from /bitcoin/src/ipc/libmultiprocess/src/mp/util.cpp:6:
In file included from /bitcoin/src/ipc/libmultiprocess/include/mp/util.h:8:
In file included from /usr/include/capnp/schema.h:39:
In fil
...
💬 stickies-v commented on pull request "build: Set AUTHOR_WARNING on warnings":
(https://github.com/bitcoin/bitcoin/pull/33144#issuecomment-3245006172)
> Sounds good.

Thanks for the feedback, I've opened #33278.

> Happy to put this in draft for now

The changes in this/your PR are small, straightforward, can easily be reverted, and offer immediate practical benefit, so I think it makes sense to not block this on my much less straightforward alternative, so my ACK still stands and I'd be happy if this makes progress.
💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3245015482)
re-ACK 755152ac819a23acf2f9e70316134d74a04d589b
👍 rkrux approved a pull request: "wallet: relax external_signer flag constraints"
(https://github.com/bitcoin/bitcoin/pull/33112#pullrequestreview-3176217358)
re-ACK 03978530ad8dc9124307d2ffc7d64c24b784be0e

```
git range-diff a973403...0397853
```
⚠️ HowHsu opened an issue: "-loadblock indicated block files cannot find Magic Bytes"
(https://github.com/bitcoin/bitcoin/issues/33280)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

I first run `bitcoind -prune=1024 -stopatheight=840000 -datadir=empty_dir`, then there is some block files in `empty_dir/blocks`.
Then I copy empty_dir to empty_dir2, run `bitcoind -prune=1024 -stopatheight=840050 -datadir=empty_dir2`
Compare two blocks dir, I now have some blk.dat files relects height 840000 to 840050.
At last I run `bitcoind -datadir=empty_dir -assumevalid=0 -prune=10240
...
💬 Sjors commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3245132183)
utACK 46860de809a1e9c7d0a1d716f77e9df7fb0bcd1c

I studied 4d07eff13002384ed59a3b7f592be56a25b88c15 from scratch since it had quite a few changes.
💬 dergoegge commented on issue "asan: GCC warning about use-after-free":
(https://github.com/bitcoin/bitcoin/issues/33188#issuecomment-3245138164)
This is a false positive. If someone is using gcc with asan we could move `ASAN_UNPOISON_MEMORY_REGION(chunk, m_chunk_size_bytes);` above the `delete` call to avoid this warning (I suspect).
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245221402)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews

Friendly ping to coordinators for addressing issues:

- @sr-gi -- Catalan (ca)
- @ostruvek -- Czech (cs)
- @pryds -- Danish (da)
- @laanwj @sipa -- Dutch (nl)
- @Emzy -- German (de)
- @cryptomeow -- Greek (el)
- @jesterhodl -- Polish (pl)
📝 ryanofsky opened a pull request: "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning"
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:

```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```

This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
📝 stickies-v opened a pull request: "cmake: fatal error when PIE not supported"
(https://github.com/bitcoin/bitcoin/pull/33282)
This simultaneously avoids silently overriding user preferences while making it harder to accidentally build without PIE support by requiring an explicit setting.

This introduces two behaviour changes:
1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF will now be respected by our build system, instead of silently overriding it like before
2. when not explicitly disabled, lack of PIE support will raise a FATAL_ERROR instead of a WARNING, with a help instruction that this can be overridd
...
💬 ryanofsky commented on issue "ci: tidy job is producing output":
(https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3245345508)
re: https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3244852511

> I don't think we should have spurious/undocumented output in the CI, so it'd be good to fix this.

Agree it's important to fix because it makes output not useful for identifying real errors. Opened #33281 to try to work around this.
💬 glozow commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2316100070)
Agree 8 places is preferable when it's BTC but 0 places would have been better here. Didn't get the chance to fix this here, but keeping in mind for the future.
💬 glozow commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3245366459)
> Not everyone knows what "33106" is. To avoid it being interpreted as trying to hide something, could you please detail the changes in the PR title and description to help the reviewers and provide full transparency?
> Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.

Agree with this practice, changed now. The intention was not to "hide something" - that's pretty hurtful.
📝 fanquake opened a pull request: "contrib: update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/33283)
Update the fixed seeds pre 30 branch off.