Bitcoin Core Github
44 subscribers
121K links
Download Telegram
βœ… fanquake closed an issue: "Release schedule for 27.0"
(https://github.com/bitcoin/bitcoin/issues/29028)
πŸ’¬ laanwj commented on pull request "doc: archive 27.0 release notes":
(https://github.com/bitcoin/bitcoin/pull/29886#issuecomment-2058543218)
ACK c08754971d207bd2b60ba9c4faf34396a97bbc26
No output for
```
git diff c08754971d207bd2b60ba9c4faf34396a97bbc26:doc/release-notes/release-notes-27.0.md v27.0:doc/release-notes.md
```
πŸ’¬ maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2058544837)
For reference, the previous bump was e1ce5b8ae9124717c00dca71a5c5b43a7f5ad177, which is in master only and not yet in a release branch.
πŸ’¬ hebasto commented on issue "`test/streams_tests.cpp` fails to compile on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058578466)
> However, my preference would be to completely avoid `signed char` and just use `int8_t` in the serialization code. That is, fix the test to use `int8_t`.

I lean to agree, considering that `signed char` is used in tests only.
πŸ‘ hebasto approved a pull request: "guix: remove `gcc-toolchain static` from Windows build"
(https://github.com/bitcoin/bitcoin/pull/29828#pullrequestreview-2003064639)
ACK 05da2460db895374ea1fd89e4b8b4b73689f8faf,

My Guix build:
```
450c0c4f45f9cb7ed7fc2ef6e7557b6a23004b82c951399da3b7635e8451a076 guix-build-05da2460db89/output/dist-archive/bitcoin-05da2460db89.tar.gz
5df68ab18636090c387bc90297356d0e148b02931d3a99c0f6d33cd268aa072b guix-build-05da2460db89/output/x86_64-w64-mingw32/SHA256SUMS.part
13e979f60d9296aa11081fbbb360404da9fbb797bb4663ed2d1189d800659b4f guix-build-05da2460db89/output/x86_64-w64-mingw32/bitcoin-05da2460db89-win64-debug.zip
d1cc
...
πŸ’¬ maflcko commented on issue "`test/streams_tests.cpp` fails to compile on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058600361)
Happy to review a pull, if someone creates one.
πŸ’¬ hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2058610414)
> FWIW I think it's a valid choice to remove what we're not using and re-introduce it when we do, the code is out there there's little point in keeping unused code in the repository.

I agree.
πŸ’¬ maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2058623912)
review ACK dd3e0fa12534c9e782dc9c24d2e30b70a0d73176
⚠️ maflcko opened an issue: "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust"
(https://github.com/bitcoin/bitcoin/issues/29889)
Happened on current master d29fc3a245c070494155dad4cf68b9c95d99c13e in ci_i686_multiprocess

```
Running tests: wallet_util_tests from wallet/test/rpc_util_tests.cpp
Running tests: scriptpubkeyman_tests from wallet/test/scriptpubkeyman_tests.cpp
Running tests: walletload_tests from wallet/test/walletload_tests.cpp
Running tests: group_outputs_tests from wallet/test/group_outputs_tests.cpp
Running tests: db_tests from wallet/test/db_tests.cpp
Running tests: ipc_tests from test/ipc_tests.c
...
πŸ’¬ maflcko commented on issue "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust":
(https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2058655310)
```
# podman exec ci_i686_multiprocess uname -a
Linux a943c4649ecd 5.14.21-150400.24.100-default #1 SMP PREEMPT_DYNAMIC Mon Dec 4 19:12:13 UTC 2023 (3f5cd84) x86_64 x86_64 x86_64 GNU/Linux
πŸ’¬ paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1567078029)
Removed the 3/4 to 6/8 change, kept only the test.
I don't yet understand how we expect this code to become more and more maintainable, if we don't regularly leave the campground cleaner than we've found it.
πŸ’¬ furszy commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2058716277)
> Is it worth it to partially revert? Why not just git revert 0faafb57f8298547949cbc0044ee9e925ed887ba?

I didn't do it because the revert is not clean. It conflicts with bbe82c116e72ca0638751e063bf564cd1fe5c4d5 and it would require an extra commit for the added doc (which, based on the issue, is a must have for me).
But np on doing it and squashing the commits down to one. One sec.
πŸ’¬ maflcko commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1567097669)
I don't see how using `auto` makes anything clearer.

There certainly are style and language features that make maintenance easier, but they need to be well motivated and explained. Also, if there is a good reason for them, they should be applied globally via CI and checks. Otherwise, they are again applied inconsistently.
πŸ’¬ paplorinc commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1567105912)
I agree. Is there any solution?

Is there any non-test change that I could delve intoβ€”preferably an optimization that doesn't require me to understand every part of the codeβ€”that would be welcome?
For example, like my other [base58 or bech32 speedups](https://github.com/bitcoin/bitcoin/pulls/paplorinc).
Or am I just barking up the wrong tree and should move on to other projects?
πŸ’¬ fanquake commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2058741052)
> I don't yet understand how we expect this code to become more and more maintainable, if we don't regularly leave the campground cleaner than we've found it.

The issue here is that even if you've found the campground, and while maybe it looks like it could use a little maintenance, if you zoom out for a birds-eye view, you'll actually notice that everyone else is busy somewhere next door, trying to fight the forest fires. Even if the code here is (maybe arbitrarily) "better" or "faster" by s
...
πŸ’¬ hebasto commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2058753271)
> > However, the changes in MSVC generated assembly code look quite significant.
> > Before stacking another performance deterioration change on top of the pile
>
> Isn't that because optimisations haven't been turned on? Otherwise, can you provide a concrete example of what you're talking about.

https://godbolt.org/z/of4T8hM8j provides examples with the [`/O2`](https://learn.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed) optimization flag.
πŸ’¬ paplorinc commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2058755998)
Thanks for the detailed answer @fanquake.

> everyone else is busy [...] trying to fight the forest fires

So how do we prevent future forest fires while extinguishing current ones?

I have looked through `good first issue` since the beginning, there's barely any, all of which were started by others, often years ago.
That's why I started working on what *I can* contribute instead.
πŸ‘ stickies-v approved a pull request: "doc: archive 27.0 release notes"
(https://github.com/bitcoin/bitcoin/pull/29886#pullrequestreview-2003249019)
ACK c08754971d207bd2b60ba9c4faf34396a97bbc26