bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
796 stars 288 forks source link

Simplify `public_descriptor` by using `get_descriptor` and remove redundant API function. #1501

Open KnowWhoami opened 3 days ago

KnowWhoami commented 3 days ago

Describe the enhancement

https://github.com/bitcoindevkit/bdk/blob/a112b4d97c02c3acb9728c55c6c150560b5cc0b7/crates/wallet/src/wallet/mod.rs#L1770

https://github.com/bitcoindevkit/bdk/blob/a112b4d97c02c3acb9728c55c6c150560b5cc0b7/crates/chain/src/keychain/txout_index.rs#L407

Currently, public_descriptor finds the descriptor from scratch. Instead, it can directly call get_descriptor to simplify its implementation.

       pub fn public_descriptor(..) -> &ExtendedDescriptor {
       match self.indexed_graph.index.get_descriptor {
          // implementation
        }
       }

https://github.com/bitcoindevkit/bdk/blob/a112b4d97c02c3acb9728c55c6c150560b5cc0b7/crates/wallet/src/wallet/mod.rs#L1882.

Both functions logically serve the same purpose:

public_descriptor: Returns the public version of the descriptor. get_descriptor_for_keychain: Returns the descriptor used to create addresses for a specific keychain

Therefore, we can eliminate get_descriptor_for_keychain and include its documentation in public_descriptor. This change will reduce redundancy in Wallet API's.

Use case

f3r10 commented 3 days ago

Hello, could I take this issue?

notmandatory commented 2 days ago

@f3r10 we already have a PR from @gnapoli23, feel free to add a PR review for #1503.