feat: restrict token registry to fail on multiple issuers #129
Conversation
src/common/errorHandler.ts
Outdated
| options: Parameters<Verifier["verify"]>[1] | ||
| ) => { | ||
| try { | ||
| // Using return await to ensure async function execute in try loop |
There was a problem hiding this comment.
| // Using return await to ensure async function execute in try loop | |
| // Using return await to ensure async function execute in try block |
| network: string; | ||
| }): Promise<{ minted: boolean; reason?: string }> => { | ||
| try { | ||
| const tokenRegistryContract = await TradeTrustErc721Factory.connect(tokenRegistry, getProvider({ network })); |
There was a problem hiding this comment.
- why getting rid of code and codeString ? that makes it a breaking change ... and harder to deal if want to handle specific errors
- why getting rid of SERVER_ERROR ? We use it on opencert to differentiate a business and a technical error
There was a problem hiding this comment.
I knew you will ask, which is why i got you to review.
For the errors which this verifier knows for sure that the result is either true or false, it returns that.
For errors which it is uncertain if the document is issued or not, it will throw.
There is no meaning in returning more information than that, oa-verify is meant to provide that level of abstraction to dependant applications.
In the cases from line 69 to 82, we are certain the token was not minted (regardless of which error it is), there is no error in those cases.
I was initially reluctant to return more information that true/false.
There was a problem hiding this comment.
As for SERVER_ERROR, I can make the change to throw that one with a code.
yehjxraymond
changed the base branch from
master
to
feat/stricterVerification
yehjxraymond
force-pushed
the
feat/stricterVerification
branch
from
ab85f21 to
5ba688d
Compare
* feat: restrict token registry to fail on multiple issuers (#129) * feat: restrict token registry to fail on multiple issuers * feat: added reasons back * feat: added server error * feat: stricter document store verifier (#133) * feat: stricter did signed document verifier (#134) * feat: stricter did identity (#135) * chore: refactored error handling * feat: restricted didIdentityProof * feat: stricter dns did identity (#136) * feat: restricted dnsDid * feat: stricter dnsTxt (#137) * feat: aligning data.reason in individual fragments (#138) * feat: aligning naming convention with files and modules (#139) BREAKING CHANGE: Modules name changes and stricter verification rules
Having stricter check with token registry to disallow matching it with other identity verification types.
tokenRegistry.withCodedErrorHandlerto wrap verify function and throw know errors withCodedError.