💬 Sjors commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/29691#issuecomment-2039240747)
@luke-jr can you GPG sign this commit?
Someone, possibly you, should also make a PR to update it here: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py (better still if it just parses `src/kernel/chainparams.cpp`).
I ran the above script locally and it's happy.
(https://github.com/bitcoin/bitcoin/pull/29691#issuecomment-2039240747)
@luke-jr can you GPG sign this commit?
Someone, possibly you, should also make a PR to update it here: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py (better still if it just parses `src/kernel/chainparams.cpp`).
I ran the above script locally and it's happy.
💬 Sjors commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-2039308803)
WIP in #29675!
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-2039308803)
WIP in #29675!
📝 brunoerg opened a pull request: "doc: i2p: improve `-i2pacceptincoming` mention"
(https://github.com/bitcoin/bitcoin/pull/29813)
In i2p documentation, it says that "the first time Bitcoin Core connects to the I2P router,
it automatically generates a persistent I2P address and its corresponding private key by
default **or if `-i2pacceptincoming=1` is set**". This is weird, because `-i2pacceptincoming=1`
by itself does not have any effect. Moreover, `-i2pacceptincoming` is 1 by default anyway.
(https://github.com/bitcoin/bitcoin/pull/29813)
In i2p documentation, it says that "the first time Bitcoin Core connects to the I2P router,
it automatically generates a persistent I2P address and its corresponding private key by
default **or if `-i2pacceptincoming=1` is set**". This is weird, because `-i2pacceptincoming=1`
by itself does not have any effect. Moreover, `-i2pacceptincoming` is 1 by default anyway.
💬 brunoerg commented on pull request "doc: i2p: improve `-i2pacceptincoming` mention":
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2039311131)
cc: @vasild
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2039311131)
cc: @vasild
✅ Sjors closed an issue: "Add taproot descriptor to existing descriptor wallet"
(https://github.com/bitcoin/bitcoin/issues/24193)
(https://github.com/bitcoin/bitcoin/issues/24193)
💬 Sjors commented on issue "Add taproot descriptor to existing descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/24193#issuecomment-2039314544)
The future is here!
(https://github.com/bitcoin/bitcoin/issues/24193#issuecomment-2039314544)
The future is here!
💬 fanquake commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039370588)
> I did run into a confusing, but unrelated (happens on https://github.com/bitcoin/bitcoin/commit/28f2ca675f89a764e1ec8eb5671b35357b8677f3 too, see https://github.com/bitcoin/bitcoin/issues/29792), failure with bdb,
Yes that is unrelated.
> CI seems unhappy for macOS cross:
Fixed up.
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039370588)
> I did run into a confusing, but unrelated (happens on https://github.com/bitcoin/bitcoin/commit/28f2ca675f89a764e1ec8eb5671b35357b8677f3 too, see https://github.com/bitcoin/bitcoin/issues/29792), failure with bdb,
Yes that is unrelated.
> CI seems unhappy for macOS cross:
Fixed up.
💬 maflcko commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039443040)
What is your `patch --version`?
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039443040)
What is your `patch --version`?
💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039482595)
patch 2.0-12u11-Apple
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039482595)
patch 2.0-12u11-Apple
💬 fanquake commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039500096)
Can you produce a more minified set of steps/environment to reproduce. i.e does running the same set of build steps outside of depends also fail?
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039500096)
Can you produce a more minified set of steps/environment to reproduce. i.e does running the same set of build steps outside of depends also fail?
👍 BrandonOdiwuor approved a pull request: "test: Bump timeouts in feature_index_prune and wallet_importdescriptors"
(https://github.com/bitcoin/bitcoin/pull/29791#pullrequestreview-1982820317)
crACK 49c0b8b2288e60ae22fcac5d03811cf36ecec058
(https://github.com/bitcoin/bitcoin/pull/29791#pullrequestreview-1982820317)
crACK 49c0b8b2288e60ae22fcac5d03811cf36ecec058
💬 Sjors commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039574673)
Also tested the guix build (made on AMD Ubuntu) for 77eaee0ba46a68268be352e7d9c39488f9339099 on macOS 14.4.1.
```
4fd8566ccc8229c4784eac5cc21d077b179b4165b457911fe142ea2e3b0069e7 guix-build-77eaee0ba46a/output/arm64-apple-darwin/SHA256SUMS.part
6cca0ae159eddbf88dd1858eec51625816346487fc5eb9e5260037c686e8e312 guix-build-77eaee0ba46a/output/arm64-apple-darwin/bitcoin-77eaee0ba46a-arm64-apple-darwin-unsigned.tar.gz
c967afbfd76efb0a16e54524734396e56afe249273620f59904253a8608d1416 guix-build
...
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039574673)
Also tested the guix build (made on AMD Ubuntu) for 77eaee0ba46a68268be352e7d9c39488f9339099 on macOS 14.4.1.
```
4fd8566ccc8229c4784eac5cc21d077b179b4165b457911fe142ea2e3b0069e7 guix-build-77eaee0ba46a/output/arm64-apple-darwin/SHA256SUMS.part
6cca0ae159eddbf88dd1858eec51625816346487fc5eb9e5260037c686e8e312 guix-build-77eaee0ba46a/output/arm64-apple-darwin/bitcoin-77eaee0ba46a-arm64-apple-darwin-unsigned.tar.gz
c967afbfd76efb0a16e54524734396e56afe249273620f59904253a8608d1416 guix-build
...
💬 naiyoma commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2039613643)
@stratospher thanks for the review . I think it would be more ideal to simply reference the seeder repo. However, for the sake of uniformity, I believe it's better to continue listing them as is currently being done.
> Concept ACK. thanks for opening this! it's easy to forget to update support for service bit filtering here and the list just keeps getting longer.
>
> another possibility could be just mentioning that the seeder supports service bit filtering and linking to the seeder repo t
...
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2039613643)
@stratospher thanks for the review . I think it would be more ideal to simply reference the seeder repo. However, for the sake of uniformity, I believe it's better to continue listing them as is currently being done.
> Concept ACK. thanks for opening this! it's easy to forget to update support for service bit filtering here and the list just keeps getting longer.
>
> another possibility could be just mentioning that the seeder supports service bit filtering and linking to the seeder repo t
...
💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039631300)
On todays master 5a5ab1d5446693ee2655860c2f2920bcc3389c83:
```sh
git clean -dfx
cd depends
make NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
```
Immediately fails:
```
Fetching db-4.8.30.NC.tar.gz from https://download.oracle.com/berkeley-db
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 21.7M 100 21.7M 0 0 19.2M 0 0:00:0
...
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039631300)
On todays master 5a5ab1d5446693ee2655860c2f2920bcc3389c83:
```sh
git clean -dfx
cd depends
make NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
```
Immediately fails:
```
Fetching db-4.8.30.NC.tar.gz from https://download.oracle.com/berkeley-db
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 21.7M 100 21.7M 0 0 19.2M 0 0:00:0
...
💬 maflcko commented on pull request "[WIP] ci: test secp256k1 MSAN asm annotations":
(https://github.com/bitcoin/bitcoin/pull/29742#discussion_r1553508634)
```suggestion
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'
```
nit: While touching, can remove the duplicate CFLAGS and CXXFLAGS. Same below.
(https://github.com/bitcoin/bitcoin/pull/29742#discussion_r1553508634)
```suggestion
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'
```
nit: While touching, can remove the duplicate CFLAGS and CXXFLAGS. Same below.
💬 maflcko commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039637101)
So can it be closed, or is there something that points to a problem in this repo that can be fixed here?
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039637101)
So can it be closed, or is there something that points to a problem in this repo that can be fixed here?
💬 fanquake commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039696450)
Guix Build (aarch64):
```bash
c39f93edbd44b97256834f1173f23c311a7f7fdf467ecebd4ec930e0691732ad guix-build-77eaee0ba46a/output/aarch64-linux-gnu/SHA256SUMS.part
4e88994692636055e105346229ad87fcb40569a984fddb278e92b78498a983ef guix-build-77eaee0ba46a/output/aarch64-linux-gnu/bitcoin-77eaee0ba46a-aarch64-linux-gnu-debug.tar.gz
f2d7c80d039ecfe9889eada5be4f701f413b22eb679511634a20545528b61a15 guix-build-77eaee0ba46a/output/aarch64-linux-gnu/bitcoin-77eaee0ba46a-aarch64-linux-gnu.tar.gz
df23e2
...
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039696450)
Guix Build (aarch64):
```bash
c39f93edbd44b97256834f1173f23c311a7f7fdf467ecebd4ec930e0691732ad guix-build-77eaee0ba46a/output/aarch64-linux-gnu/SHA256SUMS.part
4e88994692636055e105346229ad87fcb40569a984fddb278e92b78498a983ef guix-build-77eaee0ba46a/output/aarch64-linux-gnu/bitcoin-77eaee0ba46a-aarch64-linux-gnu-debug.tar.gz
f2d7c80d039ecfe9889eada5be4f701f413b22eb679511634a20545528b61a15 guix-build-77eaee0ba46a/output/aarch64-linux-gnu/bitcoin-77eaee0ba46a-aarch64-linux-gnu.tar.gz
df23e2
...
🤔 glozow reviewed a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1983063955)
Concept ACK, as I think we agree that we want to avoid *unintentional* recursion. I think it's useful to have all of our recursive functions enumerated.
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1983063955)
Concept ACK, as I think we agree that we want to avoid *unintentional* recursion. I think it's useful to have all of our recursive functions enumerated.
💬 glozow commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#discussion_r1553559809)
Not sure if the point is to discourage recursion, or to tell developers that it has to be marked explicitly? I think we can consider it implied that people should use recursion only when appropriate, just like any other practices that can lead to bugs. And disagreements about whether or not [fill in the blank] is appropriate can be resolved in review on a case-by-case basis.
```suggestion
- Recursion is checked by clang-tidy and thus must be made explicit. Use
```
(https://github.com/bitcoin/bitcoin/pull/29690#discussion_r1553559809)
Not sure if the point is to discourage recursion, or to tell developers that it has to be marked explicitly? I think we can consider it implied that people should use recursion only when appropriate, just like any other practices that can lead to bugs. And disagreements about whether or not [fill in the blank] is appropriate can be resolved in review on a case-by-case basis.
```suggestion
- Recursion is checked by clang-tidy and thus must be made explicit. Use
```
💬 maflcko commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#discussion_r1553606825)
In 1ffdf3f0ad10879cc4aab6a513af8d1c20c63151: Why? What is the point to put a single function consisting of a single `if` in a new file and module? I fail to see how more boilerplate than the actual logic is beneficial.
If your point is that large files shouldn't exist, then I tend to disagree. For example, `doc/developer-notes.md` is large and exists. Same for `./src/rpc/blockchain.cpp`.
My preference would be to not change code, unless there is a reason.
(https://github.com/bitcoin/bitcoin/pull/29744#discussion_r1553606825)
In 1ffdf3f0ad10879cc4aab6a513af8d1c20c63151: Why? What is the point to put a single function consisting of a single `if` in a new file and module? I fail to see how more boilerplate than the actual logic is beneficial.
If your point is that large files shouldn't exist, then I tend to disagree. For example, `doc/developer-notes.md` is large and exists. Same for `./src/rpc/blockchain.cpp`.
My preference would be to not change code, unless there is a reason.