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

RPC call for account nonce does not update for pending transactions #247

Closed
stevenroose opened this issue Dec 18, 2017 · 12 comments
Closed

Comments

@stevenroose
Copy link
Contributor

stevenroose commented Dec 18, 2017

Since we upgraded to Istanbul, we're experiencing issues with abigen when making multiple transactions per block. It seems that abigen is using nonces twice.

abigen uses the ethclient call PendingNonceAt to retrieve the nonce to use in a transaction.
So this means that this method is returning a wrong nonce. The JSON API identifier for this call is eth_getTransactionCount with the block parameter to pending.

Now, I remember that with Raft, there was an issue with there being no "pending state". So in order to be able to use the PendingNonceAt method with Raft, this change was required. Istanbul does (or should) have a pending state, however. The mentioned change only explicitly triggers when using Raft, but it might have been accidentally extended to Istanbul.

More likely the issue is due to a different change. I'll try to investigate, but perhaps someone more familiar with the Istanbul changes can help out.

@stevenroose stevenroose changed the title abigen fails to increase nonces when making multiple transactions per block (PendingNonceAt broken) RPC call for account nonce does not update for pending transactions Jan 23, 2018
@stevenroose
Copy link
Contributor Author

So I tried this again with a very simple setup; 3 istanbul nodes making blocks every 3 secs and a command line tool to make simple transactions. When I launch multiple ones within the same block period, I get this error:

replacement transaction underpriced

This means geth is trying to insert a new transactions by the same account with the same sequence number than an already existing one.

@markya0616
Copy link
Contributor

Thanks your feedback, @stevenroose! This issue is not related to Istanbul. It's the current design in geth. The pending nonce is from the pending block, and the pending block will be updated only if a new block is inserted. Therefore, if we call PendingNonceAt many times within a block (i.e., no new block is inserted), it will always return the same nonce for an address. Please refer:
https://github.com/jpmorganchase/quorum/blob/master/miner/worker.go#L180

@stevenroose
Copy link
Contributor Author

The pending nonce is from the pending block, and the pending block will be updated only if a new block is inserted.

This is not true. In regular Ethereum, the pending block is dynamically generated on every call by taking the latest block and adding an implicit temporary block with all unconfirmed transactions.

So PendingNonceAt changes when you submit transactions that are unconfirmed.

It is possible that this is a problem with the underlying consensus package in Ethereum that Istanbul leverages, but since Istanbul is (AFAIK) the first implementation to use this package, it should interest you.

@stevenroose
Copy link
Contributor Author

You do point out something interesting, though. The problem indeed does not occur on a node that is not a mining/validating node.

So somehow there is a problem with updating the self.current.Block in that struct. Let me look into that.

@stevenroose
Copy link
Contributor Author

This PR on geth fixes the problem for us: ethereum/go-ethereum#15963

@patrickmn
Copy link
Contributor

So, just to confirm, this is an upstream geth issue and will hopefully get resolved via the above PR?

@stevenroose
Copy link
Contributor Author

@patrickmn Correct. But it looks like there is a bigger underlying issue that they are solving with rethinking more than just this one thing. I will keep monitoring and take part in the conversation. For now, I have a tiny patch that works for us.

@kegsay
Copy link

kegsay commented Feb 28, 2018

I have a tiny patch that works for us.

Was this merged in somewhere? I'm hitting this exact failure mode.

@stevenroose
Copy link
Contributor Author

@kegsay look at the PR at geth that I posted. The go-ethereum guys told me they were thinking about redoing the whole miner package anyway, so I'm thinking it might get resolved indirectly. Not for the near future though prob.

You could try to use the PR as a patch and see if it solves things. In the meantime, I do manual nonce-handling at the client side for my projects. It reduces the HTTP round-trips for abigen as well.

@kegsay
Copy link

kegsay commented Mar 1, 2018

Ah, I thought you guys did a PR on Quorum somewhere to pull this in. Thanks for clarifying.

Indeed: I'm going to have to track nonces client side for the mean time. At least it's possible to do with bind.TransactOpts.Nonce...

@fixanoid
Copy link
Contributor

Should not be an issue as of 1.8

@sarthaktNt
Copy link

Polygon Edge migration from POA to POS
I have created a 4 nodes POA network using polygon-edge.
As mentioned ,I have tried out the approach for migration.

I have deployed the staking smart contract in block no 59357 .All the validators staking has staked before block no 59750.I have verified the list in staking contract.
I have restarted all the nodes after migration to rebuild the blocks from new genesis.json.
polygon-edge ibft switch --chain ./genesis.json --type PoS --deployment 59357 --from 59750
But the blocks are not getting generated from 59750 due to the following error.

failed to get validators when calculation quorum

please help me to resolve my issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants