Ethers.js: Why is the JsonRpcSigner sendTransaction slow?

[pointtoken]

When we call a method that requires signature on a contract, we are finding that, while MetaMask returns a transaction hash immediately upon submission, ethers.js will hang on to the tx hash and not return from awaiting for ofter 3-10 seconds. What is going on there? Clearly the transaction hash has been created from the user signing the transaction, but it doesn't return from the await. Thoughts?

[ricmoo]

The ethers.js library returns a Transaction object, not just a transaction hash. Since the JSON-RPC only returns a hash, we must query the node for the transaction (by the transaction hash it provided). We may be able to improve the performance of this for "nodes" like MetaMask, I have to look into whether it retains a consistent copy of it, or whether it needs to pass the query along to the network.

Getting the transaction back is very useful, since if you send a transaction like { to: address, value: amount}, the node has filled in all sorts of useful things for you, like nonce, gasLimit, gasPrice, etc. That means, for example, if you deploy a contract, you can immediately know its address, before it is mined. It also means if you under-priced your transaction during congestion, you have an opportunity to replace it, or at the very least keep track of it.

I will be putting together a simple MetaMask example and will test whether for sent transactions it keeps a local copy, in which case it will be much faster.

[pointtoken]

I would recommend at least providing this as configurable. It is best if SDKs like this don't make assumptions about what the user needs but provides options for lower level interaction. We are migrating from ethjs (which seems like a dead project) to ethers.js. Ethjs was great in that it was pretty much a web3 wrapper with a better interface (promises, etc) so when things work differently in ethers.js it is surprising.

[ricmoo]

Keep in mind that ethers.js is a fairly mature library, in use for about 3 years. The feature set and API are based on real-world usage.

There are many differences between Web3.js (which is a thin wrapper around JSON-RPC) and ethers.js, which is meant to be a generic Ethereum library, not specific to JSON-RPC. In general a transaction actually knows far more than just its hash, so dropping that data is the anomaly, not retaining it. This is something I'm working with various members of the community to address. All other Signers are fully aware of the transaction they are signing, only the JsonRpcSigner needs additional weird logic to deduce (discover) it.

The ethjs library was largely a cut-and-paste of ethers.js that was far more heavily influenced by Web3.js. If you merely need the JSON-RPC wrapped with Promise, the new EIP-1193 provider is exactly that, out of the box. :)

[pointtoken]

Thanks for illuminating the history.

To your knowledge, has anyone actually implemented EIP-1193? We ended up switching to ethers.js because we needed support for the tuple type, which broke ethjs, so it was our only option that we could determine.

This is the second time we've found ethers.js making rpc calls without our knowing. (The first time was with doing an estimate gas call). I still feel that SDKs should make rpc calls happening under the hood configurable.

[ricmoo]

I don't think a "configuration" for this makes sense though... It is not too often you would rather a JSON-RPC provider to outright fail, for things which should work. I've tried to seep ethers easy to extend, so you could add something like:

class AskProvider extends JsonRpcProvider {

