Consistent Python wrapper API

[skilo-sh]

Hi, I noticed that the wrapper API is not consistent at the moment, here is a table who recap current state :

In this table you can see the columns:

• Function name: the name of the function who run tests with the python wrapper's file as an input.
• Is defined in __dir__: if the function is defined in the __dir__ attribute of the parent Class.
• Take path in parameter: If the function take the path to the python wrapper as an argument or if the path is harcoded in the source code.
• Ignore compliance/resilience: If the function ignore compliance/resilience arguments.

I think that each columns should be like the Expected ? row. I also think it's a pity to have function prototypes (i.e. both function's name and type signature) who are not consistent between each primitive ; that's why I suggest the following prototype:

def run_python_wrapper(
    wrapper: Path, compliance: bool, resilience: bool,
    options: None | Dict
) -> ResultsDict:

Where options is a Dict containing options related to the primitive.

I didn't dare to open a PR because these changes are big and require discussion before.
If you agree with me and you need help, I can do it myself and open a PR.

Thank you for your reply!

skilo

[skilo-sh]

cc. @JulioLoayzaM

[JulioLoayzaM]

Hi @skilo-sh,

Thanks for opening a detailed issue. :)

To go straight to the point, the (undocumented) goal is to have a wrapper API similar to the harness API, with a common naming convention, and similar arguments. The ultimate goal is to simplify the CLI to a single test command that would take a file, determine whether it is a Python or shared library harness, and test it:

crypto-condor-cli test my_wrapper.py
crypto-condor-cli test my_harness.so

Some of the primitives already have this type of harness, see the wrapper API. This is reflected in the CLI: for given primitives, the test wrapper command no longer takes additional arguments that are used for selecting the testing parameters, everything goes through the function name.

Internally, then, the idea is indeed to have a common function prototype that all primitives must respect, similar to the existing one for the harnesses:

def test_lib(
    ffi: cffi.FFI, lib, functions: list[str], compliance: bool, resilience: bool
) -> ResultsDict:
    ...

# Maybe something like this?
def test_python_harness(
    harness: Path, functions: list[str], compliance: bool, resilience: bool
) -> ResultsDict:

This common prototype would allow us to have a function similar to harness.test_harness, which decides which primitives to test from the function names, and dynamically calls the corresponding modules. This relies on all supported modules having this test_lib function.

For additional context, this new API solves a couple of issues: with the original wrappers, you could not test more than one parameter set for a given primitive (e.g. only one mode of operation for AES), so different wrappers were required to fully test a primitive. Second issue, the parameters to run each one were passed through the CLI, making the command very verbose.

To respond to the rest of your points:

• I'm not sure whether or not to include this function in __dir__. Following the example of test_lib, it probably shouldn't, but in the case of test_lib it's mainly because users are expected to go through harness.test_harness instead of creating the cffi.FFI objects themselves. In any case, it's inclusion or exclusion from __dir__ should be consistent.
• As the suggested prototype shows, a Path argument should be included. Hard-coding the filename was definitely not the way to go.
• The compliance and resilience options should not be ignored. It's necessary for having a common interface, and ideally it would allow primitives to report that some type of test vectors are not available.

[skilo-sh]

Hi Julio, thanks for your reply :)

The new API is a very good idea, it would be super handy to automatically detect harness/wrapper !
So if I understand correctly: there's no need to open a PR to fix these problems because they'll be solved by the new API, correct?
If so, do you know approximately when this new API will be released?

PS : In the meantime, perhaps I should correct the "complience/resilience beeing ignored" column because I think it's an important issue; do you want me to do it in a PR?

--
skilo

[JulioLoayzaM]

Hi skilo,

So if I understand correctly: there's no need to open a PR to fix these problems because they'll be solved by the new API, correct?
If so, do you know approximately when this new API will be released?

Correct :) I think that harmonising the function names and whether or not they take a path parameter would be good for a uniform public API, but not quite urgent since it will be superseded by the new API. However I do not have an approximate date.

PS : In the meantime, perhaps I should correct the "complience/resilience beeing ignored" column because I think it's an important issue; do you want me to do it in a PR?

That would be appreciated! I agree that's a more important issue.