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

Tr compiler #460

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Tr compiler #460

wants to merge 4 commits into from

Conversation

kanishk779
Copy link
Contributor

Adds the logic for converting fragments to Musig and also updates the internal key extraction logic to take benefit of Musig.

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

Did not review the compiler yet.

@@ -12,7 +12,7 @@ edition = "2018"

[features]
default = ["std"]
std = ["bitcoin/std", "bitcoin/secp-recovery"]
std = ["bitcoin/std", "bitcoin/secp-recovery", "secp256k1-zkp"]
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this. But I don't think we should be using secp recovery.

Terminal::MultiA(_, ref keys) => {
if keys
.iter()
.all(|keyexpr| keyexpr.iter().any(|pk| pk.is_uncompressed()))
Copy link
Member

Choose a reason for hiding this comment

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

This should be any instead of all right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should be any. I have fixed it now.

@@ -125,8 +126,16 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
/// `miniscript.iter_pubkeys().collect()`.
pub fn get_leapk(&self) -> Vec<Pk> {
match self.node {
Terminal::PkK(ref key) | Terminal::PkH(ref key) => vec![key.clone()],
Terminal::Multi(_, ref keys) | Terminal::MultiA(_, ref keys) => keys.clone(),
Terminal::PkK(ref key) => key.iter().map(|pk| pk.clone()).collect(),
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

You can cherry pick the commits and add them here

KeyExpr::<Pk>::SingleKey(ref pk) => pk.to_public_key().to_bytes(),
KeyExpr::<Pk>::MuSig(_) => {
let agg_key = self.key_agg();
agg_key.to_public_key().to_bytes()
Copy link
Member

Choose a reason for hiding this comment

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

You should not need the .to_public_key here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I have removed to_public_key() and used the serialize() function.

)),
None => Ok(Policy::KeyHash(self.internal_key.to_pubkeyhash())),
// None => Ok(Policy::KeyHash(self.internal_key.to_pubkeyhash())),
Copy link
Member

Choose a reason for hiding this comment

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

Remove, commented line here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.


// address for the descriptor (bc1...)
let address = real_desc.address(Network::Bitcoin).unwrap();
println!("The address is {}", address);
Copy link
Member

Choose a reason for hiding this comment

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

Also print what the internal key is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
ind += 1;
Copy link
Member

Choose a reason for hiding this comment

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

Not thrilled with this part. Ideally, we should be able to get the complete Tree and parse the script spend and key spend nicely. This hack worked nicely for key spend when there was a single key and now it's an even ugly hack. I will fixup the tree decoding later

@michaelfolkson
Copy link

What's the latest status of this? Gonna be in 9.0? :)

@sanket1729
Copy link
Member

This is me wanting to figure out how to expose this as an experimental feature.

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.

3 participants