  constructor(url, askCallback) {
    super(url);
    this.askCallback = askCallback;
  }
  ask(action: string, params: {}): Promise<bool> {
    return new Promise((resolve, reject) => {
      this.askCallback(action, params, (error) => {
        if (error) { return reject(new Error('deny')); }
        resolve(true);
      }
    });
  }
  send(action: string, params: Array<any>): Promise<any> {
    return this._ask(action, params).then(() => super.send(action, params);
  }
}

Which would give you the option to pass in a callback to decide whether to allow a call or not.

What would the configuration look like anyways? provider.emulateLegacyJsonRpcBehaviour = true? That would break anything that currently uses ethers... :(

You could also override CustomProvider.prototype.sendTransaction to return a hash. There are far too many tools, frameworks, etc that depend on this functionality. But if you have a specific use case, and don't plan to use any other ethers.js features, or extended libraries you could easily do:

class CusomSigner extends JsonRpcSigner {
  sendTransaction(tx): Promise<string> {
    return this.provider.send('eth_sendTransaction', [ tx ]);
  }
}

class CustomProvider extends JsonRpcProvider {
  getSigner(indexOrAddress: number | string): CusomJsonRpcSigner {
    return new CustomJsonRpcSigner(indexOrAddress);
  }
}

EIP-1193 will be a thing. MetaMask is dropping support for non-EIP-1193 at some point. We met during DevCon4 at a steering group to discuss the future of the low-level Provider object. It is unlikely it will ever be the API that the ethers.js API exposes, since it is still very tightly coupled to JSON-RPC, but ethers.js will certainly be able to consume them and expose its richer API, part of which is returning transaction objects rather than transaction hashes. :)

One more note, if you specify the gasLimit explicitly, it will not call estimateGas.

Anyways, just a few notes... I can also be reached on the ether-io gitter channel, to have a real-time discussion sometime. :)

[pkieltyka]

another data point is we recently switched from web3.js in our tests to ethers.js for our tests, and we've seen 3x slower execution with the ethers version over web3. We've found this to be the extra calls ethers makes after a transaction is sent, in cases not necessary for our tests to parse logs, etc.

you can test yourselves too / compare versions..

web3 version: https://github.com/horizon-games/multi-token-standard/tree/v0.1.0
ethers version (master): https://github.com/horizon-games/multi-token-standard

(for either, yarn install; yarn ganache; yarn test)

I think it's very useful for developers that ethers will "do-the-right" thing (aka expected thing) by removing the common boilerplate to parse logs or get a transaction receipt, but personally I look at ethers as a very lean swiss-army knife for interacting with an ethereum node or vm. In this case, I think having a RawJsonRpcProvider to be super lightweight, as just a thin wrapper with helper methods for a developer to have explicit control over the handling of the request / response is much better. It also lets people build middleware or other systems, such as debugging tools for development, profiling tools, load balancing, etc. etc. I still think JsonRpcProvider can stay as it is, just there should be a leaner RawJsonRpcProvider with helper functions. And the nice part of ethers is anyone can develop one of these on their own too.

[ricmoo]

Is this using the JsonRpcSigner? My guess is that it has to do with the fact that nodes do not return transactions, just hashes. Since that comes back immediately, and the is all Web3.js responds with, it can be much faster than having to ask the node for the pending transaction that is represented by that hash. This may change in the future (no one I’ve talked to from Geth seems to think not returning the transaction is a bad idea).

The purpose of ethers has never been performance, but it’s something I can look into. With v4 the test suite on node runs 4x faster than v3, so I’m not against improving performance. :)

People who want “lean”, have all the methods exposed for them to do this, using the provider.send(“eth_sendRawTransaction”, [ tx ]) along with all the static wrapping method, JsonRpcProvider.wrapTransaction(tx). So people who need the performance can get it. Generally though, I have not worried about optimizing sendTransaction, since they take 15s or so to mine on a POW network, and they are fairly rare events...

[pointtoken]

Hi - We tried implementing your idea of overriding the provider but are having issues. Here's the code we wrote. However, we are getting the following error:

Error: do not call the JsonRpcSigner constructor directly; use provider.getSigner
    at CustomSigner.JsonRpcSigner [as constructor] (ethers.js:11582)

Here is our code:

import * as ethers from 'ethers';
import { TransactionResponse, TransactionRequest } from 'ethers/providers';

class CustomSigner extends ethers.providers.JsonRpcSigner {
    sendTransaction(transaction:TransactionRequest): Promise<TransactionResponse> {
      return this.provider.send('eth_sendTransaction', [ transaction ]);
    }
  }

export class CustomProvider extends ethers.providers.Web3Provider {

    getSigner(addressOrIndex?: number | string): CustomSigner {
      return new CustomSigner({},this,addressOrIndex);
    }
  }

Returning the signer was your suggestion but your code doens't seem to like it...

[ricmoo]

Ah yes. There is a constructor guard on the JsonRpcSigner. This may be a valid reason to remove it... hmm...

It would be easier for now if you extend the ethers.Signer instead, and have the constructor take in a JsonRpcSigner. So the getSigner would return new CustomSigner(super.getSigner(addressOrIndex)) and the custom signer would manipulate the transaction as needed before calling this.signer.sendTransaction.

Does that make sense? I’ll look into better ways to deal with sub-classing this. The main thing I’m trying to prevent is someone from instantiating a JsonRpcProvider with an index of an account on one provider and then accidentally using a different provider, which would spend completely unrelated funds.

[pointtoken]

