Remove checks option

[clehner]

As suggested in #8, remove use of the (unspecified) checks verification option.

With this change, I think it is expected that implementations check a credential/presentation's proof in all cases (as previously the "proof" check was passed in all cases), and check credentialStatus for verifiable credentials when the `credentialStatus property is present (Previously the "credentialStatus" check was passed for verifiable credentials case-14.json and case-15.json).

checks is still expected to be in the responses (VerificationResult) as before.

Related discussion of what should be checked during verification: w3c-ccg/vc-api#59.

[msporny]

Hrm, I think I disagree with this change as it was meant to be an extension point for sorts of checks that can be performed. Two options being 'proof', 'revocation', and 'custom-check-123' -- or something equivalent. If we remove this, we need to figure out how to instruct the verifier to do the appropriate set of checks, some of which may be extensions, and some of which might be entirely proprietary.

[peacekeeper]

The problem is that the test suite and the API definition currently conflict with each other. The test suite sends the "checks" option, but the API definition doesn't support it.

So either we need to remove it from the test suite (which this PR is doing), or we need to add it to the API. Personally I think either way is okay.

On the 05 Oct 2021 VC API Work Item call people felt that variability of checks is bad (see w3c-ccg/vc-api#74 (comment), w3c-ccg/vc-api#59 (comment), w3c-ccg/vc-api#62 (comment)), therefore I think this PR should go ahead, but I'd also be fine with discussing this topic again...

[OR13]

checks should be internalized, errors provided by the API should be made more detailed, and used to ensure conformance... it should not be possible to pass the tests by saying "don't check the proof or its status".

[mkhraisha]

Passing a list of checks that the verifier is expected to do only makes sense in the instance where the verifier is an internal only API, in those cases saying please verify X not Y seems pretty important.
The vc api needs to have a firm stance on if these calls are all internal vs external, and until we have that language in the spec, we should not have this test suite do a test the API definition doesn't support.

[peacekeeper]

@msporny can you re-review this, based on above comments? Especially the one about an existing conflict between API spec and test suite regarding the "checks" option.

[msporny]

I'm fine w/ removing this for now. I think this is going to bite us at some point, but let's wait until it bites us to put it back in. There is nothing preventing people from having their own extensions... which raises the question: How does an organization provide options that they are guaranteed won't conflict with future updates to this spec? Perhaps we need a "vendorOptions" field? Or just call it for what it is "vendorLockinOptions"? :P (kidding about the last part, there)

[peacekeeper]

How does an organization provide options that they are guaranteed won't conflict with future updates to this spec?

That's easy...

• We create a registry of extension options where each vendor can submit their own options
• Extension options MUST NOT rely on centralized services, or they will not be accepted into the registry
• We create an Extension Option Rubric to evaluate various criteria of the option
• All options MUST conform to an Abstract Option Model (AOM) which has multiple representations that can be losslessly converted
• All extension options MUST provide a CDDL to define the range of allowed values
• Vendor-specific options that have not yet been standardized MUST be prefixed with "X-"

[msporny]

That's easy...

I feel like I'm being trolled... are you trolling me, Markus? :P

[msporny]

Multiple reviews, no objections, merging.