💬 TheCharlatan commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#discussion_r1807432735)
I think your suggestion is a bit better. I'll push it if something else comes up. Looks to me like that code iterated over the chainstates in a previous un-pushed version, that would explain why only the errors are surfaced here. There is also ryanofsky's assumeutxo state cleanup PR that could catch this later if we don't get it here

💬 TheCharlatan commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#discussion_r1807433006)
fyi I also hope we can improve how to do this in ryanofsky's #30214.

💬 l0rinc commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1807434072)
> BIP173/BIP350 specify the maximum length of bech32/bech32m strings as 90 characters.

Yes, but https://en.bitcoin.it/wiki/BIP_0352 states that:
> Note: [BIP173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki) imposes a 90 character limit for Bech32 segwit addresses and limits versions to 0 through 16, whereas a silent payment address requires at least 117 characters[[12]](https://en.bitcoin.it/wiki/BIP_0352#cite_note-why_117_chars-12) and allows versions up to 31. Additional
...

💬 andrewtoth commented on pull request "validation: Make ReplayBlocks interruptible":
(https://github.com/bitcoin/bitcoin/pull/30155#issuecomment-2424107647)
Concept ACK

It appears to me that the size of the cache used in ReplayBlocks is never checked if it grows too large. Wouldn't this mean that if the node was sufficiently far behind, the rolling forward would fill up the cache to levels exceeding dbcache? This would result in a cycle of rolling forward and then OOM crashing, never being able to write any chainstate to disk.

💬 sipa commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1807450121)
Sure, but support for silent payment hasn't been merged. Today, the codebase only supports bech32 as specified by bip173/bip350 (see https://github.com/bitcoin/bitcoin/blob/master/doc/bips.md).

✅ achow101 closed a pull request: "Simple security.doc improvement"
(https://github.com/bitcoin/bitcoin/pull/31114)

💬 achow101 commented on pull request "Simple security.doc improvement":
(https://github.com/bitcoin/bitcoin/pull/31114#issuecomment-2424198074)
Thank you for your contribution. Stylistic and formatting changes to documentation are generally discouraged as they come at a cost for the project as a whole while not meaningfully changing anything. In particular, many of the changes in this PR do not change how things are rendered, nor the readability of the documentation even when viewed in plain text. The motivation provided does not justify the burden on the project that this may place on the project. A burden could be any of the following
...

💬 achow101 commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2424200438)
In general, draft means that a PR is not in a state where it could be merged. It indicates to reviewers that they may not want to do any in depth code review of the PR yet.

In the context of this PR, there does not appear to be consensus for the concept of OP_CAT in TapScript yet, so this PR cannot be merged even if the code is okay. Hence marking it as a draft.

Note that this is a result of a [Kill/Shill/Merge](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Kill-Shill-Drill) session
...

💬 davidgumberg commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2424219301)
> seeding randomness entropy from Windows performance data obtained through the Registry API during startup hangs when attempting to release a semaphore deep in Microsoft code.

The call to `RegQueryValueExA` in `RandAddSeedPerfmon`:

https://github.com/bitcoin/bitcoin/blob/e8f72aefd20049eac81b150e7f0d33709acd18ed/src/randomenv.cpp#L80-L91

⚠️ Hulamoney opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31119)

💬 andrewtoth commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2424297328)
If I understand correctly, this would allow us to reuse the parallel check queue with worker threads to fetch block inputs from disk. Then we could fetch and cache all missing inputs concurrently in ConnectBlock instead of fetching from disk one by one.

✅ achow101 closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31119)

👍 laanwj approved a pull request: "doc: replace `-?` with `-h` for bench_bitcoin help"
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2380203415)
ACK f0130ab1a1e65583637b6a362b879ea3253e7bb7
Easier to remember too, `-?` is a kind of mingling between Windows/DOS `/?` and UNIX `-h` conventions.

💬 laanwj commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1807719827)
> I did maintain the "high details levels = maximum" behavior preferred by laanwj in https://github.com/bitcoin/bitcoin/pull/21192 for values above 5.

Thank you 😄

💬 laanwj commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#discussion_r1807727134)
Could also use `-G ninja`, the ninja build system uses all available CPU threads by default 😎
(not sure we'd want to recommend it as default tho)

👍 laanwj approved a pull request: "init: Correct coins db cache size setting"
(https://github.com/bitcoin/bitcoin/pull/31064#pullrequestreview-2380260832)
Code review ACK 3a4a788ee0db83d20607f14801dbed2ee932943c
Both changes look sensible to me, but have not done any specific testing.

:lock: fanquake locked an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31119)

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807753176)
i'm very confused about this one; `begin()` returns a `unsigned char*`, and unsigned values are definitely allowed to overflow within defined behavior. Also, `m_data` is an array so by definition [0] will not be out of bounds.

👍 rkrux approved a pull request: "rpc: Add support to populate PSBT input utxos via rpc"
(https://github.com/bitcoin/bitcoin/pull/30886#pullrequestreview-2380312703)
tACK https://github.com/bitcoin/bitcoin/commit/abc835282a66495f65f342cfc113e45aac4ebb48

Successful make and functional tests.
I manually tested the new argument in `utxoupdatepsbt` using my regtest environment by creating parent transaction and a subsequent child transaction. Processing the child PSBT without passing in the raw parent transaction hex led to incomplete processing and was successful later when the parent was passed.
Also tested for duplicate parent transaction error.

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1807750310)
```suggestion
{"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A serialized previous transaction in hex"},
```