web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.3k stars 4.95k forks source link

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

Closed justuswilhelm closed 4 years ago

justuswilhelm commented 6 years ago

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()])

3) 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)

4) 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 web3@1.0.0-beta.35

sawyna commented 6 years ago

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

justuswilhelm commented 6 years ago

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.

dtmle commented 6 years ago

@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 commented 6 years ago

@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:

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 commented 5 years ago

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 :

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

sidenotes :

wafflemakr commented 5 years ago

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 commented 5 years ago

@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

cliffhall commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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[bot] commented 4 years ago

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