Core shouldn't allow multiple output for the same execution

[antho1404]

Based on this discussion https://github.com/mesg-foundation/core/issues/562#issuecomment-443436893, the core doesn't return any error when a task is submitting multiple results to the core.

Each task can return only one result per execution. All the following results should be ignored and error should be triggered.

Services should follow a good practice and not return errors from the the submitResult api. These errors should be treated differently.

[ilgooz]

And if service submits error output again with an invalid format this will cause to an endless loop between Core and service. Services may end up by making infinite requests to Core like a Dos attack.

[krhubert]

So we have this check right now. What we don't have is update execution result in case of invalid data.

• Should we have one?
• How to approach the situation when output is good but there is core error before exec.Complete?

In that case we should call exec.Complete just with the data and without data validation (of course data should be validated and error should be a return to service, but data should be saved in execution)

Services may end up by making infinite requests to Core like a Dos attack.

It might do it any time right now. You can create endless loop which submit to the core.

[antho1404]

Good catch, we are actually already raising an error here. But then it is the service responsibility to stop the communication with the core. But there is still something wrong with the current implementation because this example we still have the answer from the second output of the service.

That is probably the part missing here, the execution should have an error status because we probably don't put it as completed because there is an issue and so the next output can fill the data.

We need to handle the error on the execution and then listen this execution error. This might even solve this PR https://github.com/mesg-foundation/core/pull/596 and issue https://github.com/mesg-foundation/core/issues/600

[antho1404]

The problem is that the parsing is done on the grpc part (and then have no control on the status of the execution)

here https://github.com/mesg-foundation/core/blob/aa7f7b7a861e7fb629b76fb5e529214e4ef949b9/interface/grpc/service/submit_result.go#L13-L16

instead of here https://github.com/mesg-foundation/core/blob/aa7f7b7a861e7fb629b76fb5e529214e4ef949b9/api/submit_result.go#L27

One solution would be to move the parsing int the api package like that we have access to the execution and to add an error status to the Execution and instead of returning an error we set the error in the execution and still publish the execution.

[ilgooz]

We already have new Error field in this PR https://github.com/mesg-foundation/core/pull/596 as you pointed.

If we move the parsing part to api pkg, it means we need to pass output data as a string to api.SubmitResult() method. This is not quite nice we should be supporting actual Go types so we can use api pkg better in other places. Otherwise api pkg would become to a some kind of special pkg that can be used nicely only by grpc pkg.

Actually, currently we only support map[string]interface{} as output data input type in api.SubmitResult(), I prefer to change it to just interface{} this way we can also pass structs. But this can be discussed/implemented within another issue.

[ilgooz]

This issue looks like already solved by the #596.

Tested with:

asc:core ilgooz$ ./dev-cli service execute -d url=https://mesg.com -d data={} -d headers={} 83aeca808856c3bef0c74e68596b39a0bb1d0a35
✔ Task "call" executed
⨯ Task call cannot be completed, error:
json: cannot unmarshal string into Go value of type map[string]interface {}

Before it was returning with error output key and now it's an internal execution error because we complete the execution with the error.