Sort of makes sense. If you have time, would be great if you could write the code, to prevent us from having to poke around too much and deal w/ trial and error, as you know the codebase...

[ricmoo]

I'm on a computer now, so it's easier to type code than when I'm on my phone. :)

I'm thinking something like this (I haven't tried this, just typing off the top of my head, so may have typos):

import * as ethers from 'ethers';
import { TransactionResponse, TransactionRequest } from 'ethers/providers';

class CustomSigner extends ethers.Signer {
    readonly signer: Signer;

    constructor(signer: ethers.Signer) {
        super();
        ethers.utils.defineReadOnly(this, 'signer', signer);
        ethers.utils.defineReadOnly(this, 'provider', signer.provider);
    }

    getAddress(): Promise<string> {
        return this.signer.getAddress();
    }

    sendTransaction(transaction:TransactionRequest): Promise<TransactionResponse> {
      return this.signer.sendTransaction(transaction);
    }

    sendQuickTransaction(transaction: TransactionRequest): Promise<string> {
        return Promise.all([ ethers.utils.resolveProperties(transaction), this.getAddress()]).then((result) => {
            return this.provider.send('eth_sendTransaction', [ result[1].toLowerCase(), result[0] ]);
        }
    }

    signMessage(message: string | ethers.utils.Arrayish): Promise<string> {
        return this.signer.signMessage(message);
    }
}

export class CustomProvider extends ethers.providers.Web3Provider { 
    getSigner(addressOrIndex?: number | string): CustomSigner {
      return new CustomSigner(super.getSigner(addressOrIndex));
    }
}

Now that I think about this though, if you want this, you should be able to simply add the operation you want to the JsonRpcSigner.prototype. Not exactly sure how to do this in TypeScript, but in JavaScript you'd use:

JsonRpcSigner.prototype.sendQuickTransaction(transaction: TransactionRequest): Promise<string> {
    return Promise.all([ ethers.utils.resolveProperties(transaction), this.getAddress()]).then((result) => {
        return this.provider.send('eth_sendTransaction', [ result[1].toLowerCase(), result[0] ]);
    }
}

Then when you use signer.sendQuickTransaction, you won't get back a TransactionResponse, but a transaction hash to do with as you please, and you will get it back pretty much immediately.

[pointtoken]

Thanks for continuing to help. We are now transpiling but still not quite there. I think we have to override sendTransaction because we are using the ethers Contract class to marshall our calls from the ABI. So wouldn't the trick be to have the code inside sendTransaction call provider.send directly but then not wait for the TransactionResponse and just return a custom crafted TransactionResponse with only the txHash?

But I don't know if that is possible since a promise is embedded in the definition of TransactionResponse.

export interface TransactionResponse extends Transaction {
    blockNumber?: number;
    blockHash?: string;
    timestamp?: number;
    confirmations: number;
    from: string;
    raw?: string;
    wait: (confirmations?: number) => Promise<TransactionReceipt>;
}

[ricmoo]

That isn't a Promise, but a function that returns a Promise. You can easily add it though, since you have the hash:

let result = ...
result.hash = hash;
result.wait = (confirmations? number:) => {
    return this.provider.getTransactionReceipt(hash, confirmations);
};

I'm working right now on splitting the sendTransaction and the wrapping, to make it easier to extend things in this way... I'll let you know how it goes. :)

[ricmoo]

I've added a change to make it easier to sub-class the JsonRpcSigner. The sendUncheckedTransaction will return the hash (the sendTransaction just calls this and does its wrapping as usual, so existing code won't be affected).

You should be able to use something like:

class UncheckedJsonRpcSigner extends ethers.Signer {
    readonly signer: ethers.providers.JsonRpcSigner;

    constructor(signer: ethers.providers.JsonRpcSigner) {
        super();
        ethers.utils.defineReadOnly(this, 'signer', signer);
        ethers.utils.defineReadOnly(this, 'provider', signer.provider);
    }

    getAddress(): Promise<string> {
        return this.signer.getAddress();
    }

    sendTransaction(transaction:TransactionRequest): Promise<TransactionResponse> {
        return this.signer.sendUncheckedTransaction(transaction).then((hash) => {
            return {
                hash: hash,
                nonce: null,
                gasLimit: null,
                gasPrice: null,
                data: null,
                value: null,
                chainId: null,
                confirmations: 0,
                from: null,
                wait: (confirmations?: number) => { return this.provider.waitForTransaction(hash, confirmations); }
            };
        });
    }

