💬 davidgumberg commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2564008796)
> The node in question finished sync after I ran it inside heaptrack and is humming along just fine now. I printed out the flamegraph although I have trouble interpreting it:
>
> [ flamegraph.svg]

At first glance it seems pretty unusual that so much time (~40% of samples) is spent in the call stack above `CScriptCheck::operator()` seemingly doing allocations.

💬 mzumsande commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2564016601)
> I do not agree with this change and it should not be merged.

"[A NACK needs to include a rationale why the change is not worthwhile. NACKs without accompanying reasoning may be disregarded.](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md)"

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