💬 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" 😄
💬 laanwj commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063608628)
i think the point that it isn't actually used is a good one, and it would be good to make that clear, i see two options for that:
- Move it to directory called `example` (maybe better than `share/example`?)
- Put `example` in the file name
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063608628)
i think the point that it isn't actually used is a good one, and it would be good to make that clear, i see two options for that:
- Move it to directory called `example` (maybe better than `share/example`?)
- Put `example` in the file name
🤔 tdb3 reviewed a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2008638781)
ltgm.
cr re ACK for 7cad21a8bbd1e0b64a5e586b405c8619b9e701a5
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2008638781)
ltgm.
cr re ACK for 7cad21a8bbd1e0b64a5e586b405c8619b9e701a5
🤔 furszy reviewed a pull request: "refactor: interfaces, make 'createTransaction' less error-prone "
(https://github.com/bitcoin-core/gui/pull/807#pullrequestreview-2008661209)
Rebased after #806 merge.
(https://github.com/bitcoin-core/gui/pull/807#pullrequestreview-2008661209)
Rebased after #806 merge.
📝 furszy converted_to_draft a pull request: "index: blockfilter initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966)
The current procedure for building the block filter index involves processing filters one at a time;
Reading blocks, undo data, and previous headers from disk sequentially.
This PR introduces a new mechanism to perform the work concurrently. Dividing the filters
generation workload among a pool of workers that can be configured by the user,
significantly increasing the speed of the index construction process.
The same concurrent processing model has been applied to the transactions inde
...
(https://github.com/bitcoin/bitcoin/pull/26966)
The current procedure for building the block filter index involves processing filters one at a time;
Reading blocks, undo data, and previous headers from disk sequentially.
This PR introduces a new mechanism to perform the work concurrently. Dividing the filters
generation workload among a pool of workers that can be configured by the user,
significantly increasing the speed of the index construction process.
The same concurrent processing model has been applied to the transactions inde
...