MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.02k stars 4.91k forks source link

Third Party Wallet Integration #6054

Open MicahZoltu opened 5 years ago

MicahZoltu commented 5 years ago

TL;DR: I have a smart contract for holding funds (think of it like a multisig wallet). I would like to integrate it into MetaMask. What would the process look like for first getting approval for integration, then actually going about integrating. Assume that I'm willing to do the engineering work, but I don't want to do all of the work first only to have a PR rejected because it doesn't fit into the MM roadmap or something


A while back I wrote a wallet that was designed to be a hot wallet secured by a Ledger that had no recovery phrase written down. The idea is that if the ledger is damaged/lost, ownership of the smart contract can be recovered via a lengthy process, but as long as the ledger is functional/accessible the wallet is secured by it. The idea here is to solve the problem of "how do I securely store my recovery seed" by making the answer be, "you burn it with fire as soon as the Ledger is setup". In the wrench scenario, you merely need to give the attacker 3 wrong pin codes which will result in the ledger self destructing and the lengthy recovery process beginning (which can be secured by some form of cold storage like family member m of n multisig or a backup ledger in a bank vault). The code can be found at https://github.com/Zoltu/recoverable-wallet/blob/8ec801f15919bd04c59070a4a871f1d17cda04f9/contracts/recoverable-wallet.sol#L79-L144

The problem with it is (assuming I finish the CONSIDER on line 143) that the only way to interact with a dapp is by manually authoring a transaction and then signing it by hand with a tool like MEW or MyCrypto. The contract would become far more useful if it was integrated into a browser extension where the extension would convert any transaction the dapp requested signing for into a transaction that routes through the Recoverable Wallet contract.

One option would be for me to write my own browser extension. However, it would make more sense (ignoring coordination costs and risk for a moment) if it was instead integrated into MetaMask. Then instead of having two plugins that serve different target audiences, there was a single extension (MetaMask) that served both.

danfinlay commented 5 years ago

Hi Micah,

Great questions. We've done some development towards integrating multisig accounts as well, and currently have a roadmap that will let new contract accounts more easily integrate in the future. I don't think we're ready to approve an external contract account integration that isn't developed in coordination with our design team.

Since we aren't at that stage currently, I think a decent MVP strategy you could consider would be to use metamask-extension-provider. It does not yet support EIP 1102 support, but we will add that soon.

That provider allows you to create another extension that then communicates with MetaMask. In this way, you could provide a second API to a site for your account type, and have it relay its sendTransaction calls over to MetaMask, without having to re-write all our key management logic.

MicahZoltu commented 5 years ago

Re the metamask-extension-provider, will page requests to window.ethereum flow through my extension or the MetaMask extension? I need them to flow through my extension first, and then my extension would feed signing requests over to MetaMask after it has manipulated them. Is this possible, or will I need to tell my users to uninstall MetaMask?

MicahZoltu commented 5 years ago

Pinging again on this. I'm getting started on building my wallet and my initial thought was to hijack calls to window.ethereum.sendAsync and if they are eth_sendTransaction or eth_call calls I would convert them to be pipe through my smart wallet contract, then forward the request on to the original window.ethereum.sendAsync handler.

The problem I'm running into appears to be one that you all have previously run into, which is that my extension cannot access window.ethereum without doing some sketchy stuff (your solution appears to be to inject a script tag, which is then executed in the proper context).

Is there a better way for me to sit inbetween the user's application and MetaMask's processing of eth_sendTranscation and eth_call requests?

danfinlay commented 5 years ago

Oh man, really sorry for missing this. I've let GH slip a bit lately, had to do more organizational things.

In retrospect, I see you want to be able to add your contract account to the inpage provider, which our extension-provider does not make possible, so I redact my previous suggestion.

It seems you want the ability to amend the accounts list, and the ability to pre-process transaction and signing requests through your script.

I'm going to lay out a few options, from the longest-term nicest integration, to the shortest-term dirtiest hack:

Prep for native integration

In the longest term, we hope to have in-extension ability to add new account types, even contract accounts, and so if you want to be as forwards-compatible as possible, I would recommend structuring your work in the form of a module that accepts as parameters:

(There will probably need to be other methods, like maybe one for notifying the module of what accounts are available to it. This would partly be a research project and inform our own API surface.)

It is likely that we will require these modules to run in a SES containers, so avoid using global variables or deps that use global variables, as a rule of thumb.

Build on our Gnosis SAFE branch

