fix: Remove Incorrect Ordering from DefaultTxPriority (backport #371) (…

…#374) * fix: Remove Incorrect Ordering from DefaultTxPriority (#371) * init * replace existing tx_priorities with nop_tx_priority * fix go.mod --------- Co-authored-by: David Terpay <david.terpay@gmail.com> (cherry picked from commit 234e2ff) * fix mergify commit message * lint * Fix merge --------- Co-authored-by: Nikhil Vasan <97126437+nivasan1@users.noreply.github.com> Co-authored-by: Nikhil Vasan <nikhil@skip.money> Co-authored-by: Eric <eric.warehime@gmail.com>
skip-mev · Jan 20, 2024 · 1d39844 · 1d39844
1 parent e7581ff
commit 1d39844
Show file tree

Hide file tree

Showing 9 changed files with 211 additions and 400 deletions.
diff --git a/abci/abci_test.go b/abci/abci_test.go
@@ -101,7 +101,7 @@ func (s *ProposalsTestSuite) TestPrepareProposal() {
 		tx1, err := testutils.CreateRandomTx(
 			s.encodingConfig.TxConfig,
 			s.accounts[0],
-			0,
+			1,
 			1,
 			0,
 			1,
@@ -112,8 +112,8 @@ func (s *ProposalsTestSuite) TestPrepareProposal() {
 		// Create a second random transaction that will be inserted into the default lane
 		tx2, err := testutils.CreateRandomTx(
 			s.encodingConfig.TxConfig,
-			s.accounts[1],
-			1,
+			s.accounts[0],
+			0,
 			1,
 			0,
 			1,
@@ -1234,27 +1234,46 @@ func (s *ProposalsTestSuite) TestPrepareProcessParity() {
 	numAccounts := 25
 	accounts := testutils.RandomAccounts(s.random, numAccounts)
 
+	feeDenoms := []string{
+		s.gasTokenDenom,
+		"eth",
+		"btc",
+		"usdt",
+		"usdc",
+	}
+
 	// Create a bunch of transactions to insert into the default lane
 	txsToInsert := []sdk.Tx{}
 	validationMap := make(map[sdk.Tx]bool)
-	for _, account := range accounts {
-		for nonce := uint64(0); nonce < numTxsPerAccount; nonce++ {
-			// create a random fee amount
-			feeAmount := math.NewInt(int64(rand.Intn(100000)))
-			tx, err := testutils.CreateRandomTx(
-				s.encodingConfig.TxConfig,
-				account,
-				nonce,
-				1,
-				0,
-				1,
-				sdk.NewCoin(s.gasTokenDenom, feeAmount),
-			)
-			s.Require().NoError(err)
+	for nonce := uint64(0); nonce < numTxsPerAccount*uint64(numAccounts); nonce++ {
+		fees := []sdk.Coin{}
+		// choose a random set of fee denoms
+		perm := rand.Perm(len(feeDenoms))
+		for i := 0; i < 1+rand.Intn(len(feeDenoms)-1); i++ {
+			fees = append(fees, sdk.NewCoin(feeDenoms[perm[i]], math.NewInt(int64(rand.Intn(100000)))))
+		}
 
-			txsToInsert = append(txsToInsert, tx)
-			validationMap[tx] = true
+		// choose a random set of accounts
+		perm = rand.Perm(len(accounts))
+		signers := []testutils.Account{}
+		for i := 0; i < 1+rand.Intn(len(accounts)-1); i++ {
+			signers = append(signers, accounts[perm[i]])
 		}
+
+		// create a random fee amount
+		tx, err := testutils.CreateRandomTxMultipleSigners(
+			s.encodingConfig.TxConfig,
+			signers,
+			nonce,
+			1,
+			0,
+			1,
+			fees...,
+		)
+		s.Require().NoError(err)
+
+		txsToInsert = append(txsToInsert, tx)
+		validationMap[tx] = true
 	}
 
 	// Set up the default lane with the transactions

diff --git a/abci/utils_test.go b/abci/utils_test.go
@@ -65,7 +65,7 @@ func (s *ProposalsTestSuite) setUpCustomMatchHandlerLane(maxBlockSpace math.Lega
 
 	options := []base.LaneOption{
 		base.WithMatchHandler(mh),
-		base.WithMempoolConfigs[string](cfg, base.DefaultTxPriority()),
+		base.WithMempoolConfigs(cfg, base.DefaultTxPriority()),
 	}
 
 	lane, err := base.NewBaseLane(
@@ -129,7 +129,7 @@ func (s *ProposalsTestSuite) setUpPanicLane(name string, maxBlockSpace math.Lega
 
 	options := []base.LaneOption{
 		base.WithMatchHandler(base.DefaultMatchHandler()),
-		base.WithMempoolConfigs[string](cfg, base.DefaultTxPriority()),
+		base.WithMempoolConfigs(cfg, base.DefaultTxPriority()),
 		base.WithPrepareLaneHandler(base.PanicPrepareLaneHandler()),
 		base.WithProcessLaneHandler(base.PanicProcessLaneHandler()),
 	}

diff --git a/block/base/mempool_test.go b/block/base/mempool_test.go
@@ -1,4 +1,4 @@
-package base
+package base_test
 
 import (
 	"fmt"
@@ -10,15 +10,22 @@ import (
 	"github.com/stretchr/testify/require"
 
 	signerextraction "github.com/skip-mev/block-sdk/adapters/signer_extraction_adapter"
+	"github.com/skip-mev/block-sdk/block/base"
 	"github.com/skip-mev/block-sdk/testutils"
 )
 
+type txGen struct {
+	acc    testutils.Account
+	nonce  uint64
+	amount sdk.Coin
+}
+
 func TestMempoolComparison(t *testing.T) {
 	acct := testutils.RandomAccounts(rand.New(rand.NewSource(1)), 2)
 	txc := testutils.CreateTestEncodingConfig().TxConfig
 	ctx := testutils.CreateBaseSDKContext(t)
-	mp := NewMempool(
-		DefaultTxPriority(),
+	mp := base.NewMempool(
+		base.DefaultTxPriority(),
 		txc.TxEncoder(),
 		signerextraction.NewDefaultAdapter(),
 		1000,
@@ -66,7 +73,7 @@ func TestMempoolComparison(t *testing.T) {
 		require.NoError(t, err)
 		output, err := mp.Compare(ctx, tx1, tx2)
 		require.NoError(t, err)
-		require.Equal(t, -1, output)
+		require.Equal(t, 0, output)
 	})
 	t.Run("test diff account, tx1 lt amount, tx1 gt nonce, diff denoms", func(t *testing.T) {
 		tx1, err := testutils.CreateTx(txc, acct[0], 1, 0, nil, sdk.NewCoin("nonstake", sdkmath.NewInt(1)))
@@ -87,3 +94,80 @@ func TestMempoolComparison(t *testing.T) {
 		require.Equal(t, 0, output)
 	})
 }
+
+func TestMempoolSelect(t *testing.T) {
+	acct := testutils.RandomAccounts(rand.New(rand.NewSource(1)), 2)
+	txc := testutils.CreateTestEncodingConfig().TxConfig
+	ctx := testutils.CreateBaseSDKContext(t)
+	se := signerextraction.NewDefaultAdapter()
+	mp := base.NewMempool(
+		base.DefaultTxPriority(),
+		txc.TxEncoder(),
+		se,
+		1000,
+	)
+	tests := []struct {
+		name     string
+		inputs   []txGen
+		expected []txGen
+	}{
+		{
+			name: "test1",
+			inputs: []txGen{
+				{
+					acc:    acct[0],
+					amount: sdk.NewCoin("stake", sdkmath.NewInt(1)),
+					nonce:  0,
+				},
+				{
+					acc:    acct[0],
+					amount: sdk.NewCoin("notstake", sdkmath.NewInt(2)),
+					nonce:  1,
+				},
+			},
+			expected: []txGen{
+				{
+					acc:    acct[0],
+					amount: sdk.NewCoin("stake", sdkmath.NewInt(1)),
+					nonce:  0,
+				},
+				{
+					acc:    acct[0],
+					amount: sdk.NewCoin("notstake", sdkmath.NewInt(2)),
+					nonce:  1,
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		// insert all input txs
+		t.Run(tc.name, func(t *testing.T) {
+			for _, tx := range tc.inputs {
+				inputTx, err := testutils.CreateTx(txc, tx.acc, tx.nonce, 0, nil, tx.amount)
+				require.NoError(t, err)
+				err = mp.Insert(ctx, inputTx)
+				require.NoError(t, err)
+			}
+		})
+		// extract all txs via select
+		var output []sdk.Tx
+		for iter := mp.Select(ctx, nil); iter != nil; iter = iter.Next() {
+			output = append(output, iter.Tx())
+		}
+		// validate the order matches the expected order
+		require.Equal(t, len(tc.expected), len(output))
+		for i, tx := range output {
+
+			sigs, err := se.GetSigners(tx)
+			require.NoError(t, err)
+			require.Equal(t, 1, len(sigs))
+			require.Equal(t, tc.expected[i].acc.Address, sigs[0].Signer)
+			feeTx, ok := tx.(sdk.FeeTx)
+			require.True(t, ok)
+			require.Equal(t, tc.expected[i].amount.Denom, feeTx.GetFee().GetDenomByIndex(0))
+			require.Equal(t, tc.expected[i].amount.Amount, feeTx.GetFee().AmountOf(feeTx.GetFee().GetDenomByIndex(0)))
+			require.Equal(t, tc.expected[i].nonce, sigs[0].Sequence)
+
+		}
+	}
+}