Hinted contract

[bpbirch]

SimbaHintedContract in simba_hinted_contract.py basically adds a type hinted wrapper to calls in simba_contract.py. Hints are provided in native python data types - when reviewing, please let me know if we would like to provide custom type hinting for our structs, etc. In current form, the type hint for a struct becomes 'dict'. Type hints for uint become 'int'.

The SimbaHintedContract object outputs a new .py file that is type hinted, and represents our contract as a class. Examples of these outputs are provided in tests/ app_md_contract.py, app_md_2_contract.py, app_md_3_contract.py. The metadata that are being used to produce these .py files are provided in tests/data.

The jinja template that aids in producing our contract.py files is contained in tests/templates.

Examples of how to instantiate and use the SimbaHintedContract class object are provided in tests/test_hinted_contract.py

Regarding method interface produced in our eg app_md_contract.py files, if a method from our metadata accepts files, then the signature for that method will allow for an async_method parameter, which will invoke SimbaContract.submit_contract_method_with_files_async. If async_method == False (default), then SimbaContract.submit_contract_method_with_files is invoked.

If a method does not accept files, then the signature for the method in our output .py file will include a query_method parameter. if query_method == True, then invocations of that method will be queried using SimbaContract.query_method, rather than the method itself being invoked. If query_method == False (default), then the method is invoked suing SimbaContract.submit_method. docstrings are auto generated for each method that explain the above.

When reviewing, please provide guidance as to whether getters and setters should be included for structs, etc.

@abrinckm, @doctor-zob - mentioning you here because I can only add one official reviewer.

[whelks-chance]

Are these pprints used? Best to use something like

log = logging.getLogger(__name__)
log.debug('Some text')

[bpbirch]

No, sorry about that - I just use pprint when debugging because it makes it easier to view json objects. Will remove.

[whelks-chance]

Regarding method interface produced in our eg app_md_contract.py files, if a method from our metadata accepts files, then the signature for that method will allow for an async_method parameter, which will invoke SimbaContract.submit_contract_method_with_files_async. If async_method == False (default), then SimbaContract.submit_contract_method_with_files is invoked.

I'm a bit confused here, whether the async call is used doesn't depend on if the method accepts files - I it should be acceptable to call any method via async, and to submit files using the regular method. Though I'm happy to be persuaded otherwise.

[whelks-chance]

Method names should be snake_case

[whelks-chance]

This is complicated, as the structure is actually uint256[4][] rather than int[][]

Is there a way to do a type check when the method is called, to ensure the dimensions of the incoming lists are correct?

[whelks-chance]

Also as it's a unit this is an "unsigned integer" so would always be x >= 0, if we decide to type check inputs, rather than just hinting at them.

[bpbirch]

I can definitely add logic for value checking on the uint. I can also add logic to check the len of passed list to make sure it's correct. Shouldn't be too difficult.

[whelks-chance]

The main tricky bit there is that solidity lists the dimensions of the array backwards to how you would expect.

[bpbirch]

Right, so uint256[4][] == [[a1,b1,c1,d1], [a2,b2,c2,d2],...,[an,bn,cn,dn]].

[whelks-chance]

I've added some comments inline.

There's further discussion to be had about whether the goal of this code is to "hint" at types, or enforce them. Do we want to reject bad params, or pass in everything and allow the SEP service to check server-side during the txn creation?

Connected to this is the dimensions of lists. Should we allow any nested lists, or try to enforce the dimensionality via these templated classes?

We probably should have something to type hint structs, which this code currently treats as a dict. Can we create Classes for each type in the metadata? (Defined in contract->types with each param defined as a "component")

[abrinckm]

it should be acceptable to call any method via async,

@whelks-chance I think the confusion here is that the original contract class only provides async method calls with files.

[whelks-chance]

it should be acceptable to call any method via async,

@whelks-chance I think the confusion here is that the original contract class only provides async method calls with files.

I didn't realise this, was there reasoning for it?

[bpbirch]

it should be acceptable to call any method via async,

