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 signEip7702Authorization to KeyringController #5301

Merged
merged 20 commits into from
Feb 17, 2025

Conversation

jeffsmale90
Copy link
Contributor

@jeffsmale90 jeffsmale90 commented Feb 7, 2025

EIP-7702 defines a new struct Authorization which represents authority to set a pointer to a contract address at an EOA - effectively making the EOA perform as a smart contract.

This change integrates the new signEip7702Authorization method added to eth- simple and hd keyrings in MetaMask/accounts#182. This is exposed via KeyringController.signEip7702Authorization as well as the message handler KeyringController:SignEip7702Authorization.

See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7702.md for details

Changelog:

@metamask/keyring-controller:

Added

  • Add support for 7702 via new signEip7702Authorization method and corresponding KeyringController:signEip7702Authorization message handler. #5301

Changed

  • Bump @metamask/eth-hd-keyring from ^7.0.4 to ^10.0.0 #5301
  • Bump @metamask/eth-simple-keyring from ^6.0.5 to ^8.1.0 #5301
  • Bump @metamask/eth-sig-util from ^8.0.0 to ^8.2.0 #5301
  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/accounts-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/address-book-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/approval-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/assets-controllers:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/base-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/bridge-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/build-utils:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/ens-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/eth-json-rpc-provider:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/gas-fee-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/json-rpc-engine:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/json-rpc-middleware-stream:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/message-manager:

Changed

  • Bump @metamask/eth-sig-util from ^8.0.0 to ^8.2.0 #5301
  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/multichain-network-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/multichain-transactions-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/multichain:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/name-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/network-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/notification-services-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/permission-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/permission-log-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/polling-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/queued-request-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/rate-limit-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/remote-feature-flag-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/selected-network-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/signature-controller:

Changed

  • Bump @metamask/eth-sig-util from ^8.0.0 to ^8.2.0 #5301
  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/token-search-discovery-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/transaction-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/user-operation-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@metamask/name-controller:

Changed

  • Bump @metamask/utils from ^11.1.0 to ^11.2.0 #5301

@jeffsmale90 jeffsmale90 changed the title Add signEIP7702Authorization to KeyringController [draft] Add signEIP7702Authorization to KeyringController Feb 7, 2025
@jeffsmale90 jeffsmale90 force-pushed the feat/7702-sign-authorization branch from e225fe3 to 082e817 Compare February 12, 2025 00:53
@jeffsmale90 jeffsmale90 force-pushed the feat/7702-sign-authorization branch from 082e817 to 47935e0 Compare February 12, 2025 01:09
Copy link

socket-security bot commented Feb 12, 2025

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] None 0 140 kB metamaskbot
npm/@metamask/[email protected] None 0 78.4 kB metamaskbot

View full report↗︎

@jeffsmale90 jeffsmale90 changed the title [draft] Add signEIP7702Authorization to KeyringController Add signEIP7702Authorization to KeyringController Feb 12, 2025
@jeffsmale90 jeffsmale90 marked this pull request as ready for review February 12, 2025 01:43
@jeffsmale90 jeffsmale90 requested review from a team as code owners February 12, 2025 01:43
@jeffsmale90 jeffsmale90 requested a review from a team as a code owner February 12, 2025 06:18
@matthewwalsh0
Copy link
Member

