💬 ajtowns commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1585594779)
> This change does seem ready to merge, but I notice it conflicts with #27596 so might not be worth creating more rebase churn there. Will leave alone for now and wait for more input or a better opportunity to merge

FWIW, I think the rebase churn is pretty trivial (the PR's change neighbouring lines in validation.h -- this one introduces `m_warningcache` while assumeutxo moves `GetSnapshotBaseHeight`), #27596 needs rebase anyway (to combine the fixups at least), and it doesn't conflict with #
...

💬 MarcoFalke commented on issue "Indefinite "Bitcoin Core is shutting down..."":
(https://github.com/bitcoin/bitcoin/issues/27848#issuecomment-1585599116)
Looks like it didn't print "scheduler thread exit", indicating that this thread is still running.

💬 MarcoFalke commented on issue "Bitcoin's relay fee refactoring":
(https://github.com/bitcoin/bitcoin/issues/27847#issuecomment-1585599936)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base. Network-wide consensus and/or P2P changes first need to be discussed with the greater community, e.g. the `bitcoin-dev` mailing list. Also, they need a BIP to be implemented in Bitcoin Core and other software that connects to the bitcoin P2P network.

✅ MarcoFalke closed an issue: "Bitcoin's relay fee refactoring"
(https://github.com/bitcoin/bitcoin/issues/27847)

💬 MarcoFalke commented on pull request "contrib: docs fix --import-keys flag on verify.py":
(https://github.com/bitcoin/bitcoin/pull/27840#issuecomment-1585615215)
lgtm ACK ceb01689351e738436393cf7b8d84cb7fafc7b98

📝 pinheadmz opened a pull request: "Add unit & functional test coverage for blockstore"
(https://github.com/bitcoin/bitcoin/pull/27850)
This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1585663521)

> If this is done I think we could merge everything except the last two commits in a separate PR, and start having better test coverage in master right away.

Great plan, good idea thanks. Unit and functional tests opened in separate PR: https://github.com/bitcoin/bitcoin/pull/27850 I'll return to this after the tests and other conflicting PRs are merged or closed.

⚠️ 072222 opened an issue: "https://coinmarketcap.com/currencies/bitcoin/"
(https://github.com/bitcoin/bitcoin/issues/27851)

✅ fanquake closed an issue: "https://coinmarketcap.com/currencies/bitcoin/"
(https://github.com/bitcoin/bitcoin/issues/27851)

:lock: fanquake locked an issue: "https://coinmarketcap.com/currencies/bitcoin/"
(https://github.com/bitcoin/bitcoin/issues/27851)

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1225398417)
> You could fix and future-proof this by adding a interfaces::Node::setExitStatus() function, or just update the existing [appInitMain()](https://github.com/bitcoin/bitcoin/blob/153a6882f42fff3fdc63bf770d4c86a62c46c448/src/node/interfaces.cpp#L106-L109) function to set the error code.

Great, added it. Thanks!

> If you let appInitMain set the error code, you could also simplify GUI code even more, skipping Q_EMIT initializeResult(rv, tip_info) when appInitMain fails and getting rid of the r
...

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1225399092)
Done, added

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#issuecomment-1585690708)
Updated per feedback, thanks ryanofsky. [Tiny diff](https://github.com/bitcoin/bitcoin/compare/da60d86bfbe7fe49ce7181b5b394c0e3d8c53854..61c569ab6069d04079a0831468eb713983919636):

- Removed `BitcoinApplication::getExitStatus`.
- Moved `exit_status.store(EXIT_FAILURE)` to the interface `appInitMain()` method to be compatible with [#10102](https://github.com/bitcoin/bitcoin/pull/10102).

💬 TheCharlatan commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1585733097)
Concept ACK

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1585733171)
Please consider editing the description slightly, if my understanding is correct.

>With this PR, if the inbound connection is on our whitelist we start the eviction process by selecting a random unprotected peer.

At this point in the process, where we choose a random peer, we're considering _all_ (inbound, not noban) peers, is that correct? They are all (other than outbound or noban) "unprotected" because we haven't protected any of them yet. So maybe this should say, "... selecting a rand
...

👍 theStack approved a pull request: "util: implement `noexcept` move assignment & move ctor for `prevector`"
(https://github.com/bitcoin/bitcoin/pull/27334#pullrequestreview-1473456023)
Tested and light code-review ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110

(My understanding of move semantics is limited, but the change looks correct to me. [All the links @jonatack posted above](https://github.com/bitcoin/bitcoin/pull/27334#pullrequestreview-1371788591) for further reading were very helpful, especially this one: https://quuxplusone.github.io/blog/2022/08/26/vector-pessimization/)

@ryanofsky: you might be interested in this PR?

💬 Giszmo commented on issue "Indefinite "Bitcoin Core is shutting down..."":
(https://github.com/bitcoin/bitcoin/issues/27848#issuecomment-1585750520)
So ... may I kill it or would you want any additional information while it's still refusing to stop on its own?

🤔 LarryRuane reviewed a pull request: "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1430899133)
utACK fa78fc543cd46ee1c7181ecf6b696c2892b9f8d3
except for possible attack vectors (for example, `NoBanForce`) that I'm not really qualified to comment on. I may try to contribute to that discussion after I've learned more. Nice PR!

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1196647358)
```suggestion
// Return the last erased (not-to-be-evicted) element
```

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1225492297)
Perhaps insert a comment before this line `// This may still return std::nullopt`