ion-tools icon indicating copy to clipboard operation
ion-tools copied to clipboard

Cannot generate requests for update/recover operations due to inconsistent storing of operations as promises/objects in this.#ops

Open nikolockenvitz opened this issue 2 years ago • 1 comments

The create operation is stored as a promise in this.#ops, whereas all other operations are stored as plain objects.

Steps to reproduce / Cause of the bug

I create the following operations: create, update, update

const did = new DID({ content: {} });
await did.generateOperation("update", {});
await did.generateOperation("update", {});

For the create in the constructor, this.generateOperation() calls this.#addToOpQueue() and this returns this.#opQueue, which is a promise. This promise is pushed to this.#ops (see below). In contrast, for the update operations, I call generateOperation() myself (with commit implicitly set to true), so that the operation op is pushed as a plain object (this.#ops.push(op)).

https://github.com/decentralized-identity/ion-tools/blob/02199dd1d1c86c3f61c83253530199eee811db88/src/did.js#L10-L15

As described above, the operations are stored inconsistently. In addition, this leads to the previous property also being inconsistent.

The previous property of these operations is set as follows:

  • 1st operation / create: unset as intended
  • 2nd operation / first update: previous is a promise to the first (create) operation / opQueue
  • 3rd operation / second update: previous is a pointer to the second operation / first update (plain object)

Consequently, I cannot generate a request for the second operation (first update) because op.previous is a promise (and op.previous.update therefore undefined):

await did.generateRequest(1);

// Fails:
// .../did.js:69
//          signer: options.signer || LocalSigner.create(op.previous.update.privateJwk),
//                                                                          ^
//
// TypeError: Cannot read properties of undefined (reading 'privateJwk')
//     at DID.generateRequest (.../did.js:69:75)

Btw, personally, I think the recent refactoring with the opQueue and the promises does not make it easier to understand the code. If it is required to reference the operations as promises, then maybe some documentation/comments would help. I guess that operations passed into the constructor will be plain objects and not promises.

nikolockenvitz avatar Feb 22 '23 11:02 nikolockenvitz

I've run into the same issue. Is there a reason why @nikolockenvitz's fix hasn't been merged?

macterra avatar Jan 02 '24 18:01 macterra