👋 instagibbs's pull request is ready for review: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1839309807)
> The PR description should say that we don't use hardcoded fee rate value, but rather multiple of long-term-fee-rate. LTFR is an existing user configurable parameter. It's also a good fit for the purpose of determining when to be more "aggressive" with coin selection, because it captures forward looking fee market expectations.

Thanks, I’ve amended the description.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1414385368)
this can't be hit yet; post-v3 it can. I can remove this, or leave a note. It makes some package rbfs more useful

💬 achow101 commented on pull request "init: don't delete PID file if it was not generated":
(https://github.com/bitcoin/bitcoin/pull/28946#issuecomment-1839330822)
ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b

🚀 achow101 merged a pull request: "init: don't delete PID file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28946)

📝 instagibbs opened a pull request: "fuzz: txorphan check wtxids using GenTxid::Wtxid not GenTxid::Txid"
(https://github.com/bitcoin/bitcoin/pull/28997)
Fixes the bugs in the fuzz test with no more changes as an alternative to https://github.com/bitcoin/bitcoin/pull/28658

👋 instagibbs's pull request is ready for review: "fuzz: txorphan check wtxids using GenTxid::Wtxid not GenTxid::Txid"
(https://github.com/bitcoin/bitcoin/pull/28997)

✅ instagibbs closed a pull request: "type-safe(r) GenTxid constructors"
(https://github.com/bitcoin/bitcoin/pull/28658)

💬 instagibbs commented on pull request "type-safe(r) GenTxid constructors":
(https://github.com/bitcoin/bitcoin/pull/28658#issuecomment-1839363339)
up for grabs, opening https://github.com/bitcoin/bitcoin/pull/28997

📝 andrewtoth converted_to_draft a pull request: "validation: don't clear cache on periodic flush"
(https://github.com/bitcoin/bitcoin/pull/28233)
Since https://github.com/bitcoin/bitcoin/pull/17487 we no longer need to clear the coins cache when syncing to disk. [A warm coins cache significantly speeds up block connection](https://github.com/bitcoin/bitcoin/pull/18941#issuecomment-633684774), and only needs to be fully flushed when nearing the `dbcache` limit.

Periodic flushes occur every 24 hours, which empties the cache and causes block connection to slow down. By keeping the cache through periodic flushes a node can run for several
...

💬 hebasto commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839380230)
Another detail. The bug happens on Ubuntu 23.10, but not on Ubuntu 22.04.

💬 ryanofsky commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839386105)
I'm still working on this and not having any luck reproducing it. Can you give more complete steps to reproduce? I'm using the CI docker container and running:

```shell
cd /ci_container_base
make -C depends HOST=x86_64-apple-darwin MULTIPROCESS=1 NO_QT=1 V=1
mkdir build
cd build
CONFIG_SITE=$PWD/../depends/x86_64-apple-darwin/share/config.site ../configure
make
```

It looks like you're doing something different, but I'm not sure what exactly. Alternately if you could run make with V
...

🤔 achow101 reviewed a pull request: "wallet: skip BnB when SFFO is enabled"
(https://github.com/bitcoin/bitcoin/pull/28994#pullrequestreview-1763310634)
Approach ACK

💬 achow101 commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414434210)
In d62d49c605f89a325b929e8fb48091b0b56399ee "test: add coverage for BnB-SFFO restriction"

It seems like using `CreateTransaction` is a bit too high level for the goal of this test. Really this test is targeting the function that runs the various coin selection algorithms and chooses the result to return. This would be `ChooseSelectionResult`. Although this function is not exposed, its callers are, so this test could use either `SelectCoins` or `AutomaticCoinSelection` to test the same behavio
...

💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414394258)
Not sure if that is possible in the call, but when `emplace` doesn't create a new object because the outpoint is already present, then previously the flags didn't change. Now, the flag DIRTY is always added, regardless if the outpoint was already present in `cacheCoins` or not.

💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414243104)
While you are at it, I'd use `CCoinsCacheEntry = default;`. clang-tidy usually warns against this: https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-default.html

💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414384720)
I think `SetFlags` is a bit confusing name, since it's more a `AddFlags`, because it doesn't simply set the flags, it ORs them together.

Alternatively (and I think this is the better solution) really use `m_flags = flags` instead of `mflags |= flags`.

💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414402060)
I'd use `itUs = cacheCoins.try_emplace(it->first).first;` instead, it's just less code and does the same thing

💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414435232)
nit, you could move that line in front of the `if`, then remove the else, like so:

```cpp
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
if (it->second.coin.IsSpent()) {
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
cacheCoins.erase(it->first);
}
it = next_entry;
```

👍 brunoerg approved a pull request: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581#pullrequestreview-1763350846)
crACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3

code lgtm, will test in practice soon!