tweak: input_descriptors should be a map/object for more efficient lookups

[llorllale]

Proposal: make input_descriptors a map for constant time lookups.

Rationale: each input_descriptor is required to have a unique id. Also, submissions are only required to reference the requirement's id. Therefore the schema can be changed to this without breaking the semantics:

{
  // VP, OIDC, DIDComm, or CHAPI outer wrapper
  "presentation_definition": {
    "id": "32f54163-7166-48f1-93d8-ff217bdb0653",
    "input_descriptors": {
      "banking_input": {
        "name": "Bank Account Information",
        "purpose": "We need your bank and account information.",
        "schema": [{
            "uri": "https://bank-standards.com/customer.json"
        }],
      }, // end of banking_input
      "citizenship_input": {
          ...
      }, // end of citizenship_input
  } // end of presentation_definition

Motivation: avoid multiple O(N) lookups while evaluating submissions.

[decentralgabe]

I think this is a fine suggestion, however, is it a requirement? Performance improvements can be handled at the impl layer. What's stopping you from implementing this as a map and translating to objs at the boundaries of the API?

[llorllale]

@glcohen

I think this is a fine suggestion, however, is it a requirement? Performance improvements can be handled at the impl layer. What's stopping you from implementing this as a map and translating to objs at the boundaries of the API?

Yes, this is not a blocking issue.

I think this tweak would help in two ways:

• further disambiguate the input_descriptors structure by relying on a known data structure with the required semantics (id must be unique)
• simplify implementations that would like to improve performance this way

Consider this a proposal for v0.2.0 or greater.

[decentralgabe]

I'll start the new label 😄

[csuwildcat]

If this is desired, it should be done now, because it's a significant breaking change that I don't think we should do after 1.0, which we're trying to get stamped by mid-January.

[decentralgabe]

We discussed this on the call today and decided the change is not worth it for a few reasons:

1. The performance hit can be improved in implementations by building an index on the first iteration over the array
2. We have existing implementations that would break
3. In the future, you are right, a map would have been preferred -- we will keep this in mind for new additions
4. If at some point in the future there is concrete data that demonstrates a legitimate wide-scale performance issue due to the structure of the specification, we can reconsider

tl;dr good idea but not worth it 😄