Use nonce-tracker subprovider to allow for near-parallel transactions

[spalladino]

Sending multiple near-simultaneous transactions (ie one right after the other, without a pause inbetween) with the hdwallet-provider currently fails due to repeated nonce issues. After the first transaction is sent, the second one issues a getTransactionCount request to the node to get the nonce, but the node fails to respond with the updated value.

Adding custom logging to the underlying HttpProvider, I got the following trace:

SENDING eth_getTransactionCount REQUEST 1535568969314046
RESULT NONCE { jsonrpc: '2.0', id: 1535568969314046, result: '0x5b' }
SENDING eth_sendRawTransaction WITH ID 1535568969723097 WITH NONCE 0x5b
RESULT RAW TX { jsonrpc: '2.0',
  id: 1535568969723097,
  result: '0xb575416fb0b73995bc015d4719a05da52f999d6a6ffa16f5dba390b802f6965e' }

SENDING eth_getTransactionCount REQUEST 1535568970108830
RESULT NONCE { jsonrpc: '2.0', id: 1535568970108830, result: '0x5b' }
SENDING eth_sendRawTransaction WITH ID 1535568970456495 WITH NONCE 0x5b
RESULT RAW TX { jsonrpc: '2.0',
  id: 1535568970456495,
  error: { code: -32000, message: 'replacement transaction underpriced' } }

The first transaction is successfully sent, and right after that the second transaction is issued. However, since there is almost no time between the two, the node fails to compute the pending transaction when responding to getTransactionCount, returning the same value as before. This causes the new tx to fail, since it thas the same nonce and price as the previous one.

Inserting a nonce-tracker subprovider right before the ProviderSubprovider in the Engine fixes this issue, as it keeps track of the current nonce locally, which is correctly incremented with each raw transaction sent. Note that it must be inserted between the HookedWalletSubprovider and the ProviderSubprovider in order to work (as the HookedWallet issues the getTransactionCount requests, which should be intercepted by the NonceTracker before they reach the Provider).

Let me know if you think this solution is viable, and I'll be happy to open a PR.

[spalladino]

FWIW, here is the code I used for logging:

function getProvider(url) {
  const provider = new HDWalletProvider(mnemonic, url)
  const httpProvider = provider.engine._providers[2].provider;
  const sendAsync = httpProvider.sendAsync.bind(httpProvider);

  provider.engine._providers[2].provider.sendAsync = (payload, cb) => {
    if (payload.method === 'eth_getTransactionCount') {
      console.log("SENDING eth_getTransactionCount REQUEST", payload.id)
      sendAsync(payload, (err, res) => {
        if (err) {
          console.log("ERROR NONCE", err)
          cb(err, res)
        } else {
          console.log("RESULT NONCE", res)
          cb(err, res)
        }
      })
    } else if (payload.method === 'eth_sendRawTransaction') {
      const rawTx = payload.params[0]
      const tx = new Transaction(new Buffer(ethUtil.stripHexPrefix(rawTx), 'hex'))
      const nonce = ethUtil.bufferToInt(tx.nonce)
      console.log("SENDING eth_sendRawTransaction WITH ID", payload.id, "WITH NONCE", '0x' + nonce.toString(16))
      sendAsync(payload, (err, res) => {
        if (err) {
          console.log("ERROR RAW TX", err)
          cb(err, res)
        } else {
          console.log("RESULT RAW TX", res)
          cb(err, res)
        }
      })
    } else {
      sendAsync(payload, cb)
    }
  }

  // Adding this solves the issue
  // const nonce = new NonceSubprovider()
  // nonce.setEngine(provider.engine)
  // provider.engine._providers.splice(2, 0, nonce)
  
  return provider
}

[cgewecke]

@spalladino Thank you for this analysis and solution - incredibly helpful.

Have opened #66 which inserts a NonceSubprovider after the Hook, before the ProviderSubprovider as you suggest. It's published on npm at truffle-hdwallet-provider@nonce. If you have a chance could you see if that works as expected?

If not or you see improvements to that PR please feel free to revise.

[spalladino]

Thanks a lot for the quick response @cgewecke! We run several tests, and arrived at something quite interesting. When working with the following truffle configuration, the solution works like a charm:

rinkeby: {
      provider: new HDWalletProvider(provider, 'https://rinkeby.infura.io/'),
      network_id: '4'
}

However, if we use the configuration suggested in the truffle with infura tutorial, the solution fails with nonce too low:

    ropsten: {
      provider: function() {
        return new HDWalletProvider(mnemonic, "https://ropsten.infura.io/<INFURA_Access_Token>")
      },
      network_id: 3
    }   

Digging through the truffle code, we noted that the "function" version causes truffle to initialize a new provider every time a contract is provisioned (see here, here and here). This has the side effect of creating a new nonce subprovider for every contract, each having its own separate nonce cache.

We are considering working around this by changing the provider of artifacts.options to an instance if we find it's a function (by calling the function once, and storing its result instead of the function itself). This nasty one-liner seems to do the trick, bypassing the setter check:

artifacts.options = Object.assign({}, artifacts.options, { provider: artifacts.options.provider })

However, before going in that direction, we wanted to check with you why is it recommended to use a function as the provider in the truffle config, instead of returning an instance directly. If there is no particular reason, perhaps the easiest way around this is to suggest using an instance of HDWalletProvider instead of a function that returns one in the config, and be done with it.

On the other hand, another option would be to change the PR to create a singleton NonceTracker (or one per network), which is shared by all HDWalletProviders, instead of creating a new instance every time. I'm not a big fan of using singletons, but given that the nonce tracker is tied to something universal as the nonce of an address, it might make sense in this case.

[cgewecke]

@spalladino Ah ok interesting.

The history of wrapping the provider in a function begins at truffle 348. Without the wrapper a connection is automatically opened when the config is loaded and this has been the source of several issues. To be honest they mostly boiled down to recent changes at eth-block-tracker and a problem with how truffle managed command exits that has since been fixed. TLDR; it's probably fine to use the unwrapped form with recent Truffle, especially if tests and deployments run ok.

Only caveat to this is what Chris Hitchcott notes in the original discussion - there might still be cases where multiple connections on the same wallet don't work well.

Going to confer with @gnidan about this but I think your singleton solution makes a lot of sense given the context . . . thanks for suggesting.

[spalladino]

Great, thanks @cgewecke! Please let me know if we can help with anything.

[cgewecke]

@spalladino Sorry for the delay circling back to this. Have made the nonce subprovider singleton by default on the latest version of truffle-hdwallet-provider@nonce.

If you have a chance could you see if this is working ok?

[spalladino]

Thanks again for the fix @cgewecke!! Unfortunately, it doesn't seem to cut it. Even though transactions sent more-or-less closely together do work, transactions issued simultaneously keep failing.

Digging into the other subproviders code, the reason for this seems to be a nonce lock in the hooked wallet subprovider. This lock guarantees that reading-and-incrementing the lock for each tx is an atomic operation, but since we still have duplicated HookedWallets (even if we have a singleton nonce), two different txs on different contracts have different locks, so they read the same nonce.

I'm starting to think that the best fix would be on truffle's end, not here, by assuring that the function to create a new provider is run only once and its result cached. For now, we are using a hack on our end to work around this, so there is no rush on this issue.

[cgewecke]

@spalladino Ah ok, good to know. Thanks again for all of your work on this. Opening an issue over at Truffle re:

assuring that the function to create a new provider is run only once and its result cached.

[dB2510]

@cgewecke Why is this issue still open?