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 35 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
7 changes: 1 addition & 6 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 @@ -50,9 +51,6 @@ var ErrFailedCacheEviction = errors.New("failed eviction within cache")
// ErrImmuneItemsCapacityReached signals that capacity for immune items is reached
var ErrImmuneItemsCapacityReached = errors.New("capacity reached for immune items")

// ErrItemAlreadyInCache signals that an item is already in cache
var ErrItemAlreadyInCache = errors.New("item already in cache")

// ErrCacheSizeInvalid signals that size of cache is less than 1
var ErrCacheSizeInvalid = errors.New("cache size is less than 1")

Expand All @@ -71,9 +69,6 @@ var ErrNegativeSizeInBytes = errors.New("negative size in bytes")
// ErrNilTimeCache signals that a nil time cache has been provided
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")

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

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.20
require (
github.com/hashicorp/golang-lru v0.6.0
github.com/multiversx/concurrent-map v0.1.4
github.com/multiversx/mx-chain-core-go v1.2.21
github.com/multiversx/mx-chain-core-go v1.2.23
github.com/multiversx/mx-chain-logger-go v1.0.15
github.com/stretchr/testify v1.7.2
github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ github.com/mr-tron/base58 v1.2.0 h1:T/HDJBh4ZCPbU39/+c3rRvE0uKBQlU27+QI8LJ4t64o=
github.com/mr-tron/base58 v1.2.0/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc=
github.com/multiversx/concurrent-map v0.1.4 h1:hdnbM8VE4b0KYJaGY5yJS2aNIW9TFFsUYwbO0993uPI=
github.com/multiversx/concurrent-map v0.1.4/go.mod h1:8cWFRJDOrWHOTNSqgYCUvwT7c7eFQ4U2vKMOp4A/9+o=
github.com/multiversx/mx-chain-core-go v1.2.21 h1:+XVKznPTlUU5EFS1A8chtS8fStW60upRIyF4Pgml19I=
github.com/multiversx/mx-chain-core-go v1.2.21/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE=
github.com/multiversx/mx-chain-core-go v1.2.23 h1:8WlCGqJHR2HQ0vN4feJwb7W4VrCwBGIzPPHunOOg5Wc=
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to update before.

github.com/multiversx/mx-chain-core-go v1.2.23/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE=
github.com/multiversx/mx-chain-logger-go v1.0.15 h1:HlNdK8etyJyL9NQ+6mIXyKPEBo+wRqOwi3n+m2QIHXc=
github.com/multiversx/mx-chain-logger-go v1.0.15/go.mod h1:t3PRKaWB1M+i6gUfD27KXgzLJJC+mAQiN+FLlL1yoGQ=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
Expand Down
80 changes: 80 additions & 0 deletions testscommon/txcachemocks/accountStateProviderMock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package txcachemocks

import (
"math/big"
"sync"

"github.com/multiversx/mx-chain-storage-go/types"
)

// AccountStateProviderMock -
type AccountStateProviderMock struct {
mutex sync.Mutex

AccountStateByAddress map[string]*types.AccountState
GetAccountStateCalled func(address []byte) (*types.AccountState, error)
}

// NewAccountStateProviderMock -
func NewAccountStateProviderMock() *AccountStateProviderMock {
return &AccountStateProviderMock{
AccountStateByAddress: make(map[string]*types.AccountState),
}
}

// SetNonce -
func (mock *AccountStateProviderMock) SetNonce(address []byte, nonce uint64) {
mock.mutex.Lock()
defer mock.mutex.Unlock()

key := string(address)

if mock.AccountStateByAddress[key] == nil {
mock.AccountStateByAddress[key] = newDefaultAccountState()
}

mock.AccountStateByAddress[key].Nonce = nonce
}

// SetBalance -
func (mock *AccountStateProviderMock) SetBalance(address []byte, balance *big.Int) {
mock.mutex.Lock()
defer mock.mutex.Unlock()

key := string(address)

if mock.AccountStateByAddress[key] == nil {
mock.AccountStateByAddress[key] = newDefaultAccountState()
}

mock.AccountStateByAddress[key].Balance = balance
}

// GetAccountState -
func (mock *AccountStateProviderMock) GetAccountState(address []byte) (*types.AccountState, error) {
mock.mutex.Lock()
defer mock.mutex.Unlock()

if mock.GetAccountStateCalled != nil {
return mock.GetAccountStateCalled(address)
}

state, ok := mock.AccountStateByAddress[string(address)]
if ok {
return state, nil
}

return newDefaultAccountState(), nil
}

// IsInterfaceNil -
func (mock *AccountStateProviderMock) IsInterfaceNil() bool {
return mock == nil
}

func newDefaultAccountState() *types.AccountState {
return &types.AccountState{
Nonce: 0,
Balance: big.NewInt(1000000000000000000),
}
}
23 changes: 20 additions & 3 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, nicely sorted by nonce

// Holds selected transactions
selectedTransactions := empty
Expand Down Expand Up @@ -154,8 +154,6 @@ 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.
- **Duplicates-free:** No duplicate transactions are included.
- **Sorted by nonce:** Transactions are ordered in ascending order based on their nonce values.

- **Prepare the heap:**
Expand All @@ -181,7 +179,26 @@ 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, lazily (when needed).
raduchis marked this conversation as resolved.
Show resolved Hide resolved
- If an initial nonce gap is detected, the sender is (completely) skipped in the current selection session.
- If a middle nonce gap is detected, the sender is skipped (from now on) in the current selection session.
- Transactions with nonces lower than the current nonce of the sender are skipped.
- Transactions with duplicate nonces are skipped. See paragraph 5 for more details.
raduchis marked this conversation as resolved.
Show resolved Hide resolved
- Badly guarded transactions are skipped.
- Once the accumulated fees of selected transactions of a given sender exceed the sender's balance, the sender is skipped (from now one).


### Paragraph 5

On the node's side, the selected transactions are shuffled using a deterministic algorithm. This shuffling ensures that the transaction order remains unpredictable to the proposer, effectively preventing _front-running attacks_. Therefore, being selected first by the mempool does not guarantee that a transaction will be included first in the block. Additionally, selection by the mempool does not ensure inclusion in the very next block, as the proposer has the final authority on which transactions to include, based on **the remaining space available** in the block.

### Order of transactions of the same sender

Transactions from the same sender are organized based on specific rules to ensure proper sequencing for the selection flow:

1. **Nonce ascending**: transactions are primarily sorted by their nonce values in ascending order. This sequence ensures that the transactions are processed in the order intended by the sender, as the nonce represents the transaction number in the sender's sequence.

2. **Gas price descending (same nonce)**: if multiple transactions share the same nonce, they are sorted by their gas prices in descending order - transactions offering higher gas prices are prioritized. This mechanism allows one to easily override a pending transaction with a higher gas price.

3. **Hash ascending (same nonce and gas price)**: for transactions that have identical nonce and gas price, the tie is broken by sorting them based on their transaction hash in ascending order. This provides a consistent and deterministic ordering when other factors are equal. While this ordering isn't a critical aspect of the mempool's operation, it ensures logical consistency.
1 change: 1 addition & 0 deletions txcache/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ package txcache
const diagnosisMaxTransactionsToDisplay = 10000
const diagnosisSelectionGasRequested = 10_000_000_000
const initialCapacityOfSelectionSlice = 30000
const selectionLoopDurationCheckInterval = 16
raduchis marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 2 additions & 8 deletions txcache/crossTxCache.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func (cache *CrossTxCache) ImmunizeTxsAgainstEviction(keys [][]byte) {

// AddTx adds a transaction in the cache
func (cache *CrossTxCache) AddTx(tx *WrappedTransaction) (has, added bool) {
log.Trace("CrossTxCache.AddTx", "name", cache.config.Name, "txHash", tx.TxHash)
return cache.HasOrAdd(tx.TxHash, tx, int(tx.Size))
}

Expand Down Expand Up @@ -93,6 +94,7 @@ func (cache *CrossTxCache) Peek(key []byte) (value interface{}, ok bool) {

// RemoveTxByHash removes tx by hash
func (cache *CrossTxCache) RemoveTxByHash(txHash []byte) bool {
log.Trace("CrossTxCache.RemoveTxByHash", "name", cache.config.Name, "txHash", txHash)
return cache.RemoveWithResult(txHash)
}

Expand All @@ -115,14 +117,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
13 changes: 1 addition & 12 deletions txcache/diagnosis.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
"math"
"strings"

"github.com/multiversx/mx-chain-core-go/core"
Expand All @@ -26,7 +25,6 @@ type printedTransaction struct {
func (cache *TxCache) Diagnose(_ bool) {
cache.diagnoseCounters()
cache.diagnoseTransactions()
cache.diagnoseSelection()
}

func (cache *TxCache) diagnoseCounters() {
Expand Down Expand Up @@ -104,19 +102,10 @@ func convertWrappedTransactionToPrintedTransaction(wrappedTx *WrappedTransaction
GasPrice: transaction.GetGasPrice(),
GasLimit: transaction.GetGasLimit(),
DataLength: len(transaction.GetData()),
PPU: wrappedTx.PricePerUnit.Load(),
PPU: wrappedTx.PricePerUnit,
}
}

func (cache *TxCache) diagnoseSelection() {
if logDiagnoseSelection.GetLevel() > logger.LogDebug {
return
}

transactions, _ := cache.doSelectTransactions(diagnosisSelectionGasRequested, math.MaxInt)
displaySelectionOutcome(logDiagnoseSelection, "diagnoseSelection", transactions)
}

func displaySelectionOutcome(contextualLogger logger.Logger, linePrefix string, transactions []*WrappedTransaction) {
if contextualLogger.GetLevel() > logger.LogTrace {
return
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
8 changes: 8 additions & 0 deletions txcache/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package txcache

import "errors"

var errNilTxGasHandler = errors.New("nil tx gas handler")
var errNilAccountStateProvider = errors.New("nil account state provider")
var errItemAlreadyInCache = errors.New("item already in cache")
var errEmptyBunchOfTransactions = errors.New("empty bunch of transactions")
23 changes: 8 additions & 15 deletions txcache/eviction.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,14 @@ func (cache *TxCache) evictLeastLikelyToSelectTransactions() *evictionJournal {
heap.Init(transactionsHeap)

// Initialize the heap with the first transaction of each bunch
for i, bunch := range bunches {
if len(bunch) == 0 {
// Some senders may have no transaction anymore (hazardous concurrent removals).
for _, bunch := range bunches {
item, err := newTransactionsHeapItem(bunch)
if err != nil {
continue
}

// Items will be reused (see below). Each sender gets one (and only one) item in the heap.
heap.Push(transactionsHeap, &transactionsHeapItem{
senderIndex: i,
transactionIndex: 0,
transaction: bunch[0],
})
heap.Push(transactionsHeap, item)
}

for pass := 0; cache.isCapacityExceeded(); pass++ {
Expand All @@ -130,16 +126,13 @@ func (cache *TxCache) evictLeastLikelyToSelectTransactions() *evictionJournal {
break
}

transactionsToEvict = append(transactionsToEvict, item.transaction)
transactionsToEvictHashes = append(transactionsToEvictHashes, item.transaction.TxHash)
transactionsToEvict = append(transactionsToEvict, item.currentTransaction)
transactionsToEvictHashes = append(transactionsToEvictHashes, item.currentTransaction.TxHash)

// 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 in being "the worst").
item.transactionIndex++

if item.transactionIndex < len(bunches[item.senderIndex]) {
// Item is reused (same originating sender), pushed back on the heap.
item.transaction = bunches[item.senderIndex][item.transactionIndex]
// Item is reused (same originating sender), pushed back on the heap.
if item.gotoNextTransaction() {
heap.Push(transactionsHeap, item)
}
}
Expand Down
12 changes: 8 additions & 4 deletions txcache/eviction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ func TestTxCache_DoEviction_BecauseOfCount(t *testing.T) {
EvictionEnabled: true,
NumItemsToPreemptivelyEvict: 1,
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()

cache, err := NewTxCache(config, txGasHandler)
require.Nil(t, err)
require.NotNil(t, cache)
Expand Down Expand Up @@ -56,6 +58,7 @@ func TestTxCache_DoEviction_BecauseOfSize(t *testing.T) {
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()

cache, err := NewTxCache(config, txGasHandler)
require.Nil(t, err)
require.NotNil(t, cache)
Expand Down Expand Up @@ -91,6 +94,7 @@ func TestTxCache_DoEviction_DoesNothingWhenAlreadyInProgress(t *testing.T) {
}

txGasHandler := txcachemocks.NewTxGasHandlerMock()

cache, err := NewTxCache(config, txGasHandler)
require.Nil(t, err)
require.NotNil(t, cache)
Expand Down Expand Up @@ -216,8 +220,8 @@ func TestBenchmarkTxCache_DoEviction(t *testing.T) {
// Thread(s) per core: 2
// Core(s) per socket: 4
//
// 0.093771s (TestBenchmarkTxCache_DoEviction_Benchmark/numSenders_=_35000,_numTransactions_=_10)
// 0.424683s (TestBenchmarkTxCache_DoEviction_Benchmark/numSenders_=_100000,_numTransactions_=_5)
// 0.448017s (TestBenchmarkTxCache_DoEviction_Benchmark/numSenders_=_10000,_numTransactions_=_100)
// 0.476738s (TestBenchmarkTxCache_DoEviction_Benchmark/numSenders_=_400000,_numTransactions_=_1)
// 0.119274s (TestBenchmarkTxCache_DoEviction/numSenders_=_35000,_numTransactions_=_10)
// 0.484147s (TestBenchmarkTxCache_DoEviction/numSenders_=_100000,_numTransactions_=_5)
// 0.504588s (TestBenchmarkTxCache_DoEviction/numSenders_=_10000,_numTransactions_=_100)
// 0.571885s (TestBenchmarkTxCache_DoEviction/numSenders_=_400000,_numTransactions_=_1)
}
7 changes: 7 additions & 0 deletions txcache/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"math/big"

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

// TxGasHandler handles a transaction gas and gas cost
Expand All @@ -12,5 +13,11 @@ type TxGasHandler interface {
IsInterfaceNil() bool
}

// AccountStateProvider defines the behavior of a component able to provide the state of an account
type AccountStateProvider interface {
GetAccountState(accountKey []byte) (*types.AccountState, error)
IsInterfaceNil() bool
}

// ForEachTransaction is an iterator callback
type ForEachTransaction func(txHash []byte, value *WrappedTransaction)
Loading
Loading