@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": "23.0.1-preview-df91d059",
  "@metamask-previews/address-book-controller": "6.0.3-preview-df91d059",
  "@metamask-previews/announcement-controller": "7.0.3-preview-df91d059",
  "@metamask-previews/approval-controller": "7.1.3-preview-df91d059",
  "@metamask-previews/assets-controllers": "49.0.0-preview-df91d059",
  "@metamask-previews/base-controller": "8.0.0-preview-df91d059",
  "@metamask-previews/build-utils": "3.0.3-preview-df91d059",
  "@metamask-previews/composable-controller": "11.0.0-preview-df91d059",
  "@metamask-previews/controller-utils": "11.5.0-preview-df91d059",
  "@metamask-previews/earn-controller": "0.2.1-preview-df91d059",
  "@metamask-previews/ens-controller": "15.0.2-preview-df91d059",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-df91d059",
  "@metamask-previews/gas-fee-controller": "22.0.3-preview-df91d059",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-df91d059",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-df91d059",
  "@metamask-previews/keyring-controller": "19.0.7-preview-df91d059",
  "@metamask-previews/logging-controller": "6.0.4-preview-df91d059",
  "@metamask-previews/message-manager": "12.0.1-preview-df91d059",
  "@metamask-previews/multichain": "2.1.1-preview-df91d059",
  "@metamask-previews/multichain-transactions-controller": "0.3.0-preview-df91d059",
  "@metamask-previews/name-controller": "8.0.3-preview-df91d059",
  "@metamask-previews/network-controller": "22.2.1-preview-df91d059",
  "@metamask-previews/notification-services-controller": "0.20.1-preview-df91d059",
  "@metamask-previews/permission-controller": "11.0.6-preview-df91d059",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-df91d059",
  "@metamask-previews/phishing-controller": "12.3.2-preview-df91d059",
  "@metamask-previews/polling-controller": "12.0.3-preview-df91d059",
  "@metamask-previews/preferences-controller": "15.0.2-preview-df91d059",
  "@metamask-previews/profile-sync-controller": "7.0.1-preview-df91d059",
  "@metamask-previews/queued-request-controller": "9.0.1-preview-df91d059",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-df91d059",
  "@metamask-previews/remote-feature-flag-controller": "1.4.0-preview-df91d059",
  "@metamask-previews/selected-network-controller": "21.0.1-preview-df91d059",
  "@metamask-previews/signature-controller": "23.2.1-preview-df91d059",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-df91d059",
  "@metamask-previews/transaction-controller": "45.1.0-preview-df91d059",
  "@metamask-previews/user-operation-controller": "24.0.1-preview-df91d059"
}

ccharly
ccharly previously approved these changes Feb 14, 2025
@jeffsmale90 jeffsmale90 enabled auto-merge (squash) February 14, 2025 11:25
matthewwalsh0
matthewwalsh0 previously approved these changes Feb 14, 2025
@ccharly ccharly changed the title Add signEip7702Authorization to KeyringController feat: add signEip7702Authorization to KeyringController Feb 14, 2025
@jeffsmale90 jeffsmale90 disabled auto-merge February 14, 2025 18:35
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Could you update the changelog for the keying controller package? Certainly the keyring version bumps would be breaking changes at least. I don't see changes documented right now in either the PR description or the changelog file - we do require them to be listed in one place or the other, so that we know how to release this.


return await keyring.signEip7702Authorization(from, [
chainId,
contractAddress as Hex,
Copy link
Member

@Gudahtt Gudahtt Feb 17, 2025

Choose a reason for hiding this comment

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

Nit: This cast is suppressing a TypeScript error about the undefined case. We asserted just a couple lines above that this value can be undefined. Could you clarify why we're overriding that error here? Is the address expected to be undefined in some cases or not?

Edit: I think I see what happened now, suggestion on this here: https://github.com/MetaMask/core/pull/5301/files#r1958425721

Comment on lines +1170 to +1176
const contractAddress = ethNormalize(params.contractAddress) as
| Hex
| undefined;

return await keyring.signEip7702Authorization(from, [
chainId,
contractAddress as Hex,
Copy link
Member

@Gudahtt Gudahtt Feb 17, 2025

Choose a reason for hiding this comment

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

Nit: Perhaps this is what you meant to do:

Suggested change
const contractAddress = ethNormalize(params.contractAddress) as
| Hex
| undefined;
return await keyring.signEip7702Authorization(from, [
chainId,
contractAddress as Hex,
const contractAddress = ethNormalize(params.contractAddress) as Hex;
return await keyring.signEip7702Authorization(from, [
chainId,
contractAddress,

Ideally we'd validate the response from ethNormalize is non-falsy as well, or use a normalization function that returns Hex rather than string | undefined. But I see we make this assumption elsewhere already, so we can leave this improvement for a future PR.

@jeffsmale90 jeffsmale90 requested review from a team as code owners February 17, 2025 17:20
@jeffsmale90 jeffsmale90 merged commit d0d1556 into main Feb 17, 2025
259 checks passed
@jeffsmale90 jeffsmale90 deleted the feat/7702-sign-authorization branch February 17, 2025 17:25
ccharly pushed a commit that referenced this pull request Feb 19, 2025
…s provided (#5353)

#5301 introduced a new method for signing EIP-7702 Authorizations. 

This change adds validation to ensure that `contractAddress` is
specified, rather than type asserting the `contractAddress` to `Hex`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants