guard: no more retry logic

0xsequence · Nov 2, 2023 · a793a35 · a793a35
1 parent 282682a
commit a793a35
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 82 deletions.
diff --git a/packages/guard/src/signer.ts b/packages/guard/src/signer.ts
@@ -9,21 +9,11 @@ const fetch = typeof global === 'object' ? global.fetch : window.fetch
 
 export class GuardSigner implements signers.SapientSigner {
   private guard: Guard
-  private requests: Map<
-    string,
-    {
-      lastAttempt?: string
-      onSignature: (signature: BytesLike) => void
-      onRejection: (error: string) => void
-      onStatus: (situation: string) => void
-    }
-  > = new Map()
 
   constructor(
     public readonly address: string,
     public readonly url: string,
-    public readonly appendSuffix: boolean = false,
-    private readonly onError?: (err: Error) => void
+    public readonly appendSuffix: boolean = false
   ) {
     this.guard = new Guard(url, fetch)
   }
@@ -33,37 +23,54 @@ export class GuardSigner implements signers.SapientSigner {
   }
 
   async requestSignature(
-    id: string,
-    _message: BytesLike,
+    _id: string,
+    message: BytesLike,
     metadata: Object,
     callbacks: {
       onSignature: (signature: BytesLike) => void
       onRejection: (error: string) => void
       onStatus: (situation: string) => void
     }
   ): Promise<boolean> {
+    const { onSignature, onRejection } = callbacks
+
     if (!commons.isWalletSignRequestMetadata(metadata)) {
-      callbacks.onRejection('Expected Sequence-like metadata')
-    } else {
-      // Queue the request first, this method only does that
-      // the requesting to the API is later handled on every status change
-      this.requests.set(id, callbacks)
+      onRejection('expected sequence signature request metadata')
+      return false
     }
 
-    return true
-  }
+    const guardTotpCode = (metadata as { guardTotpCode?: string }).guardTotpCode
+
+    // Building auxData, notice: this uses the old v1 format
+    // TODO: We should update the guard API so we can pass the metadata directly
+    const coder = universal.genericCoderFor(metadata.config.version)
+    const { encoded } = coder.signature.encodeSigners(metadata.config, metadata.parts ?? new Map(), [], metadata.chainId)
+
+    try {
+      const { sig: signature } = await this.guard.signWith({
+        signer: this.address,
+        request: {
+          msg: ethers.utils.hexlify(message),
+          auxData: this.packMsgAndSig(metadata.address, metadata.digest, encoded, metadata.chainId),
+          chainId: ethers.BigNumber.from(metadata.chainId).toNumber()
+        },
+        token: guardTotpCode ? { id: AuthMethod.TOTP, token: guardTotpCode } : undefined
+      })
 
-  notifyStatusChange(id: string, status: Status, metadata: Object): void {
-    if (!this.requests.has(id)) return
+      if (ethers.utils.arrayify(signature).length === 0) {
+        throw new Error('guard response contained no signature data')
+      }
 
-    if (!commons.isWalletSignRequestMetadata(metadata)) {
-      this.requests.get(id)!.onRejection('Expected Sequence-like metadata (status update)')
-      return
+      onSignature(signature)
+      return true
+    } catch (error) {
+      onRejection(`unable to request guard signature: ${error.message ?? error.msg ?? error}`)
+      return false
     }
-
-    this.evaluateRequest(id, status.message, status, metadata)
   }
 
+  notifyStatusChange(_id: string, _status: Status, _metadata: Object): void {}
+
   async getAuthMethods(token: GetAuthMethodsToken): Promise<AuthMethod[]> {
     let response: AuthMethodsReturn
 
@@ -126,57 +133,6 @@ export class GuardSigner implements signers.SapientSigner {
     return ethers.utils.defaultAbiCoder.encode(['address', 'uint256', 'bytes', 'bytes'], [address, chainId, msg, sig])
   }
 
-  private keyOfRequest(signer: string, msg: BytesLike, auxData: BytesLike, chainId: ethers.BigNumberish): string {
-    return ethers.utils.solidityKeccak256(['address', 'uint256', 'bytes', 'bytes'], [signer, chainId, msg, auxData])
-  }
-
-  private async evaluateRequest(
-    id: string,
-    message: BytesLike,
-    _: Status,
-    metadata: commons.WalletSignRequestMetadata & { guardTotpCode?: string }
-  ): Promise<void> {
-    // Building auxData, notice: this uses the old v1 format
-    // TODO: We should update the guard API so we can pass the metadata directly
-    const coder = universal.genericCoderFor(metadata.config.version)
-    const { encoded } = coder.signature.encodeSigners(metadata.config, metadata.parts ?? new Map(), [], metadata.chainId)
-
-    try {
-      const key = this.keyOfRequest(this.address, message, encoded, metadata.chainId)
-      const lastAttempt = this.requests.get(id)?.lastAttempt
-      if (lastAttempt === key) {
-        return
-      }
-
-      this.requests.get(id)!.lastAttempt = key
-
-      const result = await this.guard.signWith({
-        signer: this.address,
-        request: {
-          msg: ethers.utils.hexlify(message),
-          auxData: this.packMsgAndSig(metadata.address, metadata.digest, encoded, metadata.chainId),
-          chainId: ethers.BigNumber.from(metadata.chainId).toNumber() // TODO: This should be a string (in the API)
-        },
-        token: metadata.guardTotpCode
-          ? {
-              id: AuthMethod.TOTP,
-              token: metadata.guardTotpCode
-            }
-          : undefined
-      })
-
-      if (ethers.utils.arrayify(result.sig).length !== 0) {
-        this.requests.get(id)!.onSignature(result.sig)
-        this.requests.delete(id)
-      }
-    } catch (e) {
-      // The guard signer may reject the request for a number of reasons
-      // like for example, if it's being the first signer (it waits for other signers to sign first)
-      // We always forward the error here and filter on client side.
-      this.onError?.(e)
-    }
-  }
-
   suffix(): BytesLike {
     return this.appendSuffix ? [3] : []
   }

diff --git a/packages/signhub/src/orchestrator.ts b/packages/signhub/src/orchestrator.ts
@@ -167,10 +167,14 @@ export class Orchestrator {
         const signer = this.signers[i]
         const promise = accepted[i]
 
-        if (promise.status === 'rejected' || promise.value === false) {
-          const prejected = promise as PromiseRejectedResult
-          console.warn(`Signer ${await signer.getAddress()} rejected the request ${prejected.reason}`)
-          status.signers[await signer.getAddress()] = { rejected: true, error: prejected.reason.toString() }
+        if (promise.status === 'rejected') {
+          const address = await signer.getAddress()
+          console.warn(`signer ${address} rejected the request: ${promise.reason}`)
+          status.signers[address] = { rejected: true, error: `signer ${address} rejected the request: ${promise.reason}` }
+        } else if (!promise.value) {
+          const address = await signer.getAddress()
+          console.warn(`signer ${address} rejected the request`)
+          status.signers[address] = { rejected: true, error: `signer ${address} rejected the request` }
         }
       }