unverify during verify with dedup

src/client/tests/webapp/auto-verifier.test.ts

@@ -1219,6 +1219,65 @@ describe('fetchAndVerifyIfNeeded tests', () => {
 		expect(verifyFunc).toHaveBeenCalledTimes(1);
 	});
 
+    test('ab: a: primary with null, b: primary dedup, ab: unverify during verify, ab: resolve with trusted anyway', async () => {
+		const {
+			nextPrimary, nextTrusted, nextVerify,
+			primaryFunc, trustedFunc, verifyFunc
+		} = getManualsAndMocks();
+		
+		const av = new AutoVerifier(primaryFunc, trustedFunc, verifyFunc);
+
+		const a = fetchAndVerifyIfNeededManually(av, nextPrimary, nextTrusted, nextVerify);
+
+		expect(av.primaryPromise).toBe(a.primary.promise);
+		expect(av.trustedPromise).toBe(a.trusted.promise);
+		expect(av.trustedStatus).toBe('fetching');
+		expect(primaryFunc).toHaveBeenCalled();
+		expect(trustedFunc).toHaveBeenCalled();
+
+		const b = fetchAndVerifyIfNeededManually(av, nextPrimary, nextTrusted, nextVerify);
+		
+		expect(av.primaryPromise).toBe(a.primary.promise); // doesn't change from a
+		expect(av.trustedPromise).toBe(a.trusted.promise); // doesn't change from a
+		expect(av.trustedStatus).toBe('fetching');
+		
+		a.primary.resolve(null);
+		await disjoint();
+
+		expect(av.primaryPromise).toBe(null);
+		expect(av.trustedPromise).toBe(a.trusted.promise);
+		expect(av.trustedStatus).toBe('verifying');
+
+		expect(verifyFunc).toHaveBeenCalledTimes(0);
+		a.trusted.resolve(new BasicWE(2));
+		await disjoint();
+
+		expect(verifyFunc).toHaveBeenCalledWith(null, new BasicWE(2));
+		expect(av.primaryPromise).toBe(null);
+		expect(av.trustedPromise).toBe(a.trusted.promise);
+		expect(av.trustedStatus).toBe('verifying');
+
+		expect(a.result).toBeUndefined();
+		expect(b.result).toBeUndefined();
+		
+        av.unverify();
+		a.verify.resolve(true);
+		await disjoint();
+		
+		expect(a.result).toEqual(new BasicWE(2));
+		expect(b.result).toEqual(new BasicWE(2));
+		expect(av.primaryPromise).toBe(null);
+		expect(av.trustedPromise).toBe(null);
+		expect(av.trustedStatus).toBe('none');
+
+		await disjoint(); // sanity check
+
+		// Even though there were two fetchAnd... calls, they were deduped such that each func was only called once
+		expect(primaryFunc).toHaveBeenCalledTimes(1);
+		expect(trustedFunc).toHaveBeenCalledTimes(1);
+		expect(verifyFunc).toHaveBeenCalledTimes(1);
+    });
+
 	// TODO: Why not have fetcher and a simple dedup wrapper to remove some complexity from this function?
 	// This would likely make it possible to get rid of the "else" block in tryResolveTrustedPromise
 	// Make sure it would work properly for retry functionality
@@ -1227,4 +1286,14 @@ describe('fetchAndVerifyIfNeeded tests', () => {
 	// Make sure to do try/catch errors/rejections, verification failures, unverifies in the middle
 	// Fetching after verified doesn't re-fetch trusted
 	// Unverify during verify w/ dedups
+    // verifyFunc returns false
+    // verifyFunc returns false while different trusted succeeds? - this is not possible
+    // verifyFunc rejects
+
+    // not possible (and therefore not tested) list
+    // verifyFunc returns false while awaiting a trusted promise
+    //  - implies it's possible to enter the status === 'fetching' block
+    //      while other verifyFunc is being called. verifyFunc is only
+    //      called in the 'fetching' state
+    //  - expected would be to return verify trusted result
 });

src/client/webapp/auto-verifier.ts

@@ -277,7 +277,8 @@ export class AutoVerifier<T> {
 									// this actually could be quite common
 									//console.warn('RARE ALERT: we got unverified during verification!');
 									// ** complexity:
-									// keep in mind that this will resolve 'unverified' promises with their resulting origTrustedPromise (which should be a good thing)
+                                    // if primaryUpToDate is false or we got unverified during verification, this path will still return
+                                    // the trustedResult that was passed to the verify function
 								}
 
 								if (!resolved) {
@@ -318,17 +319,15 @@ export class AutoVerifier<T> {
 									return;
 								}
 
-								const origVerifyPromise: Promise<boolean> | null = this.verifyPromise;
+								await this.verifyPromise; // we don't care about the result, just that we wait for the verification to finish
-								await origVerifyPromise; // we don't care about the result, just that we wait for the verification to finish
+								if (this.trustedPromise !== origTrustedPromise) {
-								if (this.verifyPromise !== origVerifyPromise) {
+									// we've been invalidated while we were waiting for the verify!
-									// we've been invalidated while we were waiting for the trusted result!
 									console.warn(
 										'ULTRA RARE ALERT: we got unverified while awaiting a verify promise another path was calling!',
-										origVerifyPromise,
-										this.verifyPromise,
 									);
-									await tryResolveTrustedPromise();
+									// ** complexity:
-									return;
+                                    // if primaryUpToDate is false or we got unverified during verification, this path will still return
+                                    // the trustedResult that was passed to the verify function
 								}
 
 								if (!resolved) {
@@ -356,7 +355,7 @@ export class AutoVerifier<T> {
 						resolved = true;
 					} else {
 						this.unverify()
-						console.warn('server request failed after returning cache value (or when already rejected)', e);
+						console.warn('rejection after returning cache value (or when already rejected)', e);
 					}
 				}
 			},