Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 300
feat: shield-controller: check signature coverage#6501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
bdc657693c68e68da1a155cef2e6a3a2197571817ba8b3145File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import type { SignatureRequest } from '@metamask/signature-controller'; | ||
| import type { TransactionMeta } from '@metamask/transaction-controller'; | ||
| import type { CoverageResult, CoverageStatus, ShieldBackend } from './types'; | ||
| @@ -16,6 +17,14 @@ export type InitCoverageCheckRequest = { | ||
| origin?: string; | ||
| }; | ||
| export type InitSignatureCoverageCheckRequest = { | ||
| chainId: string; | ||
| data: string; | ||
| from: string; | ||
| method: string; | ||
| origin?: string; | ||
| }; | ||
| export type InitCoverageCheckResponse = { | ||
| coverageId: string; | ||
| }; | ||
| @@ -59,9 +68,7 @@ export class ShieldRemoteBackend implements ShieldBackend { | ||
| this.#fetch = fetchFn; | ||
| } | ||
| checkCoverage: (txMeta: TransactionMeta) => Promise<CoverageResult> = async ( | ||
| txMeta, | ||
| ) => { | ||
| async checkCoverage(txMeta: TransactionMeta): Promise<CoverageResult> { | ||
| const reqBody: InitCoverageCheckRequest = { | ||
| txParams: [ | ||
| { | ||
| @@ -76,22 +83,46 @@ export class ShieldRemoteBackend implements ShieldBackend { | ||
| origin: txMeta.origin, | ||
| }; | ||
| const { coverageId } = await this.#initCoverageCheck(reqBody); | ||
| const { coverageId } = await this.#initCoverageCheck( | ||
| 'v1/transaction/coverage/init', | ||
| reqBody, | ||
| ); | ||
| return this.#getCoverageResult(coverageId); | ||
| }; | ||
| } | ||
| async checkSignatureCoverage( | ||
| signatureRequest: SignatureRequest, | ||
| ): Promise<CoverageResult> { | ||
| if (typeof signatureRequest.messageParams.data !== 'string') { | ||
| throw new Error('Signature data must be a string'); | ||
| } | ||
| const reqBody: InitSignatureCoverageCheckRequest = { | ||
| chainId: signatureRequest.chainId, | ||
| data: signatureRequest.messageParams.data, | ||
| from: signatureRequest.messageParams.from, | ||
| method: signatureRequest.type, | ||
| origin: signatureRequest.messageParams.origin, | ||
| }; | ||
| const { coverageId } = await this.#initCoverageCheck( | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please note that we can't always except Please check the swagger, as it has updated recently. (please select ![]() Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sample cases of 200 responses without
In such cases, we don't proceed to process the rulesets, we straight away return the Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case also applies to ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we throw an error in that case? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we please change the API backend. I do see several issues with this change. The expectation so far is that if we have a coverage result, either for signatures, or for transactions, it always has the same format. If we don't have that, a lot of complexity is added to the whole system. (Currently we can just treat the results within the same data structure. Then we might need separate data structure. Also, missing the | ||
| 'v1/signature/coverage/init', | ||
| reqBody, | ||
| ); | ||
| return this.#getCoverageResult(coverageId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Coverage ID Missing in API ResponseThe code assumes Additional Locations (1) | ||
| } | ||
| async #initCoverageCheck( | ||
| reqBody: InitCoverageCheckRequest, | ||
| path: string, | ||
| reqBody: unknown, | ||
| ): Promise<InitCoverageCheckResponse> { | ||
| const res = await this.#fetch( | ||
| `${this.#baseUrl}/v1/transaction/coverage/init`, | ||
| { | ||
| method: 'POST', | ||
| headers: await this.#createHeaders(), | ||
| body: JSON.stringify(reqBody), | ||
| }, | ||
| ); | ||
| const res = await this.#fetch(`${this.#baseUrl}/${path}`, { | ||
| method: 'POST', | ||
| headers: await this.#createHeaders(), | ||
| body: JSON.stringify(reqBody), | ||
| }); | ||
| if (res.status !== 200) { | ||
| throw new Error(`Failed to init coverage check: ${res.status}`); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.

Uh oh!
There was an error while loading. Please reload this page.