From a793a35f099aebc4d008ef6d8de141d8174a2566 Mon Sep 17 00:00:00 2001 From: William Hua Date: Thu, 2 Nov 2023 15:44:35 -0400 Subject: [PATCH] guard: no more retry logic --- packages/guard/src/signer.ts | 112 ++++++++------------------- packages/signhub/src/orchestrator.ts | 12 ++- 2 files changed, 42 insertions(+), 82 deletions(-) diff --git a/packages/guard/src/signer.ts b/packages/guard/src/signer.ts index fb6a84360..36a3807fe 100644 --- 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,8 +23,8 @@ export class GuardSigner implements signers.SapientSigner { } async requestSignature( - id: string, - _message: BytesLike, + _id: string, + message: BytesLike, metadata: Object, callbacks: { onSignature: (signature: BytesLike) => void @@ -42,28 +32,45 @@ export class GuardSigner implements signers.SapientSigner { onStatus: (situation: string) => void } ): Promise { + 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 { 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 { - // 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 index a08883d49..4f26c8e81 100644 --- 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` } } }