Skip to content

fix: Fix first time interaction alert to check real recipient and contract address #36763

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
+68 −50
Closed

fix: Fix first time interaction alert to check real recipient and contract address #36763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77a0eda
Initial
OGPoyraz Sep 24, 2025
f8297de
Fix unit tests
OGPoyraz Oct 10, 2025
01064ff
Merge branch 'main' into ogp/35595
OGPoyraz Oct 10, 2025
526afa2
Merge branch 'main' into ogp/35595
OGPoyraz Oct 10, 2025
a9b8a2b
Merge branch 'main' into ogp/35595
OGPoyraz Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 41 additions & 35 deletions ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { RowAlertKey } from '../../../../../components/app/confirm/info/row/cons
import { genUnapprovedTokenTransferConfirmation } from '../../../../../../test/data/confirmations/token-transfer';
import {
TrustSignalDisplayState,
useTrustSignal,
useTrustSignals,
} from '../../../../../hooks/useTrustSignals';
import { getExperience } from '../../../../../../shared/constants/verification';
import { EXPERIENCES_TYPE } from '../../../../../../shared/constants/first-party-contracts';
Expand All @@ -21,7 +21,7 @@ jest.mock('../../../../../hooks/useTrustSignals', () => {
const actual = jest.requireActual('../../../../../hooks/useTrustSignals');
return {
...actual,
useTrustSignal: jest.fn(),
useTrustSignals: jest.fn(),
};
});

Expand All @@ -43,18 +43,11 @@ const TRANSACTION_META_MOCK = {
type: TransactionType.contractInteraction,
txParams: {
from: ACCOUNT_ADDRESS_MOCK,
to: '0xrecipient',
},
time: new Date().getTime() - 10000,
} as TransactionMeta;

jest.mock('../../../../../hooks/useTrustSignals', () => {
const actual = jest.requireActual('../../../../../hooks/useTrustSignals');
return {
...actual,
useTrustSignal: jest.fn(),
};
});

