💬 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
...

📝 willcl-ark opened a pull request: "Permit opt-out of finalization during `walletprocesspsbt`"
(https://github.com/bitcoin/bitcoin/pull/30357)
Fixes: #30077

#28414 added an auto-finalization of PSBTs when callilng `walletprocesspsbt` if we think they're complete, which is generally useful.

However it subverted a check on whether we specifically opted out of finalizing by testing the `finalize` RPC boolean, resulting in a `CHECK_NONFATAL` abort in some situations where it can be avoided. Specifically this might happen (as reported originally) in the case that a `non_witness_utxo` was not provided and a witness signature was invali
...

💬 paplorinc commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1658676556)
Makes sense, removed

💬 willcl-ark commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-2196820695)
@grubles what's the current status of this issue?

Are you able to test with newer versions of Bitcoin Core, e.g. v27.1 and/or v26.2?

Reading the above, it seems to me like we can probably close this now otherwise.

💬 maflcko commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#discussion_r1658695155)
@sr-gi Do you want to follow-up on this?

✅ maflcko closed an issue: "make check errors on big endian OpenBSD 7.2"
(https://github.com/bitcoin/bitcoin/issues/26492)

💬 maflcko commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-2196844812)
Closing for now, due to inactivity

💬 stickies-v commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2196845660)
re-ACK 475c57ba482abd8675d0d3ccde98452808be5536, addressing outstanding review nits / touchups, no fundamental changes.