check parameters

[AxelNennker]

libindy is checking parameters when called through the API.

Additionally there are check at other places like e.g. https://github.com/hyperledger/indy-sdk/blob/master/libindy/src/commands/non_secrets.rs#L241 The 'from_json' call just checks whether the options string is deserializable but does not use the resulting object. Using the RecordOptions object does not really make sense because in the end the options end up in the wallet_storage API which requires a string anyway.

'fail-fast' is good but this looks like wasted CPU cycles. Not sure what to do about this.

One line above the RecordOptions wasted json deserialization is a call to _check_type which makes it clearer what the purpose is: check_type is checking the type and RecordOptions::from_json is checking the options string.

Should those checks be configured away when a release version of libindy is build? Should those checks be left to the code that is actually consuming the parameters, so fail-late?

[vimmerru]

• API layer performs enough checking to avoid breaking of process memory
• Command layer checks structure
• Service layer checks business conditions

I don't see any problems here. We can't avoid wasted CPU cycles without breaking encapsulation.

[AxelNennker]

Conserving encapsulation is not the problem.

Removing the RecordOption check at command level and trusting wallet_storage to detect invalid options would be the better encapsulation.

non_secret does not need to know about record options. That knowledge should be left to wallet service. I would argue considering encapsulation that RecordOptions should even not be visible outside of wallet storage (except the constants id, id_value, full).

On the level of non_secrets all that, maybe, should be checked is whether serde_json:from_str is able to produce a Value. But I even would not do that. Why not leave these checks to wallet service and remove this check altogether?

RecordOptions could be encapsulated in wallet service completely. get_indy_object is always called with a parameter of id_value() get_indy_record is sometime called with id() and sometime with full() When a DID is retrieved then only one call is with id() and all others is with full() as the record option. The one with id() is probably just a check that the did exists. So these getindy calls do not really need the record option.