Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ 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
πŸ’¬ 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
...