Skip to content

Introduce Extension trait#13

Merged
apoelstra merged 20 commits into
ElementsProject:masterfrom
sanket1729:ext
Oct 27, 2021
Merged

Introduce Extension trait#13
apoelstra merged 20 commits into
ElementsProject:masterfrom
sanket1729:ext

Conversation

@sanket1729
Copy link
Copy Markdown
Contributor

@sanket1729 sanket1729 commented Sep 23, 2021

Complete refactor of the library to support extensions. This introduces an Extension trait that be implemented by downstream libraries to add new leaf fragments to miniscripts. The trait asks user to provide type properties about the fragment, satisfaction logic, script costs, parsing(to and fro from script/string) logic, consensus checks, as well the interpreter API.

Not all script fragments can be easily converted into miniscript leaf fragments. In other words, the implementer of this trait should take care that fragments can be parsed unambiguously without conflicting with existing fragments/extensions.

All the library APIs(descriptors, interpreter) should work seamlessly once Extension trait is implemented. This PR also refactors the currently supported ver_eq and outputs_pref as extensions.

It should also be possible to extend the trait to support the compiler feature, but that is left as a todo for now.

Fixes #11 .

@apoelstra
Copy link
Copy Markdown
Member

Does this have any upstream dependencies, or is it ready to review (and theoretically merge) now?

@sanket1729
Copy link
Copy Markdown
Contributor Author

This is ready for merge after review

Comment thread src/policy/mod.rs
Terminal::True => Semantic::Trivial,
Terminal::False => Semantic::Unsatisfiable,
Terminal::Version(_) | Terminal::OutputsPref(_) => return Err(CovError::CovenantLift)?,
// Terminal::Version(_) | Terminal::OutputsPref(_) => return Err(CovError::CovenantLift)?,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

my onliy nit is tha tthis should be deleted rather than commented out

Copy link
Copy Markdown
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 1454e27

@apoelstra
Copy link
Copy Markdown
Member

To check my understanding:

  • This adds a new trait Extension on which basically all the features of this library are implemented as methods. Miniscript is then parameterized over a new type Ext: Extension and has a new variant Ext(Ext). All the methods of Miniscript, upon encountering an Ext, fall through to the Extension methods on this object.
  • There is a NoExt extension type which implements nothing. All the "normal" descriptor types fix their containing Miniscript to use this.
  • But the Cov descriptor type uses the AllExt extension type instead, which is defined to provide extensions for covenants.

I have a few followups that I'd like to add but I will PR them myself.

@apoelstra
Copy link
Copy Markdown
Member

@sanket1729 I recall asking if we could upstream this, and you said roughly "no, I think it would be too invasive". But I'm not sure it would be.

@apoelstra apoelstra merged commit 144b68e into ElementsProject:master Oct 27, 2021
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.

Miniscript fragment to check conditions on signed message

2 participants