search

gnosis / safe-browser-extension

MIT License
18 stars 8 forks source link

isMetaMask currently set to true #82

Closed villanuevawill closed 5 years ago

villanuevawill commented 5 years ago

Description

I've been playing around with the new extension and noticed isMetaMask is being set to true.

Should engine.isMetaMask = !0 be updated to engine.isMetaMask = !1 https://github.com/gnosis/safe-browser-extension/blob/master/extension/utils/SafeProvider.js#L21-L22

Is this a remnant to continue compatibility? Thanks, and I have enjoyed playing/setting up the new extension so far :)

rmeissner commented 5 years ago

This is to enable as many dapps as possible to work with the extension. This way we want to enable everybody to report to us any non-working dapps (https://github.com/gnosis/safe-browser-extension/issues/81).

In the future we will change this and won't implement the isMetaMask. there was some internal discussion on how to express to the dapp that we are the Safe extension. We would like to prevent that every provider implements their own isSomeCoolProvider.

This becomes interesting as soon as we start exposing an interface to make use of batched transactions (https://github.com/gnosis/safe-contracts/blob/development/contracts/libraries/MultiSend.sol).

Also we want to enable the use of EIP-1271 (contract signatures) to cover some cases where traditional signatures are currently used.

Because of this we were also thinking about creating some new rpc calls, that are more focused towards wallet use cases (or should probably not be implemented by every node). But this is in an very early stage and we will focus on this more now (see https://ethereum-magicians.org/t/add-wallet-methods-to-improve-dapp-to-wallet-interaction/1848)

villanuevawill commented 5 years ago

We would like to prevent that every provider implements their own isSomeCoolProvider. <- this makes sense.

Yep, our app currently would need to work with an EIP-1271-based system. We have the user sign to verify ownership of the public address associated with their account.

Very interested to see how the exposure of batch transaction interfaces move along... And will follow the wallet use cases proposal - this makes sense.