Local nonce manager #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nonce manager is used by the generated contracts to track the current nonce and update it after each successfully submitted transaction. Tracking the nonce locally is required when transactions are submitted from multiple goroutines for when Ethereum clients are deployed behind a load balancer and their mempools are not always in sync.
nkuba
reviewed
May 12, 2020
We want to be able to recover from a situation when mempool crashes. To achieve it, we cache the last local nonce value only for 5 seconds.
nkuba
reviewed
May 13, 2020
nkuba
approved these changes
May 13, 2020
nkuba
added a commit
to keep-network/keep-core
that referenced
this pull request
May 13, 2020
See keep-network/keep-common#35 The local nonce manager from keep-common is used by the generated contracts to track the current nonce and update it after each successfully submitted transaction. Tracking the nonce locally is required when transactions are submitted from multiple goroutines or when Ethereum clients are deployed behind a load balancer and their mempools are not always in sync. The nonce manager is used by the generated contracts to track the current nonce and update it after each successfully submitted transaction. Tracking the nonce locally is required when transactions are submitted from multiple goroutines for when Ethereum clients are deployed behind a load balancer and their mempools are not always in sync. In our case, we are having problems with ticket submission against both Infura and Alchemy, and we have a feeling that all load-balanced Ethereum APIs are affected. To be 100% safe, we need to handle nonce on our side.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The nonce manager is used by the generated contracts to track the current nonce and update it after each successfully submitted transaction. Tracking the nonce locally is required when transactions are submitted from multiple goroutines or when Ethereum clients are deployed behind a load balancer and their mempools are not always in sync.
One important aspect that might not be clear at first is that we need to call
IncrementNonce()only when the transaction has been successfully submitted to the mempool. Also, the nonce manager has to be a single instance stored between all contracts.NonceManageritself does not provide any synchronization. All generated contracts use the transaction lock provided by the chain and that transaction lock makes nonces manipulation safe.NonceManagerconsiders the local nonce value as no longer valid and updates it to the pending nonce value as seen by the chain after 5 seconds of inactivity in computing nonces. This is to let us recover from potential mempool crashes before the last TX has been propagated to other mempools. However, as long as we submit transactions one after another, the local nonce value is what determines the nonce used in transaction.I was considering keeping
NonceManagerincontractspackage so thatCurrentNonce()andIncrementNonce()(especially this one) are not accessible from anywhere else thancontractsbut it does not look right to me that the chain would have to createcontracts.NonceManagerand pass it to each contract as a constructor parameter. Given that we have the error resolver inethutil,NonceManagerfits better inethutilIMO.