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

💬 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_r2218970582)
Sorry but I don't understand how the second statement follows the first one above.
For a non-watch-only wallet, isn't it guaranteed that there must be at least one private key, which should turn `has_priv_key` to `true`?

💬 zaidmstrr commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3096459508)
The method `rpcRunLater` was removed in [commit](https://github.com/bitcoin/bitcoin/commit/fcfd3db563e89fd79820a4cdfa102d624d801de1#diff-8e9fef4d0718d085f31da382899d97e3bbefc8d6a72ede95916a0e2fbd1a532f), thus there are some build errors.

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2219082048)
> 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.

This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)

> It might also be possible to use fast rescan when importing descriptors if none of the descriptors are silent payment descriptors.

It wasn't referring to that and I wouldn't worry about this. Somewhere in the RPC documentation (createwallet and/or importdescr
...

💬 danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2219096854)
I also don't love the `GetAddresses`/`GetAddressesUncached` naming either, but I agree with stickies that it's important that the unsafe version stands out.

What if we instead renamed them to `GetAddressesCached` / `GetAddressesUncached`? That way, there’s no ambiguity about what each function does, and the naming feels a bit more consistent.

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2219135907)
> This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)

I added a separate commit that disables fast rescan and I added an explanation to the commit message. Will push soon.

💬 m3dwards commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2219202190)
Done

💬 maflcko commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r2219238228)

implementat -> implement [spelling error in comment “…able to implementat a ReadField…”]

💬 darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3096821342)
The behaviour already exists in a limited fashion. Even without this change, an attacker can trivially create such a transaction (as discussed in OP). Here is for instance a `p2p_invalid_tx.py` case which passes both with or without this PR:

<details>

<summary> Click to see patch </summary>

```diff
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 36b274efc27..46bb2bba367 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functiona
...

💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2219264339)
Fair, it does feel cleaner using GenTxid and avoiding the branch in `ProcessGetData`. I chose to templatize in response to this [comment](https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2877628308) from the original PR. The templating takes care of everything at compile-time, which is nice. But `m_most_recent_block_txs` holds GenTxids, so in the templated version we're creating a temporary GenTxid anyway for that map lookup vs creating a GenTxid once upfront.

I think it's larg
...

💬 danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#issuecomment-3096922403)
In my last push (df65b527f00cbf9fd6675bf22bb1285bfce24f66):
- split changes into two commits (one for rename, one for docs)

💬 totdking commented on issue "A missing import to the src/chainparamsbase.h":
(https://github.com/bitcoin/bitcoin/issues/33019#issuecomment-3096966958)
I added the import and it worked :) , thanks man 👍

💬 maflcko commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3096984862)
No, you haven't replied/addressed the feedback from June: https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2150261914

✅ pinheadmz closed an issue: "b-msghand invoked oom-killer in Debug build: Master (v28.99) crashing during IBD"
(https://github.com/bitcoin/bitcoin/issues/31561)

💬 pinheadmz commented on issue "b-msghand invoked oom-killer in Debug build: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-3096984988)
Closing this now, `Bitcoin Core daemon version v29.99.0-c43cc48aaaaa (debug build)` synced succesfully on a debian VM 8GB memory and no swap

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2219412158)
Rebased and fixed

💬 darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3097046877)
Rebased to fix a silent merge conflict that was making the "test each commit" CI job fail.

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#issuecomment-3097049530)
Rebased to switch from the old (removed) `CBlock::hash` property to the recently introduced `hash_int` and `hash_hex`

💬 glozow commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3097123255)
ACK 249889bee6b88eb9814eb969e6fb108f86a4bf98

💬 glozow commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3097142325)
@darosior could you make the release notes updates as well please? Thanks