Skip to content

Use method middleware for watchAsset#9943

Merged
rekmarks merged 7 commits intodevelopfrom
watch-asset-method-middleware
Dec 2, 2020
Merged

Use method middleware for watchAsset#9943
rekmarks merged 7 commits intodevelopfrom
watch-asset-method-middleware

Conversation

@rekmarks
Copy link
Copy Markdown
Member

@rekmarks rekmarks commented Nov 24, 2020 •
edited
Loading

This PR moves the wallet_watchAsset (also metamask_watchAsset) implementation to the method middleware. The functionality is unchanged. This was a planned change, but I did it now because an upcoming PR will remove the web3 usage logging method, which would leave the method middleware temporarily unused.

This method will be further refactored and moved out of the preferences controller once the approval controller is merged.

There are no e2e tests for this method, but I tested it manually with the following:

await ethereum.request({
    method: 'wallet_watchAsset',
    params: {
        type: 'ERC20',
        options: {
            address: '0xdac17f958d2ee523a2206206994597c13d831ec7',
            symbol: 'USDT',
            decimals: 6,
        },
    },
})

Ref: https://eips.ethereum.org/EIPS/eip-747

@rekmarks rekmarks requested a review from a team as a code owner November 24, 2020 20:18
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 24, 2020

Builds ready [7444e79]
Page Load Metrics (439 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33100642412
domContentLoaded31161943810852
load31262043910852
domInteractive31061843810852

@rekmarks rekmarks force-pushed the watch-asset-method-middleware branch from b274085 to e44a459 Compare November 24, 2020 21:12
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 24, 2020

Builds ready [a39500b]
Page Load Metrics (467 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29104592311
domContentLoaded2635494668139
load2645514678139
domInteractive2625494658139

@rekmarks
Copy link
Copy Markdown
Member Author

rekmarks commented Nov 24, 2020

I just realized that #8640 will add another method to the method middleware, so this is not urgent. Nevertheless, it's here when we're ready!

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Dec 1, 2020

Builds ready [fb817ac]
Page Load Metrics (519 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32118672612
domContentLoaded36771851712359
load36972051912359
domInteractive36771851612359

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Dec 1, 2020

Builds ready [e817079]
Page Load Metrics (456 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3090552311
domContentLoaded3015484558441
load3025494568541
domInteractive3005484548441

NiranjanaBinoy
NiranjanaBinoy approved these changes Dec 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

brad-decker
brad-decker approved these changes Dec 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

brad-decker
brad-decker approved these changes Dec 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

@rekmarks rekmarks merged commit c3eb272 into develop Dec 2, 2020
@rekmarks rekmarks deleted the watch-asset-method-middleware branch December 2, 2020 16:49
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Gudahtt Gudahtt Awaiting requested review from Gudahtt

@estebanmino estebanmino Awaiting requested review from estebanmino

2 more reviewers

@brad-decker brad-decker brad-decker approved these changes

@NiranjanaBinoy NiranjanaBinoy NiranjanaBinoy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Gudahtt Gudahtt

@estebanmino estebanmino

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rekmarks @metamaskbot @brad-decker @NiranjanaBinoy @Gudahtt @estebanmino