⚠️ Sjors opened an issue: "Log which peer sent us a header (first)"
(https://github.com/bitcoin/bitcoin/issues/27744)
Discussed in https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478368582

```
Saw new header hash=... height=...
```

It would be useful to add the node id to this message. The validation code doesn't know which peer the header came from, so we'd have to move the log entry to net_processing.

One way is to add a helper function `LogBlockHeader` and call it right after both `ProcessNewBlockHeaders(header, peer, compact_blk)` calls in net_processing, only if they return true. Sin
...

📝 amitiuttarwar opened a pull request: "addrman: select addresses by network follow-up "
(https://github.com/bitcoin/bitcoin/pull/27745)
this PR addresses outstanding review comments from #27214

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661115)
done in #27745

💬 mzumsande commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561807894)
> As such it shouldn't add any risk of such attacks if the remote address was simply added to this line, something like peer=x addr=x.x.x.x?

I agree, with `addr=x.x.x.x` being conditional on `-logips`. That could make sense in some more spots as well, I could open a PR to suggest it unless you want to.

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661204)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661284)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661358)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661423)
done in #27745

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#discussion_r1204663303)
Good catch thanks. First draft of this PR did check for `blocks` directory but of course that is unreliable because it could be customized by the user.

💬 ryanofsky commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561821162)
Thanks, added std::move as suggested.

Updated b2bcba068e2c5542ab157e62c82bb868f678beeb -> 8aa8f73adce72e1ae855b43413c1f65504423cb7 ([`pr/bresult-opt.10`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-opt.10) -> [`pr/bresult-opt.11`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-opt.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult-opt.10..pr/bresult-opt.11)) with workaround for CI errors https://cirrus-ci.com/task/6456067106799616?logs=ci#L1528 https:/
...

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1561833344)
> This change ignores non-main network data.

Ah I see, because I'm checking for `Bitcoin\chainstate` specifically but if a user only ever used signet, there would be a nested `signet` directory in there too. Is there anything else I can check for that always gets written to that root? Or maybe just check if `Bitcoin\` is empty at all?

> I'm OK with that. But it seems worth mentioning in the docs.

Sure but I feel like that's not super clean either.

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1204686737)
done

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1204687270)
Sounds good, I am using this now.

👋 fjahr's pull request is ready for review: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581)

💬 amitiuttarwar commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561847276)
it looks like this PR provides the same coverage as 8f91e5898b3571e0802f2197981c54dda9ee71fa in #27213. I'll remove the commit there so 27213 can focus on discussing the changes to node behavior

💬 amitiuttarwar commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561848392)
for the record, ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838 - only small changes from the version (previously) proposed in 27213

📝 sdaftuar opened a pull request: "Draft: rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746)
This PR proposes a clean up of the relationship between block storage and the chainstate objects, by moving the decision of whether to store a block on disk to something that is not chainstate-specific. Philosophically, the decision of whether to store a block on disk is related to validation rules that do not require any UTXO state; for anti-DoS reasons we were using some chainstate-specific heuristics, and those have been reworked here to achieve the proposed separation.

This PR also fixes
...

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#issuecomment-1561849558)
I think this is ready for review. I have included the feedback so far and I think the current extent of the checks and stats is good enough as a first step. I am currently still going through all the possible cases we could see an ASMap file be harmful to a node and this may lead to some additional checks and stats being logged in the future but I need some more time for this and it can be done as a follow-up.

💬 ArmchairCryptologist commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561850433)
> I agree, with `addr=x.x.x.x` being conditional on `-logips`. That could make sense in some more spots as well, I could open a PR to suggest it unless you want to.

By all means, please do. At least with this it will be trivial to just block any peers that repeatedly cause the node to stall.

💬 jamesob commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#issuecomment-1561858383)
Great! Thanks for this. I'm active in these neighborhoods today as well, since adding some `-stopatheight`/restart logic to the functional tests in #27596 has turned up some issues with `CheckBlockIndex()`, which are maybe related to some of the changes here. So I'll be looking at this shortly.