💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1664298064)
added

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1664298204)
done, thanks

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1664301048)
I suppose it's not super accurate to say that `UpdatedBlockTipSync` is the synchronous version of `UpdatedBlockTip`, as the point here is to fire whenever the chain tip changes at all, while `UpdatedBlockTip` skips some things. i.e. `InvalidateBlock` when our new tip is a step back instead of an advancement.

I've removed comments comparing it to `UpdatedBlockTip`, slightly changed the calling logic, and renamed it to `ActiveTipChange`.

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1664301296)
gating on whether ibd now

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2206343668)
> Changes like this are inherently difficult to review

Yes, needs careful code review. The intermediate `Assume` in 4673e04cb3 may help as a sanity check. Lmk if anybody has ideas to structure the PR in a clearer way.

👍 theStack approved a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376#pullrequestreview-2156652690)
ACK eef5dbc21b3fd52069f2f0855fb76a13234ebbf3

💬 furszy commented on pull request "[WIP] p2p: send not_found msgs for unknown, pruned or unwilling to share blocks":
(https://github.com/bitcoin/bitcoin/pull/30385#issuecomment-2206361109)
> Nodes advertise which blocks they have and what their current tip is, so the timeouts should only happen if the peer is misbehaving or if you disrespect the protocol by asking for things they don't have.

We don't ask all peers about all the blocks they know about. If the node is it in the main-chain, most of the time we only know their last advertised header only.
Once we are headers-wise sync, the block downloading procedure download blocks from different peers. It has a window of 16 in-f
...

💬 sipa commented on pull request "[WIP] p2p: send not_found msgs for unknown, pruned or unwilling to share blocks":
(https://github.com/bitcoin/bitcoin/pull/30385#issuecomment-2206384707)
> The peer who advertised the header might not be the one who ends up providing the final block.

I don't think we ever ask for a block from a peer without that specific peer having announced the block or a descendant to us?

I can imagine asking a peer for a block that is not on the main chain (due to a very recent reorg) or due to just being pruned, though I don't know how often this happens.

I think this is generally a good change, but to judge how important it is, I think it's good to
...

👍 maflcko approved a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329#pullrequestreview-2156663091)
lgtm. Thanks!

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1664319501)
style nit: Missing trailing `,`?

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1664318951)
nit: maybe also list the filename here, like in the other cases.

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1664333808)
nit: Could assert the chain tx value from the chain params constant?

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1664325174)
style-nit:
```suggestion
auto msg_start = chainman.Params().MessageStart();
```

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1664344893)
Should be fine to exceed the range here for fuzzing? Maybe `+1`, or `*3` instead of `*2`?

🤔 maflcko reviewed a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329#pullrequestreview-2156716926)
ACK b9ba1a73094f4ad593b527e23d2681f41c22539 🎯

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK b9ba1a73094f4ad593b527e23d2
...

👍 theStack approved a pull request: "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB"
(https://github.com/bitcoin/bitcoin/pull/26596#pullrequestreview-2156741773)
Code-review ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b

💬 theStack commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1664366237)
nit: was about to suggest to deduplicate shared code between this method and `::GetLegacyScriptPubKeyMan` above, but since the latter is removed soon anyway (in PR #28710, commit https://github.com/bitcoin/bitcoin/pull/28710/commits/5652a9c59f9ec793886eff00126c382003e4ce02#diff-1f2db0e4d5c12d109c7f0962333c245b49b696cb39ff432da048e9d6c08944d8), it's probably not worth it.

💬 furszy commented on pull request "[WIP] p2p: send not_found msgs for unknown, pruned or unwilling to share blocks":
(https://github.com/bitcoin/bitcoin/pull/30385#issuecomment-2206523377)
> > The peer who advertised the header might not be the one who ends up providing the final block.
>
> I don't think we ever ask for a block from a peer without that specific peer having announced the block or a descendant to us?

Yeah. I actually spent some time checking that specifically because it would allow an easy way to partition the network by sending only the new header tip, disconnecting and expect the peer requests it to someone else who does not have it etc..

What I meant wit
...

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#issuecomment-2206597223)
Windows CI fails because the error that I'm looking for `EAI_ADDRFAMILY` doesn't exist in Windows CRT.

Trying to think what to do about it.

💬 ryanofsky commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2206612992)
> Not really convinced explaining things is getting anywhere? Sometimes you have to try fully implementing your idea before you realise it doesn't work...

I think #30342 is a full implementation of making it possible the specify log instances kernel code should use. If you are looking for an implementation of a change to remove the hardcoded global logger instance from the kernel (replacing it as described with a global pointer to a default fallback log instance), I'll provide that, and hopef
...