Security provider check (OpenSea) #16584

filipsekulic · 2022-11-18T13:04:54Z

Explanation

The security provider is an optional feature for users, similar to an RPC provider for a new chain. The security provider will be called before the wallet action is presented to the user, and can flag possible malicious transactions that the wallet provider can raise to the user.
More information: https://www.notion.so/opensea/Wallet-Tx-Security-PoC-b6a926e1cb6448638d147696cd8f5937

Fixes Wallet Tx Security Provider: Send the transaction or signature request payload information to a service provider #16281

In order to use the security provider check, Transaction security check should be enabled from the Experimental tab.

This functionality will be fully implemented in the following PR (#16258) where the warning message will be shown.

Manual testing steps

Go to Experimental tab and enable the Transaction security check.
Go to test dapp.
Send a transaction or sign a message and the security provider check will be called.

github-actions · 2022-11-18T13:05:06Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2022-11-21T10:30:16Z

Builds ready [1f5db07]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2265 ± 126 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	104	2285	336	591	284
		domContentLoaded	1758	2681	2246	275	132
		load	1758	2681	2265	262	126
		domInteractive	1758	2681	2246	275	132

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2562 bytes
ui: 20 bytes
common: 18 bytes

highlights:

storybook

app/scripts/lib/personal-message-manager.js

metamaskbot · 2022-12-08T13:10:20Z

Builds ready [df954ff]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (3246 ± 225 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	122	3993	705	1141	548
		domContentLoaded	2574	4117	3221	471	226
		load	2575	4117	3246	469	225
		domInteractive	2574	4117	3221	471	226

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2554 bytes
ui: 20 bytes
common: 18 bytes

highlights:

storybook

metamaskbot · 2022-12-08T13:42:59Z

Builds ready [504ce81]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2019 ± 98 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	102	167	120	16	8
		domContentLoaded	1571	2365	1997	203	98
		load	1571	2365	2019	203	98
		domInteractive	1571	2365	1997	203	98

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2554 bytes
ui: 20 bytes
common: 18 bytes

highlights:

storybook

app/scripts/controllers/transactions/index.js

app/scripts/controllers/transactions/index.test.js

app/scripts/metamask-controller.js

metamaskbot · 2022-12-12T11:57:19Z

Builds ready [06e642f]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2159 ± 213 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	95	1255	169	250	120
		domContentLoaded	1637	3869	2137	447	215
		load	1645	3869	2159	443	213
		domInteractive	1637	3869	2137	447	215

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2578 bytes
ui: 20 bytes
common: 18 bytes

highlights:

storybook

metamaskbot · 2022-12-12T13:41:21Z

Builds ready [352f1d9]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2270 ± 170 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	110	1385	196	273	131
		domContentLoaded	1753	3440	2256	359	172
		load	1753	3440	2270	355	170
		domInteractive	1753	3440	2256	359	172

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2578 bytes
ui: 20 bytes
common: 18 bytes

highlights:

storybook

jpuri

Code changes look good, but will be nice to have some more test coverage as it touches important functionality.

metamaskbot · 2022-12-20T14:15:47Z

Builds ready [d93b98c]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2357 ± 111 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	97	221	145	30	15
		domContentLoaded	1700	2803	2334	242	116
		load	1712	2803	2357	231	111
		domInteractive	1700	2803	2334	242	116

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2587 bytes
ui: 38 bytes
common: 0 bytes

highlights:

storybook

app/scripts/controllers/transactions/index.test.js

jpuri · 2023-01-05T03:50:03Z

app/scripts/lib/security-provider-helpers.js

+  );
+
+  return await response.json();
+}


test cases can be added for above new file.

Did we ever add test cases?

jpuri

Code changes look good, some small non-blocking feedback there.

seaona · 2023-01-09T11:37:41Z

The toggle looks good when I tested using the feature flag.

There are 2 problems related to the Search Function that might/might not be related to this PR:

There is no matching when we use the description words for the search. On the other cases it does match with text description, so this might be something related with this PR?

search-security.mp4

The transaction and security words are matched but not the check word from the feature title. This seems related to an existing issue, but we should verify it, to make sure

filipsekulic · 2023-01-10T10:42:16Z

The toggle looks good when I tested using the feature flag.

There are 2 problems related to the Search Function that might/might not be related to this PR:

There is no matching when we use the description words for the search. On the other cases it does match with text description, so this might be something related with this PR?

search-security.mp4

The transaction and security words are matched but not the check word from the feature title. This seems related to an existing issue, but we should verify it, to make sure

Hey @seaona!
I rebased this PR. You can check the new design in the Experimental tab. I think the problem with description search was not introduced in this PR. I tried to reproduce the issue you mentioned from both the develop and this PR's branch. If I am right, there is a problem with all the the settings descriptions when searching it, not just the ones from the Experimental tab.

jpuri

LGTM

seaona · 2023-01-19T09:33:54Z

@filipsekulic thank you! It looks indeed a separate issue with the Search, not related to these changes

blackdevelopa

LGTM. However, just noticed we have a failing CI.

…provider

filipsekulic · 2023-01-23T14:08:39Z

LGTM. However, just noticed we have a failing CI.

@blackdevelopa
Thanks! I've just updated the branch.

metamaskbot · 2023-01-23T14:12:21Z

Builds ready [ed0cf96]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1314 ± 112 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	98	259	138	40	19
		domContentLoaded	999	1751	1293	219	105
		load	1061	1830	1314	233	112
		domInteractive	999	1751	1293	219	105

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 2676 bytes
ui: 38 bytes
common: 0 bytes

filipsekulic self-assigned this Nov 18, 2022

filipsekulic marked this pull request as ready for review November 22, 2022 11:43

