halfagg: Add Python specification and extensive test vectors #21
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of an effort to get the halfagg BIP ready for a pull to the bips repo.
This PR intends to resolve the following pending TODOs:
Instead this PR adds a Python specification. The problem I have run into is that since the specification code was written hacspec (the implementation language) has stopped development and was superseeded by hax which also implements hacspec (the specification language) follows a different philosophy than hacspec (implementation language). Hax is under active development and could still have bigger changes and documentation is mostly a work in progress. As a first step I ported the BIP340 example that was a dependency of the halfagg hacspec code to hax, that PR has not received any significant review in the last 6 months despite me reaching out to contributors. It simply isn't a big priority on their side. I even have a hax halfagg code that ran with an earlier version but it's really just working rust and I don't know if it's acceptable hax.
It seems to me that using Python is probably the best option, given that it has been using in many comparable BIPs and the advent of the secp256k1lab library has made this even more practical than it used to be.
The alternative would be to keep using the rust code that is there now, maybe updating it with a few tweaks. But I think trying to make it compatible with hax would take too long due to the lack of feedback and we would also run the risk of depending on an outdated hax version quite quickly. It would be practical to just have a specification in rust instead of Python but I would prefer to go with Python.
The Python code still has some very verbose comments that link the Python code closely to the pseudo code specification which is not the usual style for these kinds of specifications. I tend to think it should be removed but wanted to hear feedback is someone thinks differently. It was helpful for me to relate the BIP draft to the code but it's probably not something everyone is interested in.
This add a generator script which includes the tests that were part of the rust test code and adds extensive additional test vectors which are output as CSV files. There is also a runner script. This pattern follows that of several other comparable BIPs. I ran
coverageon the vectors runner and it shows that every line of the Python specification is covered except for the raise inAggregatewhich is unreachable if theVerifyAggregatecode is implemented correctly.Note that the
max_countquestion is not addressed here and there are still tests added for the current value2^16 - 1. That topic is probably best discussed in the existing PRs on this question.