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

Don't be notified about account nonces, don't store them; get them at selection-time #57

Merged
merged 36 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2231c3d
Don't notify about account nonces, don't forget etc.
andreibancioiu Nov 13, 2024
b22177e
When selecting transactions, receive an account nonce provider.
andreibancioiu Nov 13, 2024
3714684
Refactor, pass "AccountNonceProvider" in constructor.
andreibancioiu Nov 13, 2024
74e7ac5
Quick sketch of selection changes.
andreibancioiu Nov 13, 2024
d583eae
Fix tests and comments.
andreibancioiu Nov 13, 2024
94131c7
Refactoring.
andreibancioiu Nov 13, 2024
4ca46fd
Refactor, optimize.
andreibancioiu Nov 13, 2024
20d4ba4
Fix after self-review.
andreibancioiu Nov 14, 2024
ee6476d
Update readme.
andreibancioiu Nov 14, 2024
4f94f63
Merge branch 'selection-by-ppu' into MX-16107-no-more-notify
andreibancioiu Nov 14, 2024
0f01e5c
Additional tests.
andreibancioiu Nov 14, 2024
80b4979
Fix after review.
andreibancioiu Nov 14, 2024
f97351c
Receive the nonce provider in SelectTransactions, instead of receivin…
andreibancioiu Nov 15, 2024
0db9d39
Break the selection loop if it takes too long.
andreibancioiu Nov 15, 2024
3bbf408
AccountNonceProvider becomes AccountStateProvider (more information f…
andreibancioiu Nov 18, 2024
bae6b43
Additional logs on cross tx cache.
andreibancioiu Nov 18, 2024
613f5ba
Hold fee on tx, handle accumulated fees. Avoid non-executable transac…
andreibancioiu Nov 18, 2024
95a888a
Reference new core-go.
andreibancioiu Nov 18, 2024
ebe0e12
Handle guarded transactions with same nonce.
andreibancioiu Nov 18, 2024
c61ce2a
Better readme etc.
andreibancioiu Nov 18, 2024
26f0189
Fix tests.
andreibancioiu Nov 19, 2024
03c5adb
Better handling of not-executable transactions.
andreibancioiu Nov 20, 2024
679a465
A few optimizations.
andreibancioiu Nov 20, 2024
de2620b
Fix fee exceeded balance detection. Refactoring.
andreibancioiu Nov 20, 2024
fe1bd09
Additional unit tests.
andreibancioiu Nov 20, 2024
67c1c6e
Adjust benchmark output.
andreibancioiu Nov 20, 2024
e20d2e5
Fix tests.
andreibancioiu Nov 20, 2024
b2fa1ce
Additional unit tests.
andreibancioiu Nov 20, 2024
48853fb
Fix after self review.
andreibancioiu Nov 20, 2024
f159f60
Fix after self review.
andreibancioiu Nov 20, 2024
d5ef41b
Remove out-of-reality test.
andreibancioiu Nov 20, 2024
457c06c
Update readme.
andreibancioiu Nov 21, 2024
1b8274d
Merge branch 'feat/mempool' into MX-16107-no-more-notify
andreibancioiu Nov 21, 2024
92d0476
Fix linter issues.
andreibancioiu Nov 21, 2024
93c958a
Fix after review (part 1).
andreibancioiu Nov 24, 2024
8a0cb50
Fix after review (part 2).
andreibancioiu Nov 26, 2024
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
4 changes: 4 additions & 0 deletions common/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package common

import (
"errors"

"github.com/multiversx/mx-chain-core-go/core"
)

Expand Down Expand Up @@ -74,6 +75,9 @@ var ErrNilTimeCache = errors.New("nil time cache")
// ErrNilTxGasHandler signals that a nil tx gas handler was provided
var ErrNilTxGasHandler = errors.New("nil tx gas handler")

// ErrNilAccountNonceProvider signals that a nil account nonce provider was provided
var ErrNilAccountNonceProvider = errors.New("nil account nonce provider")

// ErrNilStoredDataFactory signals that a nil stored data factory has been provided
var ErrNilStoredDataFactory = errors.New("nil stored data factory")

Expand Down
37 changes: 37 additions & 0 deletions testscommon/txcachemocks/accountNonceProviderMock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package txcachemocks

import (
"errors"
)

// AccountNonceProviderMock -
type AccountNonceProviderMock struct {
NoncesByAddress map[string]uint64
GetAccountNonceCalled func(address []byte) (uint64, error)
}

// NewAccountNonceProviderMock -
func NewAccountNonceProviderMock() *AccountNonceProviderMock {
return &AccountNonceProviderMock{
NoncesByAddress: make(map[string]uint64),
}
}

// GetAccountNonce -
func (stub *AccountNonceProviderMock) GetAccountNonce(address []byte) (uint64, error) {
if stub.GetAccountNonceCalled != nil {
return stub.GetAccountNonceCalled(address)
}

nonce, ok := stub.NoncesByAddress[string(address)]
if !ok {
return 0, errors.New("cannot get nonce")
}

return nonce, nil
}

// IsInterfaceNil -
func (stub *AccountNonceProviderMock) IsInterfaceNil() bool {
return stub == nil
}
8 changes: 6 additions & 2 deletions txcache/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ The mempool selects transactions as follows (pseudo-code):
func selectTransactions(gasRequested, maxNum):
// Setup phase
senders := list of all current senders in the mempool, in an arbitrary order
bunchesOfTransactions := sourced from senders; nonces-gap-free, duplicates-free, nicely sorted by nonce
bunchesOfTransactions := sourced from senders; middle-nonces-gap-free, duplicates-free, nicely sorted by nonce

// Holds selected transactions
selectedTransactions := empty
Expand Down Expand Up @@ -154,7 +154,7 @@ Thus, the mempool selects transactions using an efficient and value-driven algor
- **Organize transactions into bunches:**
- For each sender, collect all their pending transactions and organize them into a "bunch."
- Each bunch is:
- **Nonce-gap-free:** There are no missing nonces between transactions.
- **Middle-nonces-gap-free:** There are no missing nonces between transactions.
- **Duplicates-free:** No duplicate transactions are included.
- **Sorted by nonce:** Transactions are ordered in ascending order based on their nonce values.

Expand All @@ -181,6 +181,10 @@ Thus, the mempool selects transactions using an efficient and value-driven algor
- The accumulated gas of selected transactions meets or exceeds `gasRequested`.
- The number of selected transactions reaches `maxNum`.

**Additional notes:**
- Within the selection loop, the current nonce of the sender is queryied from the blockchain, if necessary.
- If an initial nonce gap is detected, the sender is excluded from the selection process.
- Transactions with nonces lower than the current nonce of the sender are skipped (not included in the selection).

### Paragraph 5

Expand Down
8 changes: 0 additions & 8 deletions txcache/crossTxCache.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,6 @@ func (cache *CrossTxCache) GetTransactionsPoolForSender(_ string) []*WrappedTran
return make([]*WrappedTransaction, 0)
}