function runHook({
currentConfirmation,
internalAccountAddresses,
Expand Down Expand Up @@ -86,15 +79,17 @@ function runHook({
}

describe('useFirstTimeInteractionAlert', () => {
const mockUseTrustSignal = jest.mocked(useTrustSignal);
const mockUseTrustSignals = jest.mocked(useTrustSignals);
const mockIsFirstPartyContract = jest.mocked(getExperience);

beforeEach(() => {
jest.resetAllMocks();
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Unknown,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Unknown,
label: null,
},
]);
mockIsFirstPartyContract.mockReturnValue(undefined);
});

Expand Down Expand Up @@ -182,10 +177,12 @@ describe('useFirstTimeInteractionAlert', () => {
});

it('returns no alerts if transaction destination is verified', () => {
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Verified,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Verified,
label: null,
},
]);

const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
Expand All @@ -205,10 +202,12 @@ describe('useFirstTimeInteractionAlert', () => {
});

it('returns no alerts if token transfer recipient is verified', () => {
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Verified,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Verified,
label: null,
},
]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Mock Returns Single Signal, Fails Multi-Address Testing

The useTrustSignals mock in tests consistently returns a single trust signal, even though the hook can request signals for multiple addresses (like contract and recipient in token transfers). This prevents accurate testing of multi-address scenarios and conditions that rely on checking all or some trust signals.

Additional Locations (1)

Fix in Cursor Fix in Web


const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
Expand All @@ -231,6 +230,7 @@ describe('useFirstTimeInteractionAlert', () => {
it('returns alert if isFirstTimeInteraction is true', () => {
const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
type: TransactionType.simpleSend,
isFirstTimeInteraction: true,
};
const alerts = runHook({
Expand Down Expand Up @@ -270,10 +270,12 @@ describe('useFirstTimeInteractionAlert', () => {
expect(alerts).toEqual([]);
});
it('does not return alert when trust signal is still loading', () => {
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Loading,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Loading,
label: null,
},
]);

const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
Expand All @@ -294,10 +296,12 @@ describe('useFirstTimeInteractionAlert', () => {

it('returns alert when trust signal is loaded but address is not verified', () => {
// Simulate the trust signal being loaded with non-verified result
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Unknown,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Unknown,
label: null,
},
]);

const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
Expand All @@ -318,10 +322,12 @@ describe('useFirstTimeInteractionAlert', () => {
});

it('does not return alert when trust signal shows verified address', () => {
mockUseTrustSignal.mockReturnValue({
state: TrustSignalDisplayState.Verified,
label: null,
});
mockUseTrustSignals.mockReturnValue([
{
state: TrustSignalDisplayState.Verified,
label: null,
},
]);

const firstTimeConfirmation = {
...TRANSACTION_META_MOCK,
Expand Down
42 changes: 27 additions & 15 deletions ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useConfirmContext } from '../../../context/confirm';
import { getInternalAccounts } from '../../../../../selectors';
import { useTransferRecipient } from '../../../components/confirm/info/hooks/useTransferRecipient';
import {
useTrustSignal,
useTrustSignals,
TrustSignalDisplayState,
} from '../../../../../hooks/useTrustSignals';
import { getExperience } from '../../../../../../shared/constants/verification';
Expand All @@ -21,34 +21,46 @@ export function useFirstTimeInteractionAlert(): Alert[] {
const t = useI18nContext();
const { currentConfirmation } = useConfirmContext<TransactionMeta>();
const internalAccounts = useSelector(getInternalAccounts);
const to = useTransferRecipient();
const recipient = useTransferRecipient();
const { isFirstTimeInteraction, chainId, txParams } =
currentConfirmation ?? {};
const recipient = (txParams?.to ?? '0x') as Hex;
const transactionTo = txParams?.to;

const isInternalAccount = internalAccounts.some(
(account) => account.address?.toLowerCase() === to?.toLowerCase(),
(account) => account.address?.toLowerCase() === recipient?.toLowerCase(),
);

const { state: trustSignalDisplayState } = useTrustSignal(
recipient || '',
NameType.ETHEREUM_ADDRESS,
);
const trustSignalRequests = [
{ value: transactionTo || '', type: NameType.ETHEREUM_ADDRESS },
];

if (recipient !== transactionTo) {
trustSignalRequests.push({
value: recipient || '',
type: NameType.ETHEREUM_ADDRESS,
});
}

const trustSignalResults = useTrustSignals(trustSignalRequests);

const isVerifiedAddress =
trustSignalDisplayState === TrustSignalDisplayState.Verified;
const hasAnyLoadingTrustSignal = trustSignalResults.some(
(result) => result.state === TrustSignalDisplayState.Loading,
);

const isFirstPartyContract = Boolean(getExperience(recipient, chainId));
const isAllAddressesVerified = trustSignalResults.every(
(result) => result.state === TrustSignalDisplayState.Verified,
);

const isTrustSignalLoading =
trustSignalDisplayState === TrustSignalDisplayState.Loading;
const isFirstPartyContract = Boolean(
getExperience(txParams?.to as Hex, chainId),
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Undefined txParams.to Breaks Contract Checks

The getExperience function now passes txParams?.to directly, removing the '0x' fallback. If txParams.to is undefined, getExperience will receive undefined instead of a hex string, which could cause runtime errors or incorrect first-party contract checks.

Fix in Cursor Fix in Web


const showAlert =
!isInternalAccount &&
isFirstTimeInteraction &&
!isVerifiedAddress &&
!isAllAddressesVerified &&
!isFirstPartyContract &&
!isTrustSignalLoading;
!hasAnyLoadingTrustSignal;

return useMemo(() => {
// If isFirstTimeInteraction is undefined that means it's either disabled or error in accounts API
Expand Down
Loading