Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ€” 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
...
πŸ’¬ 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?
πŸ’¬ 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 πŸ‘
πŸ’¬ 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?
πŸ’¬ 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
...
πŸ’¬ 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.
πŸ’¬ 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.
πŸ’¬ t-bast commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2061480170)
> Ah so (900 + 172 * 483 * 2 + 224) / 4 = 41819vB?

That's correct, that is the maximum size of a commitment transaction today.

This size was chosen to ensure that signatures for all HTLCs could be transmitted in a single 65kB lightning p2p message. But we will likely need to transmit more data when we support PTLCs, which means we'll probably reduce the maximum number of allowed PTLCs (IIRC we'll have to divide the current limits roughly by two). That would reduce the maximum size of a com
...
πŸ’¬ ariard commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2061482383)
> So maybe a better number is 42KvB or 50KvB.

Yes, don’t forget the 2 * for anchor outputs.

> Regardless of that, I think you shouldn't care about the size of today's lightning transactions: whenever we start using TRUCs,

Still, what are TRUCs ? Do you have documentation ?
πŸš€ achow101 merged a pull request: "doc: Add example of mixing private and public keys in descriptors"
(https://github.com/bitcoin/bitcoin/pull/28373)
πŸ’¬ fanquake commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1569045988)
> Would it be possible to change these to values 0/1 instead of defined/undefined?

These are currently checked via: `AC_CHECK_HEADERS([sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h])`. If we wanted, to we could change this.
βœ… maflcko closed a pull request: "ci: Run everything in Nulgrind or Valgrind"
(https://github.com/bitcoin/bitcoin/pull/29900)
πŸ’¬ maflcko commented on pull request "ci: Run everything in Nulgrind or Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29900#issuecomment-2061593496)
I guess this is too fragile to be useful
πŸ’¬ 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-2061604188)
I guess it could make more sense if someone tried to reproduce this in `rr`.
πŸ’¬ theStack commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#issuecomment-2061659300)
Concept ACK, thanks for fixing!
⚠️ brunoerg opened an issue: "Wallet fuzzing tracking issue"
(https://github.com/bitcoin/bitcoin/issues/29901)
The wallet has poor fuzz coverage. Hopefully, some work is being done to improve it. The goal of this issue is to actively track current work and work that needs to be done to improve fuzz coverage for the wallet.

## Open PRs / Ready to review

- [ ] https://github.com/bitcoin/bitcoin/pull/29694
- [ ] https://github.com/bitcoin/bitcoin/pull/28236
- [ ] https://github.com/bitcoin/bitcoin/pull/28074

## Current wallet targets

We currently have 6 specific targets for wallet, which cover
...