💬 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

🚀 glozow merged a pull request: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827)