💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568154781)
This is already done, see https://cirrus-ci.com/task/4965521926389760?logs=ci#L356

📝 MarcoFalke opened a pull request: "ci: Enable float-divide-by-zero check"
(https://github.com/bitcoin/bitcoin/pull/27778)
Enable it, because

* It is enabled on OSS-Fuzz, so to be able to catch bugs earlier, enable it here as well.
* It makes sense to enable, because when a float is divided by zero, it may be a logic bug in our code, so it should be suppressed in the suppressions file.

💬 hebasto commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568162128)
> This is already done, see [cirrus-ci.com/task/4965521926389760?logs=ci#L356](https://cirrus-ci.com/task/4965521926389760?logs=ci#L356)

Right. I mean, no need to wait for 6 month. If numbers are the same after a week then this PR works as intended, no?

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568163647)
Ok, I'll check back a week after merge.

👍 hebasto approved a pull request: "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN"
(https://github.com/bitcoin/bitcoin/pull/27777#pullrequestreview-1450590109)
ACK fa123077bc3f39aa0969d883e2d799a054cd4543

💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210077812)
Same for `#include <boost/multi_index/hashed_index.hpp> // for hashed_index_iterator` etc ... ?

💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210080813)
Seems an implementation detail of boost/signal that should map to `boost/signals2/signal.hpp`?

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210091336)
Looking into the `boost/signals2/signal.hpp` and `boost/signals2/optional_last_value.hpp` headers, I'd say that the `boost/optional.hpp` header is an implementation details of the latter, not former.

💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1568212984)
Forget my earlier comments. I think this is correct, or at least a good step in the right direction.

lgtm ACK b2634ad1ed884ff7cd32f00cb45075c100902763

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1568216467)
> Forget my earlier comments.

Including https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210077812?

💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1210099908)
Another use case could be so that testers can purposefully load (very) old fee estimate files to populate `estimatesmartfee`? https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1531711091

📝 fanquake opened a pull request: "guix: fix CURL disable flag in osslsigncode"
(https://github.com/bitcoin/bitcoin/pull/27779)
See https://cmake.org/cmake/help/latest/module/FindCURL.html.

💬 MarcoFalke commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1568242621)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59285

💬 hebasto commented on pull request "guix: fix CURL disable flag in osslsigncode":
(https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1568258065)
Concept ACK.

💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1210171462)
We don't need `m_uri` to be a valid URI, that's what `evhttp_uri_parse` is checking, we just need it to not behave unexpectedly (eg. segfault) when passed to `evhttp_uri_parse`. "a" also would not be a valid URI, but we're not checking that here either, that would just be duplicating what `evhttp_uri_parse` is already doing.

So unless there's another reason to not pass an empty char array to `evhttp_uri_parse`, I'd leave it out yeah.

💬 hebasto commented on pull request "guix: fix CURL disable flag in osslsigncode":
(https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1568322163)
It is not clear why this is a fix. `CMAKE_DISABLE_FIND_PACKAGE_CURL` is a valid [variable](https://cmake.org/cmake/help/latest/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html) that:
> can be used to build a project without an optional package

🤔 stickies-v reviewed a pull request: "httpserver, rest: improving URI validation"
(https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1450797669)
I'm in favour of doing the `replySent` refactoring in a separate commit, but it makes more sense to have that be the first commit imo, to avoid adding logic and then removing it in the very next commit.

💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1210184105)
Don't we need to clean this up regardless of whether an exception was thrown?

📝 MarcoFalke opened a pull request: "fuzz: Avoid timeout in utxo_total_supply"
(https://github.com/bitcoin/bitcoin/pull/27780)
Looks like for high block counts it may be better to mock the chain, otherwise a high limit will lead to fuzz input bloat and timeouts, see https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1538252773.

It can be checked that the fuzz target can still find the CVE, see https://github.com/bitcoin/bitcoin/pull/17860#pullrequestreview-1410594057 with a diff of:

```diff
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index f949655909..6f4cfb5f51 100644
--- a/src
...

💬 furszy commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1568357831)
> What about a one-line fix to clear the test issue and then a follow-up with the RPC, which may take longer to review than a trivial one-line patch?

np for me. @mzumsande do you want to go ahead with your fix? I can just relabel this PR to "Implement getblockfileinfo RPC command" and done.