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

Metamask / Mist / Parity Support #59

Closed
swcdev opened this issue Nov 5, 2017 · 12 comments
Closed

Metamask / Mist / Parity Support #59

swcdev opened this issue Nov 5, 2017 · 12 comments
Assignees
Labels
enhancement New feature or improvement.

Comments

@swcdev
Copy link

swcdev commented Nov 5, 2017

As you mention in the readme, this library intends to include:

Complete functionality for all your Ethereum needs

If this is the case, one very common use case is have support Metamask (and even Mist / Parity). Is this feature on the roadmap? If not, is it worth attempting to implement?

As far as I know, it seems like it shouldn't be too complicated, but obviously we don't want to include web3 as a dependency, so we should somehow interact with the injected web3.currentProvider directly; https://github.com/MetaMask/faq/blob/master/DEVELOPERS.md

@ricmoo
Copy link
Member

ricmoo commented Nov 5, 2017

This is certainly on the road map. I have a few ideas on this, and will likely have a larger impact on how Wallet behaves in the next major release.

There will be a more generalized idea of a Signer, with multiple sub-classes. For example, a JsonSigner holds onto a JSON wallet, and will require a passwordCallback so that it can be used and only require the password for things which require signing (e.g. signMessage, signTransaction). Another abstraction will be that getAddress will return a Promise. There will be a simple Web3Signer and a MetamaskSigner (although, those might actually be both the same thing).

This is very much planned, and there will soon be a web3-bridge package that will help expose the ethers.js interface over the web3 interface, to make it easier for existing apps to use. This will be what we use in Ethers Wallet to add full support for web3 apps, so it is soon-ish on the roadmap.

The only functionality you are looking for from MetaMask is the signing, correct? I can put together a quick cookbook recipe on the documentation fro creating a MetaMaskSigner this week. All other functionality is exposed over the provider interface.

@ricmoo ricmoo added the enhancement New feature or improvement. label Nov 5, 2017
@ricmoo ricmoo self-assigned this Nov 5, 2017
@swcdev
Copy link
Author

swcdev commented Nov 6, 2017

Good to hear this is on the roadmap, thanks for the update.

I don't mind taking a crack at it myself, but there are probably some intricacies that a guide would help with (and would in general be useful documentation for future extensions, hardware wallets?). I'll take at the codebase for the existing signers for now.

The only functionality you are looking for from MetaMask is the signing, correct?

For my current requirements, yes, I would prefer to keep control over the read layer in the InfuraProvider. Still, I can imagine some potential projects where the user might want to select their own network (e.g. private network) via MM/Mist/Parity, so ideally ethers.js eventually supports both InjectedWeb3Signer and InjectedWeb3Provider.

@swcdev
Copy link
Author

swcdev commented Nov 6, 2017

@SwaroopH
Copy link

SwaroopH commented Nov 9, 2017

EDITED: Sorry, I was using the wrong library! facepalm

@chmanie
Copy link

chmanie commented Nov 14, 2017

This is my humble approach to signing / sending transactions via MetaMask (I tested it and it seems to work): https://gist.github.com/chmanie/cdf2697e42bf9adb1dc018731688b894

