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

💬 fanquake commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034408348)
> The regression is in the pre-built 27.0rc1 binary for Windows

Ok. It would be good if someone else on Windows can confirm this. Can you also let us know if you're using any particular config options etc.

💬 maflcko commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1549650210)
not sure. Just because it currently isn't called with that value, I fail to see what prevents that in the future.

💬 furszy commented on pull request "Fix create unsigned transaction fee bump":
(https://github.com/bitcoin-core/gui/pull/812#discussion_r1549656233)
The change was introduced because the OS system notification wasn't popping up on macOS. Either way is ok for me.