💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-2063369209)
I just tried this myself with `--disable-wallet`, then call `help`, and the wallet RPCs were not printed. `write_all_rpc_commands` also uses `help()`, so I wonder how you achieved this outcome.
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-2063369209)
I just tried this myself with `--disable-wallet`, then call `help`, and the wallet RPCs were not printed. `write_all_rpc_commands` also uses `help()`, so I wonder how you achieved this outcome.
🚀 hebasto merged a pull request: "Change example address from legacy (P2PKH) to bech32m (P2TR)"
(https://github.com/bitcoin-core/gui/pull/808)
(https://github.com/bitcoin-core/gui/pull/808)
💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063421412)
> > I believe this is unrelated to this PR, see #29051. `make clean` and rebuilding should fix it.
>
> I'm on Windows and using msvc but I did do a clean build but it didn't help.
Does it still happen after you clear the git repo carefully and start from scratch? `git clean -dffx`, or similar, but this will delete all non-git files, so be careful.
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063421412)
> > I believe this is unrelated to this PR, see #29051. `make clean` and rebuilding should fix it.
>
> I'm on Windows and using msvc but I did do a clean build but it didn't help.
Does it still happen after you clear the git repo carefully and start from scratch? `git clean -dffx`, or similar, but this will delete all non-git files, so be careful.
💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556)
Addressed the recent @maflcko's comments.
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556)
Addressed the recent @maflcko's comments.
💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1570360242)
An explanation has been [added](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556) to the commit message.
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1570360242)
An explanation has been [added](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556) to the commit message.
💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1570360864)
Thanks! [Implemented](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556).
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1570360864)
Thanks! [Implemented](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063432556).
💬 maflcko commented on pull request "test: Validate transaction without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#issuecomment-2063435993)
For reference, this is already covered in the total coverage: https://maflcko.github.io/b-c-cov/total.coverage/src/consensus/tx_check.cpp.gcov.html
(https://github.com/bitcoin/bitcoin/pull/29862#issuecomment-2063435993)
For reference, this is already covered in the total coverage: https://maflcko.github.io/b-c-cov/total.coverage/src/consensus/tx_check.cpp.gcov.html
💬 maflcko commented on pull request "test: Validate transaction without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#issuecomment-2063436726)
lgtm ACK 1240610fcf0651f217fd01de387e1047dc18485f
(https://github.com/bitcoin/bitcoin/pull/29862#issuecomment-2063436726)
lgtm ACK 1240610fcf0651f217fd01de387e1047dc18485f
💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063446332)
Haven't reviewed anything in build_msvc
lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
...
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063446332)
Haven't reviewed anything in build_msvc
lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
...
💬 glozow commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1570382377)
Ah, I didn't even realize they were generated on the server. Edited.
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1570382377)
Ah, I didn't even realize they were generated on the server. Edited.
💬 glozow commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1570382877)
Removed
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1570382877)
Removed
💬 fanquake commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063525458)
Something a bit more concrete. We could consolidate all packages in depends to .tar.gz (which is also what we use for releases), and drop `bzip2` and eventually `xz`.
> Oh, I see. So the motivation is to drop, possibly non-determinisically created, "bootstrapped garbage"? Concept +1 on this.
We could also start doing this now, but it may be somewhat cleaner to do when packages are using CMake, so if reviewers want we could look at switching any of the open CMake depends PRs to downloading
...
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063525458)
Something a bit more concrete. We could consolidate all packages in depends to .tar.gz (which is also what we use for releases), and drop `bzip2` and eventually `xz`.
> Oh, I see. So the motivation is to drop, possibly non-determinisically created, "bootstrapped garbage"? Concept +1 on this.
We could also start doing this now, but it may be somewhat cleaner to do when packages are using CMake, so if reviewers want we could look at switching any of the open CMake depends PRs to downloading
...
💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063544415)
@dergoegge @sipa @sipsorcery
Mind having another look into this PR?
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2063544415)
@dergoegge @sipa @sipsorcery
Mind having another look into this PR?
💬 hebasto commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063547954)
Concept ACK on consolidating archive formats.
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063547954)
Concept ACK on consolidating archive formats.
💬 maflcko commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063554137)
> Something a bit more concrete. We could consolidate all packages in depends to .tar.gz (which is also what we use for releases), and drop `bzip2` and eventually `xz`.
Again, I don't understand what the goal is. I doubt that removing xz as a dependency is doable. For example, gcc seems to be fetched as a xz archive (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/gcc.scm#n803)? Even if it wasn't, I am sure there are other guix packages that depend on xz.
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063554137)
> Something a bit more concrete. We could consolidate all packages in depends to .tar.gz (which is also what we use for releases), and drop `bzip2` and eventually `xz`.
Again, I don't understand what the goal is. I doubt that removing xz as a dependency is doable. For example, gcc seems to be fetched as a xz archive (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/gcc.scm#n803)? Even if it wasn't, I am sure there are other guix packages that depend on xz.
💬 fanquake commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063559858)
> Again, I don't understand what the goal is.
The goal is to remove things we don't use. If we don't need bzip2, then I don't see a reason to keep it in our build release environment. Same reasoning for any other dep. Also, just because some thing may be used somewhere in the Guix bootstrap chain, isn't a reason to explicitly include it in our release env unnecessarily.
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2063559858)
> Again, I don't understand what the goal is.
The goal is to remove things we don't use. If we don't need bzip2, then I don't see a reason to keep it in our build release environment. Same reasoning for any other dep. Also, just because some thing may be used somewhere in the Guix bootstrap chain, isn't a reason to explicitly include it in our release env unnecessarily.
📝 willcl-ark opened a pull request: "Move bitcoin.conf to /share/examples dir in releases"
(https://github.com/bitcoin/bitcoin/pull/29903)
Motivated by #29139
Placing the example `bitcoin.conf` file in this directory makes it clearer that it will not actively be used by the binary in `bin/`, and must be moved elsewhere.
(https://github.com/bitcoin/bitcoin/pull/29903)
Motivated by #29139
Placing the example `bitcoin.conf` file in this directory makes it clearer that it will not actively be used by the binary in `bin/`, and must be moved elsewhere.
💬 laanwj commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063582138)
Good idea, code review ACK 9256693be16cd56183557cf0bef57979fd04c39c
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063582138)
Good idea, code review ACK 9256693be16cd56183557cf0bef57979fd04c39c
💬 fanquake commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063589430)
Note previous discussion / change in https://github.com/bitcoin/bitcoin/pull/25935. cc @josibake
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063589430)
Note previous discussion / change in https://github.com/bitcoin/bitcoin/pull/25935. cc @josibake
💬 willcl-ark commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063599267)
Thanks @fanquake
> it's more correct to have it in the root directory to 1) make it easier for the user to find it and 2) be clear that it is not an example config but a full config
1. IMO people can find it just fine in share/examples, in fact I'd be more likely to `ls` that dir looking for it personally...
2. Would be happy to meet @josibake halfway and classify it as a "full example config" 😄
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063599267)
Thanks @fanquake
> it's more correct to have it in the root directory to 1) make it easier for the user to find it and 2) be clear that it is not an example config but a full config
1. IMO people can find it just fine in share/examples, in fact I'd be more likely to `ls` that dir looking for it personally...
2. Would be happy to meet @josibake halfway and classify it as a "full example config" 😄