👋 MarcoFalke's pull request is ready for review: "ci: Switch more tasks to self-hosted"
(https://github.com/bitcoin/bitcoin/pull/21652)

💬 MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1682374092)
The final step is https://github.com/bitcoin/bitcoin/pull/21652. Feel free to rebase on top of that, to test your changes.

💬 TheCharlatan commented on pull request "refactor: Remove confusing static_cast in address types":
(https://github.com/bitcoin/bitcoin/pull/28284#issuecomment-1682379299)
Concept ACK

✅ MarcoFalke closed an issue: "What is depends BUILD_ID_SALT ?"
(https://github.com/bitcoin/bitcoin/issues/28276)

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1297300532)
Actually, it seems iterators may get invalidated when a map rehash occurs, but pointers to elements don't.

I think you can however store pointers to `std::pair<const COutPoint, CoinsCacheEntry>` (the items of the map). Would that help alleviate the need for `m_outpoint`?

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1297307567)
Brilliant, thanks! Was just trying this last night, this looks like the way.

💬 MarcoFalke commented on issue "lint: MYPY_CACHE_DIR is unused":
(https://github.com/bitcoin/bitcoin/issues/28183#issuecomment-1682390715)
Let's move discussion to https://github.com/bitcoin/bitcoin/pull/28184

✅ MarcoFalke closed an issue: "lint: MYPY_CACHE_DIR is unused"
(https://github.com/bitcoin/bitcoin/issues/28183)

💬 MarcoFalke commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1682392919)
Looks like there are also other bugs with this: https://cirrus-ci.com/task/6126543575973888?logs=ci#L5061

💬 fanquake commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1682397002)
@sipa when you get a chance to respond to review comments here, we can decide to either merge as-is, and address in a followup, or touch up here.

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1682399338)
> > I think you asking these questions inappropriately gives readers the impression
> > that there actually are non-trivial examples of real businesses who are relying
> > on unconfirmed transactions. The fact is in this entire thread no-one has been
> > able to provide clear, convincing, evidence that this is true.
>
> I think you have two layers of reasoning: a) browsing the matrix of alternatives solutions for unconfirmed transactions and b) the economics traffic of the use-cases which
...

💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1682414932)
I think they should either be fixed up here or not at all. I don't think it makes sense to create a follow-up to a follow-up.

💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1682416176)
I will address them.

💬 fanquake commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1682417462)
I don't think we are going to get the Rust linter over the line here. Obviously the filesystem commit is correct, and should be merged. Do you want to split that out, and continue the lint discussion, or drop the linter from this PR for now?

💬 MarcoFalke commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1682432081)
> Do you want to split that out, and continue the lint discussion

Seems better to keep the discussion in one place for now.

💬 fanquake commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1682437724)
> Seems better to keep the discussion in one place for now.

Ok, but the discussion of introducing Rust into the codebase (in whatever form) is clearly contentious, and shouldn't be a blocker for merging obvious bug fixes.

💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297349342)
I think `!node.m_sock` can only happen when when we've disconnected in another thread, using `CloseSocketDisconnect()` (and during tests). That's also what the documentation of `m_sock_mutex` says. cc @vasild

👍 Sjors approved a pull request: "net: transport abstraction"
(https://github.com/bitcoin/bitcoin/pull/28165#pullrequestreview-1582634664)
ACK d9a91ba9606f828ef3846d2877ec56313653a109

💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297229209)
58cb50cdc60df5d7e42466213f213cf7ab54db45: IIUC in v1 `&& pnode->vSendMsg.empty()` is always `true` when `GetBytesToSend()` just told us there's nothing to send? But in v2 we might be waiting for a handshake.

💬 vasild commented on issue "Use semantic analysis in lint-logs.py":
(https://github.com/bitcoin/bitcoin/issues/27825#issuecomment-1682443788)
Fixed by https://github.com/bitcoin/bitcoin/pull/26296, thanks!