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

Adjust pending nonce update operation #21164

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/tx_noncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,12 @@ func (txn *txNoncer) setIfLower(addr common.Address, nonce uint64) {
}
txn.nonces[addr] = nonce
}

// When reset is executed, the virtual state database is cleared and
// the status of all accounts will be updated again
func (txn *txNoncer) resetAll(all map[common.Address]uint64) {
txn.lock.Lock()
defer txn.lock.Unlock()

txn.nonces = all
}
13 changes: 8 additions & 5 deletions core/tx_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -1046,16 +1046,19 @@ func (pool *TxPool) runReorg(done chan struct{}, reset *txpoolResetRequest, dirt
// because of another transaction (e.g. higher gas price).
if reset != nil {
pool.demoteUnexecutables()

// Update all accounts to the latest known pending nonce
nonces := make(map[common.Address]uint64)
for addr, list := range pool.pending {
txs := list.Flatten() // Heavy but will be cached and is needed by the miner anyway
nonces[addr] = txs[len(txs)-1].Nonce()+1
}
pool.pendingNonces.resetAll(nonces)
}
// Ensure pool.queue and pool.pending sizes stay within the configured limits.
pool.truncatePending()
pool.truncateQueue()

// Update all accounts to the latest known pending nonce
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's correct. truncatePending will drop the overflowed transactions from the pending list. In the original code, the pending nonces will be set back.

Copy link
Contributor Author

@WeiLoy WeiLoy Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. But in truncatePending, the nonce of the account will be updated through pool.pendingNonces.setIfLower while deleting the overflowed transaction. The account that needs to be updated nonce has been updated in this function, so There is no need to reset the account again outside the function.
  2. In the original code, if you are performing the reset action, first execute truncatePending. Since the value of pool.pendingNonces has been cleared in the reset function, the subsequent steps are executed. When truncatePending deletes the overflow transaction, pool.pendingNonces.setIfLower actually reads the nonce in StateDB to judge, the condition is not established and will not be updated to the highest nonce, and finally by direct update All accounts are realized
  3. So only need to update all nonce to pool.pendingNonces at the time of reset, and after the update is completed, then execute truncatePending, this time if the function deletes the overflow transaction, pool .pendingNonces.setIfLower is able to update the nonce of the corresponding account

for addr, list := range pool.pending {
txs := list.Flatten() // Heavy but will be cached and is needed by the miner anyway
pool.pendingNonces.set(addr, txs[len(txs)-1].Nonce()+1)
}
pool.mu.Unlock()

// Notify subsystems for newly added transactions
Expand Down