From afd51e206ebb27e6df6743323444996cdbb30b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 25 Nov 2024 14:21:38 +0200 Subject: [PATCH 1/3] Handle consumed balance. --- .../txcachemocks/selectionSessionMock.go | 16 +++++++++--- txcache/diagnosis.go | 2 +- txcache/interface.go | 1 + txcache/selection.go | 2 +- txcache/transactionsHeapItem.go | 26 +++++++++++-------- txcache/transactionsHeapItem_test.go | 19 ++++++++------ 6 files changed, 42 insertions(+), 24 deletions(-) diff --git a/testscommon/txcachemocks/selectionSessionMock.go b/testscommon/txcachemocks/selectionSessionMock.go index 32f852a8..c33999a7 100644 --- a/testscommon/txcachemocks/selectionSessionMock.go +++ b/testscommon/txcachemocks/selectionSessionMock.go @@ -12,9 +12,10 @@ import ( type SelectionSessionMock struct { mutex sync.Mutex - AccountStateByAddress map[string]*types.AccountState - GetAccountStateCalled func(address []byte) (*types.AccountState, error) - IsBadlyGuardedCalled func(tx data.TransactionHandler) bool + AccountStateByAddress map[string]*types.AccountState + GetAccountStateCalled func(address []byte) (*types.AccountState, error) + IsBadlyGuardedCalled func(tx data.TransactionHandler) bool + GetTransferredValueCalled func(tx data.TransactionHandler) *big.Int } // NewSelectionSessionMock - @@ -78,6 +79,15 @@ func (mock *SelectionSessionMock) IsBadlyGuarded(tx data.TransactionHandler) boo return false } +// GetTransferredValue - +func (mock *SelectionSessionMock) GetTransferredValue(tx data.TransactionHandler) *big.Int { + if mock.GetTransferredValueCalled != nil { + return mock.GetTransferredValueCalled(tx) + } + + return tx.GetValue() +} + // IsInterfaceNil - func (mock *SelectionSessionMock) IsInterfaceNil() bool { return mock == nil diff --git a/txcache/diagnosis.go b/txcache/diagnosis.go index 6f693c97..0abbcef7 100644 --- a/txcache/diagnosis.go +++ b/txcache/diagnosis.go @@ -73,7 +73,7 @@ func (cache *TxCache) diagnoseTransactions() { } // marshalTransactionsToNewlineDelimitedJSON converts a list of transactions to a newline-delimited JSON string. -// Note: each line is indexed, to improve readability. The index is easily removable for if separate analysis is needed. +// Note: each line is indexed, to improve readability. The index is easily removable for separate analysis is needed. func marshalTransactionsToNewlineDelimitedJSON(transactions []*WrappedTransaction, linePrefix string) string { builder := strings.Builder{} builder.WriteString("\n") diff --git a/txcache/interface.go b/txcache/interface.go index 6b8b97e0..d8000326 100644 --- a/txcache/interface.go +++ b/txcache/interface.go @@ -17,6 +17,7 @@ type TxGasHandler interface { type SelectionSession interface { GetAccountState(accountKey []byte) (*types.AccountState, error) IsBadlyGuarded(tx data.TransactionHandler) bool + GetTransferredValue(tx data.TransactionHandler) *big.Int IsInterfaceNil() bool } diff --git a/txcache/selection.go b/txcache/selection.go index e21e05e8..2cbdc1df 100644 --- a/txcache/selection.go +++ b/txcache/selection.go @@ -76,7 +76,7 @@ func selectTransactionsFromBunches(session SelectionSession, bunches []bunchOfTr // Transaction isn't selected, but the sender is still in the game (will contribute with other transactions). } else { accumulatedGas += gasLimit - selectedTransactions = append(selectedTransactions, item.selectCurrentTransaction()) + selectedTransactions = append(selectedTransactions, item.selectCurrentTransaction(session)) } // If there are more transactions in the same bunch (same sender as the popped item), diff --git a/txcache/transactionsHeapItem.go b/txcache/transactionsHeapItem.go index 19f8ada7..f0cbadaf 100644 --- a/txcache/transactionsHeapItem.go +++ b/txcache/transactionsHeapItem.go @@ -19,7 +19,7 @@ type transactionsHeapItem struct { latestSelectedTransaction *WrappedTransaction latestSelectedTransactionNonce uint64 - accumulatedFee *big.Int + consumedBalance *big.Int } func newTransactionsHeapItem(bunch bunchOfTransactions) (*transactionsHeapItem, error) { @@ -40,12 +40,12 @@ func newTransactionsHeapItem(bunch bunchOfTransactions) (*transactionsHeapItem, currentTransactionNonce: firstTransaction.Tx.GetNonce(), latestSelectedTransaction: nil, - accumulatedFee: big.NewInt(0), + consumedBalance: big.NewInt(0), }, nil } -func (item *transactionsHeapItem) selectCurrentTransaction() *WrappedTransaction { - item.accumulateFee() +func (item *transactionsHeapItem) selectCurrentTransaction(session SelectionSession) *WrappedTransaction { + item.accumulateConsumedBalance(session) item.latestSelectedTransaction = item.currentTransaction item.latestSelectedTransactionNonce = item.currentTransactionNonce @@ -53,13 +53,16 @@ func (item *transactionsHeapItem) selectCurrentTransaction() *WrappedTransaction return item.currentTransaction } -func (item *transactionsHeapItem) accumulateFee() { +func (item *transactionsHeapItem) accumulateConsumedBalance(session SelectionSession) { fee := item.currentTransaction.Fee - if fee == nil { - return + if fee != nil { + item.consumedBalance.Add(item.consumedBalance, fee) } - item.accumulatedFee.Add(item.accumulatedFee, fee) + transferredValue := session.GetTransferredValue(item.currentTransaction.Tx) + if transferredValue != nil { + item.consumedBalance.Add(item.consumedBalance, transferredValue) + } } func (item *transactionsHeapItem) gotoNextTransaction() bool { @@ -123,16 +126,17 @@ func (item *transactionsHeapItem) detectWillFeeExceedBalance() bool { return false } - futureAccumulatedFee := new(big.Int).Add(item.accumulatedFee, fee) + // Here, we are not interested into an eventual transfer of value (we only check if there's enough balance to pay the transaction fee). + futureConsumedBalance := new(big.Int).Add(item.consumedBalance, fee) senderBalance := item.senderState.Balance - willFeeExceedBalance := futureAccumulatedFee.Cmp(senderBalance) > 0 + willFeeExceedBalance := futureConsumedBalance.Cmp(senderBalance) > 0 if willFeeExceedBalance { logSelect.Trace("transactionsHeapItem.detectWillFeeExceedBalance", "tx", item.currentTransaction.TxHash, "sender", item.sender, "balance", item.senderState.Balance, - "accumulatedFee", item.accumulatedFee, + "consumedBalance", item.consumedBalance, ) } diff --git a/txcache/transactionsHeapItem_test.go b/txcache/transactionsHeapItem_test.go index 9ab581df..dffa9c4b 100644 --- a/txcache/transactionsHeapItem_test.go +++ b/txcache/transactionsHeapItem_test.go @@ -33,12 +33,13 @@ func TestNewTransactionsHeapItem(t *testing.T) { require.Equal(t, bunch[0], item.currentTransaction) require.Equal(t, uint64(42), item.currentTransactionNonce) require.Nil(t, item.latestSelectedTransaction) - require.Equal(t, big.NewInt(0), item.accumulatedFee) + require.Equal(t, big.NewInt(0), item.consumedBalance) }) } func TestTransactionsHeapItem_selectTransaction(t *testing.T) { txGasHandler := txcachemocks.NewTxGasHandlerMock() + session := txcachemocks.NewSelectionSessionMock() a := createTx([]byte("tx-1"), "alice", 42) b := createTx([]byte("tx-2"), "alice", 43) @@ -48,20 +49,20 @@ func TestTransactionsHeapItem_selectTransaction(t *testing.T) { item, err := newTransactionsHeapItem(bunchOfTransactions{a, b}) require.NoError(t, err) - selected := item.selectCurrentTransaction() + selected := item.selectCurrentTransaction(session) require.Equal(t, a, selected) require.Equal(t, a, item.latestSelectedTransaction) require.Equal(t, 42, int(item.latestSelectedTransactionNonce)) - require.Equal(t, "50000000000000", item.accumulatedFee.String()) + require.Equal(t, "50000000000000", item.consumedBalance.String()) ok := item.gotoNextTransaction() require.True(t, ok) - selected = item.selectCurrentTransaction() + selected = item.selectCurrentTransaction(session) require.Equal(t, b, selected) require.Equal(t, b, item.latestSelectedTransaction) require.Equal(t, 43, int(item.latestSelectedTransactionNonce)) - require.Equal(t, "100000000000000", item.accumulatedFee.String()) + require.Equal(t, "100000000000000", item.consumedBalance.String()) ok = item.gotoNextTransaction() require.False(t, ok) @@ -133,7 +134,7 @@ func TestTransactionsHeapItem_detectMiddleGap(t *testing.T) { }) } -func TestTransactionsHeapItem_detectFeeExceededBalance(t *testing.T) { +func TestTransactionsHeapItem_detectWillFeeExceedBalance(t *testing.T) { txGasHandler := txcachemocks.NewTxGasHandlerMock() a := createTx([]byte("tx-1"), "alice", 42) @@ -149,6 +150,8 @@ func TestTransactionsHeapItem_detectFeeExceededBalance(t *testing.T) { }) t.Run("known, not exceeded, then exceeded", func(t *testing.T) { + session := txcachemocks.NewSelectionSessionMock() + item, err := newTransactionsHeapItem(bunchOfTransactions{a, b}) require.NoError(t, err) @@ -158,9 +161,9 @@ func TestTransactionsHeapItem_detectFeeExceededBalance(t *testing.T) { require.False(t, item.detectWillFeeExceedBalance()) - _ = item.selectCurrentTransaction() + _ = item.selectCurrentTransaction(session) _ = item.gotoNextTransaction() - require.Equal(t, "50000000000000", item.accumulatedFee.String()) + require.Equal(t, "50000000000000", item.consumedBalance.String()) require.True(t, item.detectWillFeeExceedBalance()) }) From 00adab2de46a1089739562dd5651f2a7112c5d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 25 Nov 2024 19:59:18 +0200 Subject: [PATCH 2/3] Additional unit tests. --- txcache/transactionsHeapItem_test.go | 43 ++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/txcache/transactionsHeapItem_test.go b/txcache/transactionsHeapItem_test.go index dffa9c4b..96f7afdc 100644 --- a/txcache/transactionsHeapItem_test.go +++ b/txcache/transactionsHeapItem_test.go @@ -139,17 +139,22 @@ func TestTransactionsHeapItem_detectWillFeeExceedBalance(t *testing.T) { a := createTx([]byte("tx-1"), "alice", 42) b := createTx([]byte("tx-2"), "alice", 43) + c := createTx([]byte("tx-3"), "alice", 44).withValue(big.NewInt(1000000000000000000)) + d := createTx([]byte("tx-4"), "alice", 45) + a.precomputeFields(txGasHandler) b.precomputeFields(txGasHandler) + c.precomputeFields(txGasHandler) + d.precomputeFields(txGasHandler) t.Run("unknown", func(t *testing.T) { item, err := newTransactionsHeapItem(bunchOfTransactions{a, b}) - require.NoError(t, err) + require.NoError(t, err) require.False(t, item.detectWillFeeExceedBalance()) }) - t.Run("known, not exceeded, then exceeded", func(t *testing.T) { + t.Run("known, not exceeded, then exceeded (a)", func(t *testing.T) { session := txcachemocks.NewSelectionSessionMock() item, err := newTransactionsHeapItem(bunchOfTransactions{a, b}) @@ -163,8 +168,42 @@ func TestTransactionsHeapItem_detectWillFeeExceedBalance(t *testing.T) { _ = item.selectCurrentTransaction(session) _ = item.gotoNextTransaction() + require.Equal(t, "50000000000000", item.consumedBalance.String()) + require.True(t, item.detectWillFeeExceedBalance()) + }) + + t.Run("known, not exceeded, then exceeded (b)", func(t *testing.T) { + session := txcachemocks.NewSelectionSessionMock() + + item, err := newTransactionsHeapItem(bunchOfTransactions{a, b, c, d}) + require.NoError(t, err) + + item.senderState = &types.AccountState{ + Balance: big.NewInt(1000000000000000000 + 2*50000000000000 + 1), + } + + require.False(t, item.detectWillFeeExceedBalance()) + + // Select "a", move to "b". + _ = item.selectCurrentTransaction(session) + _ = item.gotoNextTransaction() + + require.Equal(t, "50000000000000", item.consumedBalance.String()) + require.False(t, item.detectWillFeeExceedBalance()) + + // Select "b", move to "c". + _ = item.selectCurrentTransaction(session) + _ = item.gotoNextTransaction() + + require.Equal(t, "100000000000000", item.consumedBalance.String()) + require.False(t, item.detectWillFeeExceedBalance()) + + // Select "c", move to "d". + _ = item.selectCurrentTransaction(session) + _ = item.gotoNextTransaction() + require.Equal(t, "1000150000000000000", item.consumedBalance.String()) require.True(t, item.detectWillFeeExceedBalance()) }) } From 0710e1bd5d54a884bff0d5e763a6ba02239800c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Tue, 26 Nov 2024 17:53:40 +0200 Subject: [PATCH 3/3] Delayed commit. --- txcache/testutils_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/txcache/testutils_test.go b/txcache/testutils_test.go index 165818be..2592a834 100644 --- a/txcache/testutils_test.go +++ b/txcache/testutils_test.go @@ -2,6 +2,7 @@ package txcache import ( "encoding/binary" + "math/big" "math/rand" "sync" "time" @@ -181,6 +182,12 @@ func (wrappedTx *WrappedTransaction) withGasLimit(gasLimit uint64) *WrappedTrans return wrappedTx } +func (wrappedTx *WrappedTransaction) withValue(value *big.Int) *WrappedTransaction { + tx := wrappedTx.Tx.(*transaction.Transaction) + tx.Value = value + return wrappedTx +} + func createFakeSenderAddress(senderTag int) []byte { bytes := make([]byte, 32) binary.LittleEndian.PutUint64(bytes, uint64(senderTag))