Skip to content
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

Pegin desc wildcard #96

Merged
merged 5 commits into from
Dec 7, 2024
Merged

Pegin desc wildcard #96

merged 5 commits into from
Dec 7, 2024

Conversation

RCasatta
Copy link
Collaborator

@RCasatta RCasatta commented Nov 26, 2024

On top of #95

draft PR to investigate the derivability of a bunch of addresses from a pegin descriptor.

Use the following assumption

  • federation descriptor is using only bitcoin::PublicKey

@apoelstra
Copy link
Member

Merged #95 -- can rebase and undraft this.

Since the `Pk` generic on `fed_desc` is the same as in `elem_desc` this
binds the kind of user descriptor accepted.
`fed_desc` always come in the form with `PublicKey`, thus by removing
the generic we can support different `elem_desc`.
Allowing to generate a bunch of pegin addresses from a single pegin
descriptor
@RCasatta RCasatta force-pushed the pegin_desc_wildcard branch from bd220a0 to 59362c8 Compare December 2, 2024 09:28
@@ -148,6 +160,7 @@ impl<Pk: MiniscriptKey> Pegin<Pk> {
{
// TODO
Ok(bitcoin::Address::p2shwsh(
// Should the address type taken from the top level user desc?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be the right thing to do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should come from the fed_desc field. Probably easiest to match on self.fed_desc.desc_type() (lol we should rename this) and choose the address type based on that. https://docs.rs/miniscript/latest/miniscript/descriptor/enum.Descriptor.html#method.desc_type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than this, 59362c8 looks good.

@RCasatta RCasatta marked this pull request as ready for review December 2, 2024 09:31
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 624e3f4; successfully ran local tests

@apoelstra apoelstra merged commit ffc3ea0 into master Dec 7, 2024
17 checks passed
@apoelstra apoelstra deleted the pegin_desc_wildcard branch December 7, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants