🤔 mzumsande reviewed a pull request: "validation: write chainstate to disk every hour"
(https://github.com/bitcoin/bitcoin/pull/30611#pullrequestreview-2230536256)
Did you check if this would slow down IBD noticeably when running with a large dbcache?
If i understand it right, coins that are created and destroyed more than 1 hour of syncing apart would not have been written to disk before, but will be written and deleted after this PR - though presumably this is not a frequent event, so there wouldn't be much of an impact?!

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1711719710)
Oh of course, I was confusing myself :+1:

📝 paplorinc opened a pull request: "Change `nproc` in docs to `getconf _NPROCESSORS_ONLN`"
(https://github.com/bitcoin/bitcoin/pull/30619)
Split out of https://github.com/hebasto/bitcoin/pull/316/files#r1711537463

nproc is linux only, getconf _NPROCESSORS_ONLN is used in the Linux kernel for the same task: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/rcutorture/bin/kvm-build.sh#L44

On linux:
```
# nproc
8
root@rescue ~ # getconf _NPROCESSORS_ONLN
8
```

Note that in the productivity doc the recommended core count is one less than before, seems simpler to me this way.

👍 itornaza approved a pull request: "Stratum v2 Noise Protocol"
(https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2230564992)
Code review ACK e73740fde107f79eeee8e55c6bed156713abe8a5

This pr closely follows the intersection of the original [noise protocol paper](https://noiseprotocol.org/noise.pdf) and the [Protocol Security section](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) of the Stratum V2 Specification to provide an implementation of the Noise Protocol on Bitcoin Core. In that way we can now step by step integrate the Stratum protocol Template Provider and Job Declaration Clie
...

💬 ajtowns commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1711764521)
Added a commit to use mutually exclusive groups here and in a few other places.

💬 achow101 commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#issuecomment-2278320928)
> `-assumevalid` will raise an error when input contains invalid hex characters (including `0x` prefix), or when it is not exactly 64 characters long

Why not allow `0x` prefixes as `-minimumchainwork` does? It seems plausible to me that someone might have put a `0x` prefixed block hash in their conf file and this change breaks that.

👍 pablomartin4btc approved a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2230620169)
> > Tested [d45ac03](https://github.com/bitcoin-core/gui/commit/d45ac03a89cc500cd461f878f092cf8ec99e7760). The "Migrate Wallet" menu item is still disabled when no wallet is loaded.
>
> Should be fixed now.
>

re-ACK bcf47b9f34e4f21e5ac75521ddd4f25ff3812412

💬 achow101 commented on pull request "doc, chainparams: 29775 release notes and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/30604#issuecomment-2278345688)
ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60

💬 sipa commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1711792420)
I don't have much of an opinion about how valuable this comment is, but for the same of argument, I would not say that "Disallow silent float -> int conversion" carries no meaning beyond what `std::integral I` implies.

`std::integral I` means that the function cannot be instantiated with I anything but an integer. The comment additionally clarifies that it's there because (a) specifically we do not want to allow floats and (b) that this is because of unexpected effects from silently convertin
...

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2278349582)
> Before someone suggests that it should do this every block, a reason to not do this too much is that delaying writes prevents many writes from ever happening at all because many outputs are quickly spent.

> If i understand it right, coins that are created and destroyed more than 1 hour of syncing apart would not have been written to disk before, but will be written and deleted after this PR

Correct, there will be significantly less utxo cut through with this PR.

> I don't see a partic
...

🚀 achow101 merged a pull request: "doc, chainparams: 29775 release notes and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/30604)

👍 andrewtoth approved a pull request: "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin"
(https://github.com/bitcoin/bitcoin/pull/30326#pullrequestreview-2230764899)
re-ACK 204ca67bba263018374fe86d7a6867362d09536f

💬 furszy commented on pull request "wallet: Fix listwalletdir listing of migrated default wallets and generated backup files":
(https://github.com/bitcoin/bitcoin/pull/30265#discussion_r1711906769)
this line does not seems to be related to a700b303fcba7915f.

💬 whitslack commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1711941341)
`std::integral` already means you specifically do not want to allow floats. If you did want to allow floats, you would have used `std::is_arithmetic_v`, but you didn't, so therefore you specifically want to exclude floats.

I'll concede the point that deleting the comment loses the information that you've previously had issues with silent conversions, although the presence of the constraint guarantees that you'll never have such issues again, so signaling about the previous issues would only s
...

⚠️ wydengyre opened an issue: "wallet: setting changes are subject to race conditions"
(https://github.com/bitcoin/bitcoin/issues/30620)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Creating two wallets with `load_on_startup=true` simultaneously will result in only one of the wallets being added to the startup file.

### Expected behaviour

Both wallets should be added.

### Steps to reproduce

I don't have an easy script to reproduce this, but the issue is quite visible in the code:
https://github.com/bitcoin/bitcoin/blob/389cf32aca0be6c4b01151c92cc3be79c120f197/src
...

🤔 mzumsande reviewed a pull request: "assumeutxo: Drop block height from metadata"
(https://github.com/bitcoin/bitcoin/pull/30598#pullrequestreview-2230844861)
ACK 00618e8745192d209c23e3ae873c077e58168957

💬 mzumsande commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711982543)
nit: could remove comment

💬 ryanofsky commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711935626)
In commit "util: refactor: change IsHexNumber to TrySanitizeHexNumber" (802374b4355bd1dec7a88bba6287c55f935699fe)

Would be good to change "Size of" to "Desired size of". When I first read this I was confused why this would ask for the size to specified.

🤔 ryanofsky reviewed a pull request: "node: reduce unsafe uint256S usage"
(https://github.com/bitcoin/bitcoin/pull/30569#pullrequestreview-2230780077)
Code review 855784d3a0026414159acc42fceeb271f8a28133, mild NACK.

I like the new test, and ideas behind this change, but I think some parts of this are not well documented, and are breaking things that work without a clear reason.

💬 ryanofsky commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711982050)
In commit "test: Only accept a fully valid RANDOM_CTX_SEED" (855784d3a0026414159acc42fceeb271f8a28133)

I don't understand the motivation for being so strict here. It seems better to call TrySanitizeHexNumber and keep accepting small numbers. The seed just needs to be any number, and I don't see why numbers that have 64 digits are better than other numbers, or why somebody manually setting the environment variable manually should need to pad the number and make it has 64 digits.