Skip to content
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

Add the iframe provider to support parent window based wallets #388

Closed
wants to merge 20 commits into from
Closed

Add the iframe provider to support parent window based wallets #388

wants to merge 20 commits into from

Conversation

moodysalem
Copy link

Adds the iframe provider polyfill to index.html so that wallets like Ethvault.xyz can be used to interact with ENS

@makoto
Copy link
Member

makoto commented Jul 22, 2019

Do you have anyone audited your package? I have a bit of hesitance including a package I don't know if it's secure or not. For other embedded wallets like Portis, Formatic, Universal Login, Authereum etc, we (= dapp developer) switch to their provider/sdk only if end users make conscious choice (eg: by clicking a button with the vendor logo) of using their solution. On the other hand, yours is included by default so I would like to have extra assurance that there is no known security threat .

@moodysalem
Copy link
Author

I can change the PR to be more similar to those other providers-the polyfill is a small wrapper around the iframe provider. It’s only included by default if the window is in an iframe. I haven’t had an audit but I’m aware of at least one issue that is mitigated by limiting the origins that may embed the dapp in an iframe
ethvault/iframe-provider-polyfill#1

Let me know what steps you would like to see to get this merged

jefflau and others added 3 commits July 23, 2019 15:49
* Remove contents utils file. Use @ensdomains/ui instead

* Update onion placeholder

* Add onion link
@@ -64,5 +64,6 @@
To begin the development, run `npm start` or `yarn start`.
To create a production bundle, use `npm run build` or `yarn build`.
-->
<script src="https://unpkg.com/@ethvault/[email protected]/dist/index.js" integrity="sha384-qx1eG3ocmrXxXBwEL+qnP7je980Depwy/J7keRHVnqfEFVXHJ0ruzYIsUX767NEH" crossorigin="anonymous"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Does this have to be included here rather than inside package.json while keeping the integrity hash? We prefer to manage all third party packages in one place.

@moodysalem
Copy link
Author

I did a rebase on the patch branch that github created, which for some reason added all the commits to this PR, so I opened a new one

#405

@moodysalem moodysalem closed this Jul 23, 2019
@moodysalem moodysalem deleted the patch-1 branch July 23, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants