💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1666119672)
fixed, the "," went into the hash by mistake (and was ignored). I also fixed the `hash_serialized`, which was wrong somehow.

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1666119718)
fixed, here and at one more spot.

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1666119744)
Done!

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1666119817)
Done, use `*3`

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2209669755)
Thanks for the reviews! I addressed the feedback by @maflcko and also corrected `hash_serialized` in the chainparams. Not sure why it was wrong before, I'm pretty sure I was able to activate the snapshot before opening the PR on another computer. Probably just an oversight, but maybe someone can confirm that everything is deterministic and the snapshot can be loaded now (e.g. by adding an assert and setting `base_blockheight` and `m_coins_count` to 200 it should crash almost immediately).

> I
...

💬 chennleyi commented on issue "Improve description of the `filename` parameter of `loadwallet` RPC":
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2209881209)
Hi @murchandamus, is this issue about to get resolved? can I work on this issue? I am new to this.

💬 mzumsande commented on pull request "Assumeutxo: bugfix on loadtxoutset with a divergent chain + tests":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1666308257)
I think that "chain tip" usually refers to blocks we have connected to a chainstate (so headers-only would not qualify), so I would suspect that the original intention of the TODO was just what the third commit implements.

But I agree with @fjahr that it doesn't really add too much to `test_snapshot_with_less_work` - less work is less work, no matter if the chain is divergent or not. Plus, I agree that if #30320 gets merged it would become a special case of that, because a chain tip with more
...

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1666348629)
> the "," went into the hash by mistake (and was ignored)

ugh. I didn't even notice this

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2210242534)
re-ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb 🎆

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK de71d4dece604907afc4
...

💬 paplorinc commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2210308399)
Thanks for the review @andrewtoth.
I'm not sure I see the advantage of inverting the `if (inserted) {` call, just to have two `return it;` calls after.
But I did rename `it` to `ret`, since we're getting the most likely return value immediately now.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2210322982)
Updated c1d6e525131cb9e54bbedb79ea64e2469a77aed8 -> 606a7ab862470413ced400aa68a94fd37c8ad3d3 ([noGlobalScriptCache_13](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_13) -> [noGlobalScriptCache_14](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_14), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_13..noGlobalScriptCache_14))

* Addressed @glozow's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1664039214), a
...

👍 TheCharlatan approved a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329#pullrequestreview-2160094330)
ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb

💬 ajtowns commented on pull request "wallet: use LogTrace for walletdb log messages at trace level":
(https://github.com/bitcoin/bitcoin/pull/30355#issuecomment-2210376239)
> > Could also change it to `[warn]` when the user request could not be fulfilled?
>
> @ajtowns Did you see this? Seems rfm either way, but maybe address this or reply, so that maintainers don't hold it back on this nit?

Think it would make more sense to change that along with bumping other `LogPrint{f,}()` calls (eg, "WARNING SQLite is configured to not wait for data to be flushed...", ). Also whether that should be `[warn]` seems up in the air at the moment: with #30347 or #30361 it migh
...

📝 maflcko opened a pull request: " rpc: Use untranslated error strings in loadtxoutset "
(https://github.com/bitcoin/bitcoin/pull/30395)
Motivation:
* Some are not translated at all, anyway. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663631973
* For others translation is not yet needed, because they are not called by the GUI (yet)
* For others translations will never be needed, because they are RPC code. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663611194

Also, while touching this:
* Remove the trailing `\n`. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663647981

...

💬 TheCharlatan commented on pull request "rpc: Use untranslated error strings in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/30395#issuecomment-2210422683)
Concept ACK

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2210448369)
A different approach that stays closer to the original design but does not add noise encryption and a dedicated network stack:

- (Maybe) introduce `SocketTransport` to enable p2p messages over unix sockets with less overhead (but how to handle multiple connections?)
- Add a whitelist permission `mining`
- Peers with that permission have an extra `CNode` field (akin to `Sv2Client`)
- `m_coinbase_output_data_size`
- We send these peers a `HEADERS` message for every tip update (equivalent
...

💬 HerWayBit commented on pull request "rpc: Use untranslated error strings in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/30395#issuecomment-2210450157)
> Motivation:
>
> * Some are not translated at all, anyway. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663631973
>
> * For others translation is not yet needed, because they are not called by the GUI (yet)
>
> * For others translations will never be needed, because they are RPC code. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663611194
>
>
>
> Also, while touching this:
>
> * Remove the trailing `\n`. See https://github.com/bitcoin/bitcoin/pull/3026
...

💬 maflcko commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2210469835)
Maybe something like https://github.com/OSGeo/PROJ/commit/8f29b8f48b1b78aed4cb54f19ce7ea5c6c6fb469 is needed in oss-fuzz?

💬 maflcko commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2210544601)
ACK 7c9684a5a357e300f4901167a4bb0e1bdafd075d

💬 fjahr commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2210547243)
utACK de71d4dece604907afc4fc26b7788e9c1a4cbecb