🤔 jonatack reviewed a pull request: "rpc: encryptwallet help, mention HD seed rotation and backup requirement"
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1763137265)
Concept ACK

💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414333653)
s/file, containing/file that contains/

Suggest:

"Therefore, it is crucial to make a fresh backup of the newly generated wallet file."

💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414328971)
"backup" is a noun spelled as a single word, while "back up" is a verb spelled as 2 words
```suggestion
"Please exercise caution and ensure that you securely back up the updated wallet file to safeguard your funds and wallet information.\n",
```

👍 romanz approved a pull request: "init: don't delete PID file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28946#pullrequestreview-1763165176)
ACK https://github.com/bitcoin/bitcoin/commit/8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b

👋 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