💬 ajtowns commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680721302)
Would probably be better to replace `__func__` in all these messages with something more relevant to potential users (filename, or some more general indication of what's going on). `"block index database corrupted: proof of work check failed for %s\n", pindexNew->ToString()` eg?

💬 ajtowns commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680595996)
I think all the Log statements in this function could be downgraded to `LogDebug(BCLog::PROXY, ...)` -- they just indicate a p2p connection failure? The `LogPrintf("... connect to %s:%d failed ...")` on line 448 generates a bit of noise for nodes with tor enabled, I believe:

```
2024-07-17T05:23:37.590158Z Socks5() connect to nz7rn2ukf3kwqx24iv6q6tyhiclxwzuk65thkihoo4btbdat4y5ee2qd.onion:8333 failed: host unreachable
2024-07-17T05:23:51.908547Z Socks5() connect to nz7rn2ukf3kwqx24iv6q6tyhic
...

💬 ajtowns commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680749763)
Touching all the `LogPrintLevel` warning/error calls without switching them to `LogAlert` seems a shame. A separate scripted-diff prior to "Replace log error and warning..." with:

```sh
sed -i 's/LogPrintLevel(\(BCLog::[^,]*\), BCLog::Level::Error, */LogAlert(/' $files
sed -i 's/LogPrintLevel(\(BCLog::[^,]*\), BCLog::Level::Warning, */LogAlert(/' $files
```

should work, I think.

💬 ajtowns commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680737774)
Should this be an `if (!Assume(redeemScript.size() <= MAX_SCRIPT_ELEMENT_SIZE)) return false;` ?

💬 ajtowns commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680738999)
Not much value testing the same thing twice.

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680773490)
> 3 is the peer timeout, so you'll have to change it to bump 2 here. Otherwise, the test will fail:

I didn't follow. The inactivity check trigger runs only when we bump by > 3 and not when the bump = 3 right.
I tried logging [`return node.m_connected + m_peer_connect_timeout < now;` condition in `ShouldRunInactivityChecks()`:](https://github.com/bitcoin/bitcoin/blob/6f9db1ebcab4064065ccd787161bf2b87e03cc1f/src/net.cpp#L1957)

```
[net] [net.cpp:1968] [ShouldRunInactivityChecks] [net] Inac
...

👍 stickies-v approved a pull request: "doc: getaddressinfo[isscript] is optional"
(https://github.com/bitcoin/bitcoin/pull/30457#pullrequestreview-2182506187)
ACK fa6390df205513319f28e35e3e17c40ecaa7d731

💬 hebasto commented on pull request "depends: Set `CMAKE_SYSTEM_VERSION` for CMake builds":
(https://github.com/bitcoin/bitcoin/pull/30465#issuecomment-2232957753)
> What change in behaviour should be expected here?

For example, when building for macOS, the `CMAKE_SYSTEM_VERSION` value is used to define `DARWIN_MAJOR_VERSION` and `DARWIN_MINOR_VERSION`, which in turn affect the default build options.

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1680816119)
I took the patch.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2232977932)
Rebased to include the latest interface changes and to take advantage of mocked sockets in #30205.

📝 fanquake opened a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30467)
Backports:
* https://github.com/bitcoin/bitcoin/pull/29855
* https://github.com/bitcoin/bitcoin/pull/30357
* https://github.com/bitcoin/bitcoin/pull/30394 (modified test commit)
* https://github.com/bitcoin/bitcoin/pull/30435

💬 fanquake commented on pull request "init: change shutdown order of load block thread and scheduler":
(https://github.com/bitcoin/bitcoin/pull/30435#issuecomment-2232979945)
Backported to 27.x in #30467.

💬 fanquake commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2232980520)
Backported to 27.x in #30467.

💬 fanquake commented on pull request "psbt: Check non witness utxo outpoint early":
(https://github.com/bitcoin/bitcoin/pull/29855#issuecomment-2232980891)
Backported to 27.x in #30467.

💬 fanquake commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2232981582)
Backported to 27.x in #30467.

💬 hebasto commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#discussion_r1680830600)
Can our previous [conversation](https://github.com/hebasto/bitcoin/pull/226#discussion_r1632362329) be helpful?

📝 stratospher opened a pull request: "test: bump mocktime only after node has received and sent bytes"
(https://github.com/bitcoin/bitcoin/pull/30468)
Fixes an intermittent failure for `p2p_v2_misbehaving.py` reported in https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680462164.

A [different error message](https://github.com/bitcoin/bitcoin/blob/262260ce1e919613ba60194a5861b0b0a51dfe01/src/net.cpp#L1970) `"socket no message in first %i seconds"` will be displayed if `m_last_send=0` or if `m_last_recv is 0`. Fix this by:
1. mocktime bump is done after all the bytes are received. (`m_last_recv is not 0 now`)
2. wait until bytes
...

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680837624)
done in https://github.com/bitcoin/bitcoin/pull/30468

📝 fjahr opened a pull request: "index: Fix coinstats overflow and introduce index versioning"
(https://github.com/bitcoin/bitcoin/pull/30469)
Closes https://github.com/bitcoin/bitcoin/issues/26362

This continues the work that was started with #26426. It fixes the overflow issue by switching most of the values tracked in the index from historical totals to values per block. The only effect of this is that it requires iterating over all the entries to get to these historical values, however nothing changes for the capabilities of the RPCs we have today because the switched values were always report per block and the totals were used
...

💬 fjahr commented on pull request "WIP: Fix coinstatsindex overflow issue":
(https://github.com/bitcoin/bitcoin/pull/26426#issuecomment-2233006443)
#30469 continues the work here