diff --git a/extension/src/popup/components/BlockaidBanner/__tests__/BlockaidBanner.test.tsx b/extension/src/popup/components/BlockaidBanner/__tests__/BlockaidBanner.test.tsx index 0f1f1db413..a03c310251 100644 --- a/extension/src/popup/components/BlockaidBanner/__tests__/BlockaidBanner.test.tsx +++ b/extension/src/popup/components/BlockaidBanner/__tests__/BlockaidBanner.test.tsx @@ -32,6 +32,12 @@ describe("getBlockaidBannerTitle", () => { ); }); + it("labels a simulation revert as likely to fail, not suspicious", () => { + expect( + getBlockaidBannerTitle(t, "transaction", SecurityLevel.LIKELY_TO_FAIL), + ).toBe("This transaction is likely to fail"); + }); + it("uses a soft caution title for unable-to-scan (site gets its own copy)", () => { expect( getBlockaidBannerTitle(t, "token", SecurityLevel.UNABLE_TO_SCAN), diff --git a/extension/src/popup/components/BlockaidBanner/index.tsx b/extension/src/popup/components/BlockaidBanner/index.tsx index 7e7d7e564b..1234b988cf 100644 --- a/extension/src/popup/components/BlockaidBanner/index.tsx +++ b/extension/src/popup/components/BlockaidBanner/index.tsx @@ -24,6 +24,7 @@ export type BlockaidEntity = /** * Resolves the banner title from the flagged entity and its severity. * UNABLE_TO_SCAN is a soft "proceed with caution" (site has its own copy). + * LIKELY_TO_FAIL is a simulation revert, not a risk flag. * SAFE never produces a title. */ export const getBlockaidBannerTitle = ( @@ -31,6 +32,10 @@ export const getBlockaidBannerTitle = ( entity: BlockaidEntity, securityLevel: SecurityLevel, ): string => { + if (securityLevel === SecurityLevel.LIKELY_TO_FAIL) { + return t("This transaction is likely to fail"); + } + if (securityLevel === SecurityLevel.UNABLE_TO_SCAN) { return entity === "site" ? t("Unable to scan site for malicious behavior") @@ -73,8 +78,9 @@ interface BlockaidBannerProps { /** * The single reusable Blockaid warning banner. Filled style, colored by - * severity (red for malicious, amber for suspicious / unable-to-scan), with a - * chevron when it opens a detail sheet. Renders nothing for a SAFE verdict. + * severity (red for malicious, amber for suspicious / likely-to-fail / + * unable-to-scan), with a chevron when it opens a detail sheet. Renders + * nothing for a SAFE verdict. */ export const BlockaidBanner = ({ securityLevel, diff --git a/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx b/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx index dc085b300a..834909ec93 100644 --- a/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx +++ b/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx @@ -233,10 +233,11 @@ export const ReviewTx = ({ const isMalicious = securityLevel === SecurityLevel.MALICIOUS; const isSuspicious = securityLevel === SecurityLevel.SUSPICIOUS; - // Determine if a security warning should be shown (tx- or token-driven) + // Determine if a warning should be shown (tx risk, a revert, or a token). const shouldShowTxWarning = isMalicious || isSuspicious || + securityLevel === SecurityLevel.LIKELY_TO_FAIL || securityLevel === SecurityLevel.UNABLE_TO_SCAN; // Banner copy for a flagged destination token (null when the token is clean @@ -289,10 +290,9 @@ export const ReviewTx = ({ // Which single Blockaid banner to render, by priority cascade: // tx-malicious > tx-suspicious > token-malicious > token-suspicious > - // any unable-to-scan. A flagged TOKEN outranks a tx that merely couldn't be - // scanned (common on mainnet), so a malicious-token warning is never - // downgraded to the soft "proceed with caution". Only the tx banner opens - // the expandable pane. + // tx likely-to-fail > any unable-to-scan. A flagged token outranks a revert + // and a tx that merely couldn't be scanned, so a malicious-token warning is + // never downgraded. Only the tx banner opens the expandable pane. const blockaidBannerKind: "tx" | "token" | null = (() => { if ( txSecurityLevel === SecurityLevel.MALICIOUS || @@ -306,6 +306,9 @@ export const ReviewTx = ({ ) { return "token"; } + if (txSecurityLevel === SecurityLevel.LIKELY_TO_FAIL) { + return "tx"; + } if (txSecurityLevel && shouldShowTxWarning) { return "tx"; // tx unable-to-scan } diff --git a/extension/src/popup/components/WarningMessages/index.tsx b/extension/src/popup/components/WarningMessages/index.tsx index b3da93d19a..126052fe7a 100644 --- a/extension/src/popup/components/WarningMessages/index.tsx +++ b/extension/src/popup/components/WarningMessages/index.tsx @@ -905,14 +905,29 @@ export const BlockAidScanExpanded = ({ return null; } + // A revert with no risk verdict is a failure prediction. The raw simulation + // error is already one of the rows; the title and subtitle should not call + // it unsafe. + const isSimulationFailureOnly = + !mergedIsMalicious && + !mergedIsSuspicious && + !isAssetScan && + !!scanResult && + "simulation" in scanResult && + !!scanResult.simulation && + "error" in scanResult.simulation; const title = mergedIsMalicious ? t("Do not proceed") : mergedIsSuspicious ? t("Suspicious Request") - : t("Proceed with caution"); - const subtitle = isAssetScan - ? t("This token does not appear safe for the following reasons.") - : t("This transaction does not appear safe for the following reasons."); + : isSimulationFailureOnly + ? t("Likely to fail") + : t("Proceed with caution"); + const subtitle = isSimulationFailureOnly + ? t("This transaction is expected to fail for the following reasons.") + : isAssetScan + ? t("This token does not appear safe for the following reasons.") + : t("This transaction does not appear safe for the following reasons."); return (
{ ).toBe(SecurityLevel.MALICIOUS); }); + it("ranks a revert above UNABLE_TO_SCAN and below SUSPICIOUS", () => { + expect( + mergeSecurityLevels([ + SecurityLevel.UNABLE_TO_SCAN, + SecurityLevel.LIKELY_TO_FAIL, + ]), + ).toBe(SecurityLevel.LIKELY_TO_FAIL); + expect( + mergeSecurityLevels([ + SecurityLevel.LIKELY_TO_FAIL, + SecurityLevel.SUSPICIOUS, + ]), + ).toBe(SecurityLevel.SUSPICIOUS); + }); + it("ranks SUSPICIOUS above UNABLE_TO_SCAN", () => { expect( mergeSecurityLevels([ diff --git a/extension/src/popup/constants/blockaid.ts b/extension/src/popup/constants/blockaid.ts index 2b24eb783b..c383b867a7 100644 --- a/extension/src/popup/constants/blockaid.ts +++ b/extension/src/popup/constants/blockaid.ts @@ -7,6 +7,7 @@ export enum SecurityLevel { SUSPICIOUS = "SUSPICIOUS", MALICIOUS = "MALICIOUS", UNABLE_TO_SCAN = "UNABLE_TO_SCAN", + LIKELY_TO_FAIL = "LIKELY_TO_FAIL", } /** @@ -28,8 +29,9 @@ export interface BlockaidWarning { const SECURITY_LEVEL_SEVERITY: Record = { [SecurityLevel.SAFE]: 0, [SecurityLevel.UNABLE_TO_SCAN]: 1, - [SecurityLevel.SUSPICIOUS]: 2, - [SecurityLevel.MALICIOUS]: 3, + [SecurityLevel.LIKELY_TO_FAIL]: 2, + [SecurityLevel.SUSPICIOUS]: 3, + [SecurityLevel.MALICIOUS]: 4, }; /** diff --git a/extension/src/popup/helpers/__tests__/blockaid.test.tsx b/extension/src/popup/helpers/__tests__/blockaid.test.tsx index 71dee7ad37..2752852061 100644 --- a/extension/src/popup/helpers/__tests__/blockaid.test.tsx +++ b/extension/src/popup/helpers/__tests__/blockaid.test.tsx @@ -8,6 +8,7 @@ import { shouldTreatAssetAsUnableToScan, isTxSuspicious, useIsTxSuspicious, + getTransactionSecurityLevel, useShouldTreatTxAsUnableToScan, isBlockaidEnabled, getSiteSecurityStates, @@ -222,7 +223,7 @@ describe("BlockAid Helper Functions", () => { expect(isTxSuspicious(null, null)).toBe(false); }); - it("should return true when simulation has error", () => { + it("should return false when the only finding is a simulation error", () => { expect( isTxSuspicious( { @@ -230,7 +231,7 @@ describe("BlockAid Helper Functions", () => { } as BlockAidScanTxResult, null, ), - ).toBe(true); + ).toBe(false); }); it("should return true when validation result_type is not Benign", () => { @@ -286,6 +287,47 @@ describe("BlockAid Helper Functions", () => { }); }); + describe("getTransactionSecurityLevel", () => { + it("classifies a simulation revert as likely to fail", () => { + expect( + getTransactionSecurityLevel( + { + simulation: { error: "insufficient balance for asset USDC" }, + validation: null, + } as unknown as BlockAidScanTxResult, + false, + null, + ), + ).toBe(SecurityLevel.LIKELY_TO_FAIL); + }); + + it("keeps a malicious verdict when the simulation also reverted", () => { + expect( + getTransactionSecurityLevel( + { + simulation: { error: "insufficient balance for asset USDC" }, + validation: { result_type: "Malicious" }, + } as unknown as BlockAidScanTxResult, + false, + null, + ), + ).toBe(SecurityLevel.MALICIOUS); + }); + + it("keeps a warning verdict as suspicious", () => { + expect( + getTransactionSecurityLevel( + { + simulation: {}, + validation: { result_type: "Warning" }, + } as unknown as BlockAidScanTxResult, + false, + null, + ), + ).toBe(SecurityLevel.SUSPICIOUS); + }); + }); + describe("useIsTxSuspicious hook", () => { beforeEach(() => { process.env.DEV_EXTENSION = "true"; diff --git a/extension/src/popup/helpers/blockaid.ts b/extension/src/popup/helpers/blockaid.ts index ae749b040d..12fb1fe2a5 100644 --- a/extension/src/popup/helpers/blockaid.ts +++ b/extension/src/popup/helpers/blockaid.ts @@ -607,11 +607,7 @@ export const isTxSuspicious = ( return false; } - const { simulation, validation } = blockaidData; - - if (simulation && "error" in simulation) { - return true; - } + const { validation } = blockaidData; if ( validation && @@ -728,9 +724,11 @@ export const getSiteSecurityLevel = ({ /** * Collapses a transaction Blockaid scan into a single SecurityLevel (or null - * when nothing is flagged) for the reusable BlockaidBanner. A simulation error - * is treated as suspicious. The dev override takes precedence. Shared by the - * swap review and the dApp sign-transaction flow so both read one source. + * when nothing is flagged) for the reusable BlockaidBanner. A risk verdict + * wins over a simulation revert: Malicious and Warning stay malicious / + * suspicious, and a revert with no risk verdict is LIKELY_TO_FAIL. The dev + * override takes precedence. Shared by the swap review and the dApp + * sign-transaction flow so both read one source. */ export const getTransactionSecurityLevel = ( txScanResult: BlockAidScanTxResult | null | undefined, @@ -748,11 +746,6 @@ export const getTransactionSecurityLevel = ( const { simulation, validation } = txScanResult; - // Handle simulation error - treat as suspicious - if (simulation && "error" in simulation) { - return SecurityLevel.SUSPICIOUS; - } - if (validation && "result_type" in validation) { const resultType = validation.result_type; if (resultType === "Malicious") { @@ -763,6 +756,11 @@ export const getTransactionSecurityLevel = ( } } + // A revert is "this will fail". It is not a risk verdict. + if (simulation && "error" in simulation) { + return SecurityLevel.LIKELY_TO_FAIL; + } + if (isUnableToScan) { return SecurityLevel.UNABLE_TO_SCAN; } diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index 45fc6b4ac5..7a4975a88d 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -370,6 +370,7 @@ "Leave feedback about Blockaid warnings and messages": "Leave feedback about Blockaid warnings and messages", "Ledger will not display the transaction details in the device display prior to signing so make sure you only interact with applications you know and trust.": "Ledger will not display the transaction details in the device display prior to signing so make sure you only interact with applications you know and trust.", "Let’s go": "Let’s go", + "Likely to fail": "Likely to fail", "Limit": "Limit", "Limited trading data": "Limited trading data", "Links": "Links", @@ -703,6 +704,8 @@ "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.": "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.", "This transaction does not appear safe for the following reasons": "This transaction does not appear safe for the following reasons", "This transaction does not appear safe for the following reasons.": "This transaction does not appear safe for the following reasons.", + "This transaction is expected to fail for the following reasons.": "This transaction is expected to fail for the following reasons.", + "This transaction is likely to fail": "This transaction is likely to fail", "This transaction was flagged as malicious": "This transaction was flagged as malicious", "This transaction was flagged as malicious (override active)": "This transaction was flagged as malicious (override active)", "This transaction was flagged as suspicious": "This transaction was flagged as suspicious", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 34b51ea54c..294fa544ac 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -370,6 +370,7 @@ "Leave feedback about Blockaid warnings and messages": "Deixar feedback sobre avisos e mensagens do Blockaid", "Ledger will not display the transaction details in the device display prior to signing so make sure you only interact with applications you know and trust.": "O Ledger não exibirá os detalhes da transação na tela do dispositivo antes de assinar, então certifique-se de interagir apenas com aplicativos que você conhece e confia.", "Let’s go": "Vamos lá", + "Likely to fail": "Provável falha", "Limit": "Limite", "Limited trading data": "Dados de negociação limitados", "Links": "Links", @@ -703,6 +704,8 @@ "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.": "Esta transação desativa a chave mestra da sua conta. Você pode perder permanentemente o acesso a esta conta, a menos que outro assinante com peso suficiente seja adicionado.", "This transaction does not appear safe for the following reasons": "Esta transação não parece segura pelos seguintes motivos", "This transaction does not appear safe for the following reasons.": "Esta transação não parece segura pelos seguintes motivos.", + "This transaction is expected to fail for the following reasons.": "Esta transação deve falhar pelos seguintes motivos.", + "This transaction is likely to fail": "Esta transação provavelmente vai falhar", "This transaction was flagged as malicious": "Esta transação foi marcada como maliciosa", "This transaction was flagged as malicious (override active)": "Esta transação foi sinalizada como maliciosa (substituição ativa)", "This transaction was flagged as suspicious": "Esta transação foi marcada como suspeita", diff --git a/extension/src/popup/views/SignTransaction/index.tsx b/extension/src/popup/views/SignTransaction/index.tsx index 2f06c75b38..33dcbc4b2c 100644 --- a/extension/src/popup/views/SignTransaction/index.tsx +++ b/extension/src/popup/views/SignTransaction/index.tsx @@ -54,7 +54,11 @@ import { MemoRequiredLabel, } from "popup/components/WarningMessages"; import { BlockaidBanner } from "popup/components/BlockaidBanner"; -import { SecurityLevel, mergeSecurityLevels } from "popup/constants/blockaid"; +import { + BlockaidWarning, + SecurityLevel, + mergeSecurityLevels, +} from "popup/constants/blockaid"; import { useShouldTreatTxAsUnableToScan, useIsTxSuspicious, @@ -322,8 +326,9 @@ export const SignTransaction = () => { (scanResult.validation.result_type === "Malicious" || scanResult.validation.result_type === "Warning") ); - const hasSimulationError = - scanResult && scanResult.simulation && "error" in scanResult.simulation; + const hasSimulationError = !!( + scanResult?.simulation && "error" in scanResult.simulation + ); const showBlockAidDetails = isUnableToScan || hasSimulationError || @@ -333,7 +338,6 @@ export const SignTransaction = () => { (scanResult?.validation && "result_type" in scanResult.validation && scanResult.validation.result_type === "Malicious") || - hasSimulationError || isSiteMalicious; // Single Blockaid verdict for the banner: the worst of the transaction scan @@ -357,6 +361,22 @@ export const SignTransaction = () => { ]); const blockaidBannerEntity = txSecurityLevel ? "transaction" : "site"; + // The details sheet only receives the transaction scan, so carry the site + // verdict into it. Otherwise a reverting tx from a malicious site would open + // a "Likely to fail" sheet that never mentions the site risk the banner + // reported. + const isSiteFlagged = isSiteMalicious || isSiteSuspicious; + const blockaidSheetSiteWarnings: BlockaidWarning[] = isSiteFlagged + ? [ + { + description: isSiteMalicious + ? t("This site was flagged as malicious") + : t("This site was flagged as suspicious"), + isError: isSiteMalicious, + }, + ] + : []; + if (_networkPassphrase !== networkPassphrase) { return ( { {isOnBlockaidSheet ? ( setIsOnBlockaidSheet(false)} /> ) : ( diff --git a/extension/src/popup/views/__tests__/SignTransaction.test.tsx b/extension/src/popup/views/__tests__/SignTransaction.test.tsx index 31edaedf2d..338a75250a 100644 --- a/extension/src/popup/views/__tests__/SignTransaction.test.tsx +++ b/extension/src/popup/views/__tests__/SignTransaction.test.tsx @@ -35,6 +35,7 @@ import { RequestState } from "constants/request"; import { TRANSACTION_WARNING } from "constants/transaction"; import { AppDataType } from "helpers/hooks/useGetAppData"; import * as SigningFlowHooks from "popup/helpers/useSetupSigningFlow"; +import * as BlockaidHelpers from "popup/helpers/blockaid"; import { ShowOverlayStatus } from "popup/ducks/transactionSubmission"; import { WalletType } from "@shared/constants/hardwareWallet"; import { sortBalances } from "popup/helpers/account"; @@ -1190,10 +1191,147 @@ describe("SignTransactions", () => { ); await waitFor(() => screen.getByTestId("SignTransaction")); expect(screen.getByTestId("sign-tx-blockaid-banner")).toHaveTextContent( - "This transaction was flagged as suspicious", + "This transaction is likely to fail", ); expect(screen.getByText("Confirm anyway")).toBeDefined(); }); + it("keeps the malicious site verdict in the details sheet when a benign tx is expected to fail", async () => { + const getSiteSecurityStatesSpy = jest + .spyOn(BlockaidHelpers, "getSiteSecurityStates") + .mockReturnValue({ + isMalicious: true, + isSuspicious: false, + isUnableToScan: false, + }); + let currentSignTxDataMock = { + state: { + state: RequestState.SUCCESS, + data: { + type: AppDataType.RESOLVED, + scanResult: { + simulation: { + status: "Error" as any, + error: "This transaction is expected to fail" as any, + }, + validation: null, + request_id: "1", + }, + icons: {}, + balances: { + balances: sortBalances(mockBalances.balances), + isFunded: true, + subentryCount: 0, + }, + publicKey: mockAccounts[0].publicKey, + signFlowState: { + allAccounts: mockAccounts, + accountNotFound: false, + currentAccount: mockAccounts[0], + }, + applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, + networkDetails: { + ...defaultSettingsState.networkDetails, + networkPassphrase: "Test SDF Network ; September 2015", + }, + siteScanData: null, + blockaidOverrideState: null, + }, + error: null, + }, + fetchData: jest.fn(), + } as ReturnType; + jest + .spyOn(SignTxDataHooks, "useGetSignTxData") + .mockReturnValue(currentSignTxDataMock); + jest.spyOn(ApiInternal, "loadSettings").mockImplementation(() => + Promise.resolve({ + networkDetails: { + ...defaultSettingsState.networkDetails, + networkPassphrase: "Test SDF Network ; September 2015", + networkName: "Test Net", + }, + networksList: DEFAULT_NETWORKS, + hiddenAssets: {}, + allowList: { + "Test Net": { + [mockAccounts[0].publicKey]: ["laboratory.stellar.org"], + }, + }, + error: "", + isDataSharingAllowed: false, + isMemoValidationEnabled: false, + isHideDustEnabled: true, + isOpenSidebarByDefault: false, + settingsState: SettingsState.SUCCESS, + isSorobanPublicEnabled: false, + isRpcHealthy: true, + userNotification: { + enabled: false, + message: "", + }, + isExperimentalModeEnabled: false, + isHashSigningEnabled: false, + isNonSSLEnabled: false, + experimentalFeaturesState: SettingsState.SUCCESS, + assetsLists: DEFAULT_ASSETS_LISTS, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + }), + ); + const transaction = TransactionBuilder.fromXdr( + transactions.classic, + Networks.TESTNET, + ) as Transaction; + const op = transaction.operations[0]; + jest.spyOn(Stellar, "getTransactionInfo").mockImplementation(() => ({ + ...mockTransactionInfo, + transactionXdr: transactions.classic, + transaction: { + ...mockTransactionInfo.transaction, + _networkPassphrase: Networks.TESTNET, + _operations: [op], + }, + isHttpsDomain: false, + domain: "laboratory.stellar.org", + uuid: "123-123-123-123-123", + })); + render( + + + , + ); + await waitFor(() => screen.getByTestId("SignTransaction")); + fireEvent.click(screen.getByTestId("sign-tx-blockaid-banner")); + await waitFor(() => screen.getByText("Do not proceed")); + expect(screen.queryByText("Likely to fail")).toBeNull(); + expect( + screen.getByText("This site was flagged as malicious"), + ).toBeDefined(); + expect( + screen.getByText("This transaction is expected to fail"), + ).toBeDefined(); + getSiteSecurityStatesSpy.mockRestore(); + }); it("renders blockaid scan label when tx is malicious", async () => { let currentSignTxDataMock = { state: {