Make comment property optional in (issue-credential) messages

[TimoGlastra]

ACA-Py requires the comment property for at least the propose-credential and offer-credential messages (maybe also other messages / protocols?). However this is not required per the RFC.

When interacting with the .NET framework this is now handled using an empty string, but it would be nice to be able to leave the comment out.

2020-08-03 12:10:19,529 aries_cloudagent.messaging.models.base ERROR CredentialProposal message validation error:
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/aries_cloudagent/messaging/models/base.py", line 127, in deserialize
    return schema.loads(obj) if isinstance(obj, str) else schema.load(obj)
  File "/usr/local/lib/python3.7/site-packages/marshmallow/schema.py", line 723, in load
    data, many=many, partial=partial, unknown=unknown, postprocess=True
  File "/usr/local/lib/python3.7/site-packages/marshmallow/schema.py", line 904, in _do_load
    raise exc
marshmallow.exceptions.ValidationError: {'comment': ['Field may not be null.']}
2020-08-03 12:10:19,530 aries_cloudagent.core.dispatcher ERROR Message parsing failed: Error deserializing message: CredentialProposal schema validation failed, sending problem report
2020-08-03 12:10:19,581 aries_cloudagent.transport.outbound.manager ERROR >>> Posting error: http://192.168.65.3:9030; Re-queue failed message ...

[sklump]

It isn't mandatory: leave it out and it will be fine. What it can't be is present with a null value. The standard says it's a string if present, if I remember correctly. I will take a look and adjust it Tuesday if necessary.

[TimoGlastra]

ah that makes sense.

I'll also open an issue on the .NET side to not include the property if the value is null.

Thanks

[sklump]

https://github.com/hyperledger/aries-cloudagent-python/pull/649 allows for null values in protocol message comments.