search

darosior / python-bip380

Bitcoin Output Script Descriptors with Miniscript extension
MIT License
20 stars 5 forks source link

Fix the mess with malleability-tracking properties #18

Closed darosior closed 2 years ago

darosior commented 2 years ago

We actually don't use them! But parts of the analysis logic (such as in c: or thresh()) still used them... Clean up this mess.

It was uncovered by #16

stickies-v commented 2 years ago

Confirmed that it fixes the tests in #16. I'm unsure why we don't use the malleability-tracking properties anymore though, they're still listed on the website and the C++ implementation, do you have any links to help me understand that?

darosior commented 2 years ago

See the commit message. The malleability analysis is tracked using members of Node objects: https://github.com/darosior/python-bip380/blob/8aa1e2ff1f2092e249c16a559b1cb4a2b6835f6c/bip380/miniscript/fragments.py#L70-L79

stickies-v commented 2 years ago

Right, that makes sense - sorry I missed the commit message. Does that mean we should now introduce a Node.check_valid() method to check for incompatibility between Property's attributes and Node's malleability properties?

darosior commented 2 years ago

It's not necessary, but i think that would make sense to check for inconsistencies at the node level yes. ------- Original Message ------- Le jeudi 5 mai 2022 à 11:46 AM, stickies-v @.***> a écrit :

Right, that makes sense - sorry I missed the commit message. Does that mean we should now introduce a Node.check_valid() method to check for incompatibility between Property's attributes and Node's malleability properties?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>