π€ shahsb reviewed a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2797611994)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2797611994)
Concept ACK
π¬ shahsb commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2833412008)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2833412008)
Approach ACK
π¬ hebasto commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2833427483)
> > the changes in this PR could be much smaller and easier to review
>
>
>
> You mean to avoid the use of the switch statement? If you feel strongly about it I can revert that, but the code seemed easy to review in either way.
I mean to squash all changes into a single commit with functionality changes only. Refactoring changes, such as introducing the `switch` statement, could be added in a separate commit.
But not a strong opinion, though.
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2833427483)
> > the changes in this PR could be much smaller and easier to review
>
>
>
> You mean to avoid the use of the switch statement? If you feel strongly about it I can revert that, but the code seemed easy to review in either way.
I mean to squash all changes into a single commit with functionality changes only. Refactoring changes, such as introducing the `switch` statement, could be added in a separate commit.
But not a strong opinion, though.
π¬ l0rinc commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2833428659)
Since CI is checking every commit, I have split out the introduction of the new algorithm, demonstrating that it results in the same output as the original.
I'm not sure how to do that in a single commit - unless I add it as a test, which didn't seem necessary here.
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2833428659)
Since CI is checking every commit, I have split out the introduction of the new algorithm, demonstrating that it results in the same output as the original.
I'm not sure how to do that in a single commit - unless I add it as a test, which didn't seem necessary here.
π ismaelsadeeq approved a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355#pullrequestreview-2797625801)
ACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398
Using the `m_options.block_reserved_weight` wonβt cause issues, but generally, I think itβs best to separate concerns and make this a `constexpr`.
This also makes the mining code cleaner.
(https://github.com/bitcoin/bitcoin/pull/32355#pullrequestreview-2797625801)
ACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398
Using the `m_options.block_reserved_weight` wonβt cause issues, but generally, I think itβs best to separate concerns and make this a `constexpr`.
This also makes the mining code cleaner.
π¬ jesterhodl commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833441349)
> The erroneous format specs still remain: [#32295 (comment)](https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827238571)
Ok, for Polish there was one. I just corrected it.
I'm happy to correct other languages for this sort of issue, but I'm only granted access to Polish team in Transifex.
If someone grants me temporary access I can fix others and go back to being just Polish Team.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833441349)
> The erroneous format specs still remain: [#32295 (comment)](https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827238571)
Ok, for Polish there was one. I just corrected it.
I'm happy to correct other languages for this sort of issue, but I'm only granted access to Polish team in Transifex.
If someone grants me temporary access I can fix others and go back to being just Polish Team.
π€ romanz reviewed a pull request: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2797634240)
LGTM, many thanks!
Added some comments/suggestions above :)
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2797634240)
LGTM, many thanks!
Added some comments/suggestions above :)
π¬ ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2062624207)
Done.
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2062624207)
Done.
π¬ Ashkar776 commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-2833469762)
What this
On Sun, 27 Apr, 2025, 5:11 pm Abubakar Sadiq Ismail, <
***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/policy/fees/mempool_forecaster.cpp
> <https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2062624207>:
>
> > @@ -28,6 +28,13 @@ ForecastResult MemPoolForecaster::ForecastFeeRate(int target, bool conservative)
> return result;
> }
>
> + const auto cached_estimate = cache.get_cached_fo
...
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-2833469762)
What this
On Sun, 27 Apr, 2025, 5:11 pm Abubakar Sadiq Ismail, <
***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/policy/fees/mempool_forecaster.cpp
> <https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2062624207>:
>
> > @@ -28,6 +28,13 @@ ForecastResult MemPoolForecaster::ForecastFeeRate(int target, bool conservative)
> return result;
> }
>
> + const auto cached_estimate = cache.get_cached_fo
...
π¬ maflcko commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2833472275)
Re-running the CentOS task shows that 1 in 5 runs fail
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2833472275)
Re-running the CentOS task shows that 1 in 5 runs fail
π€ mabu44 reviewed a pull request: "doc: Fix fuzz test_runner.py path"
(https://github.com/bitcoin/bitcoin/pull/32353#pullrequestreview-2797651133)
Tested ACK 61f238e84ac6d24d8f420c2eabcbb2980d7fcb1e
(https://github.com/bitcoin/bitcoin/pull/32353#pullrequestreview-2797651133)
Tested ACK 61f238e84ac6d24d8f420c2eabcbb2980d7fcb1e
π hebasto approved a pull request: "doc: Fix fuzz test_runner.py path"
(https://github.com/bitcoin/bitcoin/pull/32353#pullrequestreview-2797667681)
ACk 61f238e84ac6d24d8f420c2eabcbb2980d7fcb1e.
(https://github.com/bitcoin/bitcoin/pull/32353#pullrequestreview-2797667681)
ACk 61f238e84ac6d24d8f420c2eabcbb2980d7fcb1e.
π hebasto merged a pull request: "doc: Fix fuzz test_runner.py path"
(https://github.com/bitcoin/bitcoin/pull/32353)
(https://github.com/bitcoin/bitcoin/pull/32353)
π¬ jesterhodl commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833518903)
check_translations shouldn't complain for any polish labels as of now. Transifex is updated.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833518903)
check_translations shouldn't complain for any polish labels as of now. Transifex is updated.
π¬ maflcko commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833525020)
> check_translations shouldn't complain for any polish labels as of now. I ran it.
Thanks. Just for reference, the script doesn't *need* to report no complaints. Sometimes it complains about perfectly fine translations. Also, it may not even be deterministic if called several times with a cleared cache.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833525020)
> check_translations shouldn't complain for any polish labels as of now. I ran it.
Thanks. Just for reference, the script doesn't *need* to report no complaints. Sometimes it complains about perfectly fine translations. Also, it may not even be deterministic if called several times with a cleared cache.
π¬ jesterhodl commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833526071)
> > check_translations shouldn't complain for any polish labels as of now. I ran it.
>
> Thanks. Just for reference, the script doesn't _need_ to report no complaints. Sometimes it complains about perfectly fine translations. Also, it may not even be deterministic if called several times with a cleared cache.
OK. By the way I noticed it. It complained A should be B, and then B should be A. Neither form would make it happy. I opted for something else.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833526071)
> > check_translations shouldn't complain for any polish labels as of now. I ran it.
>
> Thanks. Just for reference, the script doesn't _need_ to report no complaints. Sometimes it complains about perfectly fine translations. Also, it may not even be deterministic if called several times with a cleared cache.
OK. By the way I noticed it. It complained A should be B, and then B should be A. Neither form would make it happy. I opted for something else.
π hebasto opened a pull request: "subprocess: Backport upstream changes"
(https://github.com/bitcoin/bitcoin/pull/32358)
Most of these changes were developed during work on https://github.com/bitcoin/bitcoin/pull/29868 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for https://github.com/bitcoin/bitcoin/pull/29868.
(https://github.com/bitcoin/bitcoin/pull/32358)
Most of these changes were developed during work on https://github.com/bitcoin/bitcoin/pull/29868 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for https://github.com/bitcoin/bitcoin/pull/29868.
π¬ hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2833536931)
Rebased on https://github.com/bitcoin/bitcoin/pull/32358 and drafted until the latter is landed.
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2833536931)
Rebased on https://github.com/bitcoin/bitcoin/pull/32358 and drafted until the latter is landed.
π hebasto converted_to_draft a pull request: "Reintroduce external signer support for Windows"
(https://github.com/bitcoin/bitcoin/pull/29868)
Based on https://github.com/bitcoin/bitcoin/pull/32358.
Partially reverts:
- https://github.com/bitcoin/bitcoin/pull/28967
- https://github.com/bitcoin/bitcoin/pull/29489
After this PR, we can proceed to actually remove the [unused code](https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752) from `src/util/subprocess.hpp`.
(https://github.com/bitcoin/bitcoin/pull/29868)
Based on https://github.com/bitcoin/bitcoin/pull/32358.
Partially reverts:
- https://github.com/bitcoin/bitcoin/pull/28967
- https://github.com/bitcoin/bitcoin/pull/29489
After this PR, we can proceed to actually remove the [unused code](https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752) from `src/util/subprocess.hpp`.
π petertodd opened a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359)
As per recent bitcoindev mailing list discussion.
Also removes the code to enforce those limits, including the `-datacarrier` and `-datacarriersize` config options.
These limits are easily bypassed by both direct submission to miner mempools (e.g. MARA Slipstream), and forks of Bitcoin Core that do not enforce them (e.g. Libre Relay). Secondly, protocols are bypassing them by simply publishing data in other ways, such as unspendable outputs and scriptsigs.
The *form* of datacarrier outp
...
(https://github.com/bitcoin/bitcoin/pull/32359)
As per recent bitcoindev mailing list discussion.
Also removes the code to enforce those limits, including the `-datacarrier` and `-datacarriersize` config options.
These limits are easily bypassed by both direct submission to miner mempools (e.g. MARA Slipstream), and forks of Bitcoin Core that do not enforce them (e.g. Libre Relay). Secondly, protocols are bypassing them by simply publishing data in other ways, such as unspendable outputs and scriptsigs.
The *form* of datacarrier outp
...