Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 glozow converted_to_draft a pull request: "26.x: backport #29853 ("sign: don't assume we are parsing a sane Miniscript")"
(https://github.com/bitcoin/bitcoin/pull/29854)
Backports #29853.
💬 glozow commented on pull request "26.x: backport #29853 ("sign: don't assume we are parsing a sane Miniscript")":
(https://github.com/bitcoin/bitcoin/pull/29854#issuecomment-2061140568)
Thanks. Converted to draft until the PR for master is merged.
💬 fanquake commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2061152077)
> I can add it to the CI in this change if that's preferred?

I'm not sure. We ended up removing security/symbol checks from the CI, because most of them wont pass there, and even if they do pass in the CI, that doesn't really matter unless they pass in Guix too. I'm going to think about this a bit more, now that there is some renewed interest, and ideas for new checks (£29874).
📝 glozow opened a pull request: "[26.x] archive 26.1 release notes + backports"
(https://github.com/bitcoin/bitcoin/pull/29899)
Archives 26.1 release notes and backports:
- #29691
- #29869
📝 maflcko opened a pull request: "ci: Run everything in Nulgrind or Valgrind"
(https://github.com/bitcoin/bitcoin/pull/29900)
Currently segmentation faults or core dumps do not result in a stacktrace. This is problematic when debugging issues, such as https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2059515027

Fix it by running everything in Nulgrind, which has less overhead than Valgrind, unless Valgrind is already selected.
💬 maflcko commented on pull request "ci: Run everything in Nulgrind or Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29900#issuecomment-2061177361)
I looked into many alternatives, such as `gdb --batch --ex ...`, or `catchsegv`, but none of them worked in the CI setting.

I think the only reasonable alternative would be to implement a signal handler in the C++ code.

Another alternative would be to wait for C++ (or compilers) to port `RUST_BACKTRACE`, but that seems unlikely, or at least far out in the feature.
💬 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-2061178176)
> stack trace

#29900
👍 fanquake approved a pull request: "build: Fix false positive `CHECK_ATOMIC` test"
(https://github.com/bitcoin/bitcoin/pull/29859#pullrequestreview-2005982397)
ACK dd3e0fa12534c9e782dc9c24d2e30b70a0d73176
🚀 fanquake merged a pull request: "build: Fix false positive `CHECK_ATOMIC` test"
(https://github.com/bitcoin/bitcoin/pull/29859)
👍 fanquake approved a pull request: "chore: fix some typos in comments"
(https://github.com/bitcoin/bitcoin/pull/29875#pullrequestreview-2005985412)
ACK b1ee4a557beb1b4c65eca81c567a4afa2a7a23ca
🚀 fanquake merged a pull request: "chore: fix some typos in comments"
(https://github.com/bitcoin/bitcoin/pull/29875)
💬 fanquake commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2061219985)
Backported to 27.x in #29888.
💬 paplorinc commented on pull request "ci: Run everything in Nulgrind or Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29900#discussion_r1568817395)
Not sure I fully understand the consequences here, but do we want to replace the files with empty ones when `USE_VALGRIND` is false?
Shouldn't we execute the old ones in that case?
💬 mzumsande commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568834899)
would it make sense to match peers and (if possible) always pick a child from the orphanage that was sent to us by the same peer that sent us the parent (instead of a random one)?
That way, it wouldn't be possible that a third peer could send us multiple low-fee children that we'd store in the orphanage, in the hope that we pick one of those and reject the package.
💬 tobtoht commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2061263419)
(Testing a script I'm developing on this PR)

- [x] This package was signed with a GPG key that was used to sign a previous release: `3176EF7DB2367F1FCA4F306B1F9B0E909AF37285`
- [x] All files in common with the git repository have matching hashes.
- [ ] 26 files present in the tarball are not present in the git repository for a total of 57709 lines: https://paste.debian.net/plainh/ba0edfca
- [ ] After removal of the files above, this package contains no binaries, no archives and 4 generate
...
🤔 Sjors reviewed a pull request: "deploy: remove some tools when cross-compiling for macOS"
(https://github.com/bitcoin/bitcoin/pull/29890#pullrequestreview-2006019805)
Just tested that both the guix build and local `make deploy` still work on Intel macOS 14.4.1.

```
751ede1b4f680d44c97a9aab396e0a485e3a47c88ecc30ec8b83e53784fc3f50 guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/SHA256SUMS.part
871cf387d5d60efc0ec9e50f975a9f44b2e2f9b7d92d1f2744affc3c8a5e1655 guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin-unsigned.tar.gz
6a8de4ac9647549d146a53a9167b00f72ac09168939284b76fa9b6cf81595fea guix-build-1a9aa8d4eedf/o
...
💬 Sjors commented on pull request "deploy: remove some tools when cross-compiling for macOS":
(https://github.com/bitcoin/bitcoin/pull/29890#discussion_r1568828132)
78b6b5c485191b85ae201df9d5ef0bcdaaa9c190: note for other reviewers

`BUILD_OS` is set to `darwin` when either (see `configure.ac`):
1. we're _not_ cross-compiling and `TARGET_OS=darwin`; or
2. we are cross-compiling and `$build_os=darwin`

`BUILD_DARWIN` is set when `[test "$BUILD_OS" = "darwin"]`. So the `!BUILD_DARWIN` code path here is taken when we are cross compiling (from a non-darwin system).

After this commit `$STRIP` is only used for Windows stuff.

It's not clear to me what
...
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568861661)
I think I made this point in person from the philosophical standpoint that orphanage-churning aside, we probably shouldn't allow peers to "cross-talk" when it comes to package evaluation and possible punishment.

In practice I think an adversary can just churn the orphanage, but still might be a good conceptual framework to adhere to?
💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2061285959)
CI should be bumped as well, if this is taken out of draft. Just leaving a comment here, so that it isn't forgotten.
💬 naiyoma commented on pull request "Test/rpc whitelistdefault test":
(https://github.com/bitcoin/bitcoin/pull/29858#issuecomment-2061289481)
> Thank you for picking this up. Enhancing testing for of the whitelist capability is important to notice regressions associated with this security-minded feature.
>
> Concept ACK. Looks like there could be opportunities to enhance the existing approach taken. Some inline comments were left.
>
> Built and ran all functional tests (all passed).
>
> Checked that the following comments were addressed from PR #17805.
>
> * The additional tests appear to be integrated into rpc_whitelist.p
...