💬 maflcko commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1485967010)
nit: While touching this, could switch to LogInfo in this file, to avoid having to touch it yet again?

💬 maflcko commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1485966299)
Not sure about the argument dependent lookup here. Why not simply fix it the way the CI has instructed? (Add the include and use the fs namespace)?

If the CI error was unclear, suggestions to make it clearer are welcome.

💬 maflcko commented on pull request "redeclare nChainTx to use uint64_t":
(https://github.com/bitcoin/bitcoin/pull/29331#issuecomment-1938380704)
Are you still working on this? If not, I am happy to pick up.

💬 glozow commented on issue "Update security.md with all PGP fingerprints":
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1938397734)
> in the past I’ve been asked by LN maintainers to segregate some security information from the usual set of Core people handling issues. There is no need to distribute the information to a wider set of people than strictly necessary (see _need to know policy_).

One potential solution to this problem is to stop CCing unrelated people on your disclosure emails.

✅ maflcko closed a pull request: "test: Add and use option for tx-version in MiniWallet methods"
(https://github.com/bitcoin/bitcoin/pull/28972)

💬 maflcko commented on pull request "test: Add and use option for tx-version in MiniWallet methods":
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1938431311)
Merged as 27c8786ba918a42c860e6a50eaee9fdf56d7c646

💬 dergoegge commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1486013425)
I'm not sure if that is simpler though. Yes it's less code but harder to read/understand imo (e.g. you forgot the `!` in your suggestion).

I'm actually not a fan of my `std::optional<bool>` choice, so I might try to change that.

💬 kilrau commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1938459985)
> Plus, a default Replace-By-Fee will just make BTC harder to use as a transactional currency

That's part of the goal. (Micro-) transactional use cases should use Layer 2.

Anyhow, it doesn't matter. I can only speak for [Boltz](https://boltz.exchange/), but for us this train has left the station. Only one pool accepting fullrbf and it's not safe to accept 0-conf anymore. Game over. It took a while to grow on me, give it some time.

ACK

🤔 hebasto reviewed a pull request: "Intro: Have user choose assumevalid"
(https://github.com/bitcoin-core/gui/pull/149#pullrequestreview-1875020927)
Approach ACK cf940f0e5f5ffb14ec7b6f751879bd70b3c350be.

This branch has to be rebased due to the conflicts with the master branch.

💬 hebasto commented on pull request "QRImageWidget: Sizing and font fixups":
(https://github.com/bitcoin-core/gui/pull/506#issuecomment-1938496294)
Closing due to lack of support from the PR author (https://github.com/bitcoin-core/gui/pull/506#issuecomment-1549832808, https://github.com/bitcoin-core/gui/pull/506#pullrequestreview-1630119899).

✅ hebasto closed a pull request: "QRImageWidget: Sizing and font fixups"
(https://github.com/bitcoin-core/gui/pull/506)

💬 Sjors commented on issue "Add taproot descriptor to existing descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/24193#issuecomment-1938499238)
Which is superseeded by #29130

✅ Sjors closed an issue: "More coin stats (for dumpcoinstats & gettxoutsetinfo)"
(https://github.com/bitcoin/bitcoin/issues/24581)

💬 Sjors commented on issue "More coin stats (for dumpcoinstats & gettxoutsetinfo)":
(https://github.com/bitcoin/bitcoin/issues/24581#issuecomment-1938500792)
Not much happening here. Can reopen is anyone has a wish list.

👍 hebasto approved a pull request: "Intro: Never change the prune checkbox after the user has touched it"
(https://github.com/bitcoin-core/gui/pull/658#pullrequestreview-1875086606)
ACK bee0ffbecf4f95b65f4084893924a1ab250ca77c, both commits are improvements of the current behaviour. Tested on Ubuntu 23.10.

🚀 fanquake merged a pull request: "test: Fix utxo set hash serialisation signedness"
(https://github.com/bitcoin/bitcoin/pull/29399)

🚀 hebasto merged a pull request: "Intro: Never change the prune checkbox after the user has touched it"
(https://github.com/bitcoin-core/gui/pull/658)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1938574388)
Rebased.

I agree adding `start_height` could be useful. I grabbed the commits and set start height to the taproot activation height. Will leave it up to someone else to make it a PR.

Looks like I'm using it incorrectly though:

```
2024-02-12T12:17:04Z silentpaymentindex thread start
2024-02-12T12:17:04Z Syncing silentpaymentindex with block chain from height 0
2024-02-12T12:17:04Z ERROR: Commit: Failed to commit latest silentpaymentindex state
2024-02-12T12:17:04Z ERROR: UndoReadFro
...

💬 maflcko commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#issuecomment-1938577167)
```
wallet/scriptpubkeyman.cpp:2161:42: error: no member named 'GetEncryptionKey' in 'wallet::WalletStorage'
if (!Assume(DecryptKey(m_storage.GetEncryptionKey(), crypted_secret, it->second.first, key))) {
~~~~~~~~~ ^

💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1938581651)
Interesting, your first error seems related to `UndoData`, which start height shouldn't affect?

> The compiler is also warning about the block filter index:

Ah, this is a shadowing error, I can fix that. I think I'll go ahead and open a `start_height` index PR since this seems generally useful and I'll fix the error there. @fjahr adding you as co-author unless you have any objections?