💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2062578451)
nit:
```c++
std::string_view RequestMethodString(HTTPRequestMethod m)
```

💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2062578725)
nit: wouldn't it copy the substrings?

💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2062578806)
nit:
```
std::pair<bool, std::string_view> HTTPRequest::GetHeader(const std::string& hdr) const
```

🤔 shahsb reviewed a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(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

💬 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.

💬 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.

👍 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.

💬 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.

🤔 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 :)

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(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
...

💬 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

🤔 mabu44 reviewed a pull request: "doc: Fix fuzz test_runner.py path"
(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.

🚀 hebasto merged a pull request: "doc: Fix fuzz test_runner.py path"
(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.

💬 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.

💬 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.

📝 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.