💬 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)
```

💬 Ataraxia009 commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#issuecomment-3644985274)
This is good LogPrintLevel just leads to more confusion, standardise the macros!

💬 stratospher commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#issuecomment-3644999563)
planning on picking this up in a separate PR - unless someone already has a branch ready (happy to review in that case!).

💬 Ataraxia009 commented on pull request "refactor: inline constant `f_obfuscate = false` parameter":
(https://github.com/bitcoin/bitcoin/pull/34048#issuecomment-3645025381)
> if it's needed in the future, it's trivial to reintroduce it.

It seems like a strong enough option to leave as is still but ill defer to others i dont have a strong opinion

💬 Chand-ra commented on pull request "fuzz: Add tests for `CCoinControl` methods":
(https://github.com/bitcoin/bitcoin/pull/34026#discussion_r2613095907)
`GetInputWeight()` returns a `std::optional<int64_t>`, so I don't think it can return a `nullptr`. It **does** return a `std::nullopt` when the coin is not selected, but I think adding a check for that would be too close to duplicating the implementation.

👍 vasild approved a pull request: "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3570511275)
ACK 325f98bca07c3a0c48a46dc18f67376d44c5341d

💬 vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2613108267)
There are about 60k ports to choose from. There must be a way to pick in a collision free manner even without reuse...

💬 Chand-ra commented on pull request "fuzz: Add tests for `CCoinControl` methods":
(https://github.com/bitcoin/bitcoin/pull/34026#discussion_r2613109116)
Yes, that is correct and intentional.

`Select()` assigns a default sequential position, but the goal of this test is to exercise the `SetPosition()` setter directly. We want to verify that the `PreselectedInput` object handles arbitrary position values correctly, even if they differ from the default sequence assigned by `Select()`.

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

That'd conceptually re-introduce the CVE
...

💬 maflcko commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613241442)
I've added a new `LogDebug(BCLog::HTTP,` in the line below, to clarify this in the test itself. If that is not sufficient, I am happy to push any diff or commit here, if someone writes one. Also, I am happy to review a separate pull, or a follow-up.

👍 vasild approved a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497#pullrequestreview-3570749656)
ACK 9f39a8309cb0a23df03937b0b225aff8ce9e1ec6

📝 maflcko opened a pull request: "lint: Remove confusing, redundant, and brittle lint-spelling"
(https://github.com/bitcoin/bitcoin/pull/34053)
`codespell` was a fun experiment. However, it has many issues, when used in this project:

* The number of false-positives and true-positives are in the same ballpark. There are also many false-negatives, so the overall net-benefit is questionable.
* There is often confusion around spelling errors leading to a failing CI (they do not, which was intended).
* LLMs released this year are capable to detect typos with less false-positives and less false-negatives, so the `codespell` integration i
...

💬 l0rinc commented on pull request "fuzz: Add tests for `CCoinControl` methods":
(https://github.com/bitcoin/bitcoin/pull/34026#discussion_r2613401863)
I meant `nullopt` of course. We should at least exercise that code path.

💬 l0rinc commented on pull request "lint: Remove confusing, redundant, and brittle lint-spelling":
(https://github.com/bitcoin/bitcoin/pull/34053#issuecomment-3645520119)
ACK fa904fc683c0892eb800ddb986fdc0c646721077

💬 sedited commented on pull request "lint: Remove confusing, redundant, and brittle lint-spelling":
(https://github.com/bitcoin/bitcoin/pull/34053#issuecomment-3645523026)
Concept ACK