search

openwallet-foundation / credo-ts

Typescript framework for building decentralized identity and verifiable credential solutions
https://credo.js.org
Apache License 2.0
262 stars 199 forks source link

Error retrieving tails file to check for revocation #1926

Open icc-romeu opened 3 months ago

icc-romeu commented 3 months ago

We are facing an error during a proof presentation while downloading the tails file to check the revocation status of anoncreds.

The proof includes multiple fields from the same credential, and it seems Credo is trying to download the tails file for each of the fields, although it is the same credential. Hence, it may end up storing the same file multiple times, and I think that if the hash is calculated each time, at some point it may be calculated while one of those downloads are being stored, and then it fails with Hash of downloaded file does not match expected hash.

This does not always happen, and it makes sense if it is a race condition. 

 LOG  2024-06-28T07:24:06.104Z [ARIESAGENT][LOG] Using ledger 'did:sov' to retrieve revocation registry deltas with revocation registry definition id 'did:indy:did:sov:VecAQ6Q2TdWo9h3vVqTaA4/anoncreds/v0/REV_REG_DEF/27/emergencycontact_emergencycontact_1.0/0bbc6214-c2b5-45fe-b153-8a51adc90748' until 1719395248
 LOG  2024-06-28T07:24:06.105Z [ARIESAGENT][LOG] Submitting get transaction request to ledger 'did:sov'
 LOG  2024-06-28T07:24:06.110Z [ARIESAGENT][LOG] Saved tails file to FileSystem at path /data/user/0/org.unicc.undigitalid/cache/.afj/cache/anoncreds/tails/4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT
 LOG  2024-06-28T07:24:06.114Z [ARIESAGENT][LOG] Using ledger 'did:sov' to retrieve revocation registry deltas with revocation registry definition id 'did:indy:did:sov:VecAQ6Q2TdWo9h3vVqTaA4/anoncreds/v0/REV_REG_DEF/27/emergencycontact_emergencycontact_1.0/0bbc6214-c2b5-45fe-b153-8a51adc90748' until 1719395248
 LOG  2024-06-28T07:24:06.116Z [ARIESAGENT][LOG] Submitting get transaction request to ledger 'did:sov'
 LOG  2024-06-28T07:24:06.122Z [ARIESAGENT][LOG] Saved tails file to FileSystem at path /data/user/0/org.unicc.undigitalid/cache/.afj/cache/anoncreds/tails/4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT
 LOG  2024-06-28T07:24:06.123Z [ARIESAGENT][LOG] Using ledger 'did:sov' to retrieve revocation registry deltas with revocation registry definition id 'did:indy:did:sov:VecAQ6Q2TdWo9h3vVqTaA4/anoncreds/v0/REV_REG_DEF/27/emergencycontact_emergencycontact_1.0/0bbc6214-c2b5-45fe-b153-8a51adc90748' until 1719395248
 LOG  2024-06-28T07:24:06.124Z [ARIESAGENT][LOG] Submitting get transaction request to ledger 'did:sov'
 LOG  2024-06-28T07:24:06.132Z [ARIESAGENT][LOG] Saved tails file to FileSystem at path /data/user/0/org.unicc.undigitalid/cache/.afj/cache/anoncreds/tails/4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT
 LOG  2024-06-28T07:24:06.133Z [ARIESAGENT][LOG] Using ledger 'did:sov' to retrieve revocation registry deltas with revocation registry definition id 'did:indy:did:sov:VecAQ6Q2TdWo9h3vVqTaA4/anoncreds/v0/REV_REG_DEF/27/emergencycontact_emergencycontact_1.0/0bbc6214-c2b5-45fe-b153-8a51adc90748' until 1719395248
 LOG  2024-06-28T07:24:06.134Z [ARIESAGENT][LOG] Submitting get transaction request to ledger 'did:sov'
 LOG  2024-06-28T07:24:06.139Z [ARIESAGENT][LOG] Error while retrieving tails file from URL https://tailserver.../VecAQ6Q2TdWo9h3vVqTaA4:4:VecAQ6Q2TdWo9h3vVqTaA4:3:CL:27:emergencycontact_emergencycontact_1.0:CL_ACCUM:0bbc6214-c2b5-45fe-b153-8a51adc90748 {"error": [CredoError: Hash of downloaded file does not match expected hash. Expected: 4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT, Actual: EaRWN78m86Kfvj3QJtn4XqtaAWvWLASsgZS2sDx854zL]}
 LOG  2024-06-28T07:24:06.141Z [ARIESAGENT][LOG] Error retrieving revocation registry for proof request {"error": [CredoError: Hash of downloaded file does not match expected hash. Expected: 4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT, Actual: EaRWN78m86Kfvj3QJtn4XqtaAWvWLASsgZS2sDx854zL], "proofRequest": {"name": "Emergency contact UNJSPF v1", "non_revoked": {"from": 1719559430, "to": 1719559430}, "nonce": "1197045659185484348975297", "requested_attributes": {"email": [Object], "first_name": [Object], "last_name": [Object], "middle_name": [Object], "phone_number": [Object], "staff_relationship": [Object]}, "requested_predicates": {}, "version": "1.0"}, "selectedCredentials": {"attributes": {"email": [Object], "first_name": [Object], "last_name": [Object], "middle_name": [Object], "phone_number": [Object], "staff_relationship": [Object]}, "predicates": {}}}
 WARN  2024-06-28T07:24:06.160Z [ARIESAGENT][WARN] [useAgentProofs] Error reading link  [CredoError: Hash of downloaded file does not match expected hash. Expected: 4jxyNBbTFQEjtxij7FUnDkQ9jUhZSPQJ67dzf6jjDmnT, Actual: EaRWN78m86Kfvj3QJtn4XqtaAWvWLASsgZS2sDx854zL]

I reviewed the code and I think this should be sequential and not a Promise.all:

https://github.com/openwallet-foundation/credo-ts/blob/6faff7dd08458053af43d58cf16e6e7854648cb6/packages/anoncreds/src/anoncreds-rs/AnonCredsRsHolderService.ts#L848

TimoGlastra commented 3 months ago

We should probably dedupe upfront based on timestamp and id, which can help with downloading it multiple times. And then we should probably look when downloading to also include the timestamp in the filename. Then we'll never have clashes.

The promise.all does help in keeping things faster instead of doing it sequentially.

What do you think?

icc-romeu commented 3 months ago

The problem with the Promise.all approach is that:

I just added a patch in the meanwhile to do a for loop. As it is checking the filesystem for the file it only downloads once. It is sequential, but it is only downloading once per credential.

TimoGlastra commented 3 months ago

Ah of course, we don't need the timestamp for it. Well then we can just dedupe up front to only download the tails file for each registry once. There's still a slight chance of error if multiple processes (different presentation flows) are executing at the same then