types: export EIP1193Provider and LegacyEthereumProvider interfaces #140

legobeat · 2023-09-13T06:15:53Z

Description

This exports two new TypeScript declarations:

EIP1993Provider: Encodes a provider for EIP1993
LegacyEthereumProvider: Encodes a legacy Ethereum JSON RPC provider. Based on original work in Introduce LegacyEthereumProvider interface eth-json-rpc-provider#14

Motivation

As far as I know, there is no common explicit interface for the de-facto interface implemented by various Ethereum Json RPC providers. Across the MetaMask npm packages, there are currently use of differing implementations with mostly overlapping interfaces.

This PR introduces a common interface that can be applied regardless of implementations and used in MetaMask packages.

Example of situation where this will be useful

socket-security · 2023-09-13T06:16:58Z

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

socket-security · 2023-09-13T06:17:01Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

legobeat · 2023-09-13T06:19:16Z

@SocketSecurity ignore @metamask/[email protected]

Mrtenz

In Snaps we typically just import MetaMaskInpageProvider from @metamask/providers to get types for this. Do you think it's useful to export this from @metamask/utils?

src/index.ts

src/eth-provider-types.ts

legobeat · 2023-09-13T21:51:34Z

In Snaps we typically just import MetaMaskInpageProvider from @metamask/providers to get types for this. Do you think it's useful to export this from @metamask/utils?

~~MetaMaskInpageprovider is a class which would satisfy the interface here.~~ The intention here is to supply that common interface that can be used regardless of implementation when interfacing with internal providers like EthQuery et al.

legobeat · 2023-09-13T22:02:12Z

Related: MetaMask/eth-json-rpc-provider#14

jiexi · 2023-10-03T18:05:45Z

looks good. I think we can do without EthJsonRpcProvider as I don't see when we would use the extra properties being defined

legobeat · 2023-10-04T04:36:25Z

Updated:

Removed EthJsonRpcProvider
Added LegacyEthereumProvider
- Unification of https://github.com/MetaMask/eth-json-rpc-provider/pull/14/files#diff-eb5ad9e4a58317c1917e5ce8ce71ffca663c150c81ad4ed36706d7295821249dR10 and https://github.com/MetaMask/eth-query/blob/main/index.d.ts

legobeat · 2023-10-04T06:33:42Z

@SocketSecurity ignore [email protected]
@SocketSecurity ignore @ethersproject/[email protected]
@SocketSecurity ignore [email protected]

src/json-rpc-provider-types.ts

MajorLift · 2023-10-04T17:06:31Z

src/json-rpc-provider-types.ts

+  /*
+  send<Result extends Json = Json>(
+    method: string,
+    params: any[],
+  ): Promise<Result>;
+  */


Does defining send as a generic cause issues? I think this comment should be removed either way.

mcmire

In general I'm good with defining a unified legacy type like this as I think it brings our knowledge of these concepts into one place, but considering the abstract nature of the utils package, we should make sure that this type is grounded, i.e., we've tested this type with the libraries we want to support before we merge this to ensure that no type errors occur.

mcmire · 2023-10-04T18:43:24Z

src/json-rpc-provider-types.ts

+
+type LegacyEthersProvider = {
+  /**
+   * Send a provider request asynchronously. (ethers v5 Web3Provider)


To make this a little cleaner, what do you think about adding a comment above LegacyEthersProvider which explains what it represents? My understanding is that this interface represents the portion of Ethers v5's ExternalProvider interface which intersects with our legacy provider — is that correct?

mcmire · 2023-10-04T18:43:51Z

src/json-rpc-provider-types.ts

+   * @param params - Array with method parameters.
+   * @returns A promise resolving with the result of the RPC call, or rejecting on failure.
+   */
+  send(method: string, params: any[]): Promise<Json>;


Are we trying to align with Ethers v5 here? If so, its signature for send is here: https://github.com/ethers-io/ethers.js/blob/v5.7.2/packages/providers/src.ts/web3-provider.ts#L19

src/json-rpc-provider-types.ts

mcmire · 2023-10-04T18:55:36Z

src/json-rpc-provider-types.ts

+   * @returns A Promise that resolves with the result of the RPC method,
+   * or rejects if an error is encountered.
+   */
+  request<Params extends JsonRpcParams, Result extends Json>(


Is this method signature always compatible with the one in BaseProvider? If not, do we plan on changing BaseProvider to match this?

mcmire · 2023-10-04T18:58:22Z

src/json-rpc-provider-types.ts

+    params: any[],
+  ): Promise<Result>;
+  */
+};