We have an experimental branch where we implemented Gnosis SAFE support, and it provides basically exactly what you want: A new abstraction on top of our KeyringController called the AccountsController, which allows contracts to be represented as modules.

You could probably basically edit that file there and get a basic MVP.

Please know that branch is under active development/cleanup, although if you were making good changes, we might adopt your approach.

Make a Proxy Wrapper extension

A third way you could do this with today's production MetaMask, in a way other people could easily install, would be to create a new extension that:

  1. Has a contentscript that at load polls the page for a few seconds for an ethereum provider.
  2. If an ethereum provider is detected, it is wrapped in a Proxy object that diverts the account and signing requests through your extension before passing them to MetaMask.

This might actually benefit from the metamask-extension-provider mentioned above, but just as an optimization: Once the requests were forwarded to your extension's background script, that provider would allow you to complete the transaction by forwarding it directly to the MetaMask extension.

This one is obviously the most hacky, and I know how you hate mutating the page context, so you'll probably jump at option number two, which is probably sensible.

MicahZoltu commented 5 years ago

Prep for native integration: Note that not all signing can be done from contracts. In particular, a contract cannot do an eth_sign with the address it supplies in its eth_accounts list. There are certainly some dapps that will break with this, in particular dapps that assume that if you can submit a transaction, you can sign a message (not true for contracts).

Ideally, the pre-processor would receive a function signature + parameters, rather than a byte array. However, I recognize that this is information that MM doesn't currently have due to the window.ethereum architecture. The desire for this is part of why I went down the rabbit hole of creating a better communication channel between page and extension, but I would rather have my stuff integrated into MM than have that problem solved so I can accept if I am forced to work with just a byte array.

I definitely would prefer the native integration solution, but I also don't want to do a bunch of work and have it sit around for 9 months unable to be utilized because your team ends up not scheduling the other side of that channel for a long time. Do you have any kind of roadmap or prioritization that can give me an idea of what the time line for supporting extensibility is?

danfinlay commented 5 years ago

It’s on the roadmap for this year, and we hope to have something you could try out within a month or two. but doing it safely for production will take time, maybe that 9 months.

I totally agree about the params point, and we can absolutely talk about the possibility of your module exporting an arbitrary API into the site. It may not resemble the current provider (and in turn work on legacy sites), but I guess that isn’t a hard requirement for you.

I would encourage you to consider defining your contract’s ideal injected API, and we can work with that.

frozeman commented 5 years ago

This is a very good request, and I am thinking about this for ERC725v2

The idea of 725 is that all accounts that hold stuff become 725 proxy accounts, that have exactly one owner. With that the key management can be separated from the account holding the funds, which makes the security upgradeable. While the ERC725.getData allows for any information to be attached to the account, which is great for verified accounts, or repetitional data.

IT would be great if MM supports such accounts natively, as they can dramatically increase the security and UX of the web3 (relay transaction can be accepted by they owner contract, that make on boarding easier)

The integration shouldn't be done on the dapp level, imo, but rather on the account manager level (what MM is).

The flow for a 725 is as follows: MM -> owner contract/key manager -> 725 proxy -> any other contract

In more detail:

dapp send tx
-> MM checks if "from" is a 725 contract
-> call 725.owner to get owner
-> call owner.execute, using one of the owner keys
-> owner calls 725.execute
-> any other account/smart contract

The only important thing here is to standardise the calls for the owner smart contract (which could also be a simple key), the rest is funnelled automatically by the owner (if smart contract) through the 725 to any other smart contract. Those then see the 725 as the actor (msg.sender), and that's what matters.

With the generic 725 as accounts, rather than keys we are able to abstract key management out, and are able to upgrade users security over time. This helps greatly for onboarding (currently we need to set fort knox security from the start, as we could accumulate a lot of value on any key).

While allowing any future information to be attached (can only be set by the owner).

This creates manageable, verifiable accounts and key management can be a separate discussion and improved over time, without changing your 725 proxy that owns and controls your stuff.

MicahZoltu commented 5 years ago

@danfinlay Is there anything that a motivated engineer could do to reduce that 9 month timeline? I would very much prefer to do things the "right" way, that is best aligned with long-term health of the system, but I'm concerned that a 9 month timeline may be outside of my desired acceptability range.

Note: I'm a big fan of getting honest time estimates, so if 9 months is what it is then I would rather know that now that be told 3 months and have it end up being 9 months.