📝 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

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