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

web3+HttpProvider reusing nonce when doing parallel contract deploy using Promise.all #1846

Closed
justuswilhelm opened this issue Aug 6, 2018 · 13 comments
Assignees
Labels
1.x 1.0 related issues Enhancement Includes improvements or optimizations Feature Request Stale Has not received enough activity

Comments

@justuswilhelm
Copy link

justuswilhelm commented Aug 6, 2018

Here's something that worked without any issues in MetaMask but did not work using web3 + http rpc on a private go ethereum blockchain:

  1. Add a wallet to web3 using a private key
  2. Deploy two contracts at the same time using
Promise.all([contract1.deploy({...}).send(), contract2.deploy({...}).send()])
  1. Get error
Error: Returned error: replacement transaction underpriced
    at Object.ErrorResponse (/.../node_modules/web3-core-helpers/src/errors.js:29:16)
    at /.../node_modules/web3-core-requestmanager/src/index.js:140:36
    at XMLHttpRequest.request.onreadystatechange /.../node_modules/web3-providers-http/src/index.js:79:13)
    at XMLHttpRequestEventTarget.dispatchEvent (/.../node_modules/xhr2-cookies/dist/xml-http-request-event-target.js:34:22)
    at XMLHttpRequest._setReadyState (/.../node_modules/xhr2-cookies/dist/xml-http-request.js:208:14)
    at XMLHttpRequest._onHttpResponseEnd (/.../node_modules/xhr2-cookies/dist/xml-http-request.js:318:14)
    at IncomingMessage.<anonymous> (/.../node_modules/xhr2-cookies/dist/xml-http-request.js:289:61)
    at emitNone (events.js:111:20)
    at IncomingMessage.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1064:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)
  1. Ethereum blockchain continues to deploy only one contract

Workaround: Instead of using Promise.all(), running the deploy in sequence works, albeit at a much slower speed.

Question: What is different in how MetaMask handles nonces vs. web3?

Edit: I'm using [email protected]

@nivida nivida self-assigned this Aug 10, 2018
@nivida nivida added Bug Addressing a bug Enhancement Includes improvements or optimizations labels Aug 10, 2018
@sawyna
Copy link
Contributor

sawyna commented Aug 16, 2018

I'd like to take this up and come up with why web3 is failing on deploying two or more contracts in parallel

@justuswilhelm
Copy link
Author

I've meanwhile understood that Metamask has its own dedicated nonce manager, while "plain" web3 does not support any fancy increment-nonce-upon-pending-transaction logic. Web3 would obviously benefit from being much more pluggable in this regard.

For example, being able to provide a parameter when calling new Web3({nonceManager: myNonceManager}) would increase the value of web3 by a lot. I've resorted to monkey patching any occurrence of nonce generation in my code, but it's certainly not the cleanest way to do it.

@nivida nivida added Feature Request and removed Bug Addressing a bug labels Aug 17, 2018
@dtmle
Copy link

dtmle commented Aug 22, 2018

@justuswilhelm, I've been able to send multiple transactions using web3.eth.getTransactionCount(address, "pending"). Note that if any fail, I assume a nonce gap will be created.

@justuswilhelm
Copy link
Author

@dtmle yes, that's how I approached the problem as well: I've created a NonceManager singleton that keeps a copy of the last nonce used and every time you make a transaction you call the NonceManager to give you a new one. This is still buggy under the following circumstances:

  • You use your account concurrently from different applications (then, pending nonces would not be caught correctly)
  • You interrupt transactions and restart them. (then, you will resubmit the same transaction accidentally)

In other words, the nonce manager that I've implemented knows nothing of the real world and is just a glorified counter. Studying Metamask's source code, I've seen that they've put a lot of thought into this:

https://github.com/MetaMask/metamask-extension/blob/955ec2dca620f25beb2c7cd24c059f6ac22fdebe/app/scripts/controllers/transactions/nonce-tracker.js

-- albeit to a point where it would make me feel uncomfortable to think that this is the way to solve it. It feels like sending transactions shouldn't be such a painful process.

As a sidenote: Given all the other projects that do something with blockchains, perhaps web3.js and Ethereum will continuously face more and more competition. Adding comfortable features, like better nonce management, will really help web3.js and Ethereum as a whole stand out far more.

@Asone
Copy link

Asone commented Jan 25, 2019

I've been facing the same problem than described as i'm writing an oracle which would need to batch an important amount of contract deployments ( up to many hundreds per day, maybe more).

As a prependum, note the following :

  • the batches i've been making were done using latest version of geth.
  • they were using an internal job creating a delay beetween each smart-contract deployment (shortest try was 400ms).

Below are a few notes, questions and suggestions from my tests around this topic :

  • When using a node's unlocked account, it seems this problem does not exists. So far, at first i was using a node's unlocked account from which i could pile more than a thousand deploy transactions of the same smart-contract. Once i switched to a non node account and used a wallet with private key i faced this problem. I can't understand why this difference occurs.

  • as already mentionned here, latest geth (v1.8.21) versions supports including the pending transactions in the nonce retrieved with getTransactionCount

  • it seems possible to provide manually the nonce value within parameters of send. However this is never mentioned in the docs . @nivida @frozeman , could you confirm about that ? If so, it might be useful to mention this possibility in the documentation.

  • I've been able to solve this problem on my side by getting the transactionCount for the account used by the oracle on its initialization, storing it in local and then increasing in each iteration. I don't see it as a problem as long as i can be quite sure that my transaction gets in the pool.

  • However, this could be only a partial solution as it doesn't involve concurrence of transactions with the same address.

