🚀 glozow merged a pull request: "test: add conflicting topology test case"
(https://github.com/bitcoin/bitcoin/pull/30066)
(https://github.com/bitcoin/bitcoin/pull/30066)
👍 hebasto approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065686592)
ACK a057869aa3c42457570765966cb66accb2375b13, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size = 86 KiB` in the configure summary.
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065686592)
ACK a057869aa3c42457570765966cb66accb2375b13, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size = 86 KiB` in the configure summary.
🤔 glozow reviewed a pull request: "test: remove unneeded `-maxorphantx=1000` settings"
(https://github.com/bitcoin/bitcoin/pull/30133#pullrequestreview-2065702888)
ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 From skimming the tests, it appears that none of these need a larger `-maxorphantx`.
It looks like the extra `-maxorphantx=1000`s have existed since each of the tests were introduced. My best guess is this was common when package limits were higher and/or tx relay and orphanage worked a bit differently, and then not cleaned up. For example, descendant packages of 1000 were allowed when mempool_packages.py was first added (along with package trackin
...
(https://github.com/bitcoin/bitcoin/pull/30133#pullrequestreview-2065702888)
ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 From skimming the tests, it appears that none of these need a larger `-maxorphantx`.
It looks like the extra `-maxorphantx=1000`s have existed since each of the tests were introduced. My best guess is this was common when package limits were higher and/or tx relay and orphanage worked a bit differently, and then not cleaned up. For example, descendant packages of 1000 were allowed when mempool_packages.py was first added (along with package trackin
...
🚀 glozow merged a pull request: "test: remove unneeded `-maxorphantx=1000` settings"
(https://github.com/bitcoin/bitcoin/pull/30133)
(https://github.com/bitcoin/bitcoin/pull/30133)
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441254)
Removed from this location
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441254)
Removed from this location
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606440905)
done
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606440905)
done
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441158)
Thanks, taken here and elsewhere it was being used.
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441158)
Thanks, taken here and elsewhere it was being used.
👍 jonasnick approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065791232)
utACK
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065791232)
utACK
💬 glozow commented on pull request "policy: restrict all TRUC (v3) transactions to 10KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1606540111)
Added. Ran fuzzer for a bit, seems happy
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1606540111)
Added. Ran fuzzer for a bit, seems happy
🤔 glozow reviewed a pull request: "policy: restrict all TRUC (v3) transactions to 10KvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2065844531)
fixed commit message, thanks :+1:
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2065844531)
fixed commit message, thanks :+1:
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2120121913)
Rebased for #29817
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2120121913)
Rebased for #29817
💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120147856)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120147856)
Concept ACK
💬 fanquake commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2120154716)
Guix build (aarch64):
```bash
e7f19ceb7286ff81d1464575418d494dc2143d43cdc8b0e9a1bff7e47cffc773 guix-build-a057869aa3c4/output/aarch64-linux-gnu/SHA256SUMS.part
f7e9fa7ada1cc470342e7851862b25a026add7dbd2a441f22327f516268eabb3 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu-debug.tar.gz
30e3001bf9e9ea03defd21f094887e7c9a5f5b4c95162fbf5ea7d9cbe7458458 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu.tar.gz
3c520e
...
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2120154716)
Guix build (aarch64):
```bash
e7f19ceb7286ff81d1464575418d494dc2143d43cdc8b0e9a1bff7e47cffc773 guix-build-a057869aa3c4/output/aarch64-linux-gnu/SHA256SUMS.part
f7e9fa7ada1cc470342e7851862b25a026add7dbd2a441f22327f516268eabb3 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu-debug.tar.gz
30e3001bf9e9ea03defd21f094887e7c9a5f5b4c95162fbf5ea7d9cbe7458458 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu.tar.gz
3c520e
...
👍 cbergqvist approved a pull request: "cli: Detect port errors in rpcconnect and rpcport"
(https://github.com/bitcoin/bitcoin/pull/29521#pullrequestreview-2065901985)
re ACK 24bc46c83b39149f4845a575a82337eb46d91bdb
Inspected `git range-diff 867a673~2..867a673 24bc46c~2..24bc46c`.
Ran same tests as last time (https://github.com/bitcoin/bitcoin/pull/29521/#pullrequestreview-2046768298) with same results.
Tried to repro a boiled down version of the mysterious compiler warning but was unable to reproduce it. Here is what I ended up with: https://godbolt.org/z/fh5KcbbeW
(Not sure about the legalese around uploading Bitcoin Core source to Godbolt but just
...
(https://github.com/bitcoin/bitcoin/pull/29521#pullrequestreview-2065901985)
re ACK 24bc46c83b39149f4845a575a82337eb46d91bdb
Inspected `git range-diff 867a673~2..867a673 24bc46c~2..24bc46c`.
Ran same tests as last time (https://github.com/bitcoin/bitcoin/pull/29521/#pullrequestreview-2046768298) with same results.
Tried to repro a boiled down version of the mysterious compiler warning but was unable to reproduce it. Here is what I ended up with: https://godbolt.org/z/fh5KcbbeW
(Not sure about the legalese around uploading Bitcoin Core source to Godbolt but just
...
💬 glozow commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2120203238)
Rebased for #29817 and added a "ensure we can always download a tx as long as we have 1 good outbound peer" fuzz test
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2120203238)
Rebased for #29817 and added a "ensure we can always download a tx as long as we have 1 good outbound peer" fuzz test
💬 maflcko commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1606611069)
I think the key point of both sentences is that all dependency versions must match exactly. Installing via distro packages or via pip are just two ways. Both can fail in the same way (outdated or too-recent dependency versions), so communicating them in the same section makes sense, but again, just a nit.
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1606611069)
I think the key point of both sentences is that all dependency versions must match exactly. Installing via distro packages or via pip are just two ways. Both can fail in the same way (outdated or too-recent dependency versions), so communicating them in the same section makes sense, but again, just a nit.
📝 hebasto opened a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/30143)
This PR updates the NetBSD Build Guide to reflect:
- the recent NetBSD Release
- GCC minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28348 and https://github.com/bitcoin/bitcoin/pull/29091)
- Python minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28211)
Also a smaller package set has been suggested:
- `boost-headers` instead of the full `boost`
- `qt5-qtbase qt5-qttools` instead of the full `qt5` (similar to https://github.com/bitcoin/b
...
(https://github.com/bitcoin/bitcoin/pull/30143)
This PR updates the NetBSD Build Guide to reflect:
- the recent NetBSD Release
- GCC minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28348 and https://github.com/bitcoin/bitcoin/pull/29091)
- Python minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28211)
Also a smaller package set has been suggested:
- `boost-headers` instead of the full `boost`
- `qt5-qtbase qt5-qttools` instead of the full `qt5` (similar to https://github.com/bitcoin/b
...
💬 maflcko commented on pull request "doc: Update NetBSD Build Guide":
(https://github.com/bitcoin/bitcoin/pull/30143#issuecomment-2120337184)
utACK 85e480a41ad0901a8b46759e921c187ebbbcccdf
(https://github.com/bitcoin/bitcoin/pull/30143#issuecomment-2120337184)
utACK 85e480a41ad0901a8b46759e921c187ebbbcccdf
💬 dergoegge commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606683947)
Structs that are part of the public interface should probably go into `txdownloadman.h`. Otherwise one needs to include the impl header, e.g. like `txdownloadman.h` does right now, which defeats the purpose of pimpling to some extend.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606683947)
Structs that are part of the public interface should probably go into `txdownloadman.h`. Otherwise one needs to include the impl header, e.g. like `txdownloadman.h` does right now, which defeats the purpose of pimpling to some extend.
💬 dergoegge commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606717505)
Iiuc correctly, clearing the filters here has been removed as we don't want `TxDownloadManager` to depend on `ChainstateManager`.
An alternative to the new synchronous validation interface callback could be to preserve the previous behavior (clearing the filters in `AlreadyHaveTx` on a tip change) by introducing an interface for TxDownloadMan to fetch the latest tip hash. For example, `TxDownloadOptions` could be extended to hold a lambda that returns the latest tip hash, which in production
...
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606717505)
Iiuc correctly, clearing the filters here has been removed as we don't want `TxDownloadManager` to depend on `ChainstateManager`.
An alternative to the new synchronous validation interface callback could be to preserve the previous behavior (clearing the filters in `AlreadyHaveTx` on a tip change) by introducing an interface for TxDownloadMan to fetch the latest tip hash. For example, `TxDownloadOptions` could be extended to hold a lambda that returns the latest tip hash, which in production
...