💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062770)
I've updated it to this, this is much more clear.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062811)
I've added a check for this.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062833)
Done

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062927)
I agree, the naming here was a bit confusing. It should be fixed now.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062954)
Done both here and one other place.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062971)
Fixed

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1500792114)
Thanks for the review @glozow, I have addressed your comments and also made a modification to `RecursiveUpdateTxState` which will allow it to be more extensible.

⚠️ geenutts opened an issue: "Gee"
(https://github.com/bitcoin/bitcoin/issues/27439)
https://github.com/geenutts/consensus-specs/commit/e1ae7ec2ad141cf18940566d5070c1f146a7c2a9

✅ fanquake closed an issue: "Gee"
(https://github.com/bitcoin/bitcoin/issues/27439)

:lock: fanquake locked an issue: "Gee"
(https://github.com/bitcoin/bitcoin/issues/27439)

👋 pinheadmz's pull request is ready for review: "validation: implement MaybeInvalidateFork() and call from rpc getchaintips"
(https://github.com/bitcoin/bitcoin/pull/27434)

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161121018)
The style here matches the neighboring descriptors but I see your point about using `.at(0)` -- do you think all the `descriptor::MakeScripts()` should be cleaned up like this?

Also this chunk is the patch alluded to in the comment
> This PR also patches PKDescriptor from https://github.com/bitcoin/bitcoin/pull/22051 where matching public keys were not returned.

When we expand a bare pubkey descriptor we weren't populating `out.pubkeys` and so `GetAffectedKeys()` wouldn't return anything

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130735)
thanks will add this -- also I think these could be `[[nodiscard]]` as well? But I don't see that for any of the other `bool Is...()` members in SPKM...

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130743)
Thanks, updating.

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130747)
ok, moving `isactive` next to `ismine`. You have a good point about legacy wallet deprecation. I suppose when that happens most of this new code will be deprecated as well since its mainly implemented only in `LegacyScriptPubKeyMan`. Would it make sense to only add the field to RPC response in legacy wallets?

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130750)
thanks, I do prefer!

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130751)
thanks got it

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130754)
ah really cool with `any_of` thanks

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130756)
okay yes

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1161130759)
ok?

> "If the address is reserved for use as a change output. It may or may not have already been used as change."