sidenotes :

  • I made a few tries with sendTransaction ( see doc). However, i couldn't find any indication about how to use correctly this method for contract deployment when the contract needs some parameters in its constructor. Digging through search engines and stackoverflow, i found out that the parameters have to be passed encoded along the data field. This is not really mentionned quite clearly and might be confusing for many people. May be the doc could be a bit more explainful about this ( and how to achieve to build the whole datafield with web3 ? )

  • When using sendTransaction without any parameter, it seemed that contract deployed anyway, however, retrieving the state variable that would reflect parameters given in constructor, the retrieved state variable was 0x000... . Even if it might be logical at some point, i thought that mentionning it for further developpers getting the same kind problems or using sendTransaction the same way might be useful.

@wafflemakr
Copy link

This works, although I don't think it will be scalable. And flushing the array after certain time?

let nonce = await App.web3.eth.getTransactionCount(acc.address);

 //already used that nonce, increase 1
 if (App.nonceArray.includes(nonce)) nonce++;
 //if not, add to used nonce Array
 else App.nonceArray.push(nonce);

@Asone
Copy link

Asone commented Feb 27, 2019

@wafflemakr : I have some doubts about the flushing approach and the code you provided but maybe i'm confused :

    let nonce = await App.web3.eth.getTransactionCount(acc.address);

The instruction does not provide the pending argument to retrieve the nonce based on pending transactions. In other words you'll only retrieve processed transactions, which means if you have pending transactions you might submit transactions conflicting with pending ones, which could end up in some kind of mess.

maybe this would be more correct :

let nonce = await App.web3.eth.getTransactionCount(acc.address,'pending');

please see here and here

@nivida nivida added the 1.x 1.0 related issues label Jun 20, 2019
@cliffhall
Copy link

cliffhall commented Mar 21, 2020

@wafflemakr transaction count is 1 based and nonce is 0 based. so transaction count + 1 is one greater than the next nonce, leaving an unacceptable gap. The tx using that nonce will not be mined until the gap is filled. see here: https://ethereum.stackexchange.com/a/52586/44463

@sssubik
Copy link

sssubik commented Jun 25, 2020

@ryanio @cliffhall @justuswilhelm Hey I am using promise.all so that I can parallely invoke my smart contract function but I am confused about nonce? Is this issue over? If it is what is proper way to manage nonce?

Also Handling nonce in this way is very unhealthy but is there a way around using similar logic?

let nonce = await web3.eth.getTransactionCount(fromAccount);
const results = await Promise.all(hashes.map(async (hash, index) => {
  let thisNonce = nonce + index;
.......
    )).catch(error => {
    console.log(error)
});

@cliffhall
Copy link

cliffhall commented Jun 25, 2020

@sssubik If you try to invoke transactions in parallel from the same address, you'll find that they come out serial anyway. A transaction with nonce: 10 cannot be processed by the network until the transaction with nonce: 9 is completed. You'll be better off to feed them to the network and wait for their completion in serial order.

I ended up using a pool of addresses to execute transactions in parallel. When I go to create a transaction, I take an available address from the pool and mark it as being in use, or wait until one becomes available if need be. When the transaction completes, I mark the address as available again. And I pick the address at random, in an effort to keep their balances somewhat similar during operation. It's not difficult and, there's no need to attempt to manage the nonce for any of the addresses.

I do have to keep all the addresses funded, so I have a script that will distribute any given amount across them so that they either all end up with the same amount, or if not distributing enough to do that, it levels up the poorest ones as far as it can.

@sssubik
Copy link

sssubik commented Jun 25, 2020

Hey I want to call the function submitHash many times and it could be that it is not parallel. I want to call the function as quickly as possible one after the other so that I dont have to wait to send another transaction after it is mined. But the problem is nonce doesnt get managed when I try to use a map expression to call the function. I need a way to manage the nonce or is there any inbuilt one provided by web3.js?

 let nonce = await web3.eth.getTransactionCount(fromAccount);
const results = await Promise.all(hashes.map(async (hash, index) => {
    
    let thisNonce = nonce + index;
    const account = await web3.eth.accounts.privateKeyToAccount(process.env.PRIVATE_KEY_1);
    const transaction = await contract.methods.submitHash(hash);
    const options = {
        nonce   : web3.utils.toHex(thisNonce),
        to      : transaction._parent._address,
        data    : transaction.encodeABI(),
        gas     : await transaction.estimateGas({from: account.address}),
        gasPrice: web3.utils.toHex(web3.utils.toWei('20', 'gwei')),
    };
    const signed  = await web3.eth.accounts.signTransaction(options, account.privateKey);
    const receipt = await web3.eth.sendSignedTransaction(signed.rawTransaction);
    //console.log(receipt);
    return receipt

@cliffhall
Copy link

cliffhall commented Jun 25, 2020

@sssubik You can send in the nonce like you're doing, but you're going to tie your shoelaces together and fall over. That was my conclusion when faced with this same issue some months back and why I went with an address pool instead of trying to manage the nonce for a single address. And it allows multiple invocations to happen in the same block, since the transactions don't have to wait for lower nonce'd transactions to complete before being included in a new block.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 26, 2020
@github-actions github-actions bot closed this as completed Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Enhancement Includes improvements or optimizations Feature Request Stale Has not received enough activity
Projects
None yet
Development

No branches or pull requests

8 participants