search

ElementsProject / elements-miniscript

Creative Commons Zero v1.0 Universal
11 stars 14 forks source link

revamp `Extension` and `ExtParam` traits #30

Closed apoelstra closed 2 years ago

apoelstra commented 2 years ago

The existing code made Descriptor generic over ExtParam rather than Extension. This made the Extension trati basically useless since every Descriptor always had CovenantExt as its extension type.

It also caused me a lot of confusion because I tried to call the address method on a Descriptor<Pk, NoExt>, which I was able to parse, just not call any methods on. I pulled on this string and 1000 lines later here we are.

apoelstra commented 2 years ago

cc @sanket1729

sanket1729 commented 2 years ago

@apoelstra , the goal of Extension was so that downstream projects can implement their own extensions. ExtParam is a parameter for the data type used in Extension. For example, if CheckSigFromStack were an extension, then

1) The struct CheckSigFromStack would implement Extension 2) CsfsMsg and CsfsKey would implement the ExtParam .

It probably might be best to abandon the goal for downstream projects to implement custom extensions. Will offer detailed comments after reviewing this PR

apoelstra commented 2 years ago

@sanket1729 we can abandon that goal, but then we should drop the Extension trait, the NoExt object, and probably a bunch of other stuff. I can alternately open a PR do that if you want.

But what then will we do for Simplicity? Fork the library a second time?

sanket1729 commented 2 years ago

@apoelstra, l will actually try to write a simple downstream library. Or perhaps an example in this repo itself that implements the extension trait. Will identify a lot of pain points. So far, the PR is looking great. Will comment more after review of this PR in about 30 mins

sanket1729 commented 2 years ago

@apoelstra I was able to create APO + SighashAll extension by implementing Extension in a downstream repo. I think this should do for simplicity translation.