💬 billymcbip commented on pull request "script: Remove dead code from OP_CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33977#issuecomment-3596782767)
I think it wouldn't hurt to have an `assert(!stack.empty());` instead, just in case the code above is ever refactored.

🤔 sipa reviewed a pull request: "Cluster mempool followups"
(https://github.com/bitcoin/bitcoin/pull/33591#pullrequestreview-3525225192)
ACK 48c29fbe0974664e28564af07fbfd753cf125c36

See nits above.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577293052)
That would be racy. We could have the worker thread count be a variable instead of hard coded, then for a test we could make it zero.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577297663)
Oh, we override so we make sure all threads are stopped before we do the batch write. This is in a comment two lines below.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577299367)
Looks like this needs more work, it's not as easy as I though - we could use an `std::deque` instead of a `vector` here:
```patch
Subject: [PATCH] std::deque<InputToFetch> m_inputs
---
diff --git a/src/coinsviewcacheasync.h b/src/coinsviewcacheasync.h
--- a/src/coinsviewcacheasync.h (revision a3f56354d6e3f64eaca84a16e4951e6073090f60)
+++ b/src/coinsviewcacheasync.h (revision 462408b897197de3a7067dcbdee318ad9dc1e546)
@@ -17,6 +17,7 @@
#include <atomic>
#include <barrier>
#include <cs
...

💬 vostrnad commented on pull request "script: Remove dead code from OP_CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33977#issuecomment-3596822343)
The script interpreter code is so thoroughly tested, so infrequently refactored, and any change to it reviewed with such extreme caution that I don't think we need to bother with an assert, but I can add it if others think it's a good idea.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577321551)
you sure it would be racy?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577345226)
> I disagree, I think the current move construction is incorrect and if I understand it correctly, we should reserve instead and delete (or ignore) the other constructors.

I think you are disagreeing with my statement:

> since it only happens before we start doing work, might as well make it simple and not bother moving the other fields

because the other things I said were facts. Do you think we should also construct new `atomic_flag`s when doing the move construction? As I mentioned I
...

💬 fanquake commented on pull request "guix: reduce allowed exported symbols":
(https://github.com/bitcoin/bitcoin/pull/33950#issuecomment-3596900808)
Guix Build (aarch64):
```bash
b507c965417a1adcd6b10000f9a8bae29e18ba9a1e76060c38af7fe1fbbcd0e4 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/SHA256SUMS.part
c60c33e4ececbc650bbeda06c7aa7dad714ace312a08c804afd750fc0a0c3d68 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/bitcoin-7b90b4f5bb10-aarch64-linux-gnu-debug.tar.gz
77bdfc199c87064ee166c6c124aa9d586fb24b144d8408bdd3d0e2e0065577a5 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/bitcoin-7b90b4f5bb10-aarch64-linux-gnu.tar.gz
5b6c18
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577362125)
There is a `ConnectBlock` benchmark already. But, it only does internal spends so it won't be that great for this. I can maybe extend it to also work on a block with inputs from leveldb. WDYT?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577369275)
If we have a backing cache that blocks, how can we know if it's the main thread or worker threads that need to be blocked? And if we block the main thread by mistake, it will make no progress even though the worker thread can fetch all inputs

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577376037)
We want to specifically test that we do not access the main cache's backing view, by e.g. calling `GetCoin` on it. If we return a nullopt here then it would correctly go to the db layer, while we want to make sure we blow up here because it is a failed test.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577394394)
> NoAccessCoinsView is designed to abort on access

It is designed to abort on accessing the db via the main cache. We want to access the db only via our `m_db` ref and not go through the main cache's `base` pointer. This is unrelated to the test of short txid collisions. For those, we want to go to successfully go to disk on the main thread, while getting a nullopt from our m_input coin.

💬 willcl-ark commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3596972244)
Backports all look good to me.

We are missing a release note for https://github.com/bitcoin/bitcoin/pull/33229 currently, I think it's one worth adding too.

📝 kevkevinpal opened a pull request: "refactor/p2p: Decouple CNode from PeerManagerImpl::MaybeDiscourageAndDisconnect"
(https://github.com/bitcoin/bitcoin/pull/33983)
## Description
Refactor MaybeDiscourageAndDisconnect to accept individual parameters (`has_no_ban`, `is_manual`, `is_local_addr`, `is_inbound_onion`, `peer_addr`) instead of taking a CNode& reference directly.

This decouples the function from CNode while preserving all original logic paths and behavior.

This change is just a refactor, so no tests were modified

## Linked issue
This is a part of #33958.
Let me know if this is not needed and I can close this PR

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577433809)
> when a block has e.g. a double-spend to make sure it's not propagated to the main cache

That would be a necessary condition of a double-spending block test. If the double spend is propagated to the main cache it is part of the utxo set and the test is a failure. The main cache is treated as the source of truth.

⚠️ mbarulli opened an issue: "When 30.0 for Start9"
(https://github.com/bitcoin/bitcoin/issues/33984)
### Please describe the feature you'd like to see added.

Just a reminder that people running Core on Start9 machines are still on 29.2.
When is 30.0 going to be packaged for [Start9](https://start9.com/)?
Thanks!

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_

✅ willcl-ark closed an issue: "When 30.0 for Start9"
(https://github.com/bitcoin/bitcoin/issues/33984)

💬 willcl-ark commented on issue "When 30.0 for Start9":
(https://github.com/bitcoin/bitcoin/issues/33984#issuecomment-3597046249)
@mbarulli this is out of scope for this project.

I think the correct place to open such an issue would be in the Start9 repo.

https://github.com/Start9Labs

👍 brunoerg approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3525526629)
reACK 189437a3dedefce3945ab2a8c05785eb283c2c6f