💬 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

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

`0fa9f17332a6d9b2eb6e3d9f9102bfd2d9c6f516`

📝 vasild opened a pull request: "Logging cleanup"
(https://github.com/bitcoin/bitcoin/pull/29798)
* Move special cases from `LOG_CATEGORIES_BY_STR` to `GetLogCategory()` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1547990373)).

* Remove `"none"` and `"0"` from RPC `logging` help because that help text was wrong. `"none"` resulted in an error and `"0"` was ignored itself (contrary to what the help text suggested).

* Remove unused `LOG_CATEGORIES_BY_STR[""]` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1548018694)).

This i
...

💬 naiyoma commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1549560026)
would it be beneficial to add an assertion for a small scriptSig with an empty witness stack?

```
# small scriptSig, empty witness stack
self.assertEqual(calculate_input_weight(scriptSig_small, []),
(SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR)

```

💬 vasild commented on pull request "log: deduplicate category names and improve logging.cpp":
(https://github.com/bitcoin/bitcoin/pull/29419#issuecomment-2034378458)
@ryanofsky, thanks for the review! Addressed your pending suggestions in https://github.com/bitcoin/bitcoin/pull/29798.

💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2034388617)
@maflcko yes, e.g. it built boost and zmq just fine.

💬 glozow commented on pull request "refactor: Use typesafe Wtxid in compact block encodings":
(https://github.com/bitcoin/bitcoin/pull/29752#discussion_r1549592075)
A relatively simple test could be to a `extra_txn.resize(1)` within src/test/blockencodings_tests.cpp (currently it is empty)?

This bug description looks correct to me.

🤔 glozow reviewed a pull request: "refactor: Use typesafe Wtxid in compact block encodings"
(https://github.com/bitcoin/bitcoin/pull/29752#pullrequestreview-1976573938)
ACK e178a52647fb6a8844191961fbf74266e22d5df8

The second commit seems sensible as wtxid is cached instead of being calculated on the fly since fac1223a568fa1ad6dd602350598eed278d115e8 (nit maybe mention in the commit message), though I don't think having the pair is that bad either.

💬 glozow commented on pull request "refactor: Use typesafe Wtxid in compact block encodings":
(https://github.com/bitcoin/bitcoin/pull/29752#discussion_r1549601987)
nit: I generally prefer to add braces or put these on one line (also [see](https://stackoverflow.com/questions/12193170/whats-the-purpose-of-using-braces-i-e-for-a-single-line-if-or-loop))