💬 l0rinc commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2564016916)
@ismaelsadeeq, I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the `TxVerbosity::SHOW_TXID` bench - while the rest were exactly the same as before.
But if it's *that* important here, we could easily get a 30% speedup as well there by deduplicating the `GetSerializeSize` calls:
```C++
static int64_t GetBlockWeight(const size_t size_no_witness, const size_t size_with_witness)
{
return size_no_witness * (WITNE
...

🤔 furszy reviewed a pull request: "validation: Send correct notification during snapshot completion"
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2524378694)
Code review ACK bc43ecaf6dc

💬 brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898710992)
Done.

💬 brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#issuecomment-2564026547)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898539802

📝 brunoerg opened a pull request: "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`"
(https://github.com/bitcoin/bitcoin/pull/31577)
`DecodeBase58` and `DecodeBase58Check` are fuzzed with a fixed value for `max_ret_len` (100). However, different values are used for this parameter depending on the usage (see key_io and unit tests).

💬 l0rinc commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564034989)
Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746/files instead?

💬 brunoerg commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564037774)
> Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746 instead?

Feel free to cherry-pick this change there, then I close this one.

💬 l0rinc commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564038129)
Moved the change over, added you as a co-author - thanks

✅ brunoerg closed a pull request: "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`"
(https://github.com/bitcoin/bitcoin/pull/31577)

💬 daniel1302 commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31575#discussion_r1898758548)
This function is declared but it looks unused in entire codebase.

💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2564139270)
Tackled review comments from the follow-up PR #31560 (https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896811520, https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896861901, https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896864227, https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1897341483).

💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776033)
Good idea, done in #27432 (referring now to the module docstring, which is also visible as `--help` output of the script).

💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776061)
Added a fixed timeout of 10 seconds, which should be more than enough given the tiny regtest UTXO set. I felt that it's not worth it to introduce a constant for that, but happy to add if others feel strongly.

💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776099)
Great idea, done in #27432.

💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776469)
The idea is to skip a test rather than fail, if the `sqlite3` module is not available (as far as I'm aware, the only supported distro where this could happen currently is FreeBSD). However, this indeed didn't work as expected since I was using `skip_if_no_sqlite` rather than `skip_if_no_py_sqlite3` (see #26882). Fixed in #27432.

💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2564143915)
@tdb3 @romanz: Thanks for your reviews, much appreciated! Note that the first two commits which introduce the utxo-to-sqlite.py tool (+test) are part of the base PR #27432, so further comments on those changes would better fit there in the future. I took all of your suggestions and updated #27432 and rebased this PR on top of that again accordingly.

✅ SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31575)

💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898892756)
Should probably replace `// Support up to x-of-3 multisig txns as standard` with
`// Support up to x-of-MAX_BARE_MULTISIG_PUBKEYS_NUM multisig txns as standard`

💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898893779)
https://github.com/bitcoin/bitcoin/pull/31404/commits/088bbb81e34dfbe56fdeafaba2ddf756017e86e1: `MAX_BARE_MULTISIG_PUBKEYS_NUM` is an unsigned int so you can use `%u` format specifier instead of `%d`

💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898901503)
https://github.com/bitcoin/bitcoin/pull/31404/commits/50ebd73aeeb853328d1f4eadba77f2ca80a27a8c: AFAICT At this point, the `addmultisigaddress` operation is already complete (the scripts have been imported into the wallet and the destination is in the address book), throwing an error here may mislead the user into thinking the operation failed.

If you want the operation to fail if an invalid descriptor is inferred then this code needs to be moved up before the scripts are imported into the wal
...