💬 hebasto commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034267106)
Speaking of MSVC builds, it's worth noting that they still don't have a hardware-accelerated SHA256 implementation (see https://github.com/bitcoin/bitcoin/pull/24773).

💬 naiyoma commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#issuecomment-2034278715)
CONCEPT ACK , Using `calculate_input_weight` is a better approach than manual estimations, also increases readability.

💬 maflcko commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1549504485)
Could also print the result on success? IIUC, stdout will be hidden by the test runner, and when running this test independently, having some feedback is better than no feedback?

💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034285249)
@maflcko

> I forgot to mention that optimization was enabled in commit https://github.com/bitcoin/bitcoin/commit/41e378a0a1f0729b423034f47c28a4e83287a1e8. So for a fair comparison, one would have to backport https://github.com/bitcoin/bitcoin/commit/41e378a0a1f0729b423034f47c28a4e83287a1e8 to 26.0.

The original regression is also in variance which doesn't appear with the MSVC binary, so I don't think it's necessary to test the backport.

@fanquake

> It's not clear to me if there is st
...

📝 fanquake opened a pull request: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796)
We currently have two issues in relation to debugging optimisation flags:

* A depends build with `DEBUG=1` and using `./configure --enable-debug` do not align, which is inconsistent/confusing. Depends currently uses `-O1`, while `--enable-debug` will set `-O0`.
* `-O0` is unusable in various ways:
* Compiling certain assembly under `-O0` doesn't currently work: https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-2027843446 (probably an upstream issue?).
* Windows cross-compi
...

💬 dergoegge commented on pull request "refactor: Use typesafe Wtxid in compact block encodings":
(https://github.com/bitcoin/bitcoin/pull/29752#discussion_r1549507036)
> as I understand it, this was sort-of a bug in the previous implementation but didn't cause a crash, just 'potentially bad' behaviour?

It would actually be a crash. Just to recap the bug that exists on master:

* If the ring buffer is not fully filled, then it contains default initialized `uint256`s (as the first pair element)
* We loop over the **entire** ring buffer in `PartiallyDownloadedBlock::InitData`, [compute the short id](https://github.com/bitcoin/bitcoin/blob/0d509bab45d292caea
...

📝 hebasto opened a pull request: "guix: Remove another leftover from #29648"
(https://github.com/bitcoin/bitcoin/pull/29797)
It was overlooked in bitcoin/bitcoin#29787.

💬 hebasto commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2034313126)
> * A depends build with `DEBUG=1` and using `./configure --enable-debug` do not align, which is inconsistent/confusing.

Concept ACK on fixing that.

💬 maflcko commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034314366)
> I presume that benchmarking IBD is expensive. So maybe comparing the micro-benchmarks can provide a hint at which code is slower?

I forgot to mention that benchmarks are disabled in guix. So something like `sed -i -e 's/--disable-bench //g' $( git grep -l disable-bench ./contrib/guix/ )` may be needed before a guix build.

🤔 sr-gi requested changes to a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623#pullrequestreview-1973887290)
ACK https://github.com/bitcoin/bitcoin/commit/2ef71c73582be554e565ada3f8a6ca77c2cd79f1

Left some comments inline, mostly nits/things that may need renaming.

Also, `qt/guiutils.h` still refers to CNodeCombinedStats.nTimeOffset after 5084ad48462b3169c99a81aeb21a339c3a9a4728, in a couple of places, but it may be better to change that for `time_offset` given `nTimeOffset` is not a thing anymore.

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1547927278)
5084ad48462b3169c99a81aeb21a339c3a9a4728 nit: sends/sent

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548002240)
nit: Not sure what commit this may better fit in, but there is no such thing as adjusted time anymore, so the comment may need updating

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1547973617)
In 16dededcd8d52b6d0fdbe9e0ffab6a177aa8aa17, why is this defined as `0s`, but `time_offset` is defined as `0` in `net_processing.cpp`?

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548018941)
e6d7bcf7689e38dcda45a1cdbff98fd610864e47 nit: full stop (for consistency with the rest).

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548019018)
e6d7bcf7689e38dcda45a1cdbff98fd610864e47 nit: full stop (for consistency with the rest)

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548030687)
e6d7bcf7689e38dcda45a1cdbff98fd610864e47 nit: theoretically, you only need to hold the mutex up to here, right?

Not that it matters much given the small size of the collection anyway

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548013616)
e6d7bcf7689e38dcda45a1cdbff98fd610864e47 doesn't look like `chrono_literals` is required here

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549492464)
Since the warn threshold was reduced from 20 to 10, it may make more sense to change this to 11

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1548041767)
e6d7bcf7689e38dcda45a1cdbff98fd610864e47 nit: _appear_

💬 kosuodhmwa commented on issue "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?":
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2034361865)
**The source code from the master branch i've compiled is:**
**_root@debian-12-btc-node:~/bitcoin# git rev-parse HEAD_** gives me the following output:
0fa9f17332a6d9b2eb6e3d9f9102bfd2d9c6f516