Refactor RPC getAccounts usage
#5620
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gudahtt
force-pushed
the
refactor-rpc-get-accounts-usage
branch
from
a43e181
to
2c47bef
Compare
Gudahtt
added
the
needs-dev-review
Gudahtt
force-pushed
the
refactor-rpc-get-accounts-usage
branch
from
2c47bef
to
c823904
Compare
Gudahtt
commented
Mar 16, 2023
Gudahtt
force-pushed
the
refactor-rpc-get-accounts-usage
branch
from
c823904
to
c0ac2a6
Compare
The `getAccounts` function is defined and used in two different places in the RPC pipeline: `RPCMethodMiddleware.ts` and `web3-provider-engine`. The second one has been removed, so now all usage is in `RPCMethodMiddleware.ts`. `getAccounts` is passed into `web3-provider-engine` and used in the `HookedWalletSubprovider`: https://github.com/MetaMask/web3-provider-engine/blob/cf612f898002833c36730d23972fe4c4dd483c76/subproviders/hooked-wallet.js It is used in these methods: * `eth_coinbase` * `eth_accounts` * `parity_defaultAccount` It's also called to validate the sender, for the following methods: * `eth_signTypedData` * `eth_signTypedData_v3` * `eth_signTypedData_v4` * `encryption_public_key` * `eth_decryptMessage` * `personal_sign` * `eth_sign` * `eth_signTransaction` * `eth_sendTransaction` Of these methods, most of them are intercepted in `RPCMethodMiddleware.ts`, so the requests never make it to `web3-provider-engine`. These three methods will make their way there: `parity_defaultAccount`, `encryption_public_key`, and `eth_decryptMessage`. The decryption- related messages rely on constructor parameters that mobile does not pass in, so those always throw an error. The only method this hook is used for in practice is `parity_defaultAccount`. The `parity_defaultAccount` method was added to `RPCMethodMiddleware.ts, so now that method won't make it to `web3-provider-engine` either. Functionally it is the same as `eth_coinbase`, so the same implementation has been used. This relates to #5513
Gudahtt
force-pushed
the
refactor-rpc-get-accounts-usage
branch
from
c0ac2a6
to
ecc33fc
Compare
Cal-L
approved these changes
Apr 6, 2023
Cal-L
added
team-mobile-client
team-wallet-framework
Spot Check on the Release Build
|
Also applied spot check on release since no functional changes |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Development & PR Process
release-xxlabel to identify the PR slated for a upcoming release (will be used in release discussion)needs-dev-reviewlabel when work is completedneeds-qalabel when dev review is completedQA Passedlabel when QA has signed offDescription
The
getAccountsfunction is defined and used in two different places in the RPC pipeline:RPCMethodMiddleware.tsandweb3-provider-engine. The second one has been removed, so now all usage is inRPCMethodMiddleware.ts.getAccountsis passed intoweb3-provider-engineand used in theHookedWalletSubprovider: https://github.com/MetaMask/web3-provider-engine/blob/cf612f898002833c36730d23972fe4c4dd483c76/subproviders/hooked-wallet.jsIt is used in these methods:
eth_coinbaseeth_accountsparity_defaultAccountIt's also called to validate the sender, for the following methods:
eth_signTypedDataeth_signTypedData_v3eth_signTypedData_v4encryption_public_keyeth_decryptMessagepersonal_signeth_signeth_signTransactioneth_sendTransactionOf these methods, most of them are intercepted in
RPCMethodMiddleware.ts, so the requests never make it toweb3-provider-engine.These three methods will make their way there:
parity_defaultAccount,encryption_public_key, andeth_decryptMessage. The decryption- related messages rely on constructor parameters that mobile does not pass in, so those always throw an error. The only method this hook is used for in practice isparity_defaultAccount.The
parity_defaultAccountmethod was added toRPCMethodMiddleware.ts, so now that method won't make it toweb3-provider-engineeither. Functionally it is the same aseth_coinbase, so the same implementation has been used.This should have no functional changes.
Issue
This relates to #5513
Checklist