// NotifyAccountNonce does nothing, only to respect the interface
func (cache *CrossTxCache) NotifyAccountNonce(_ []byte, _ uint64) {
}

// ForgetAllAccountNonces does nothing, only to respect the interface
func (cache *CrossTxCache) ForgetAllAccountNonces() {
}

// IsInterfaceNil returns true if there is no value under the interface
func (cache *CrossTxCache) IsInterfaceNil() bool {
return cache == nil
Expand Down
8 changes: 0 additions & 8 deletions txcache/disabledCache.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,6 @@ func (cache *DisabledCache) RegisterHandler(func(key []byte, value interface{}),
func (cache *DisabledCache) UnRegisterHandler(string) {
}

// NotifyAccountNonce does nothing
func (cache *DisabledCache) NotifyAccountNonce(_ []byte, _ uint64) {
}

// ForgetAllAccountNonces does nothing
func (cache *DisabledCache) ForgetAllAccountNonces() {
}

// ImmunizeTxsAgainstEviction does nothing
func (cache *DisabledCache) ImmunizeTxsAgainstEviction(_ [][]byte) {
}
Expand Down
22 changes: 15 additions & 7 deletions txcache/eviction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ func TestTxCache_DoEviction_BecauseOfCount(t *testing.T) {
EvictionEnabled: true,
NumItemsToPreemptivelyEvict: 1,
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()
cache, err := NewTxCache(config, txGasHandler)
accountNonceProvider := txcachemocks.NewAccountNonceProviderMock()

cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)
require.NotNil(t, cache)

Expand Down Expand Up @@ -56,7 +59,9 @@ func TestTxCache_DoEviction_BecauseOfSize(t *testing.T) {
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()
cache, err := NewTxCache(config, txGasHandler)
accountNonceProvider := txcachemocks.NewAccountNonceProviderMock()

cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)
require.NotNil(t, cache)

Expand Down Expand Up @@ -91,7 +96,9 @@ func TestTxCache_DoEviction_DoesNothingWhenAlreadyInProgress(t *testing.T) {
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()
cache, err := NewTxCache(config, txGasHandler)
accountNonceProvider := txcachemocks.NewAccountNonceProviderMock()

cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)
require.NotNil(t, cache)

Expand Down Expand Up @@ -129,11 +136,12 @@ func TestBenchmarkTxCache_DoEviction(t *testing.T) {
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()
accountNonceProvider := txcachemocks.NewAccountNonceProviderMock()

sw := core.NewStopWatch()

t.Run("numSenders = 35000, numTransactions = 10", func(t *testing.T) {
cache, err := NewTxCache(config, txGasHandler)
cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)

cache.config.EvictionEnabled = false
Expand All @@ -151,7 +159,7 @@ func TestBenchmarkTxCache_DoEviction(t *testing.T) {
})

t.Run("numSenders = 100000, numTransactions = 5", func(t *testing.T) {
cache, err := NewTxCache(config, txGasHandler)
cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)

cache.config.EvictionEnabled = false
Expand All @@ -169,7 +177,7 @@ func TestBenchmarkTxCache_DoEviction(t *testing.T) {
})

t.Run("numSenders = 400000, numTransactions = 1", func(t *testing.T) {
cache, err := NewTxCache(config, txGasHandler)
cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)

cache.config.EvictionEnabled = false
Expand All @@ -187,7 +195,7 @@ func TestBenchmarkTxCache_DoEviction(t *testing.T) {
})

t.Run("numSenders = 10000, numTransactions = 100", func(t *testing.T) {
cache, err := NewTxCache(config, txGasHandler)
cache, err := NewTxCache(config, txGasHandler, accountNonceProvider)
require.Nil(t, err)

cache.config.EvictionEnabled = false
Expand Down
6 changes: 6 additions & 0 deletions txcache/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,11 @@ type TxGasHandler interface {
IsInterfaceNil() bool
}

// AccountNonceProvider defines the behavior of a component able to provide the nonce for an account
type AccountNonceProvider interface {
GetAccountNonce(accountKey []byte) (uint64, error)
IsInterfaceNil() bool
}

// ForEachTransaction is an iterator callback
type ForEachTransaction func(txHash []byte, value *WrappedTransaction)
62 changes: 58 additions & 4 deletions txcache/selection.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package txcache

import (
"container/heap"

logger "github.com/multiversx/mx-chain-logger-go"
)

func (cache *TxCache) doSelectTransactions(gasRequested uint64, maxNum int) (bunchOfTransactions, uint64) {
Expand All @@ -12,11 +14,11 @@ func (cache *TxCache) doSelectTransactions(gasRequested uint64, maxNum int) (bun
bunches = append(bunches, sender.getSequentialTxs())
}

return selectTransactionsFromBunches(bunches, gasRequested, maxNum)
return cache.selectTransactionsFromBunches(bunches, gasRequested, maxNum)
}

// Selection tolerates concurrent transaction additions / removals.
func selectTransactionsFromBunches(bunches []bunchOfTransactions, gasRequested uint64, maxNum int) (bunchOfTransactions, uint64) {
func (cache *TxCache) selectTransactionsFromBunches(bunches []bunchOfTransactions, gasRequested uint64, maxNum int) (bunchOfTransactions, uint64) {
selectedTransactions := make(bunchOfTransactions, 0, initialCapacityOfSelectionSlice)

// Items popped from the heap are added to "selectedTransactions".
Expand Down Expand Up @@ -45,6 +47,7 @@ func selectTransactionsFromBunches(bunches []bunchOfTransactions, gasRequested u
// Always pick the best transaction.
item := heap.Pop(transactionsHeap).(*transactionsHeapItem)
gasLimit := item.transaction.Tx.GetGasLimit()
nonce := item.transaction.Tx.GetNonce()

if accumulatedGas+gasLimit > gasRequested {
break
Expand All @@ -53,8 +56,40 @@ func selectTransactionsFromBunches(bunches []bunchOfTransactions, gasRequested u
break
}

accumulatedGas += gasLimit
selectedTransactions = append(selectedTransactions, item.transaction)
cache.askAboutAccountNonceIfNecessary(item)

isInitialGap := item.transactionIndex == 0 && item.senderNonceTold && nonce > item.senderNonce
if isInitialGap {
if logSelect.GetLevel() <= logger.LogTrace {
logSelect.Trace("TxCache.selectTransactionsFromBunches, initial gap",
"tx", item.transaction.TxHash,
"nonce", nonce,
"sender", item.transaction.Tx.GetSndAddr(),
"senderNonce", item.senderNonce,
)
}

// Item was popped from the heap, but not used downstream.
// Therefore, the sender is completely ignored in the current selection session.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense to not select any transaction of a given sender, if we've detected an initial gap.

continue
}

isLowerNonce := item.senderNonceTold && nonce < item.senderNonce
if isLowerNonce {
if logSelect.GetLevel() <= logger.LogTrace {
logSelect.Trace("TxCache.selectTransactionsFromBunches, lower nonce",
"tx", item.transaction.TxHash,
"nonce", nonce,
"sender", item.transaction.Tx.GetSndAddr(),
"senderNonce", item.senderNonce,
)
}

// Transaction isn't selected, but the sender is still in the game (will contribute with other transactions).
} else {
accumulatedGas += gasLimit
selectedTransactions = append(selectedTransactions, item.transaction)
}

// If there are more transactions in the same bunch (same sender as the popped item),
// add the next one to the heap (to compete with the others).
Expand All @@ -69,3 +104,22 @@ func selectTransactionsFromBunches(bunches []bunchOfTransactions, gasRequested u

return selectedTransactions, accumulatedGas
}

func (cache *TxCache) askAboutAccountNonceIfNecessary(item *transactionsHeapItem) {
Copy link
Contributor

Choose a reason for hiding this comment

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

requestAccountNonceIfNecessary ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, renamed.

if item.senderNonceAsked {
return
}

item.senderNonceAsked = true

sender := item.transaction.Tx.GetSndAddr()
senderNonce, err := cache.accountNonceProvider.GetAccountNonce(sender)
if err != nil {
// Hazardous; should never happen.
logSelect.Debug("TxCache.askAboutAccountNonceIfNecessary: nonce not available", "sender", sender, "err", err)
return
}

item.senderNonceTold = true
item.senderNonce = senderNonce
}
Loading
Loading