bcgov / bc-wallet-mobile

BC Wallet to hold Verifiable Credentials
Apache License 2.0
62 stars 50 forks source link

Continued performance improvement action items. #1633

Closed jleach closed 7 months ago

jleach commented 12 months ago

While Andrew W is going to make some IndyVDR improvements, the following action items can be done in AFJ

No need to lookup schema

There is no need to lookup both the credential definition ID and schema ID as the cred definition ID contains the schema. This would remove some redundant AFJ ledger lookups and help speed up the transaction process.

Caching We should cache calls to the ledger as much as possible Make sure to cache cred defn lookups, not just TX IDs. We need to make sure we're cashing TX ID, cred definition, and schema lookups. Not just any one of these.

IndyVDR Options

IndyVDR has the option to "refresh" the pool on startup. If this is not passed in as a parameter when the pool is initialized the default time to do this is 120m (2h). We need to make sure this is being done on pool initialization.

References https://github.com/hyperledger/aries-framework-javascript/issues/1613

Future Work

IndyVDR Time Out

There is a timeout, I think it was if a node does not respond to a query, that has a default timeout of 20sec. This should be optimized in IndyVDR to be closer to 3-5 seconds.

No need for duplicate lookups

When AFJ accepts a credential the logs show the following ledger lookups:

  1. Get credential definition
  2. Get transaction
  3. Get credential definition
  4. Get transaction
  5. Get schema
  6. Get revocation registry definition

There is no need to "get transaction" and "get credential definition" to be called 2x. Although cashing may improve this it may be better to just remove duplicate network calls.

jleach commented 12 months ago

@wadeking98 FYI.

swcurran commented 12 months ago

@jleach — I suspect that the schema is needed, or will be a pain if you don’t get it, so I would skip that one until you really know the functionality. I misspoke on that one — technically correct, but likely not practical.

Getting rid of the reads 2-4 in your list above would be the priority — easpecially if they are not cached.

cvarjao commented 12 months ago

Hey team! Please add your planning poker estimate with Zenhub @bryce-mcmath @jleach @wadeking98

swcurran commented 12 months ago

I suggest that this be moved up in priority, as per this note in an Indy VDR issue. I think the change should be pretty easy (I hope) -- adding a call (or perhaps updating the data if it already being used).

From the description above:

IndyVDR Time Out

There is a timeout, I think it was if a node does not respond to a query, that has a default timeout of 20sec. This should be optimized in IndyVDR to be closer to 3-5 seconds.

knguyenBC commented 7 months ago

Closing this ticket and creating a new one to update indyvdr to incorporate performance enhancements.