Refactor remaining web3-provider-engine methods
#5627
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-remaining-web3-provider-engine-methods
branch
from
f3f8850
to
e469a70
Compare
Gudahtt
force-pushed
the
refactor-remaining-web3-provider-engine-methods
branch
from
e469a70
to
d9b319b
Compare
BelfordZ
reviewed
Feb 16, 2023
Gudahtt
force-pushed
the
refactor-remaining-web3-provider-engine-methods
branch
from
d9b319b
to
75510d1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Gudahtt
added
the
needs-dev-review
The remaining non-static methods from `web3-provider-engine` have been migrated to `RPCMethodMiddleware.ts` with the rest of the method handlers. The two methods left were `personal_ecRecover` and `parity_checkRequest`. `personal_ecRecover` will recover the signing account from a signature. This operation requires no private keys. Commonly dapps will do this themselves. Persumably it was added to the dapp API at some point for convenience. It has been preserved just to avoid making breaking changes to the dapp API. `parity_checkRequest` would return the result of a parity signature or transaction. It returned `null` if none were found. The parity signature and request methods have been throwing an error for some time now, so there is never any result to check. As a result, this method has been returning `null` for all requests in practice. It has been preserved just to avoid making breaking changes to the dapp API. This relates to #5513
Gudahtt
force-pushed
the
refactor-remaining-web3-provider-engine-methods
branch
from
75510d1
to
47672a2
Compare
Cal-L
approved these changes
Apr 6, 2023
There was a problem hiding this comment.
Currently discussing next steps on removing parity rpc methods. https://consensys.slack.com/archives/CBW7S9FSN/p1680805775457939
Since there is no change in functionality here, LGTM
Gudahtt
removed
the
needs-dev-review
Gudahtt
added
Spot Check on the Release Build
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 remaining non-static methods from
web3-provider-enginehave been migrated toRPCMethodMiddleware.tswith the rest of the method handlers. The two methods left werepersonal_ecRecoverandparity_checkRequest.personal_ecRecoverwill recover the signing account from a signature. This operation requires no private keys. Commonly dapps will do this themselves. Persumably it was added to the dapp API at some point for convenience. It has been preserved just to avoid making breaking changes to the dapp API.parity_checkRequestwould return the result of a parity signature or transaction. It returnednullif none were found. The parity signature and request methods have been throwing an error for some time now, so there is never any result to check. As a result, this method has been returningnullfor all requests in practice. It has been preserved just to avoid making breaking changes to the dapp API.Issue
This relates to #5513
Checklist