:bug: indy_credx error in validation of presentation when `non_revoked.to` is None

[ff137]

Good day

We're noticing a regression in behavior from 0.11 to 0.12, and seems to still be present in the latest nightly build.

In one of our proof request tests, we assert that a non-revoked (revocable) credential is valid. We do this with the verifier setting up a proof request like so:

    request_body = {
        "protocol_version": "v2",
        "comment": "Test cred is not revoked",
        "type": "indy",
        "indy_proof_request": {
            "non_revoked": {},  # {"to": int(time.time())},
            "requested_attributes": {
                "speed": {
                    "name": "speed",
                    "restrictions": [
                        {"cred_def_id": credential_definition_id_revocable}
                    ],
                }
            },
            "requested_predicates": {},
        },
        "save_exchange_record": True,
        "connection_id": acme_and_alice_connection.acme_connection_id,
    }

    send_proof_response = await send_proof_request(acme_client, request_body)

The above uses our own models, but indy_proof_request represents the ACA-Py IndyProofRequest model, which gets posted to /present-proof-2.0/create-request

Notice that non_revoked is an empty dict (which is different to None -- None would mean to not validate if the credential is revoked or not).

The expected behaviour, which we observe in 0.11, is that an empty non_revoked (not specifying from or to) would default to proving that the cred is not revoked at the time of presentation. After a prover sends a presentation of a non-revoked credential (posting to /present-proof-2.0/records/{pres_ex_id}/send-presentation), in 0.11 we observe that verified returns as True in the verifier's presentation exchange record.

When attempting to upgrade to 0.12.1, we observe the following error in the agent, after prover sends presentation:

2024-06-05 13:20:44,349 aries_cloudagent.indy.credx.verifier ERROR Validation of presentation on nonce=960779015519508537837623 failed with error
Traceback (most recent call last):
  File "/home/aries/.local/lib/python3.9/site-packages/aries_cloudagent/indy/credx/verifier.py", line 72, in verify_presentation
    verified = await asyncio.get_event_loop().run_in_executor(
  File "/usr/local/lib/python3.9/asyncio/futures.py", line 284, in __await__
    yield self  # This tells Task to wait for completion.
  File "/usr/local/lib/python3.9/asyncio/tasks.py", line 328, in __wakeup
    future.result()
  File "/usr/local/lib/python3.9/asyncio/futures.py", line 201, in result
    raise self._exception
  File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/aries/.local/lib/python3.9/site-packages/indy_credx/types.py", line 467, in verify
    return bindings.verify_presentation(
  File "/home/aries/.local/lib/python3.9/site-packages/indy_credx/bindings.py", line 797, in verify_presentation
    do_call(
  File "/home/aries/.local/lib/python3.9/site-packages/indy_credx/bindings.py", line 465, in do_call
    raise get_current_error(True)
indy_credx.error.CredxError: Missing referent

This above error vanishes when I change non_revoked.to, in the verifier's initial proof request, to have some timestamp. e.g. "non_revoked": {"to": int(time.time())}

Making this above change to the verifier's proof request resolves the error and verified is shown to be True again.

The reason for this bug is unclear to me. I can't see why the indy_credx rust code would say referent is missing when non_revoked.to is None, but it passes when it is set. So I'm just posting an issue in the meantime -- maybe someone has encountered the same behaviour. If it's new, then I'll investigate more in due time. Any feedback will be appreciated.

[jamshale]

I don't think any of this code would have effected this but I recently merged something somewhat related. https://github.com/hyperledger/aries-cloudagent-python/pull/2995

[ff137]

@jamshale I tested with 0.12.1, which didn't include that change yet, and also tested the latest nightly build - both cases raise that error. But indeed works with 0.11

[ianco]

Sounds like it's a change in behaviour of aca-py, so we should probably fix it, but we should verify that we're following best practices: https://github.com/hyperledger/aries-rfcs/blob/main/concepts/0441-present-proof-best-practices/README.md

[dbluhm]

A bit of a different angle that this made me think about is:

Should we permit implicit to == now() in the non_revoked object if it is an empty object? This seems like it could backfire if, for instance, a controller accidentally fumbles calculating a timestamp and it's request library turns {to: undefined} into {}. Should this be a validation error?

If we're okay with this implicit behavior then cool, I agree we should figure out what changed between 0.11.x and now and fix it if possible.

[jamshale]

@dbluhm @ff137 @swcurran Do we think this should be addressed for version 1.0.0? I don't want to keep adding more and more to do before we get the release done. Not exactly sure if this is important enough to tackle asap, or not?

[dbluhm]

I think it's acceptable for this to come in a later patch

[jamshale]

I'm kind of thinking that Daniel is right about this, and even though there has been a change to require revoked.to to be set, that this is what the behaviour should actually be.

I'll leave this open for now. If we want to change it back to the old behaviour than I can tackle this.