@whelks-chance I think the confusion here is that the original contract class only provides async method calls with files.

I didn't realise this, was there reasoning for it?

@whelks-chance , @abrinckm , yes, this was my reasoning behind only including async functionality in methods that accept files. I wasn't sure why it was written that way, but figured there was a reason for it.

[bpbirch]

I've added some comments inline.

There's further discussion to be had about whether the goal of this code is to "hint" at types, or enforce them. Do we want to reject bad params, or pass in everything and allow the SEP service to check server-side during the txn creation?

Connected to this is the dimensions of lists. Should we allow any nested lists, or try to enforce the dimensionality via these templated classes?

We probably should have something to type hint structs, which this code currently treats as a dict. Can we create Classes for each type in the metadata? (Defined in contract->types with each param defined as a "component")

@whelks-chance -- the class-based approach for creating custom types for structs was actually what I was considering, if we wanted custom type hints. I will go ahead read in each type as a new class, with defaulted values for components / attributes.

[whelks-chance]

I've added some comments inline.
There's further discussion to be had about whether the goal of this code is to "hint" at types, or enforce them. Do we want to reject bad params, or pass in everything and allow the SEP service to check server-side during the txn creation?
Connected to this is the dimensions of lists. Should we allow any nested lists, or try to enforce the dimensionality via these templated classes?
We probably should have something to type hint structs, which this code currently treats as a dict. Can we create Classes for each type in the metadata? (Defined in contract->types with each param defined as a "component")

@whelks-chance -- the class-based approach for creating custom types for structs was actually what I was considering, if we wanted custom type hints. I will go ahead read in each type as a new class, with defaulted values for components / attributes.

Great, I think this is what we had in mind originally. Each type of struct becomes a Class with the types already known. Then the SDK user can instantiate these classes and add submit using that. You may need some code to convert the class object to a dict for sending, I guess. Something extra for your examples/tests

[whelks-chance]

Speaking of, we should separate the examples and tests into distinct dirs, and have the examples show how to do things, and the tests actually testing things with assert etc

[whelks-chance]

@abrinckm as it stands, are you happy to merge what we have here, and the extras discussed above can be a new feature/issue?

[bpbirch]

Speaking of, we should separate the examples and tests into distinct dirs, and have the examples show how to do things, and the tests actually testing things with assert etc

@whelks-chance - will definitely do this soon. Just wanted to make sure the templated code I was producing wasn't terribly off the mark.

[bpbirch]

I've added some comments inline.
There's further discussion to be had about whether the goal of this code is to "hint" at types, or enforce them. Do we want to reject bad params, or pass in everything and allow the SEP service to check server-side during the txn creation?
Connected to this is the dimensions of lists. Should we allow any nested lists, or try to enforce the dimensionality via these templated classes?
We probably should have something to type hint structs, which this code currently treats as a dict. Can we create Classes for each type in the metadata? (Defined in contract->types with each param defined as a "component")

@whelks-chance -- the class-based approach for creating custom types for structs was actually what I was considering, if we wanted custom type hints. I will go ahead read in each type as a new class, with defaulted values for components / attributes.

Great, I think this is what we had in mind originally. Each type of struct becomes a Class with the types already known. Then the SDK user can instantiate these classes and add submit using that. You may need some code to convert the class object to a dict for sending, I guess. Something extra for your examples/tests

@whelks-chance Yeah, the reason I took the dict approach is that the API expects a dict object, so what I'll do is write templated logic such that if a method parameter is a struct, then the method will convert to a dict of component : value pairs, then include in the inputs.

[abrinckm]

are you happy to merge what we have here, and the extras discussed above can be a new feature/issue?
Yeah, I think that's a good idea.

[whelks-chance]

are you happy to merge what we have here, and the extras discussed above can be a new feature/issue?
Yeah, I think that's a good idea.

ok @bpbirch once you push the pprint and snake_case clean-ups, I'll merge this, and we can start with a fresh issue.

[bpbirch]

@abrinckm @whelks-chance - I think everything is good to go here.