The Wallet.is_mine() function should return K

[notmandatory]

Describe the enhancement

The Wallet.is_mine(&self, script: &[Script]) function currently returns a bool representing "whether or not a script is part of this wallet (either internal or external)". It would be more useful to know which KeychainKind the script was found for, or None if not one of my scripts:

pub fn is_mine(&self, script: &[Script]) -> Option<KeychainKind>

Use case

A request came up on discord about how to figure out which output is the change. Having is_mine() tell you which chain a script is for would make this easy to figure out.

[LLFourn]

Note is_mine just takes a &Script. You probably want the keychain and dervation index. We have a method for that called derivation_of_spk so this can be fixed by just updating the docs to point to that function if you want it.

[notmandatory]

@nondiremanuel we didn't discussed this one yesterday but I think it belongs in alpha.3.

[nondiremanuel]

@notmandatory great, thanks for letting me know!

[realeinherjar]

As discussed in W39 Lib Team Call, I think that the methods prefixed with is_* should always return a bool for consistency sake.
We should create a new Wallet method that returns Option<KeychainKind>.

[notmandatory]

I agree we should add a new function. What should we call it? does something like this make sense? in_keychain(&self, &Script) -> Option<KeychainKind>

[LLFourn]

which_keychain_derived(&self, &Script)

[vladimirfomene]

Let me take this up, @notmandatory.

[LLFourn]

Is there actually anything to do here. What's wrong with derivation_of_spk?

[evanlinjin]

Idiomatically, I think is_mine should return a boolean. I think we can update the docs (as @LLFourn suggested) to use the deriation_of_spk method of Wallet.

[notmandatory]

A likely small change but I think we should push to 2.0 milestone.

[notmandatory]

Since we have deriation_of_spk closing the issue. Can re-open if someone finds a reason not to use the existing function.