💬 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
(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
(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)
(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
(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)
(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.
(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?
(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.
(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
...
(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
...
(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
...
(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?
(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.
(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
...
(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
...
💬 fanquake commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2061297943)
> 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 generated scripts: https://paste.debian.net/plainh/ed6bbbc7
I've pushed a change that switches to downloading the unbootstraped source tarball. Want to re-run your script?
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2061297943)
> 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 generated scripts: https://paste.debian.net/plainh/ed6bbbc7
I've pushed a change that switches to downloading the unbootstraped source tarball. Want to re-run your script?
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568894409)
great idea, will change 👍
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568894409)
great idea, will change 👍
💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568898268)
Got it, thanks. Worth opening a new PR?
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568898268)
Got it, thanks. Worth opening a new PR?
💬 tobtoht commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2061349222)
>Want to re-run your script?
- [x] No signature file found, however git tag `R_2_6_2` is 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.
- [x] Tarball does not contain any files that are not present in the git repository.
- [ ] This package contains no binaries, no archives and 4 generated scripts: https://paste.debian.net/plainh/beafa9aa
Those generated scr
...
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2061349222)
>Want to re-run your script?
- [x] No signature file found, however git tag `R_2_6_2` is 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.
- [x] Tarball does not contain any files that are not present in the git repository.
- [ ] This package contains no binaries, no archives and 4 generated scripts: https://paste.debian.net/plainh/beafa9aa
Those generated scr
...
💬 maflcko commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568925505)
If you want, yes, I am happy to review. Though, I won't create a pull myself.
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568925505)
If you want, yes, I am happy to review. Though, I won't create a pull myself.
💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2061378522)
> Here's the tweaks I get on a recent signet block:
>
Do you have a mainnet block before height 834761 indexed, then we could compare. I don't have signet data available.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2061378522)
> Here's the tweaks I get on a recent signet block:
>
Do you have a mainnet block before height 834761 indexed, then we could compare. I don't have signet data available.