Do we also need to include sendAsync in this type? Or do we need to define two types for Ethers v5, one that contains send, another that contains sendAsync?

Ethers itself (which this is intended to represent) only has send, not sendAsync: https://docs.ethers.org/v5/api/providers/jsonrpc-provider/#JsonRpcProvider-send

src/json-rpc-provider-types.ts

mcmire · 2023-10-04T19:05:20Z

src/json-rpc-provider-types.ts

+  ): void;
+};
+
+type LegacyWeb3Provider = {


Do we need this type? Our provider technically supports send, but it's even more deprecated than sendAsync is. I can't recall a time where we've used it internally.

mcmire · 2023-10-04T19:06:30Z

src/json-rpc-provider-types.ts

+   * @param callback - A function that is called upon the success or failure of the request.
+   */
+  sendAsync<Result extends Json = Json>(
+    req: Partial<JsonRpcRequest>,


Hmm, this seems overly strict. The params here can be anything — eth-query / ethjs-query doesn't put a restriction on what they can be.

Where did you derive this type from?

From here: https://github.com/MetaMask/eth-query/blob/main/index.d.ts

First, we can see here that the missing parts are auto-filled in ethjs-rpc (actual provider for eth-query) so we can at least see that the same interface is implicit.

As for extraneous parameters: On one hand, the more backwards-compatible approach would be something like Json, EverythingButNull or even unknown. But at the same time I'm thinking that users of this type could benefit from spotting unintentionally unsupported API-usage (as any non-standard fields would have to be recognized by the actual provider).

Are we aware of any places where that could already be a thing?

Hmm... I'm wary of doing this, but I don't know why yet. I'll have to think about this.

- Remove EthJsonRpcProvider - Add LegacyEthereumProvider defining sendAsync providers

Co-authored-by: Elliot Winkler <[email protected]>

Co-authored-by: Jongsun Suh <[email protected]>

src/json-rpc-provider-types.ts

- For proof of correctness see https://tsplay.dev/mpQX7W. - TODO: move `EverythingButNull` into `misc.ts` and export.

src/json-rpc-provider-types.test-d.ts

…metamask/eth-query`

… repo `@metamask/eth-query`" This reverts commit 6a4ab9b.

This reverts commit 7b176aa.

## Explanation This PR implements the following incremental steps in the process for migrating `eth-json-rpc-provider` into the core monorepo: *** ### Phase B: Staging from `merged-packages/` #### 5. Port tags - See: #1800 <details> <summary>Push ported tags to core repo</summary> - [x] https://github.com/MetaMask/core/releases/tag/@metamask/[email protected] - [x] https://github.com/MetaMask/core/releases/tag/@metamask/[email protected] - [x] https://github.com/MetaMask/core/releases/tag/@metamask/[email protected] - [x] https://github.com/MetaMask/core/releases/tag/@metamask/[email protected] - [x] https://github.com/MetaMask/core/releases/tag/@metamask/[email protected] </details> <details> <summary>Verify that the tag diff links in CHANGELOG are working</summary> - [x] **WONTFIX**: https://github.com/MetaMask/core/compare/@metamask/[email protected] - [x] https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected] - [x] https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected] - [x] https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected] - [x] https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected] </details> ### Phase C: Integration into `packages/` #### 1. The big leap - [x] **Move migration target from `migrated-packages/` to `packages/`.** - [x] Run `yarn install` in the root directory. - [x] Check that all tests are passing in migration target by running `yarn workspace @metamask/<package-name> test`. #### 2. Update downstream repos - [x] Add tsconfig reference paths for migration target in downstream packages and root. - [x] Bump migration target version in downstream packages and root. #### 3. Linter fixes - [x] Apply yarn constraints fixes to migration target package.json file: `yarn constraints --fix` (run twice). - [x] Identify validator fixes for CHANGELOG using `yarn workspace @metamask/<package-name> changelog:validate` and apply the diffs. #### 4. Resolve downstream errors - [x] #1653 - If introducing the migration target breaks any downstream repos: - [x] Resolve simple errors - [x] Mark and ignore complex errors using `@ts-expect-error TODO:` annotations. - [x] Create a separate issue for resolving the marked errors as soon as the migration is completed. #### 5. Finalize merge - [x] Check that all tests are passing in all subpackages of core and CI. - [x] Merge `packages/<package-name>` directory into core main branch. *** See #1551 (comment) for an outline of the entire process. ## Next Steps - The next PR(s) will implement the final steps of the migration process (D-1 in the migration checklist). ## Blocked by - Dependencies: - [x] typescript bump: #1718 - [x] `@metamask/utils` bump: #1639 - Downstream type errors: - [x] #1653 - [ ] MetaMask/eth-json-rpc-provider#14 (ignored) - [ ] MetaMask/utils#140 (ignored) - Tag porting: - [x] #1802 - [x] "Unreleased" tag diff link shows entire history of core: https://github.com/MetaMask/core/compare/@metamask/[email protected] ## References - Contributes to #1685 - Contributes to #1551 ## Changelog  ### `@metamask/eth-json-rpc-provider` - **ADDED**: Migrated into the core monorepo. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate

legobeat added the question Further information is requested label Sep 13, 2023

legobeat marked this pull request as ready for review September 13, 2023 06:19

legobeat requested a review from a team as a code owner September 13, 2023 06:19

legobeat requested review from rekmarks, kumavis, Gudahtt and FrederikBolding September 13, 2023 06:28

Mrtenz requested changes Sep 13, 2023

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

src/eth-provider-types.ts Outdated Show resolved Hide resolved

legobeat mentioned this pull request Sep 13, 2023

Introduce LegacyEthereumProvider interface MetaMask/eth-json-rpc-provider#14

Open

legobeat marked this pull request as draft September 13, 2023 22:02

legobeat requested a review from mcmire September 13, 2023 22:02

legobeat mentioned this pull request Oct 3, 2023

deps: Bump @metamask/{eth-json-rpc-provider,rpc-errors} MetaMask/core#1653

Merged

14 tasks

MajorLift mentioned this pull request Oct 3, 2023

eth-json-rpc-provider migration - Integration into packages/ MetaMask/core#1738

Merged

33 tasks

MajorLift mentioned this pull request Oct 4, 2023

eth-json-rpc-provider migration - D: Repair breaking packages and finalize merge MetaMask/core#1770

Closed

9 tasks

deps: add @metamask/safe-event-emitter@^3.0.0

1ea71a6

legobeat force-pushed the types-provider branch 2 times, most recently from 0a8d867 to b759b1a Compare October 4, 2023 04:34

legobeat requested review from a team, Mrtenz and jiexi October 4, 2023 04:36

legobeat removed the question Further information is requested label Oct 4, 2023

legobeat marked this pull request as ready for review October 4, 2023 06:35

MajorLift reviewed Oct 4, 2023

View reviewed changes

src/json-rpc-provider-types.ts Show resolved Hide resolved

MajorLift reviewed Oct 4, 2023

View reviewed changes

src/json-rpc-provider-types.ts Outdated Show resolved Hide resolved

MajorLift reviewed Oct 4, 2023

View reviewed changes

mcmire reviewed Oct 4, 2023

View reviewed changes

legobeat marked this pull request as draft October 5, 2023 01:39

legobeat added 5 commits October 5, 2023 10:42

feat/types: export EIP1193Provider and EthJsonRpcProvider interfaces

35ec38a

chore: rename eth-provider-types.ts to json-rpc-provider-types.ts

de8d6ef

replace EthJsonRpcProvider with LegacyEthereumProvider

28a8129

- Remove EthJsonRpcProvider - Add LegacyEthereumProvider defining sendAsync providers

update LegacyEthereumProvider; add type-integration tests

a53994f

update provider types, add type-tests

e3c3771

legobeat force-pushed the types-provider branch from 049d23e to e3c3771 Compare October 5, 2023 01:45

Update LegacyEthereumProvider typedoc

47f7023

Co-authored-by: Elliot Winkler <[email protected]>

legobeat changed the title ~~types: export EIP1993Provider and LegacyEthereumProvider interfaces~~ types: export EIP1193Provider and LegacyEthereumProvider interfaces Oct 5, 2023

Update refs in comment

a8ddce8

Co-authored-by: Jongsun Suh <[email protected]>

MajorLift reviewed Oct 5, 2023

View reviewed changes

src/json-rpc-provider-types.ts Show resolved Hide resolved

legobeat and others added 2 commits October 10, 2023 23:34

Merge branch 'main' into types-provider

bbffb39

Update EverythingButNull to use less brittle type definition

7b176aa

- For proof of correctness see https://tsplay.dev/mpQX7W. - TODO: move `EverythingButNull` into `misc.ts` and export.

MajorLift reviewed Oct 11, 2023

View reviewed changes

src/json-rpc-provider-types.test-d.ts Show resolved Hide resolved

MajorLift and others added 3 commits October 11, 2023 14:11

Add @ts-expect-error for typing inconsistency with upstream repo `@…

6a4ab9b

…metamask/eth-query`

Revert "Add @ts-expect-error for typing inconsistency with upstream…

90c23d6

… repo `@metamask/eth-query`" This reverts commit 6a4ab9b.

Revert "Update EverythingButNull to use less brittle type definition "

199f1de

This reverts commit 7b176aa.

MajorLift mentioned this pull request Oct 11, 2023

Fix provider type errors in NetworkController and its downstream packages MetaMask/core#1823

Closed

legobeat and others added 3 commits October 16, 2023 11:34

Merge branch 'main' into types-provider

fe10d3c

Merge branch 'main' into types-provider

940bdc9

yarn dedupe

82ea00b

legobeat mentioned this pull request Nov 9, 2023

Align types with @metamask/eth-json-rpc-provider MetaMask/eth-query#21

Merged

rekmarks removed their request for review February 9, 2024 22:02

Mrtenz removed their request for review March 1, 2024 09:01

FrederikBolding removed their request for review April 19, 2024 08:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

types: export EIP1193Provider and LegacyEthereumProvider interfaces #140

types: export EIP1193Provider and LegacyEthereumProvider interfaces #140

legobeat commented Sep 13, 2023 •

edited

Loading

socket-security bot commented Sep 13, 2023 •

edited

Loading

socket-security bot commented Sep 13, 2023 •

edited

Loading

legobeat commented Sep 13, 2023

Mrtenz left a comment

legobeat commented Sep 13, 2023 •

edited

Loading

legobeat commented Sep 13, 2023

jiexi commented Oct 3, 2023

legobeat commented Oct 4, 2023 •

edited

Loading

legobeat commented Oct 4, 2023

MajorLift Oct 4, 2023 •

edited

Loading

mcmire left a comment

mcmire Oct 4, 2023

mcmire Oct 4, 2023

mcmire Oct 4, 2023

mcmire Oct 4, 2023

legobeat Oct 5, 2023

mcmire Oct 4, 2023

mcmire Oct 4, 2023

legobeat Oct 5, 2023

mcmire Oct 11, 2023

types: export EIP1193Provider and LegacyEthereumProvider interfaces #140

Are you sure you want to change the base?

types: export EIP1193Provider and LegacyEthereumProvider interfaces #140

Conversation

legobeat commented Sep 13, 2023 • edited Loading

Description

Motivation

Example of situation where this will be useful

socket-security bot commented Sep 13, 2023 • edited Loading

socket-security bot commented Sep 13, 2023 • edited Loading

legobeat commented Sep 13, 2023

Mrtenz left a comment

Choose a reason for hiding this comment

legobeat commented Sep 13, 2023 • edited Loading

legobeat commented Sep 13, 2023

jiexi commented Oct 3, 2023

legobeat commented Oct 4, 2023 • edited Loading

Updated:

legobeat commented Oct 4, 2023

MajorLift Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

mcmire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legobeat commented Sep 13, 2023 •

edited

Loading

socket-security bot commented Sep 13, 2023 •

edited

Loading

socket-security bot commented Sep 13, 2023 •

edited

Loading

legobeat commented Sep 13, 2023 •

edited

Loading

legobeat commented Oct 4, 2023 •

edited

Loading

MajorLift Oct 4, 2023 •

edited

Loading