Some concerns:

  • web3.js as well as Ethjs are using BigNumber.js (https://github.com/MikeMcl/bignumber.js/) whereas ethers.js is using BN.js (https://github.com/indutny/bn.js/) and these are not compatible. So I am converting those using strings for now (which seems terrible). Is there are a reason why you went for BN.js in particular?
  • When calling the sign() function of the Web3Signer the transaction is immediately being sent. This is due to the way web3 works which doesn't let you just sign a transaction.
  • This is definitely just some experimentation and by far not tested thoroughly or used in production even. This should be treated as inspiration.

@ricmoo
Copy link
Member

ricmoo commented Nov 14, 2017

The reasons to use BN are:

  1. It is already required by the elliptic curve libraries, so rather than using multiple Big Number libraries, we consolidate on the one that would be far too difficult and dangerous to remove
  2. BigNumber.js used in web3 is not actually BigNumber.js, but a fork of it, with several changes, and is not actively maintained and does not get bug fixes
  3. BigNumber is a decimal library, not an integer library, so it does not accurately reflect the operations required in Ethereum and its use can disregard underflow

The custom signer should instead of overriding sign, could instead override sendTransaction, which would give you the opportunity to intercept signing. This is how ethers.io creates a custom signer as well as the ethers-cli TestRPC wrappers. I haven't had time to get a simple Custom Signer for MetaMask together, but I will try this week. The ethjs library is a thin wrapper around several ethers.js functions and cut and paste of some web3 functions. The easiest way to do what you wish is to extend the JsonRpcProvider, and override the send function to pass the calls off to the web3.sendAsync instead of call make a request to the network.

Sorry, I have 4 projects I'm trying to catch up on (ethers.js, the firefly hardware wallet, Ethers Wallet and ethers.io) but am hoping to get at least a cookbook entry this week on how to create a custom signer that bridges into Metamask. We certainly need to add more hours in a day... :)

@ricmoo
Copy link
Member

ricmoo commented Nov 14, 2017

Here is a quick and dirty example I've thrown together. Probably needs some flushing out, and obviously not production ready, but give it a try:

function MetamaskSigner(web3, provider) {
    this.provider = provider;

    this.getAddress = function() {
        return web3.eth.accounts[0];
    }
    
    Object.defineProperty(this, 'address', {
        get: function() {
            return web3.eth.accounts[0];
        }
    });
    
    this.sendTransaction = function(transaction) {
        var tx = {
            from: this.address
        };
        ['to', 'data'].forEach(function(key) {
            if (transaction[key] != null) {
                tx[key] = transaction[key];
            }
        });
        ['gasLimit', 'gasPrice', 'nonce', 'value'].forEach(function(key) {
            if (transaction[key] != null) {
                tx[key] = ethers.utils.hexlify(transaction[key]);
            }
        });
        return new Promise(function(resolve, reject) {
            var payload = {
                jsonrpc: "2.0",
                method: 'eth_sendTransaction',
                id: 1,
                params: [ tx ]
            };
            web3.currentProvider.sendAsync(payload, function(error, hash) {
                if (error) {
                    reject(error);
                } else {
                    tx.hash = hash;
                    resolve(tx);
                }
            });
        });
    }
}
var provider = ethers.getDefaultProvider('homestead');
var wallet = new MetamaskSigner(web3, provider);

@jennazenk
Copy link

jennazenk commented Nov 16, 2017

Hello - was wondering is parity compatibility for signing/sending tx in your roadmap ?

@ricmoo
Copy link
Member

ricmoo commented Nov 16, 2017

Heya!

I think something very similar to the above solution will work with a local parity JSON-RPC will work. I will add a recipe for it as well.

Or are you referring to their UI module? I haven’t used that before, but if you point me to some documentation I can figure that out too.

The ethers-app API will soon transparently roll Metamask, ethers.io and local Ethereum noses up into one convenient API so that you no longer need to handle various signers in your code. Things will just work, no matter what backend the user uses.

Once Ethers Wallet is updated to iOS 11 and for iPhone X (hopefully by the end of this week) I’ll be adding these to ethers-app.

@chmanie
Copy link

chmanie commented Nov 17, 2017

@ricmoo Thank you for your example! Sadly it doesn't work for me as it complains about a missing signer here:

if (!signer) { return Promise.reject(new Error('missing signer')); }

@ricmoo
Copy link
Member

ricmoo commented Nov 17, 2017

@chmanie Ack! You are right, I forgot to set a provider, which is how it determines the difference between a signer and provider.

I have updated the above comment. Please try that one out. Update the provider to the network you are connecting to. I will update it once I have a Web3Provider, which will proxy all requests through the web3 wrapped provider.

@ricmoo
Copy link
Member

ricmoo commented Jan 17, 2018

This should be good to go now. See 67bb0b7.

Here is a modified version of the Metamask documentation for you to use ethers.js instead of using web3.

window.addEventListener('load', function() {

  // Checking if Web3 has been injected by the browser (Mist/MetaMask/EthersWallet)
  if (typeof web3 !== 'undefined') {
    // Use Mist/MetaMask's provider
    provider = new ethers.providers.Web3Provider(web3.currentProvider);
    signer = provider.getSigner();
  } else {
    console.log('No web3? You should consider trying MetaMask!')
    // Allow read-only access to the blockchain if no Mist/Metamask/EthersWallet
    provider = ethers.providers.getDefaultProvider();
  }

  // Now you can start your app & access the ethers provider and signer freely:
  startApp()
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement.
Projects
None yet
Development

No branches or pull requests

5 participants