💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951571563)
You're right, but that's exactly what the "delay chunking while sub-acceptable" commit is about. In the initial commit, the invariant is that the chunk feerates are maintained at all times (and the sanity check will fail if not). In this "delay chunking while sub-acceptable" commit, some updates are avoided while they are unnecessary. The commit after it ("special-case removal of tail of cluster") reintroduces some, because with that, performing a Split() can in fact immediately introduce optima
...
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951571563)
You're right, but that's exactly what the "delay chunking while sub-acceptable" commit is about. In the initial commit, the invariant is that the chunk feerates are maintained at all times (and the sanity check will fail if not). In this "delay chunking while sub-acceptable" commit, some updates are avoided while they are unnecessary. The commit after it ("special-case removal of tail of cluster") reintroduces some, because with that, performing a Split() can in fact immediately introduce optima
...
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951573976)
No, the order of removals is irrelevant. The comment is just about pointing out that while TxGraph is allowed to reorder dependency additions and transaction removals w.r.t. each other, this doesn't matter in practice. As long as every transaction removal is combined (doesn't matter when) with also removal of all its ancestors, or with all its descendants, this reordering does not affect its observable behavior.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951573976)
No, the order of removals is irrelevant. The comment is just about pointing out that while TxGraph is allowed to reorder dependency additions and transaction removals w.r.t. each other, this doesn't matter in practice. As long as every transaction removal is combined (doesn't matter when) with also removal of all its ancestors, or with all its descendants, this reordering does not affect its observable behavior.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951574878)
Same comment as before: in the initial commit, the invariant is that the chunk feerates are maintained at all times. It is relaxed later on.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1951574878)
Same comment as before: in the initial commit, the invariant is that the chunk feerates are maintained at all times. It is relaxed later on.
👍 theuni approved a pull request: "depends: Make default `host` and `build` comparable"
(https://github.com/bitcoin/bitcoin/pull/30584#pullrequestreview-2610020007)
utACK b28917be363fb5a82effffeadbe4ba27bb1c70ce.
Makes sense, thanks for fixing.
(https://github.com/bitcoin/bitcoin/pull/30584#pullrequestreview-2610020007)
utACK b28917be363fb5a82effffeadbe4ba27bb1c70ce.
Makes sense, thanks for fixing.
💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2652056781)
Fixed the typo. I'll add the other features and a fix for the `m_next_inv_to_inbounds` non-determinism that you mentioned in a follow-up, if that is all right?
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2652056781)
Fixed the typo. I'll add the other features and a fix for the `m_next_inv_to_inbounds` non-determinism that you mentioned in a follow-up, if that is all right?
💬 theuni commented on pull request "depends: Make default `host` and `build` comparable":
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2652074197)
@laanwj Interesting! Looks like cross builds of libevent for kqueue platforms suffer because of the wonky `check_c_source_runs` that can only succeed for native builds. That's probably worth addressing itself.
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2652074197)
@laanwj Interesting! Looks like cross builds of libevent for kqueue platforms suffer because of the wonky `check_c_source_runs` that can only succeed for native builds. That's probably worth addressing itself.
👍 pinheadmz approved a pull request: "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries"
(https://github.com/bitcoin/bitcoin/pull/31407#pullrequestreview-2610053597)
ACK 46e44a35b85830a60cf622e039db19ccf1989008
Tested on arm64/macos, but did not review code or test other platforms. This updated process closes the referenced issues and successfully creates signed binaries for `bitcoind`, `bitcoin-cli` and all other utilities including Bitcoin-Qt.
----
## Detached sigs (`tar xf` and commit locally in `bitcoin-detached-sigs` to test:
[signature-osx-arm64.tar.gz](https://github.com/user-attachments/files/18758816/signature-osx-arm64.tar.gz)
--
...
(https://github.com/bitcoin/bitcoin/pull/31407#pullrequestreview-2610053597)
ACK 46e44a35b85830a60cf622e039db19ccf1989008
Tested on arm64/macos, but did not review code or test other platforms. This updated process closes the referenced issues and successfully creates signed binaries for `bitcoind`, `bitcoin-cli` and all other utilities including Bitcoin-Qt.
----
## Detached sigs (`tar xf` and commit locally in `bitcoin-detached-sigs` to test:
[signature-osx-arm64.tar.gz](https://github.com/user-attachments/files/18758816/signature-osx-arm64.tar.gz)
--
...
💬 pinheadmz commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2652085416)
Bonus verbose code sign verification of `bitcoind`:
```
codesign -dv --verbose=4 /Users/matthewzipkin/Desktop/work/bitcoin/guix-build-46e44a35b858/output/arm64-apple-darwin/arm64-apple-darwin-codesigned/bitcoin-46e44a35b858/bin/bitcoind
Executable=/Users/matthewzipkin/Desktop/work/bitcoin/guix-build-46e44a35b858/output/arm64-apple-darwin/arm64-apple-darwin-codesigned/bitcoin-46e44a35b858/bin/bitcoind
Identifier=bitcoind
Format=Mach-O thin (arm64)
CodeDirectory v=20500 size=23284 flags=0
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2652085416)
Bonus verbose code sign verification of `bitcoind`:
```
codesign -dv --verbose=4 /Users/matthewzipkin/Desktop/work/bitcoin/guix-build-46e44a35b858/output/arm64-apple-darwin/arm64-apple-darwin-codesigned/bitcoin-46e44a35b858/bin/bitcoind
Executable=/Users/matthewzipkin/Desktop/work/bitcoin/guix-build-46e44a35b858/output/arm64-apple-darwin/arm64-apple-darwin-codesigned/bitcoin-46e44a35b858/bin/bitcoind
Identifier=bitcoind
Format=Mach-O thin (arm64)
CodeDirectory v=20500 size=23284 flags=0
...
💬 pinheadmz commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951625820)
not an issue anymore on 46e44a35b85830a60cf622e039db19ccf1989008
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951625820)
not an issue anymore on 46e44a35b85830a60cf622e039db19ccf1989008
✅ hebasto closed an issue: "Wallet symlinks are not rejected as expected"
(https://github.com/bitcoin/bitcoin/issues/31842)
(https://github.com/bitcoin/bitcoin/issues/31842)
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2652236274)
> My understanding is that m_total_orphan_usage exists to limit memory usage, and m_total_announcements to limit CPU usage - but why the number of announcements instead of number of orphans?
Yep, to limit CPU usage. The complexity of eviction for example is bounded by the total number of announcements: in the worst case, each orphan has many announcers and the `MaybeTrimOrphans` loop first removes announcements until each orphan just has 1 left, and then finally can remove transactions. See c
...
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2652236274)
> My understanding is that m_total_orphan_usage exists to limit memory usage, and m_total_announcements to limit CPU usage - but why the number of announcements instead of number of orphans?
Yep, to limit CPU usage. The complexity of eviction for example is bounded by the total number of announcements: in the worst case, each orphan has many announcers and the `MaybeTrimOrphans` loop first removes announcements until each orphan just has 1 left, and then finally can remove transactions. See c
...
🤔 mzumsande reviewed a pull request: "validation: set BLOCK_FAILED_CHILD correctly"
(https://github.com/bitcoin/bitcoin/pull/31835#pullrequestreview-2610319771)
Concept ACK
This is a straightforward bugfix.
In general, I'm in favor of removing the `BLOCK_FAILED_CHILD` / `BLOCK_FAILED_VALID` distinction as a follow-up, as suggested in https://github.com/bitcoin/bitcoin/pull/16856#issuecomment-565506585. It's been introduced in 2012 (#1677) and (as far as I know) has never been used for anything. I also can't think of a legitimate use - block indexes that are marked as invalid are invalid indefinitely (outside of the `reconsiderblock` rpc, but that
...
(https://github.com/bitcoin/bitcoin/pull/31835#pullrequestreview-2610319771)
Concept ACK
This is a straightforward bugfix.
In general, I'm in favor of removing the `BLOCK_FAILED_CHILD` / `BLOCK_FAILED_VALID` distinction as a follow-up, as suggested in https://github.com/bitcoin/bitcoin/pull/16856#issuecomment-565506585. It's been introduced in 2012 (#1677) and (as far as I know) has never been used for anything. I also can't think of a legitimate use - block indexes that are marked as invalid are invalid indefinitely (outside of the `reconsiderblock` rpc, but that
...
💬 yancyribbens commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2652350051)
I tracked down the PR that add this: https://github.com/bitcoin/bitcoin/pull/27585 and commit https://github.com/bitcoin/bitcoin/pull/27585/commits/6d9b26d56ab5295dfcfe0f80a3069046a263fb2f. I'm a little confused by this since this is counter to what I would expect. BnB seeks to find a changelss solution, however only if the change created is less than the cost of creating a new output. That is, if it's more expansive to add the change leftover to a new output, don't bother.
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2652350051)
I tracked down the PR that add this: https://github.com/bitcoin/bitcoin/pull/27585 and commit https://github.com/bitcoin/bitcoin/pull/27585/commits/6d9b26d56ab5295dfcfe0f80a3069046a263fb2f. I'm a little confused by this since this is counter to what I would expect. BnB seeks to find a changelss solution, however only if the change created is less than the cost of creating a new output. That is, if it's more expansive to add the change leftover to a new output, don't bother.
👋 achow101's pull request is ready for review: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247)
(https://github.com/bitcoin/bitcoin/pull/31247)
💬 achow101 commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2652378647)
Updated with the test vectors from https://github.com/bitcoin/bips/pull/1764
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2652378647)
Updated with the test vectors from https://github.com/bitcoin/bips/pull/1764
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951831500)
If I need to retouch
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951831500)
If I need to retouch
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951831539)
If I need to retouch
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951831539)
If I need to retouch
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2652428792)
Windows code signature:
[signature-win.tar.gz](https://github.com/user-attachments/files/18760608/signature-win.tar.gz)
It looks like I have a mismatch.
```
$ find guix-build-46e44a35b858/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c92e7d0b4d0d1f929bfe8d61e15a953738725d530fc64eb936634571c17936b7 guix-build-46e44a35b858/output/aarch64-linux-gnu/SHA256SUMS.part
ad3992a66f9de8039cabbc9f222f7369f8002fc5350a01b03a3194d574100770 guix-build-46e44a35b858/output/aarch
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2652428792)
Windows code signature:
[signature-win.tar.gz](https://github.com/user-attachments/files/18760608/signature-win.tar.gz)
It looks like I have a mismatch.
```
$ find guix-build-46e44a35b858/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c92e7d0b4d0d1f929bfe8d61e15a953738725d530fc64eb936634571c17936b7 guix-build-46e44a35b858/output/aarch64-linux-gnu/SHA256SUMS.part
ad3992a66f9de8039cabbc9f222f7369f8002fc5350a01b03a3194d574100770 guix-build-46e44a35b858/output/aarch
...
📝 luke-jr opened a pull request: "Add -pruneduringinit option to temporarily use another prune target during IBD"
(https://github.com/bitcoin/bitcoin/pull/31845)
Low prune sizes can force flushing long before dbcache is full, making a significant impact on real-world IBD time.
This new option allows a higher (or lower) prune size during IBD to mitigate this. When assumeutxo is active, the entire additional budget is allocated to the background chainstate.
(https://github.com/bitcoin/bitcoin/pull/31845)
Low prune sizes can force flushing long before dbcache is full, making a significant impact on real-world IBD time.
This new option allows a higher (or lower) prune size during IBD to mitigate this. When assumeutxo is active, the entire additional budget is allocated to the background chainstate.
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951862122)
Done
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1951862122)
Done