🚀 hebasto merged a pull request: "Add settings.json prune-prev, proxy-prev, onion-prev settings "
(https://github.com/bitcoin-core/gui/pull/603)

✳️ hebasto pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/68e484afbbc2...e43ff4eab2fe)
Merge bitcoin-core/gui#603: Add settings.json prune-prev, proxy-prev, onion-prev settings

9d3127b11e34131409dab7c08bde5b444d90b2cb Add settings.json prune-prev, proxy-prev, onion-prev settings (Ryan Ofsky)

Pull request description:

With #602, if proxy and pruning settings are disabled in the GUI and the GUI is restarted, proxy and prune values are not stored anywhere. So if these settings are enabled in the future, default values will be shown, not previous values.

This PR stores previous values so they will preserved across restarts. I'm not sure I like this behavior because showing default values seems simpler and safer to me. Previous values may just have been set temporarily and may have never actually worked, and it adds some code complexity to store them.

This PR is one way of resolving #596. Other solutions are possible and could be implemented as alternatives.

ACKs for top commit:
hebasto:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb, tested on Ubuntu 22.04.
vasild:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb
jarolrod:
tACK 9d3127b11e34131409dab7c08bde5b444d90b2cb

Tree-SHA512: 1778d1819443490c880cfd5c1711d9c5ac75ea3ee8440e2f0ced81d293247163a78ae8aba6027215110aec6533bd7dc6472aeead6796bfbd51bf2354e28f24a9

✳️ fanquake pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/e43ff4eab2fe...2b0cd7679f82)
Merge bitcoin/bitcoin#27076: verify-commits: Bump trusted git root to after most recent laanwj merge

6ada37d44cce7fa3a729de72cede4c1f3bc675ce verify-commits: Bump trusted git root to after most recent laanwj merge (Andrew Chow)

Pull request description:

To prepare for the removal of laanwj's key from trusted key (#27054), the trusted git root needs to be newer than the most recent merge commit signed by his key.

This can be tested by removing the laanwj's key from trusted keys (e.g. by merging with #27054) and running `verify-commits.py` with `--clean-merge 0`: `./contrib/verify-commits/verify-commits.py --clean-merge 0 HEAD~`. (`--clean-merge 0` disables the clean merge check which will checkout some commits, which results in the `trusted-keys` used in checking of subsequent commits to be different than expected).

ACKs for top commit:
fanquake:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce
hebasto:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce, I've verified the history of laanwj's merge commits.

Tree-SHA512: 55cafeddd54aa2b62d7b7cd41c542f4fd974b322a0405de546600d88658575714ebc893b087eb31f28c205559a7b213f88d9038de431271fca00be866610df74

🚀 fanquake merged a pull request: "verify-commits: Bump trusted git root to after most recent laanwj merge"
(https://github.com/bitcoin/bitcoin/pull/27076)

👍 MarcoFalke approved a pull request: "Remove laanwj from trusted-keys"
(https://github.com/bitcoin/bitcoin/pull/27054)

💬 MarcoFalke commented on pull request "Remove laanwj from trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27054#discussion_r1107102745)
Looks like this can be dropped after commit 2b0cd7679f826af13e809df889093b4371c3e972

Probably the whole file can be dropped, but this can be done either here or in a follow-up.

💬 ishaanam commented on pull request "wallet: ensure the wallet is unlocked when needed for rescanning":
(https://github.com/bitcoin/bitcoin/pull/26347#issuecomment-1431407746)
Rebased, I also increased the `walletpassphrase` timeout in both of the tests.

👍 MarcoFalke approved a pull request: "validation: Improve error handling when VerifyDB fails due to insufficient dbcache"
(https://github.com/bitcoin/bitcoin/pull/25574)

💬 MarcoFalke commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107161592)
nit in cf307244db97e7b9e29a3d0ff948a6fa7877b548: In the future it could make sense to return the error right away for better UX? But would probably be too much for this pull here? :man_shrugging:

💬 MarcoFalke commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107159246)
nit in 65067ee3d9730b9a121ed36d6a720cc7530a5d77: Could check that the return value is now `false`, instead of `true`?

💬 MarcoFalke commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107162432)
nit in cf307244db97e7b9e29a3d0ff948a6fa7877b548, if you retouch: clang-format?


```diff
diff --git a/src/node/chainstate.h b/src/node/chainstate.h
index db8b4f945e..1e885a22f1 100644
--- a/src/node/chainstate.h
+++ b/src/node/chainstate.h
@@ -36,11 +36,12 @@ struct ChainstateLoadOptions {
//! case, and treat other cases as errors. More complex applications may want to
//! try reindexing in the generic failure case, and pass an interrupt callback
//! and exit cleanly in the interru
...

💬 MarcoFalke commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107164412)
nit in 42b192f0cbf9c04da111145c921344b0881b3ce3:
```suggestion
dbcache, Bitcoin Core will now return an error at startup. (#25574)
```

(holds for bitcoin-qt as well)

👍 hebasto approved a pull request: "refactor: wallet, remove global 'ArgsManager' dependency"
(https://github.com/bitcoin/bitcoin/pull/26889)

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1107165674)
nanonit: slightly prefer
```suggestion
walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
```

as it was before, and is used recently in our code base.

👍 john-moffett approved a pull request: "refactor: Disable unused special members functions in `UnlockContext`"
(https://github.com/bitcoin-core/gui/pull/711)

⚠️ darosior opened an issue: "Disallow duplicate leaves inside `tr()` descriptors"
(https://github.com/bitcoin/bitcoin/issues/27104)
While working on adapting Miniscript to Tapscript, i [noticed](https://gist.github.com/sipa/06c5c844df155d4e5044c2c8cac9c05e?permalink_comment_id=4434770#gistcomment-4434770) signatures in Tapscript only commit to the leaf (i previously assumed they committed to the whole branch). It's been discussed lately on the mailing list as well ([1](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-February/021452.html), [2](https://github.com/bitcoin-inquisition/bitcoin/issues/19)). This mean
...

💬 darosior commented on issue "Disallow duplicate leaves inside `tr()` descriptors":
(https://github.com/bitcoin/bitcoin/issues/27104#issuecomment-1431453521)
In the case of the Bitcoin Core wallet it would be a backward incompatible change though..

💬 glozow commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1431494681)
post merge ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107231839)
Right that's a good catch. We still _behave_ the same as previously, either fetching `conf` from an absolute path or completing the path with default datadir, but now because I simplified [reading the conf into a stream](https://github.com/bitcoin/bitcoin/pull/27073/commits/6207b73e508d6cfc7583483d00261c66b0c348c3#diff-19427b0dd1a791adc728c82e88f267751ba4f1c751e19262cac03cccd2822216L993), we return the full path we were trying in the error.

I agree that therefore there is a slight behaviour c
...

🚀 fanquake merged a pull request: "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements"
(https://github.com/bitcoin/bitcoin/pull/26153)

💬 fanquake commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#issuecomment-1431505288)
Going to move this forward. @real-or-random would still be great for you to leave any post-merge commentary, if you get the time.