👍 dergoegge approved a pull request: "fuzz: Make process_message(s) more deterministic"
(https://github.com/bitcoin/bitcoin/pull/32822#pullrequestreview-3037221638)
reACK fa1a14a13a15ecfb7587a94ee86b4ace7c819519

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2218671774)
IIUC The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which `scriptPubKeys` to use in a GCS filter. Since we can't skip blocks, fast rescans shouldn't provide any advantages here.

💬 stickies-v commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2218745362)
> I'm wondering if it makes more sense to signal the lack of something ... instead of the additional functionality in the pair

I considered that too, but I don't think that's better. The "Unchached" one is unsafe, and for that it should stand out. My initial thought was to name it `GetAddressesUnsafe` or `GetAddressesInternal`, but those names carry less meaning, so I eventually landed on just describing what it does, which I think at least gives people somewhat familiar with the code a good
...

📝 maflcko opened a pull request: "NOMERGE DEBUG WIP ignore"
(https://github.com/bitcoin/bitcoin/pull/33028)

💬 fanquake commented on pull request "ci: Use APT_LLVM_V in msan task":
(https://github.com/bitcoin/bitcoin/pull/32999#issuecomment-3096172593)
Should probably adjust the timeout / comment:
https://github.com/bitcoin/bitcoin/blob/5878f35446ae260ccb0ab5051b795b4364f77951/.cirrus.yml#L160
but given it's being moved in #32989, left a note there.

💬 fanquake commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2218810639)
Note to remember to drop the timeout post #32999.

🤔 ajtowns reviewed a pull request: "refactor: GenTxid type safety followups"
(https://github.com/bitcoin/bitcoin/pull/33005#pullrequestreview-3037443575)
Approach ACK

💬 ajtowns commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2218771665)
Maybe add a comment to call attention to the `const&`and the reason for it?

💬 ajtowns commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2218799503)
Passing in the `GenTxid` and keeping the `visit` inside the function (rather than an `if` outside it along with templatizing) seems nicer?

🚀 fanquake merged a pull request: "ci: Use APT_LLVM_V in msan task"
(https://github.com/bitcoin/bitcoin/pull/32999)

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2218854573)
Unrelatedly, the `USE_CIRRUS_RUNNERS` can probably be removed:

* It is only there for forks, but the comment says it doesn't work for forks anyway in the expected way.
* If someone maintains a fork (presumably to add patches), it should be trivial for them to make the required patch to the CI as well.

🚀 fanquake merged a pull request: "test: fix `ReadTopologicalSet` unsigned integer overflow"
(https://github.com/bitcoin/bitcoin/pull/33007)

💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3096237139)
Rebased after the merge of #32521 to pick up recent changes to `AreInputsStandard`.

A new verbose message will be returned when the BIP54 legacy sigops limit is violated:

* `bad-txns-non-witness-sigops-exceeds-bip54-limit`: This occurs when the total number of non-witness sigops across the entire transaction exceeds `MAX_TX_LEGACY_SIGOPS`. Additionally, a debug message is also returned with the actual number of sigops and the limit, for example: `sigops 2501 > limit 2500`.


[3cde7f1852
...

🤔 ismaelsadeeq reviewed a pull request: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827#pullrequestreview-3037622642)
reACK 249889bee6b88eb9814eb969e6fb108f86a4bf98

💬 ismaelsadeeq commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2218883957)
In "mempool: Avoid expensive loop in removeForBlock during IBD" 41ad2be4340d7b18a4669aa9becef8936597f000

nit: change commit title to "mempool: avoid looping `blk` `txs` when mempool is empty in `removeForBlock`"

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2218901137)
nit in https://github.com/bitcoin/bitcoin/commit/ecc6c07e583842a2afe1a8b5bb8ec0bb9f997fdb: Could add and use a small helper: `TxoutType GetTxoutType(const CScript& output_script)`? This makes the code shorter below and wraps this dummy.

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2218546827)
nit in the first commit: could be using `using` for new code?

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2218582070)


nit in the first commit: Should this be called `script_base_size` instead? There are other uses of prevector (`CompressedScript`, netaddr, ...), so defining this globally could be confusing.

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2218877265)
nit in ecc6c07e583842a2afe1a8b5bb8ec0bb9f997fdb: Use snake_case for new code?

💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2218957649)
In 2551fdfdd662dfb4077c3059b437acf56bdbaf5b "wallet/migration: use count private key count to identify watchonly descs"

This evaluation of `watchonly` at a barebones level looks great to me! Though maybe we can put this inside a `IsWatchOnly` function in the Descriptor{Impl} class? Reading all these details in the migration flow might be distracting, and moreover this generic function might have its own use cases separate from this later. Following patch builds and tests fine on my system. I
...