    signMessage(message: string | ethers.utils.Arrayish): Promise<string> {
        return this.signer.signMessage(message);
    }
}

var provider = new ethers.providers.JsonRpcProvider();
var signer = new UncheckedJsonRpcSigner(provider.getSigner());
var contract = new Contract(address, abi, signer);

Note that the ContractFactory won't work with this, as it requires the from and nonce be populated. This is certainly possible to do, in limited cases, in the above by resolving the address and transaction count and including that in the result (instead of null), but then you won't be able to rely on the node to populate the nonce for you.

The tests are running right now, but once they are done, I will publish this to npm.

[ricmoo]

The JsonRpcSigner now has a sendUncheckedTransaction(transaction) method, which will just return a transaction hash, for anyone that wishes to skip wrapping (or defer wrapping; you can still in the future wrap a transaction once it is mined using provider._wrapTransaction(tx, hash)) the transaction.

@pkieltyka quick note, the "3x slower" you are experiencing has nothing to do with parsing the logs, etc. It was entirely due to having to poll and wait for the node to index the transaction in the memory pool and signer queue with its transaction hash.

Hopefully this should provide a solution everyone is happy enough with?

[ricmoo]

(whoops... had this issue open in 2 tabs, thinking they were different issues... apologies for the overlap in comments)

[pointtoken]

Thanks! Can you push 4.0.20 to npm? It is still on .19

[ricmoo]

Done. :)

[pkieltyka]

awesome stuff @ricmoo :) thanks. Btw, regarding examples above, its best developers don't have to monkey-patch a class via injecting a prototype, instead it should be designed to be subclassed, or even better with some composition, but subclassing is better than nothing. thanks again for all the work+support

[pointtoken]

Not out of the woods yet. I tried using your wrapped class (btw, I agree with @pkieltyka that it would nice if it were subclassed and not wrapper) and I am getting the following error when instantiating a Contract:

Error: invalid signer or provider (arg="signerOrProvider", value="[object Object]", version=4.0.20)

I am guessing that more methods need to be wrapped or that the signer has to be truly subclassed? Here is the wrapped code I used, fundamentally your code but removing the super (since it isn't a subclass) and assigning the signer to a local variable:

import * as ethers from 'ethers';
import { TransactionResponse, TransactionRequest } from 'ethers/providers';


export class UncheckedJsonRpcSigner {
   signer: ethers.providers.JsonRpcSigner;

  constructor(signer: ethers.providers.JsonRpcSigner) {
//      super();
        this.signer = signer;
        ethers.utils.defineReadOnly(this, 'signer', signer);
        ethers.utils.defineReadOnly(this, 'provider', signer.provider);
    }

  getAddress(): Promise<string> {
      return this.signer.getAddress();
  }

  sendTransaction(transaction:TransactionRequest): Promise<TransactionResponse> {
      return this.signer.sendUncheckedTransaction(transaction).then((hash) => {
          return {
              hash: hash,
              nonce: null,
              gasLimit: null,
              gasPrice: null,
              data: null,
              value: null,
              chainId: null,
              confirmations: 0,
              from: null,
              wait: (confirmations?: number) => { return this.signer.provider.waitForTransaction(hash, confirmations); }
          };
      });
  }

  signMessage(message: string | ethers.utils.Arrayish): Promise<string> {
      return this.signer.signMessage(message);
  }
}

[ricmoo]

oh yes, you cannot comment out signer(); and it should extend ethers.Signer. I’ll update the example when I’’ back at a computer. Those should fix it, I think.

I just typed that out as an example skeleton, there may be other typos.

[ricmoo]

(updated to include the extends)

[pointtoken]

That did the trick! It was confusing since it both inherits and gets it pass in the ctor(), but it works. Closing the issue and thanks for all your help. This represents a huge UX issue overcome and makes the ethers library much more viable for dApps.

[ricmoo]

Just seeking some quick feedback; in v5, I've added a JsonRpcSigner.prototype.connectUnchecked() which will return a sub-class of JsonRpcSigner which is an UncheckedJsonRpcSigner, which uses the above wrapped sendTransaction.

Sound good? Better idea for a method name? :)