filipsekulic requested a review from a team as a code owner November 22, 2022 11:43

filipsekulic requested a review from brad-decker November 22, 2022 11:43

jpuri reviewed Dec 6, 2022

View reviewed changes

app/scripts/lib/personal-message-manager.js Show resolved Hide resolved

filipsekulic mentioned this pull request Dec 7, 2022

Update UI for transaction security check in settings #16830

Merged

filipsekulic force-pushed the send-tx-or-signature-request-to-security-provider branch from df954ff to 504ce81 Compare December 8, 2022 13:12

filipsekulic requested a review from jpuri December 8, 2022 13:34

jpuri reviewed Dec 12, 2022

View reviewed changes

app/scripts/controllers/transactions/index.js Show resolved Hide resolved

jpuri reviewed Dec 12, 2022

View reviewed changes

app/scripts/controllers/transactions/index.js Outdated Show resolved Hide resolved

jpuri reviewed Dec 12, 2022

View reviewed changes

app/scripts/controllers/transactions/index.test.js Show resolved Hide resolved

jpuri reviewed Dec 12, 2022

View reviewed changes

app/scripts/metamask-controller.js Outdated Show resolved Hide resolved

filipsekulic force-pushed the send-tx-or-signature-request-to-security-provider branch from 06e642f to 352f1d9 Compare December 12, 2022 13:10

filipsekulic requested a review from jpuri December 12, 2022 13:32

jpuri reviewed Dec 14, 2022

View reviewed changes

filipsekulic requested a review from jpuri December 19, 2022 14:47

filipsekulic force-pushed the send-tx-or-signature-request-to-security-provider branch from c082a3c to d93b98c Compare December 20, 2022 13:44

jpuri reviewed Jan 5, 2023

View reviewed changes

app/scripts/controllers/transactions/index.test.js Show resolved Hide resolved

jpuri reviewed Jan 5, 2023

View reviewed changes

jpuri previously approved these changes Jan 5, 2023

View reviewed changes

filipsekulic dismissed jpuri’s stale review via b9fe62f January 10, 2023 08:30

filipsekulic force-pushed the send-tx-or-signature-request-to-security-provider branch from d93b98c to b9fe62f Compare January 10, 2023 08:30

filipsekulic added 18 commits January 10, 2023 09:34

Initial commit

8551446

Added more code

9f235c9

Code refactor

05e8a3f

Lint fixed

1a8cce2

Enabled fetchind data from the OpenSea server

16d5b61

Code refactor

d9b59d2

Lint fixed

a72a48e

Fixed a unit test

671713b

Fixed tests

b836cea

Fixed tests

ac94c26

Mocha fixed

0d27e09

Refactored code and deleted comments

1eefe0f

Lint fixed

b261083

Fixed e2e test

fd3efdc

Fixed e2e test

2680557

Fetch msgId within the check method

007ea5e

Applied reqested changes

31fe72d

Added a test case and refactored a piece of code

1ba6012

filipsekulic force-pushed the send-tx-or-signature-request-to-security-provider branch from b9fe62f to 1ba6012 Compare January 10, 2023 08:34

jpuri approved these changes Jan 13, 2023

View reviewed changes

blackdevelopa approved these changes Jan 23, 2023

View reviewed changes

Merge branch 'develop' into send-tx-or-signature-request-to-security-…

ed0cf96

…provider

jpuri merged commit 27d3416 into develop Jan 23, 2023

jpuri deleted the send-tx-or-signature-request-to-security-provider branch January 23, 2023 14:32

github-actions bot locked and limited conversation to collaborators Jan 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security provider check (OpenSea) #16584

Security provider check (OpenSea) #16584

filipsekulic commented Nov 18, 2022 •

edited

Loading

github-actions bot commented Nov 18, 2022

metamaskbot commented Nov 21, 2022

metamaskbot commented Dec 8, 2022

metamaskbot commented Dec 8, 2022

metamaskbot commented Dec 12, 2022

metamaskbot commented Dec 12, 2022

jpuri left a comment

metamaskbot commented Dec 20, 2022

jpuri Jan 5, 2023

brad-decker Jan 23, 2023

jpuri left a comment

seaona commented Jan 9, 2023

filipsekulic commented Jan 10, 2023

jpuri left a comment

seaona commented Jan 19, 2023

blackdevelopa left a comment

filipsekulic commented Jan 23, 2023

metamaskbot commented Jan 23, 2023

Security provider check (OpenSea) #16584

Security provider check (OpenSea) #16584

Conversation

filipsekulic commented Nov 18, 2022 • edited Loading

Explanation

Manual testing steps

github-actions bot commented Nov 18, 2022

metamaskbot commented Nov 21, 2022

highlights:

metamaskbot commented Dec 8, 2022

highlights:

metamaskbot commented Dec 8, 2022

highlights:

metamaskbot commented Dec 12, 2022

highlights:

metamaskbot commented Dec 12, 2022

highlights:

jpuri left a comment

Choose a reason for hiding this comment

metamaskbot commented Dec 20, 2022

highlights:

jpuri Jan 5, 2023

Choose a reason for hiding this comment

brad-decker Jan 23, 2023

Choose a reason for hiding this comment

jpuri left a comment

Choose a reason for hiding this comment

seaona commented Jan 9, 2023

filipsekulic commented Jan 10, 2023

jpuri left a comment

Choose a reason for hiding this comment

seaona commented Jan 19, 2023

blackdevelopa left a comment

Choose a reason for hiding this comment

filipsekulic commented Jan 23, 2023

metamaskbot commented Jan 23, 2023

filipsekulic commented Nov 18, 2022 •

edited

Loading