π¬ 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
...
(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
...
(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
...
(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)
(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
(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.
(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)
(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
(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
(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
(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.
(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
(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)
```
(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!
(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!).
(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
(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.
(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
(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...
(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()`.
(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
...
(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
...