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

Normalize transaction data length #3990

Merged
merged 4 commits into from
Mar 1, 2024

Conversation

matthewwalsh0
Copy link
Member

Explanation

The transaction parameters include the data property which is a hex string.

When processed by the RPC provider / node, data with an odd-length is treated the same as an even-length value, meaning the data value of 0x123 is treated the same as 0x0123.

This PR applies equivalent normalization in the TransactionController to ensure the client UIs receive the data in a consistent format with an even length hex string.

Also export the normalizeTransactionParams method so it can be utilised directly by any layer of the client to format transaction parameters with the same logic.

Changelog

@metamask/transaction-controller

  • ADDED: Add normalizeTransactionParams method.
  • CHANGED: Normalize data property into an even length hex string.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@matthewwalsh0 matthewwalsh0 requested a review from a team as a code owner February 28, 2024 09:37
@matthewwalsh0 matthewwalsh0 added the team-confirmations-system DEPRECATED: please use "team-confirmations" label instead label Feb 28, 2024
@matthewwalsh0
Copy link
Member Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "10.0.0-preview.0a7589e7",
  "@metamask-previews/address-book-controller": "3.1.7-preview.0a7589e7",
  "@metamask-previews/announcement-controller": "5.0.2-preview.0a7589e7",
  "@metamask-previews/approval-controller": "5.1.2-preview.0a7589e7",
  "@metamask-previews/assets-controllers": "25.0.0-preview.0a7589e7",
  "@metamask-previews/base-controller": "4.1.1-preview.0a7589e7",
  "@metamask-previews/build-utils": "1.0.2-preview.0a7589e7",
  "@metamask-previews/composable-controller": "5.0.1-preview.0a7589e7",
  "@metamask-previews/controller-utils": "8.0.3-preview.0a7589e7",
  "@metamask-previews/ens-controller": "9.0.0-preview.0a7589e7",
  "@metamask-previews/eth-json-rpc-provider": "2.3.2-preview.0a7589e7",
  "@metamask-previews/gas-fee-controller": "13.0.1-preview.0a7589e7",
  "@metamask-previews/json-rpc-engine": "7.3.2-preview.0a7589e7",
  "@metamask-previews/json-rpc-middleware-stream": "6.0.2-preview.0a7589e7",
  "@metamask-previews/keyring-controller": "12.2.0-preview.0a7589e7",
  "@metamask-previews/logging-controller": "2.0.2-preview.0a7589e7",
  "@metamask-previews/message-manager": "7.3.8-preview.0a7589e7",
  "@metamask-previews/name-controller": "5.0.0-preview.0a7589e7",
  "@metamask-previews/network-controller": "17.2.0-preview.0a7589e7",
  "@metamask-previews/notification-controller": "4.0.2-preview.0a7589e7",
  "@metamask-previews/permission-controller": "8.0.1-preview.0a7589e7",
  "@metamask-previews/permission-log-controller": "0.0.0-preview.0a7589e7",
  "@metamask-previews/phishing-controller": "8.0.2-preview.0a7589e7",
  "@metamask-previews/polling-controller": "5.0.0-preview.0a7589e7",
  "@metamask-previews/preferences-controller": "7.0.0-preview.0a7589e7",
  "@metamask-previews/queued-request-controller": "0.5.0-preview.0a7589e7",
  "@metamask-previews/rate-limit-controller": "4.0.2-preview.0a7589e7",
  "@metamask-previews/selected-network-controller": "8.0.0-preview.0a7589e7",
  "@metamask-previews/signature-controller": "12.0.0-preview.0a7589e7",
  "@metamask-previews/transaction-controller": "23.1.0-preview.0a7589e7",
  "@metamask-previews/user-operation-controller": "4.0.0-preview.0a7589e7"
}

Copy link
Contributor

@dbrans dbrans left a comment

Choose a reason for hiding this comment

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

Perfection!

@matthewwalsh0 matthewwalsh0 merged commit c5dbb37 into main Mar 1, 2024
139 checks passed
@matthewwalsh0 matthewwalsh0 deleted the fix/normalize-transaction-data-even-length branch March 1, 2024 10:43
mcmire added a commit to MetaMask/metamask-extension that referenced this pull request Mar 7, 2024
We need to fix this because of
<MetaMask/core#3990>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-confirmations-system DEPRECATED: please use "team-confirmations" label instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants