🤔 pablomartin4btc reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3569365873)
re-ACK d9319b06cf82664d55f255387a348135fd7f91c7

Checked the diffs since my last [review](https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3302612544).

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3643843248)
Nice. Only non-test change is adding the missing `std::move` to avoid the copy.

review ACK 07135290c1720a14c9d2f18a5700bb6565ae7a10 🏪

<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+SkW9a8N9
...

🤔 brunoerg reviewed a pull request: "Implementation of SwiftSync"
(https://github.com/bitcoin/bitcoin/pull/34004#pullrequestreview-3569068972)
I've ran a mutation analysis on this PR. Unkilled mutants should be killed (covered) by an unit or functional test (if make sense). However, I noticed that there are part of the code that doesn't even have test coverage (corecheck.dev would have show it but did not generated the report for this PR) and should have it. As soon as more tests are added to this PR, I can re-run the analysis again.

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2611914615)
We could exercise these errors (file doesn't exist and open failure) in the functional tests. An (obvious) unkilled mutant:

```diff
diff --git a/src/init.cpp b/muts-pr-34004-init-cpp/init.mutant.1.cpp
index fb7336f9ae..59de0f6b7a 100644
--- a/src/init.cpp
+++ b/muts-pr-34004-init-cpp/init.mutant.1.cpp
@@ -1812,7 +1812,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
AutoFile afile{file};
if (afile.IsNull()) {
LogError
...

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2611893305)
Unkilled mutant:
```diff
diff --git a/src/validation.cpp b/muts-pr-34004-validation-cpp/validation.mutant.16.cpp
index 3d84576e7d..efc5037570 100644
--- a/src/validation.cpp
+++ b/muts-pr-34004-validation-cpp/validation.mutant.16.cpp
@@ -2613,7 +2613,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// * legacy (always)
// * p2sh (when P2SH enabled in flags and excludes coinbase)
// * witness (when witness enabled in flags an
...

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2611981935)
This error isn't covered by any test and could be addressed in `feature_swiftsync`. Perhaps by deleting a block file before calling the `generatetxohints` RPC.

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2611879532)
> these failures could be exercised in the provided unit test

+1. Some unkilled mutants for obvious lack of testing:

```diff
--- a/src/swiftsync.cpp
+++ b/muts-pr-34004-swiftsync-cpp/swiftsync.mutant.25.cpp
@@ -82,7 +82,7 @@ HintsfileReader::HintsfileReader(AutoFile& file) : m_file(file.release())
}
uint8_t version{};
m_file >> version;
- if (version != FILE_VERSION) {
+ if (1==0) {
throw std::ios_base::failure("HintsfileReader: Unsupported file versi
...

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2612145025)
nit:
```suggestion
for _ in range(NUM_SWIFTSYNC_BLOCKS):
```

💬 brunoerg commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2612266788)
Unkilled mutant:
```diff
diff --git a/src/swiftsync.h b/muts-pr-34004-swiftsync-h/swiftsync.mutant.0.h
index 2c60e96895..ae229d18d8 100644
--- a/src/swiftsync.h
+++ b/muts-pr-34004-swiftsync-h/swiftsync.mutant.0.h
@@ -36,7 +36,7 @@ private:
public:
Aggregate();
/** Is the internal state zero, representing the empty set. */
- bool IsZero() const { return m_limb0 == 0 && m_limb1 == 0 && m_limb2 == 0 && m_limb3 == 0; }
+ bool IsZero() const { return m_limb0 == 0 || m_lim
...

🤔 mzumsande reviewed a pull request: "net: Waste less time in socket handling"
(https://github.com/bitcoin/bitcoin/pull/34025#pullrequestreview-3569594119)
ACK 5f5c1ea01955d277581f6c2acbeb982949088960

Looks like an improvement, but I wonder if instead of doing micro-optimizations like the ones here, it wouldn't be better to have the entire `InactivityCheck()` procedure run somewhere else than in the `SocketHandlerConnected`, or at least less often - it seems a bit absurd to check every `50ms` for a timeout of 20 minutes. Even the initial waiting time `m_peer_connect_timeout` from `ShouldRunInactivityChecks()` for a new peer doesn't really be che
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3644102756)
I've rebased due to https://github.com/bitcoin/bitcoin/pull/33602, and added several touchups. Thank you @l0rinc for the suggestions!

Thank you also @l0rinc for your very thorough measurements. I think 4 threads is a decent choice for now, but as @TheBlueMatt suggests I will try and run benchmarks in a cloud environment with network connected storage.

I would like to reproduce the memory findings, but https://github.com/bitcoin/bitcoin/issues/33351 makes it a little difficult to determine
...

💬 stickies-v commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2612421141)
nit: for people unfamiliar with these tests, it's not obvious that the relevant debug/category settings are set in `LogSetup`. would be a nice improvement to make the test more self contained by explicitly setting them at the beginning of the test (even if it's duplication)

👍 stickies-v approved a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3569753904)
ACK fa35682637e7848106189e314a963d8f6c45b2bc

⚠️ memeticae opened an issue: "Bitcoin to send field is nearly invisible"
(https://github.com/bitcoin/bitcoin/issues/34052)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [x] I still think this issue should be opened here

### Report

Bitcoin to send field is nearly invisible, it works, but the up and down arrows completely obfuscate the text area.

✅ pinheadmz closed an issue: "Bitcoin to send field is nearly invisible"
(https://github.com/bitcoin/bitcoin/issues/34052)

💬 pinheadmz commented on issue "Bitcoin to send field is nearly invisible":
(https://github.com/bitcoin/bitcoin/issues/34052#issuecomment-3644359946)
See https://github.com/bitcoin-core/gui/issues/906

💬 pinheadmz commented on issue "Bitcoin to send field is nearly invisible":
(https://github.com/bitcoin/bitcoin/issues/34052#issuecomment-3644362867)
Or otherwise search open issues in https://github.com/bitcoin-core/gui/issues before opening a new issue

🤔 w0xlt reviewed a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3569919476)
ACK https://github.com/bitcoin/bitcoin/pull/34051/commits/fa35682637e7848106189e314a963d8f6c45b2bc

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3644700618)
I have added an extensive comment inside the `SpanningForestState::Activate()` function.

🤔 Ataraxia009 reviewed a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3570363089)
Concept Ack

💬 Ataraxia009 commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2612981132)
Why not write this like?
```
#define detail_LogIfCategoryAndLevelEnabled(category, level, ...)
do {
Assume(level >= BCLog::Level::Info);
if (LogAcceptCategory((category), (level))) {
LogPrintLevel_(category, level, rate_limit, __VA_ARGS__);
}
} while (0)
```