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

Mempool: completely handle fees and transferred balances within "detectWillFeeExceedBalance()" #6637

Merged
merged 8 commits into from
Nov 28, 2024
88 changes: 83 additions & 5 deletions process/block/preprocess/selectionSession.go
Original file line number Diff line number Diff line change
@@ -1,32 +1,58 @@
package preprocess

import (
"bytes"
"errors"
"math/big"

"github.com/multiversx/mx-chain-core-go/core"
"github.com/multiversx/mx-chain-core-go/core/check"
"github.com/multiversx/mx-chain-core-go/data"
"github.com/multiversx/mx-chain-core-go/data/transaction"
"github.com/multiversx/mx-chain-core-go/marshal"
"github.com/multiversx/mx-chain-go/process"
"github.com/multiversx/mx-chain-go/state"
"github.com/multiversx/mx-chain-go/storage/txcache"
vmcommon "github.com/multiversx/mx-chain-vm-common-go"
"github.com/multiversx/mx-chain-vm-common-go/parsers"
)

type selectionSession struct {
accountsAdapter state.AccountsAdapter
transactionsProcessor process.TransactionProcessor
callArgumentsParser process.CallArgumentsParser
esdtTransferParser vmcommon.ESDTTransferParser
}

func newSelectionSession(accountsAdapter state.AccountsAdapter, transactionsProcessor process.TransactionProcessor) (*selectionSession, error) {
if check.IfNil(accountsAdapter) {
type argsSelectionSession struct {
accountsAdapter state.AccountsAdapter
transactionsProcessor process.TransactionProcessor
marshalizer marshal.Marshalizer
}

func newSelectionSession(args argsSelectionSession) (*selectionSession, error) {
if check.IfNil(args.accountsAdapter) {
return nil, process.ErrNilAccountsAdapter
}
if check.IfNil(transactionsProcessor) {
if check.IfNil(args.transactionsProcessor) {
return nil, process.ErrNilTxProcessor
}
if check.IfNil(args.marshalizer) {
return nil, process.ErrNilMarshalizer
}

argsParser := parsers.NewCallArgsParser()

esdtTransferParser, err := parsers.NewESDTTransferParser(args.marshalizer)
if err != nil {
return nil, err
}

return &selectionSession{
accountsAdapter: accountsAdapter,
transactionsProcessor: transactionsProcessor,
accountsAdapter: args.accountsAdapter,
transactionsProcessor: args.transactionsProcessor,
callArgumentsParser: argsParser,
esdtTransferParser: esdtTransferParser,
}, nil
}

Expand Down Expand Up @@ -72,6 +98,58 @@ func (session *selectionSession) IsBadlyGuarded(tx data.TransactionHandler) bool
return errors.Is(err, process.ErrTransactionNotExecutable)
}

// GetTransferredValue returns the value transferred by a transaction.
func (session *selectionSession) GetTransferredValue(tx data.TransactionHandler) *big.Int {
hasValue := tx.GetValue() != nil && tx.GetValue().Sign() != 0
if hasValue {
// Early exit (optimization): a transaction can either bear a regular value or be a "MultiESDTNFTTransfer".
return tx.GetValue()
}

hasData := len(tx.GetData()) > 0
if !hasData {
// Early exit (optimization): no "MultiESDTNFTTransfer" to parse.
return tx.GetValue()
}

maybeMultiTransfer := bytes.HasPrefix(tx.GetData(), []byte(core.BuiltInFunctionMultiESDTNFTTransfer))
if !maybeMultiTransfer {
// Early exit (optimization).
return nil
}

function, args, err := session.callArgumentsParser.ParseData(string(tx.GetData()))
if err != nil {
return nil
}

if function != core.BuiltInFunctionMultiESDTNFTTransfer {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Superfluous (somehow)?

// Early exit (optimization).
return nil
}

esdtTransfers, err := session.esdtTransferParser.ParseESDTTransfers(tx.GetSndAddr(), tx.GetRcvAddr(), function, args)
if err != nil {
return nil
}

accumulatedNativeValue := big.NewInt(0)

for _, transfer := range esdtTransfers.ESDTTransfers {
Copy link
Contributor

Choose a reason for hiding this comment

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

does this add a negligible overhead? A lot of transactions are with MultiESDTNFTTransfer
can the entire method be parallelized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Very good question, will benchmark ASAP 🙌

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

// NOTE: 20% is also due to the require() / assert() calls.
// 0.012993s (TestBenchmarkSelectionSession_GetTransferredValue/numTransactions_=_5_000)
// 0.024580s (TestBenchmarkSelectionSession_GetTransferredValue/numTransactions_=_10_000)
// 0.048808s (TestBenchmarkSelectionSession_GetTransferredValue/numTransactions_=_20_000)

Thus, if we select ~10k, about 20 milliseconds (a bit much) will be spent in GetTransferredValue(). The selection duration is capped, though. There is still room for optimization.

We can move the call to GetTransferredValue() to the moment the transaction is added to the pool (additions are concurrent, also).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MX-16211

if transfer.ESDTTokenNonce != 0 {
continue
}
if string(transfer.ESDTTokenName) != vmcommon.EGLDIdentifier {
// We only care about native transfers.
continue
}

_ = accumulatedNativeValue.Add(accumulatedNativeValue, transfer.ESDTValue)
}

return accumulatedNativeValue
}

// IsInterfaceNil returns true if there is no value under the interface
func (session *selectionSession) IsInterfaceNil() bool {
return session == nil
Expand Down
77 changes: 72 additions & 5 deletions process/block/preprocess/selectionSession_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package preprocess
import (
"bytes"
"fmt"
"math/big"
"testing"

"github.com/multiversx/mx-chain-core-go/data/transaction"
"github.com/multiversx/mx-chain-core-go/marshal"
"github.com/multiversx/mx-chain-go/process"
"github.com/multiversx/mx-chain-go/state"
"github.com/multiversx/mx-chain-go/testscommon"
Expand All @@ -17,15 +19,27 @@ import (
func TestNewSelectionSession(t *testing.T) {
t.Parallel()

session, err := newSelectionSession(nil, &testscommon.TxProcessorStub{})
session, err := newSelectionSession(argsSelectionSession{
accountsAdapter: nil,
transactionsProcessor: &testscommon.TxProcessorStub{},
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.Nil(t, session)
require.ErrorIs(t, err, process.ErrNilAccountsAdapter)

session, err = newSelectionSession(&stateMock.AccountsStub{}, nil)
session, err = newSelectionSession(argsSelectionSession{
accountsAdapter: &stateMock.AccountsStub{},
transactionsProcessor: nil,
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.Nil(t, session)
require.ErrorIs(t, err, process.ErrNilTxProcessor)

session, err = newSelectionSession(&stateMock.AccountsStub{}, &testscommon.TxProcessorStub{})
session, err = newSelectionSession(argsSelectionSession{
accountsAdapter: &stateMock.AccountsStub{},
transactionsProcessor: &testscommon.TxProcessorStub{},
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.NoError(t, err)
require.NotNil(t, session)
}
Expand Down Expand Up @@ -57,7 +71,11 @@ func TestSelectionSession_GetAccountState(t *testing.T) {
return nil, fmt.Errorf("account not found: %s", address)
}

session, err := newSelectionSession(accounts, processor)
session, err := newSelectionSession(argsSelectionSession{
accountsAdapter: accounts,
transactionsProcessor: processor,
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.NoError(t, err)
require.NotNil(t, session)

Expand Down Expand Up @@ -95,7 +113,11 @@ func TestSelectionSession_IsBadlyGuarded(t *testing.T) {
return nil
}

session, err := newSelectionSession(accounts, processor)
session, err := newSelectionSession(argsSelectionSession{
accountsAdapter: accounts,
transactionsProcessor: processor,
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.NoError(t, err)
require.NotNil(t, session)

Expand All @@ -108,3 +130,48 @@ func TestSelectionSession_IsBadlyGuarded(t *testing.T) {
isBadlyGuarded = session.IsBadlyGuarded(&transaction.Transaction{Nonce: 44, SndAddr: []byte("alice")})
require.False(t, isBadlyGuarded)
}

func TestSelectionSession_GetTransferredValue(t *testing.T) {
t.Parallel()

session, err := newSelectionSession(argsSelectionSession{
accountsAdapter: &stateMock.AccountsStub{},
transactionsProcessor: &testscommon.TxProcessorStub{},
marshalizer: &marshal.GogoProtoMarshalizer{},
})
require.NoError(t, err)
require.NotNil(t, session)

t.Run("with value", func(t *testing.T) {
value := session.GetTransferredValue(&transaction.Transaction{
Value: big.NewInt(1000000000000000000),
})
require.Equal(t, big.NewInt(1000000000000000000), value)
})

t.Run("with value and data", func(t *testing.T) {
value := session.GetTransferredValue(&transaction.Transaction{
Value: big.NewInt(1000000000000000000),
Data: []byte("data"),
})
require.Equal(t, big.NewInt(1000000000000000000), value)
})

t.Run("native transfer within MultiESDTNFTTransfer", func(t *testing.T) {
value := session.GetTransferredValue(&transaction.Transaction{
SndAddr: testscommon.TestPubKeyAlice,
RcvAddr: testscommon.TestPubKeyAlice,
Data: []byte("MultiESDTNFTTransfer@8049d639e5a6980d1cd2392abcce41029cda74a1563523a202f09641cc2618f8@03@4e46542d313233343536@0a@01@544553542d393837363534@01@01@45474c442d303030303030@@0de0b6b3a7640000"),
})
require.Equal(t, big.NewInt(1000000000000000000), value)
})

t.Run("native transfer within MultiESDTNFTTransfer; transfer & execute", func(t *testing.T) {
value := session.GetTransferredValue(&transaction.Transaction{
SndAddr: testscommon.TestPubKeyAlice,
RcvAddr: testscommon.TestPubKeyAlice,
Data: []byte("MultiESDTNFTTransfer@00000000000000000500b9353fe8407f87310c87e12fa1ac807f0485da39d152@03@4e46542d313233343536@01@01@4e46542d313233343536@2a@01@45474c442d303030303030@@0de0b6b3a7640000@64756d6d79@07"),
})
require.Equal(t, big.NewInt(1000000000000000000), value)
})
}
6 changes: 5 additions & 1 deletion process/block/preprocess/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,11 @@ func (txs *transactions) computeSortedTxs(
sortedTransactionsProvider := createSortedTransactionsProvider(txShardPool)
log.Debug("computeSortedTxs.GetSortedTransactions")

session, err := newSelectionSession(txs.basePreProcess.accounts, txs.txProcessor)
session, err := newSelectionSession(argsSelectionSession{
accountsAdapter: txs.accounts,
transactionsProcessor: txs.txProcessor,
marshalizer: txs.marshalizer,
})
if err != nil {
return nil, nil, err
}
Expand Down