-
Notifications
You must be signed in to change notification settings - Fork 389
Add function to check internal descriptor #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I am still unsure why would a lib user won't know if his wallet has a change_desc or not, but I think its no harm to explicitly have an API to determine that.
ACK 64acb93
@w0xlt we discussed about this in the meeting today. Can you give us an idea what you would use this for? |
@@ -1059,6 +1059,11 @@ where | |||
&self.secp | |||
} | |||
|
|||
/// Checks if the wallet has internal descriptor | |||
pub fn has_internal_descriptor(&self) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think has_descriptor_for_keychain(&self, keychain: KeychainKind)
would be better. I have in my mind to add other KeychainKind
s later on and this would be helpful to that end.
Sure. This PR is related to how descriptors are implemented in the BDK. This behavior is described in #522 (review) and #522 (comment). There is currently no way to retrieve internal (change) addresses. With #522, this will be possible, but if there's no internal descriptor, This PR adds the ability to know if there is an internal descriptor and how to better process the results of The use case is: building an Electrum wallet using BDK. As the image below shows, it is necessary to distinguish between internal and external addresses. |
What if we add the |
I agree. I think there is no need for a new function ( This way, it maintains backwards compatibility and still adds this new functionality. I also don't see a problem if the BDK throws an exception if the user requires a |
It turns out we already have a method that gives you this anyway: |
As discussed in #522 (comment) and suggested in #525 (comment), there is no way of knowing whether the descriptor returned in
get_descriptor_for_keychain
is external or internal.According to the first comment, this is a deliberate decision. So this PR adds a new function to check if internal descriptor exists.
This information can be important when showing a list of addresses and indicating which descriptor the address comes from, for example.