💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2096309054)
Nice! I guess it manages to see through the references after all. My testing shows that:
```c++
auto& queue = m_chainman.GetCheckQueue();
CCheckQueueControl<CScriptCheck> control(queue);
CCheckQueueControl<CScriptCheck> control2(queue)
```

```c++
CCheckQueueControl<CScriptCheck> control(m_chainman.GetCheckQueue());
CCheckQueueControl<CScriptCheck> control2(m_chainman.GetCheckQueue());
```

Both of those indeed work.

I suppose I came to the conclusion that the
...

💬 darosior commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2891985815)
> against an accidental soft fork

You mean against a tightening of the rules, which in the context of standardness is harder than a loosening of the rules? :p

👍 hebasto approved a pull request: "Use WitnessV0KeyHash in TestAddAddressesToSendBook"
(https://github.com/bitcoin-core/gui/pull/875#pullrequestreview-2851735518)
ACK fa982f14254433a969499e93c1c3f0db31dce6ab, I have reviewed the code along with the related changes from https://github.com/bitcoin/bitcoin/pull/32511.

✅ hebasto closed an issue: "`AddressBookTests` failure"
(https://github.com/bitcoin-core/gui/issues/874)

✅ hebasto closed an issue: "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]"
(https://github.com/bitcoin/bitcoin/issues/32558)

🚀 hebasto merged a pull request: "Use WitnessV0KeyHash in TestAddAddressesToSendBook"
(https://github.com/bitcoin-core/gui/pull/875)

🤔 darosior reviewed a pull request: "script: short-circuit `GetLegacySigOpCount` for known scripts"
(https://github.com/bitcoin/bitcoin/pull/32532#pullrequestreview-2851775828)
This is a significant refactoring of tricky consensus critical routines that have essentially never been modified since Satoshi wrote them 15 years ago. The exploration is interesting but it seems ill-advised to completely overhaul this critical code for a marginal IBD speedup.

I feel bad for being stop energy here, but since i have been an advocate for us voicing our opinion instead of just ignoring PRs i'll go and own it. Concept NACK: i don't think the risk-benefit ratio of this change is
...

🤔 mzumsande reviewed a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage"
(https://github.com/bitcoin/bitcoin/pull/32516#pullrequestreview-2851588776)
Concept ACK

💬 mzumsande commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2096386541)
This comment is outdated, because `DEFAULT_ANCESTOR_LIMIT = 25` is no longer after the last commit. Not sure about the reason why it was used before, is it no longer valid?

Also, changing DEFAULT_ANCESTOR_LIMIT to 100 could be mentioned in the commit message, it also affects the other subtests.

💬 mzumsande commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2096229309)
I don't understand this comment. Did you use prioritisetransaction in an older version, but dropped that?

🚀 ryanofsky merged a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221)

✅ ryanofsky closed an issue: "wallet: wrong balance and crash after reorg and unclean shutdown"
(https://github.com/bitcoin/bitcoin/issues/31824)

💬 ryanofsky commented on pull request "script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2892128673)
Might also help to add all the benchmark and test changes and simpler improvements like inlining in a separate PR, so it's a more clear how tricky the sensitive changes here are, and if the sensitive changes are really worth making.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096403730)
I understand the slight additional complexity concern but for a transaction submitted to us with no PoW i think we should discard it as soon as we know we won't accept it. I really don't think we should keep doing unnecessary work by iterating, solving scripts and counting.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096405194)
Don't you think it would be confusing to point here since this function does not concern itself with the witness? Especially as it's pointed right before.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096415934)
Done, added a sentence to this effect.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096416043)
Done.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096416080)
That's actually what i had initially but i reduced it to `MAX_LEGACY_SIGOPS` in an attempt to avoid too long variable names. With the comment expliciting it's about transaction inputs, i think it should be fine as-is.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2096416254)
Expanded the test to check the original, non-standard, one can still be mined.

💬 darosior commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2096422126)
If it's set to the maximum RAM capacity on 32-bit system the process will start and then just randomly crash after having received enough blocks. 1 GiB seems like a reasonable maximum for a 32-bit system.