Skip to content

Conversation

@mathieuartu
Copy link
Contributor

@mathieuartu mathieuartu commented Dec 3, 2025

Related to: https://consensyssoftware.atlassian.net/browse/MUL-1316

Examples


Note

Introduces EthKeyringWrapper with shared Ethereum submitRequest logic and updates HdKeyringV2 to extend it, adding RPC param structs, an EthKeyring alias, tests, and required deps.

  • Keyring API:
    • New: eth/v2/EthKeyringWrapper abstract class with common Ethereum request routing (submitRequest) and EthKeyringMethod enum.
    • RPC: Add EthGetEncryptionPublicKeyOptionsStruct and EthGetEncryptionPublicKeyParamsStruct (address, optional options).
    • Exports: Re-export from eth/v2 and include new wrapper in eth/index.
    • Deps: Add @ethereumjs/tx and @metamask/eth-sig-util.
    • Tests: Add comprehensive unit tests for wrapper routing and errors.
  • Keyring ETH HD:
    • Refactor: HdKeyringV2 now extends EthKeyringWrapper; removes duplicate routing, uses shared methods list including encryption/app-key/EIP-7702.
    • Tests: Adjust to pass address/options for eth_getEncryptionPublicKey and align with wrapper behavior.
    • Changelog: Note extension of EthKeyringWrapper.
  • Keyring Utils:
    • New: Export EthKeyring type alias for legacy Keyring.
    • Changelog: Document alias addition.

Written by Cursor Bugbot for commit 22051b5. This will update automatically on new commits. Configure here.

@mathieuartu mathieuartu self-assigned this Dec 3, 2025
@mathieuartu mathieuartu added the team-accounts This should be handled by the Accounts Team label Dec 3, 2025
@mathieuartu mathieuartu force-pushed the feat/base-ethkeyring-v2 branch from 6944620 to e75db9c Compare December 3, 2025 20:40
@mathieuartu mathieuartu changed the title feat: add EthKeyringWrapper for common Eth logic feat: add EthKeyringWrapper for common Eth logic around KeyringV2 implementation Dec 3, 2025
@mathieuartu mathieuartu marked this pull request as ready for review December 3, 2025 20:49
@mathieuartu mathieuartu requested a review from a team as a code owner December 3, 2025 20:49
@mathieuartu mathieuartu added this pull request to the merge queue Dec 4, 2025
Merged via the queue into main with commit d6a0c20 Dec 4, 2025
29 checks passed
@mathieuartu mathieuartu deleted the feat/base-ethkeyring-v2 branch December 4, 2025 13:41
* @returns The result of the signing operation.
*/
async submitRequest(request: KeyringRequest): Promise<Json> {
const { method, params } = request.request;
Copy link

Choose a reason for hiding this comment

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

Bug: Missing default value for params may cause unexpected errors

The params destructuring lacks a default value. The previous implementation in HdKeyringV2.submitRequest used const { method, params = [] } = request.request; but EthKeyringWrapper uses const { method, params } = request.request;. Since params is defined as exactOptional in KeyringRequestStruct, requests without params are valid and will result in params being undefined. This could cause confusing validation errors when assert is called on undefined instead of an empty array.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-accounts This should be handled by the Accounts Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants