Long running operation set to done before dependencies are done

[lenkan]

Note sure if you would consider this a bug, but I thought I would flag it because the behavior is not expected to me.

Actual Behaviour

When an operation has dependencies, it's done status is set to true before the dependencies are done. For example, when an AID with witnesses is creating a registry, you get:

{
  name: 'registry.EMr2ZK2Y5CpACEM6iHJQPrI4oHk7xg6QjUK_Uw0YFFld',
  metadata: {
    anchor: {
      i: 'EHmDHffYsNVW8-T-IRD2zcLEopTcE96ZugeKWY_GdxRK',
      s: '0',
      d: 'EHmDHffYsNVW8-T-IRD2zcLEopTcE96ZugeKWY_GdxRK'
    },
    depends: {
      name: 'witness.EMr2ZK2Y5CpACEM6iHJQPrI4oHk7xg6QjUK_Uw0YFFld',
      metadata: [Object],
      done: false,
      error: null,
      response: null
    }
  },
  done: true,
  error: null,
  response: {
    anchor: {
      i: 'EHmDHffYsNVW8-T-IRD2zcLEopTcE96ZugeKWY_GdxRK',
      s: '0',
      d: 'EHmDHffYsNVW8-T-IRD2zcLEopTcE96ZugeKWY_GdxRK'
    }
  }
}

Notice how the top-level operation is done, but it's dependencies are not. The same happens when issuing credentials:

{
  name: 'credential.EAM5C0JG9_zpMhKg97t-Rw9QaeKiYpPPXhmQQXr0GM8m',
  metadata: {
    ced: {
      ...
    },
    depends: {
      name: 'witness.EAM5C0JG9_zpMhKg97t-Rw9QaeKiYpPPXhmQQXr0GM8m',
      metadata: { sn: 2 },
      done: false,
      error: null,
      response: null
    }
  },
  done: true,
  error: null,
  response: {
    ced: {
      ...
    }
  }
}

Expected behaviour

Top level operation done should also depend on its child operations. So something like this done = depends.done === true ? self.done : false

Notes

This could be implemented at client level of course. But i suspect others will run into the same issue unless this is well documented.

[psteniusubi]

There's a client side workaround for this in the integration tests

https://github.com/WebOfTrust/signify-ts/blob/6574f3089fdc95abd52ff08b43f569107ec1009f/examples/integration-scripts/utils/test-util.ts#L64-L82

[iFergal]

So depends isn't really a "thing" in KERIA. It's just metadata stored with the operation that the client interprets. Given the current code structure, it could make sense to recursively check to make sure operations are done.

However, I'm a bit torn though as we will eventually need event driven support (see #290). It would be problematic to not be able to emit the event because the dependent events are not done, and this adds complexity of checking if the dependencies are done at emit time.

So in an event driven architecture, the client ends up needing to aggregate the event completions anyway, I think. Maybe it's better if they are consistent and better documented. What do you think?

[lenkan]

Ok. Yes I had a similar feeling when I looked into that code recently.

For now, I think it is probably fine to leave as is and it would be up to the client to do the right thing.

But I still find it a bit odd that the registry operation is considered done without the witness receipts. That is at least how I interpret the first example.

[iFergal]

The registry operation calls findAnchoringSealEvent - which from a quick glance, seems like it waits until it's fully witnessed. Perhaps there's a bug there.

[SmithSamuelM]

It depends on what the role of the particular participant is. FindAnchoringSealEvent finds events anchored in KELs in the LMDB database. The rules for what events are "accepted" or first seen in a KEL depend on the relation of the participant to a given KEL. If the participant is a Controller of the KEL then it will be accepted without witness receipts. If the participant is a Delegator of the KEL then is will NOT be accepted without witness receipts. If the participant is a witness of the KEL then is will be accepted with only the self generated receipt not fully receipted. IF the participant is anyone else i.e a Watcher the the event will NOT be accepted into the KEL until is is fully receipted and if delegated until the anchoring seal has been find in the Delegators's KEL.

[SmithSamuelM]

THis is one reason why I strongly suggest that KERIA agents should run local watchers and then make decisions based on the status of the local watcher's KEL not a local controller's KEL.

[SmithSamuelM]

Or at least unit tests should understand what they are testing for. Is is the view of the KEL that a watcher will see or the view of the KEL as some other participant will see? It matters for the test.