📝 ajtowns opened a pull request: "wallet: use LogTrace for walletdb log messages at trace level"
(https://github.com/bitcoin/bitcoin/pull/30355)
Wallet sqlite logging is enabled by `-debug=walletdb -loglevel=walletdb:trace` however the actual log messages are sent at `BCLog::Level::Info`. Switch to the trace level to make this consistent. This adds `[walletdb:trace]` to the log output, eg:

```
[httpworker.3] [wallet/sqlite.cpp:55] [TraceSqlCallback] [/tmp/bitcoin_func_test_4fsnatpg/node0/regtest/wallets/boring/wallet.dat] SQLite Statement: BEGIN EXCLUSIVE TRANSACTION
```

becomes

```
[httpworker.0] [wallet/sqlite.cpp:55] [Trac
...

👍 rkrux approved a pull request: "test: fix inconsistency in fundrawtransaction weight limits test"
(https://github.com/bitcoin/bitcoin/pull/30353#pullrequestreview-2147327181)
tACK [4f571fb](https://github.com/bitcoin/bitcoin/pull/30353/commits/4f571fb73526b55a83af3c04fbc662ccdd1307ae)

`make, test/functional` are successful.

Verified that the default value of `add_inputs` is `True` in `fundrawtransaction` that might have caused the inconsistency in the CI: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/spend.cpp#L751

Also verified that the corresponding scenario in `wallet_send.py` has `add_inputs: False` passed already here: https://github.com
...

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049)
unrelated nit: The debug mode part has been fixed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85828#c3 / https://github.com/gcc-mirror/gcc/commit/c2fb0a1a2e7a0fb15cf3cf876f621902ccd273f0 for all version supported by Bitcoin Core, so this part can be removed.

About whether the stdlib is faster or uses more entropy, I haven't benchmarked it, but wanted to note that it generates two ints with one 64-bit rng call and uses https://github.com/lemire/FastShuffleExperiments

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658346921)
nit: Could also require `concept std::uniform_random_bit_generator` inside of `RandomNumberGenerator`?

📝 Sjors opened a pull request: "refactor: add coinbase constraints to BlockAssembler::Options"
(https://github.com/bitcoin/bitcoin/pull/30356)
When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs.

At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually [produced an invalid block](https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426) which exceeded the sigops limit.

The existince of such patches suggests it may be useful to make this va
...

💬 Sjors commented on pull request "Mining interface followups, reduce cs_main locking, test rpc bug fix":
(https://github.com/bitcoin/bitcoin/pull/30335#discussion_r1658383394)
Added to #30356.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2196451678)
I opened #30356 with some `BlockAssembler::Options` and mining interface refactoring that should make this PR smaller.

💬 maflcko commented on pull request "wallet: use LogTrace for walletdb log messages at trace level":
(https://github.com/bitcoin/bitcoin/pull/30355#issuecomment-2196451807)
ACK 46819f5df6de2a860c3ec87d55527b617a3b128f

Makes sense to use the severity level and category that the user asked for, instead of a different one.

Could also change it to `[warn]` when the user request could not be fulfilled?

https://github.com/bitcoin/bitcoin/blob/46819f5df6de2a860c3ec87d55527b617a3b128f/src/wallet/sqlite.cpp#L270

👍 maflcko approved a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2147384765)
Left some more nits. Feel free to ignore.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658360003)
nit: This is used in 5 places, in 3 of them "incorrectly". (BIP 330 doesn't disallow a uint64_t max as salt, same for `CSipHasher` in `PriorityComputer`). So you could replace them with a call to `rand`. The remaining two could just be inlined with a call to randrange.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658362617)
nit in b128d1d0ab48d2c4e535ff0e90225d6aae626817: When making `GetOSRand` private, you should make this one private as well?

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658389327)
I benchmarked this suggested change and it was an improvement for my CPU.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2196470058)
The MSan, depends job [failed](https://cirrus-ci.com/task/5872614726434816?logs=ci#L2697) the test which issues the certificate 1 second in the future. I increased the time difference so such failure is less likely.

The fuzzer could also pick 1 second in the past or future for the certificate validity test. But it uses `SetMockTime`, so presumably doesn't have this problem.

💬 maflcko commented on pull request "Make GUI and CLI tools use the same datadir":
(https://github.com/bitcoin/bitcoin/pull/27409#issuecomment-2196478179)
@ryanofsky What is the status of this?

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658405291)
Good catch, this is indeed wrong.

💬 maflcko commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1658448718)
NACK on refactoring code that will be removed anyway in the future. The change has no benefit and is causing merge conflicts and hassle down the line.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658468860)
Does this even need to be a fresh random value every time? Couldn't it just be any nothing up my sleeve number?

💬 darosior commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2196596411)
Will re-review from scratch next week to finally get this over the finish line.

💬 darosior commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#issuecomment-2196665299)
Rebased.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2196705221)
Thank you for the reviews @stickies-v and @theuni!

Updated 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a -> 269afa0bf02650067a7507af026165d83678786e ([noGlobalScriptCache_7](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_7) -> [noGlobalScriptCache_8](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_7..noGlobalScriptCache_8))

* Addressed @theuni's [comment](https://github.com/bitco
...