๐ ismaelsadeeq opened a pull request: "interfaces: enable cancelling running `waitNext` calls "
(https://github.com/bitcoin/bitcoin/pull/33676)
This is an attempt to fix issue #32387 see the issue for background and the usefulness of this feature.
This PR uses one of the suggested approaches: adding a new `interruptWaitNext()` method to the mining interface.
It introduces a new boolean variable, `waiting`, which is set to `true` when the thread starts waiting. The `interruptWaitNext()` method wakes the thread and sets `waiting` to `false`.
Whenever the thread wakes up, it checks whether the wait was aborted; if so, it simply retu
...
(https://github.com/bitcoin/bitcoin/pull/33676)
This is an attempt to fix issue #32387 see the issue for background and the usefulness of this feature.
This PR uses one of the suggested approaches: adding a new `interruptWaitNext()` method to the mining interface.
It introduces a new boolean variable, `waiting`, which is set to `true` when the thread starts waiting. The `interruptWaitNext()` method wakes the thread and sets `waiting` to `false`.
Whenever the thread wakes up, it checks whether the wait was aborted; if so, it simply retu
...
๐ฌ waketraindev commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3431625011)
> ...When you see that a new contributor used an LLM, you can have your guard up.
Wouldn't it make sense to add those tags to all commits, regardless of whether assistance was used, to help encourage more thorough and attentive reviews? It seems important that reviewers stay thoughtful and cautious with all changes, no matter the level of assistance, the contributor's reputation, or their role in the project
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3431625011)
> ...When you see that a new contributor used an LLM, you can have your guard up.
Wouldn't it make sense to add those tags to all commits, regardless of whether assistance was used, to help encourage more thorough and attentive reviews? It seems important that reviewers stay thoughtful and cautious with all changes, no matter the level of assistance, the contributor's reputation, or their role in the project
๐ fanquake merged a pull request: "ci: Only write docker build images to Cirrus cache"
(https://github.com/bitcoin/bitcoin/pull/33639)
(https://github.com/bitcoin/bitcoin/pull/33639)
๐ fanquake merged a pull request: "ci: add Valgrind fuzz"
(https://github.com/bitcoin/bitcoin/pull/33461)
(https://github.com/bitcoin/bitcoin/pull/33461)
๐ฌ mzumsande commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-3431866885)
[df1c9c2](https://github.com/bitcoin/bitcoin/commit/df1c9c2e2f4b21578b79b08301bd10279ba0e22f) to [9806bc5](https://github.com/bitcoin/bitcoin/commit/9806bc54b7c053a5ea132a8abe0d27c8f9c5a65c): rebased (there was a silent conflict for `p2p_opportunistic_1p1c.py`, together with the `-maxconnections=94` default limit used in the functional tests)
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-3431866885)
[df1c9c2](https://github.com/bitcoin/bitcoin/commit/df1c9c2e2f4b21578b79b08301bd10279ba0e22f) to [9806bc5](https://github.com/bitcoin/bitcoin/commit/9806bc54b7c053a5ea132a8abe0d27c8f9c5a65c): rebased (there was a silent conflict for `p2p_opportunistic_1p1c.py`, together with the `-maxconnections=94` default limit used in the functional tests)
โ
maflcko closed a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308)
(https://github.com/bitcoin/bitcoin/pull/31308)
๐ maflcko reopened a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308)
This PR is the first step towards treating IWYU warnings as errors. At this stage, it applies only to the `crypto` and `index` directories.
(https://github.com/bitcoin/bitcoin/pull/31308)
This PR is the first step towards treating IWYU warnings as errors. At this stage, it applies only to the `crypto` and `index` directories.
๐ฌ maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3431891840)
Only small changes:
* Rebase
* one minimal doc fixup
* one minimal symbol rename in the ci script
re-ACK 02d2b5a11c921ef71c971ee80eb3dfbc75c8cb0d ๐ฎ
<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/Kly
...
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3431891840)
Only small changes:
* Rebase
* one minimal doc fixup
* one minimal symbol rename in the ci script
re-ACK 02d2b5a11c921ef71c971ee80eb3dfbc75c8cb0d ๐ฎ
<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/Kly
...
โ
fjahr closed a pull request: "init: Improve -asmap option behavior and documentation"
(https://github.com/bitcoin/bitcoin/pull/33632)
(https://github.com/bitcoin/bitcoin/pull/33632)
๐ฌ TheCharlatan commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3431918569)
Thanks for the review @stringintech,
Updated edf99b88e644c7d2a2db434c8db298c9c5303cf9 -> 20be96ee696ba8c3085da0877156e534a58c39bf ([kernelApi_75](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_75) -> [kernelApi_76](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_76), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_75..kernelApi_76))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2447802589), properly catch an
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3431918569)
Thanks for the review @stringintech,
Updated edf99b88e644c7d2a2db434c8db298c9c5303cf9 -> 20be96ee696ba8c3085da0877156e534a58c39bf ([kernelApi_75](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_75) -> [kernelApi_76](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_76), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_75..kernelApi_76))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2447802589), properly catch an
...
๐ฌ laanwj commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3431922921)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3431922921)
Concept ACK
๐ฌ laanwj commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#discussion_r2451792938)
nit: AS mapping versus "asn mapping", let's settle on one spelling.
(https://github.com/bitcoin/bitcoin/pull/33631#discussion_r2451792938)
nit: AS mapping versus "asn mapping", let's settle on one spelling.
๐ฌ laanwj commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3431948513)
Concept ACK. Splitting it to a separate filename option makes sense. imo it's better to avoid overloading the type (boolean or path) of options and introducing ambiguity. This is conceptually simpler for the user and simplifies the code.
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3431948513)
Concept ACK. Splitting it to a separate filename option makes sense. imo it's better to avoid overloading the type (boolean or path) of options and introducing ambiguity. This is conceptually simpler for the user and simplifies the code.
๐ฌ w0xlt commented on pull request "Add libbitcoinkernel example files":
(https://github.com/bitcoin/bitcoin/pull/33669#issuecomment-3431972960)
I wasnโt aware of the Charlatanโs website, but thatโs literally the idea behind this PR โ the user shouldnโt need to look for any external resources.
Also, many other Bitcoin libraries use the `examples` folder pattern.
That said, Iโm not opposed to having these examples in another place.
(https://github.com/bitcoin/bitcoin/pull/33669#issuecomment-3431972960)
I wasnโt aware of the Charlatanโs website, but thatโs literally the idea behind this PR โ the user shouldnโt need to look for any external resources.
Also, many other Bitcoin libraries use the `examples` folder pattern.
That said, Iโm not opposed to having these examples in another place.
๐ฌ laanwj commented on issue "[flatpak] - I cannot choose or change datadir when running from flatpak":
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3431981297)
> Allow flatpak to use the new data folder:
>
> ```
> > flatpak override --user --filesystem=/my/new/folder/for-btc-data org.bitcoincore.bitcoin-qt
> ```
This is the correct solution. Flatpak applications are sandboxed, and there's nothing bitcoin-qt can do to reach outside that, unless given explicit permission.
i would guess that after giving it permission, as above, you can also just use the qt chooser dialog to pick the datadir, so that it gets picked automatically in the future:
```
> f
...
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3431981297)
> Allow flatpak to use the new data folder:
>
> ```
> > flatpak override --user --filesystem=/my/new/folder/for-btc-data org.bitcoincore.bitcoin-qt
> ```
This is the correct solution. Flatpak applications are sandboxed, and there's nothing bitcoin-qt can do to reach outside that, unless given explicit permission.
i would guess that after giving it permission, as above, you can also just use the qt chooser dialog to pick the datadir, so that it gets picked automatically in the future:
```
> f
...
๐ฌ laanwj commented on issue "[flatpak] - I cannot choose or change datadir when running from flatpak":
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3432030077)
> I don't know much about the flatpak, but I'm guessing the "flatpak run" command is invoking bitcoin-qt with a hardcoded -datadir= argument that preempts the -choosedatadir argument:
Looks like you're right. Flatpak creates a wrapper script that simply passes `-datadir` unconditionally.
https://github.com/flathub/org.bitcoincore.bitcoin-qt/blob/master/install_wrappers.sh
There would be multiple ways to solve this. One way to fix it without changes to this repository would be to adapt the scri
...
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3432030077)
> I don't know much about the flatpak, but I'm guessing the "flatpak run" command is invoking bitcoin-qt with a hardcoded -datadir= argument that preempts the -choosedatadir argument:
Looks like you're right. Flatpak creates a wrapper script that simply passes `-datadir` unconditionally.
https://github.com/flathub/org.bitcoincore.bitcoin-qt/blob/master/install_wrappers.sh
There would be multiple ways to solve this. One way to fix it without changes to this repository would be to adapt the scri
...
๐ maflcko opened a pull request: " ci: Retry image building once on failure "
(https://github.com/bitcoin/bitcoin/pull/33677)
This should fix https://github.com/bitcoin/bitcoin/issues/33640.
It also contains a few refactor cleanups, which are explained in the corresponding commits.
(https://github.com/bitcoin/bitcoin/pull/33677)
This should fix https://github.com/bitcoin/bitcoin/issues/33640.
It also contains a few refactor cleanups, which are explained in the corresponding commits.
๐ฌ l0rinc commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432095621)
I don't really see any speedup for `AssembleBlock`
<img width="679" height="476" alt="image" src="https://github.com/user-attachments/assets/fc804b80-241b-446f-bfc1-28b222d3190e" />
Change: gcc=-0.71%, clang=-0.82%
------
`CCoinsCaching` does seem to be improved a bit
<img width="664" height="477" alt="image" src="https://github.com/user-attachments/assets/fc248746-1716-41b1-ae0c-ac08b6c3dbd5" />
Change: gcc=+18.87%, clang=+11.79%
------
<details>
<summary>Benchmark</sum
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432095621)
I don't really see any speedup for `AssembleBlock`
<img width="679" height="476" alt="image" src="https://github.com/user-attachments/assets/fc804b80-241b-446f-bfc1-28b222d3190e" />
Change: gcc=-0.71%, clang=-0.82%
------
`CCoinsCaching` does seem to be improved a bit
<img width="664" height="477" alt="image" src="https://github.com/user-attachments/assets/fc248746-1716-41b1-ae0c-ac08b6c3dbd5" />
Change: gcc=+18.87%, clang=+11.79%
------
<details>
<summary>Benchmark</sum
...
๐ฌ laanwj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3432096233)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3432096233)
Concept ACK
๐ ismaelsadeeq's pull request is ready for review: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676)
(https://github.com/bitcoin/bitcoin/pull/33676)