Code refactoring in Desmo module

[iosonopersia]

• Do we really need two separate functions to execute a query?

Why cannot we merge together buyQuery and getQueryResult into an atomic function called executeQuery?

In this case, we wouldn't need to store the dealid into the this.dealid member...
Here there's an example of why the current code can be a source of problems:

const desmo = new Desmo(...);

await desmo.buyQuery(...); // this.dealid <-- DEAL_A
await desmo.buyQuery(...); // this.dealid <-- DEAL_B

// The following function call sees this.dealid == DEAL_B,
// hence it returns RESULT_B instead of RESULT_A:
const resultA = await desmo.getQueryResult(...);

• Also, this is what we return when executing a query (see Code refactoring in Desmo module #12):

{
  requestID: string;
  taskID: string;
  result: number | string;
}

Why cannot we simply return the result? Are the other values really useful for users of the SDK?

• What's the purpose of the verifyCallbackAddress method? Do we really need it in the Desmo module?

[relu91]

Do we really need two separate functions to execute a query?

I have to investigate the role of dealId before answering. I am puzzled as I was here. Having dealID saved as an instance variable seems like a bad design to me too. But I have to dig further. @iotondato?

Also, this is what we return when executing a query

I think we returned those IDs for easy accessing and referring to the transaction ID from the frontend.

What's the purpose of the verifyCallbackAddress method? Do we really need it in the Desmo module?

It should be a sanity check that the DApp is not compromised but I need another pass of iExec documentation reading to be sure if it is really needed.

[iosonopersia]

I have to investigate the role of dealId before answering. I am puzzled as I was #1 (comment). Having dealID saved as an instance variable seems like a bad design to me too. But I have to dig further. @iotondato?

I may have found a partial answer to why we select the first appOrder and the first workerpoolOrder. I strongly believe that relevant code was taken by @iotondato from this demo during the initial stages of the SDK development. Have a look at file src/index.js , lines 154 to 165...

Unluckily, this doesn't explain why it's done in the first place, maybe is just a simplification which is OK for a demo but not for production (or maybe is OK for production also, I have no clue about this).

I think we returned those IDs for easy accessing and referring to the transaction ID from the frontend.

Then we'll leave it as it is.