-
-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PhishingDetect to support IPFS CID blocking #4465
Conversation
…ead of only hostname now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I just had one comment, but this otherwise looks good to me. You may need to run yarn lint:fix
to handle the lint failures.
* @param hostname - the hostname of the URL. | ||
* @returns the href property of a URL object. | ||
*/ | ||
export const formatHostnameToUrl = (hostname: string): string => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this function is only used in the tests. Do we want the package to export this function? If we only want to share it in tests, we've typically solved this by:
- extracting it to a file in
packages/phishing-controller/tests/
(we may have to create that directory first) - adding
./tests
toinclude
inpackages/phishing-controller/tsconfig.json
And now we can import this file from the tests/
directory instead of the src/
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Elliot Winkler <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Explanation
This allows us to push IPFS cIDs to the blocklist and have them blocked by the eth-phishing-detect service. We only need to note the CIDs and not block the gateways.
Related issue: MetaMask/eth-phishing-detect#19532
After merged, then we can merge:
References
Changelog
@metamask/package-a
@metamask/package-b
Checklist