…
|
||
---|---|---|
.. | ||
eth_sign.png | ||
footer.png | ||
header.png | ||
personal_sign.png | ||
README.md | ||
signature_request_old.png | ||
signature_request_proposed.png | ||
siwe.png | ||
v1.png | ||
v3.png | ||
v4.png |
Refactoring - Signature Request Pages
This document details the plan to refactor and cleanup Signature Request pages in Metamask.
The current structure of Signature Request pages look like:
-
Simple ETH Signature
-
Personal Signature
-
Typed Data - V1
-
Typed Data - V3
-
Typed Data - V4
-
SIWE Signature
The current flow of control for Signature Request looks like:
The proposed flow of control:
Proposed Refactoring:
There are many areas in above flow where the code can be improved upon to cleanup and make it more extensible:
-
Refactor message managers:
Currently we have 3 different message managers:
Above message managers handle different types of message requests sent by DAPP. There is a lot of code duplication between the 3 classes. We can extract out a parent class and move duplicated code to it. Functions that can be moved to parent class:
-
constructor
- variable initialisation:this.messages = []; this.metricsEvent = metricsEvent;
-
unapprovedMsgCount
-
getUnapprovedMsgs
-
addUnapprovedMessageAsync
- partially -
addUnapprovedMessage
- partially -
addMsg
-
getMsg
-
approveMessage
-
setMsgStatusApproved
-
setMsgStatusSigned
-
prepMsgForSigning
-
rejectMsg
-
errorMessage
-
clearUnapproved
-
_setMsgStatus
-
_updateMsg
-
_saveMsgList
Much de-duplication can be achieved in message menagers.
-
-
Refactoring Routing to Signature Request pages:
Current navigation to Signature Request pages is un-necessarily complicated. It can be simplified to great extent.
- To the navigation code in Home component add condition to check if there are unapproved messages and route to path
/singature-request
. - In Routes component render pages/confirm-signature-request for path
/singature-request
. - Refactor out conf-tx.js into pages/confirm-signature-request component. #17240
- To the navigation code in Home component add condition to check if there are unapproved messages and route to path
-
Refactoring in conf-tx.js
- While fixing routing conf-tx.js will be renamed to pages/confirm-signature-request component
- We are getting rid of confirm-transaction component from the flow. Thus, we need to ensure that any required logic from the component is extracted into a reusable hook and included in pages/confirm-signature-request. This check for valid transaction id should be made re-usable and extracted out.
- The component can be converted to a function react component and use selectors to get state and get rid of
mapStatToProps
. #17239 - Various callbacks to sign message, cancel request, etc for different type of messaged can be moved to respective child components.
- On component mount/update if there are no unapproved messages redirect to
mostRecentlyOverviewedPage
as here. - Not pass values like these to child components which can be obtained in child components using selectors.
- Extract logic here to show success modal for previously confirmed transaction into separate hook.
- Question - this check for message params look confusing - is it possible for a message request to not have message params or for other transactions to have message params. Here we could have just checked for unapproved messages, why are we checking for pending transactions also ?
-
Refactoring component rendering Signature Request Pages
There are 3 different signature request components responsible to render different signature request pages:
- signature-request-original - ETH sign, personal sign, sign typed data V1
- signature-request - Sign typed data V3, V4
- signature-request-siwe - SignatureRequestSIWE (Sign-In with Ethereum)
All, the signature request pages (except SIWE) are very similar, the differing part in these pages is the message section. And there is a lot of code duplication between components - signature-request-original and signature-request.
-
Refactoring in signature-request-original
-
Rename, this component takes care of ETH sign, personal sign, sign typed data V1 requests. Let's rename it accordingly.
-
Get rid of container components and for other components migrate to functional react components.
-
Move this metrics event to pages/confirm-signature-request as it is applicable to all the signature requests types.
-
Header or we can say upper half of the page of all signature request pages (except SIWE) are very similar, this can be extracted into a reusable component used across both signature-request-original and signature-request:
-
LedgerInstructions can also be moved to the header.
-
Create a reuable footer component and use it across all confirmation pages. #17237
-
Create a reuable component for Cancel All requests for use across signature request pages Code.
-
Extract getNetrowkName into a reuable hook / utility method.
-
msgHexToText to be made a utility method.
-
Extract renderBody into a reusable component.
-
-
Refactoring in signature-request
- Get rid of container components and for other components migrate to functional react components.
- Reuse the Header component created for signature-request pages
- Reuse the footer component created for confirmation pages.
- Extract formatWallet into a utility method.
-
Refactoring in signature-request-siwe
- Footer component use
PageContainerFooter
can be converted into a footer component for all confirmation pages. #17237
- Footer component use