Updated is_mine documentation
Description
To clarify the purpose of a method and to indicate how to get more information from the wallet
Notes to the reviewers
We can also change the how is_mine works by calling derivation_of_spk directly, so its more clear how the internals work. This way rather than having the docs explicitly call out the method, they can jump to source.
Checklists
All Submissions:
- [ x] I've signed all my commits
- [ x] I followed the contribution guidelines
- [ x] I ran
cargo fmtandcargo clippybefore committing
Sure I support reimplementing in terms of derivation_of_spk. Also there might be a better name for it.
/// Finds how the wallet derived the script pubkey `spk`.
///
/// Will only return `Some(_)` if the wallet has given out the spk.
pub fn derivation_of_spk(&self, spk: &Script) -> Option<(KeychainKind, u32)> {
self.indexed_graph.index.index_of_spk(spk)
}
Also there might be a better name for it
@LLFourn rename this? Could be find_spk_derivation or find_spk?
Also with regards to;
Will only return
Some(_)if the wallet has given out the spk
I think this means to say that it will return Some(_) if the spk is findable/derivable, to me, "has given out" means its tracking which spks its been giving out which sounds inaccurate.
/// Finds how the wallet derived the script pubkey `spk`. /// /// Will only return `Some(_)` if the wallet has given out the spk. pub fn derivation_of_spk(&self, spk: &Script) -> Option<(KeychainKind, u32)> { self.indexed_graph.index.index_of_spk(spk) }Also there might be a better name for it
@LLFourn rename this? Could be
find_spk_derivationorfind_spk?
I don't like find because it implies it's not just doing a lookup. find in rust is used in iterators to go through each element and return the first match.
Also with regards to;
Will only return
Some(_)if the wallet has given out the spkI think this means to say that it will return
Some(_)if the spk is findable/derivable, to me, "has given out" means its tracking which spks its been giving out which sounds inaccurate.
I think it was accurate. That's one of the main functions of the KeychainTxOutIndex is to track which has given out. However I think this will now return results for things that have not yet been given out because of the "lookahead" feature so I do think the docs needs to be updated to say that if it matches a script pubkey that is has derived internally.
Since this is just a docs and internal implementation change that doesn't change the API I'd like to move it to a post 1.0 milestone.
I closed this but would like to revisit this post 1.0 release.