Skip to content
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

feat: Add transaction alert when sending data to EOA #30141

Merged
merged 1 commit into from
Feb 6, 2025
Merged

Conversation

pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Feb 5, 2025

Description

Adds new alert for when the user sends a "simple send" native asset transaction with hex data to an EOA account.

This intends to alert the user when they think they are doing a contract interaction but perhaps trigger the confirmation on the wrong network, in which the recipient address is in fact an EOA.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2926

Manual testing steps

// 1. Switch to the chain you want to broadcast the message on.
// 2. Open the browser console, paste this code and execute it.
// 3. Confirm the transaction on MetaMask.

const ON_CHAIN_MESSAGE = `Why, let the stricken deer go weep,
The hart ungallèd play.
For some must watch while some must sleep.
So runs the world away.`;

async function connectMetaMask() {
  try {
    const accounts = await window.ethereum.request({
      method: 'eth_requestAccounts',
    });
    console.log('Connected account:', accounts[0]);
    return accounts[0];
  } catch (error) {
    console.error('User rejected the request:', error);
    throw error;
  }
}

function stringToHex(str) {
  const encoder = new TextEncoder();
  const bytes = encoder.encode(str);
  return '0x' + Array.from(bytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
}

async function createTransactionWithHexData() {
  try {
    const fromAddress = await connectMetaMask();
    const hexData = stringToHex(ON_CHAIN_MESSAGE);

    const transactionParameters = {
      to: fromAddress,
      from: fromAddress,
      value: '0x0', // Optional: Amount of ETH to send
      data: hexData,
    };

    const txHash = await window.ethereum.request({
      method: 'eth_sendTransaction',
      params: [transactionParameters],
    });

    console.log('Transaction hash:', txHash);
    return txHash;
  } catch (error) {
    console.error('Error creating transaction:', error);
  }
}

createTransactionWithHexData();

Screenshots/Recordings

Before

After

Screenshot 2025-02-05 at 16 43 59 Screenshot 2025-02-05 at 16 44 02 Screenshot 2025-02-05 at 16 44 05

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@pedronfigueiredo pedronfigueiredo added the team-confirmations Push issues to confirmations team label Feb 5, 2025
@pedronfigueiredo pedronfigueiredo self-assigned this Feb 5, 2025
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner February 5, 2025 18:22
Copy link
Contributor

github-actions bot commented Feb 5, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@pedronfigueiredo pedronfigueiredo changed the title feat: Add transaction alert for sending data for EOA feat: Add transaction alert for sending data to EOA Feb 5, 2025
@pedronfigueiredo pedronfigueiredo changed the title feat: Add transaction alert for sending data to EOA feat: Add transaction alert when sending data to EOA Feb 5, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [7e91a59]
Page Load Metrics (1654 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14432052166117483
domContentLoaded14262000163015675
load14392015165416579
domInteractive1990482512
backgroundConnect1080292110
firstReactRender1598433014
getState54713147
initialActions00000
loadScripts10161473119113263
setupStore893212512
uiStartup167126531910251121
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 4.81 KiB (0.06%)
  • common: 271 Bytes (0.00%)

Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Changes look good 👍

@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Feb 6, 2025
Merged via the queue into main with commit 6d5b3dd Feb 6, 2025
75 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/2926 branch February 6, 2025 14:24
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
@metamaskbot metamaskbot added the release-12.13.0 Issue or pull request that will be included in release 12.13.0 label Feb 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.13.0 Issue or pull request that will be included in release 12.13.0 team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants