skip same-sender non-sequential sequence and then add others txs

CHANGELOG.md

@@ -80,6 +80,7 @@ Every Module contains its own CHANGELOG.md. Please refer to the module you are i
 
 ### Bug Fixes
 
+* (baseapp) [#19119](https://github.com/cosmos/cosmos-sdk/pull/19119) Fix baseapp DefaultProposalHandler same-sender non-sequential sequence
 * (baseapp) [#19058](https://github.com/cosmos/cosmos-sdk/pull/19058) Fix baseapp posthandler branch would fail if the `runMsgs` had returned an error.
 * (baseapp) [#18609](https://github.com/cosmos/cosmos-sdk/issues/18609) Fixed accounting in the block gas meter after BeginBlock and before DeliverTx, ensuring transaction processing always starts with the expected zeroed out block gas meter.
 * (baseapp) [#18727](https://github.com/cosmos/cosmos-sdk/pull/18727) Ensure that `BaseApp.Init` firstly returns any errors from a nil commit multistore instead of panicking on nil dereferencing and before sealing the app.

baseapp/abci_utils.go

@@ -15,6 +15,7 @@ import (
 	"github.com/cosmos/gogoproto/proto"
 
 	"cosmossdk.io/math"
+	"cosmossdk.io/x/auth/signing"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/mempool"
@@ -227,9 +228,23 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
 		}
 
 		iterator := h.mempool.Select(ctx, req.Txs)
+		skipSenders := make(map[string]any)
+		var selectedTxsNums int
 		for iterator != nil {
 			memTx := iterator.Tx()
-
+			sigs, err := memTx.(signing.SigVerifiableTx).GetSignaturesV2()
+			if err != nil || len(sigs) == 0 {
+				err := h.mempool.Remove(memTx)
+				if err != nil && !errors.Is(err, mempool.ErrTxNotFound) {
+					return nil, err
+				}
+			}
+			sig := sigs[0]
+			sender := sdk.AccAddress(sig.PubKey.Address()).String()
+			if _, ok := skipSenders[sender]; ok {
+				iterator = iterator.Next()
+				continue
+			}
 			// NOTE: Since transaction verification was already executed in CheckTx,
 			// which calls mempool.Insert, in theory everything in the pool should be
 			// valid. But some mempool implementations may insert invalid txs, so we
@@ -245,6 +260,11 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
 				if stop {
 					break
 				}
+				txsLen := len(h.txSelector.SelectedTxs(ctx))
+				if txsLen <= selectedTxsNums {
+					skipSenders[sender] = nil
+				}
+				selectedTxsNums = txsLen
 			}
 
 			iterator = iterator.Next()

baseapp/abci_utils_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil"
 	"github.com/cosmos/cosmos-sdk/baseapp/testutil/mock"
+	"github.com/cosmos/cosmos-sdk/client"
 	codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -415,6 +416,98 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection()
 	}
 }
 
+func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSelection() {
+	cdc := codectestutil.CodecOptions{}.NewCodec()
+	baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry())
+	txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)
+	ctrl := gomock.NewController(s.T())
+	app := mock.NewMockProposalTxVerifier(ctrl)
+	mp := mempool.NewPriorityMempool(
+		mempool.PriorityNonceMempoolConfig[int64]{
+			TxPriority:      mempool.NewDefaultTxPriority(),
+			MaxTx:           0,
+			SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(),
+		},
+	)
+	ph := baseapp.NewDefaultProposalHandler(mp, app)
+	handler := ph.PrepareProposalHandler()
+	var (
+		secret1 = []byte("secret1")
+		secret2 = []byte("secret2")
+		tx1     = buildMsg(s.T(), txConfig, []byte(`1`), secret1, 1)
+		ctx1    = s.ctx.WithPriority(10)
+		tx2     = buildMsg(s.T(), txConfig, []byte(`12345678910`), secret1, 2)
+		tx3     = buildMsg(s.T(), txConfig, []byte(`12`), secret1, 3)
+
+		ctx2 = s.ctx.WithPriority(8)
+		tx4  = buildMsg(s.T(), txConfig, []byte(`12`), secret2, 1)
+	)
+	err := mp.Insert(ctx1, tx1)
+	s.Require().NoError(err)
+	err = mp.Insert(ctx1, tx2)
+	s.Require().NoError(err)
+	err = mp.Insert(ctx1, tx3)
+	s.Require().NoError(err)
+	err = mp.Insert(ctx2, tx4)
+	s.Require().NoError(err)
+
+	txBz1, err := txConfig.TxEncoder()(tx1)
+	s.Require().NoError(err)
+	txBz2, err := txConfig.TxEncoder()(tx2)
+	s.Require().NoError(err)
+	txBz3, err := txConfig.TxEncoder()(tx3)
+	s.Require().NoError(err)
+	txBz4, err := txConfig.TxEncoder()(tx4)
+	s.Require().NoError(err)
+
+	app.EXPECT().PrepareProposalVerifyTx(tx1).Return(txBz1, nil).AnyTimes()
+	app.EXPECT().PrepareProposalVerifyTx(tx2).Return(txBz2, nil).AnyTimes()
+	app.EXPECT().PrepareProposalVerifyTx(tx3).Return(txBz3, nil).AnyTimes()
+	app.EXPECT().PrepareProposalVerifyTx(tx4).Return(txBz4, nil).AnyTimes()
+
+	txDataSize1 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz1}))
+	txDataSize2 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz2}))
+	txDataSize3 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz3}))
+	txDataSize4 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz4}))
+	s.Require().Equal(txDataSize1, 111)
+	s.Require().Equal(txDataSize2, 121)
+	s.Require().Equal(txDataSize3, 112)
+	s.Require().Equal(txDataSize4, 112)
+
+	testCases := map[string]struct {
+		ctx         sdk.Context
+		req         *abci.RequestPrepareProposal
+		expectedTxs [][]byte
+	}{
+		"skip same-sender non-sequential sequence and then add others txs": {
+			ctx: s.ctx,
+			req: &abci.RequestPrepareProposal{
+				Txs:        [][]byte{txBz1, txBz2, txBz3, txBz4},
+				MaxTxBytes: 111 + 112,
+			},
+			expectedTxs: [][]byte{txBz1, txBz4},
+		},
+	}
+
+	for name, tc := range testCases {
+		s.Run(name, func() {
+			resp, err := handler(tc.ctx, tc.req)
+			s.Require().NoError(err)
+			s.Require().EqualValues(resp.Txs, tc.expectedTxs)
+		})
+	}
+}
+
+func buildMsg(t *testing.T, txConfig client.TxConfig, value, secret []byte, nonce uint64) sdk.Tx {
+	t.Helper()
+	builder := txConfig.NewTxBuilder()
+	_ = builder.SetMsgs(
+		&baseapptestutil.MsgKeyValue{Value: value},
+	)
+	setTxSignatureWithSecret(t, builder, nonce, secret)
+	return builder.GetTx()
+}
+
 func marshalDelimitedFn(msg proto.Message) ([]byte, error) {
 	var buf bytes.Buffer
 	if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil {

baseapp/utils_test.go

@@ -414,3 +414,17 @@ func wonkyMsg(t *testing.T, cfg client.TxConfig, tx signing.Tx) signing.Tx {
 	require.NoError(t, err)
 	return builder.GetTx()
 }
+
+func setTxSignatureWithSecret(t *testing.T, builder client.TxBuilder, nonce uint64, secret []byte) {
+	t.Helper()
+	privKey := secp256k1.GenPrivKeyFromSecret(secret)
+	pubKey := privKey.PubKey()
+	err := builder.SetSignatures(
+		signingtypes.SignatureV2{
+			PubKey:   pubKey,
+			Sequence: nonce,
+			Data:     &signingtypes.SingleSignatureData{},
+		},
+	)
+	require.NoError(t, err)
+}

go.mod

@@ -37,7 +37,7 @@ require (
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
 	github.com/hashicorp/go-metrics v0.5.3
 	github.com/hashicorp/golang-lru v1.0.2
-	github.com/hdevalence/ed25519consensus v0.1.0
+	github.com/hdevalence/ed25519consensus v0.2.0
 	github.com/huandu/skiplist v1.2.0
 	github.com/improbable-eng/grpc-web v0.15.0
 	github.com/magiconair/properties v1.8.7
@@ -66,7 +66,7 @@ require (
 )
 
 require (
-	filippo.io/edwards25519 v1.0.0 // indirect
+	filippo.io/edwards25519 v1.1.0 // indirect
 	github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
 	github.com/DataDog/datadog-go v4.8.3+incompatible // indirect
 	github.com/DataDog/zstd v1.5.5 // indirect

go.sum

@@ -15,8 +15,8 @@ cosmossdk.io/store v1.0.2/go.mod h1:EFtENTqVTuWwitGW1VwaBct+yDagk7oG/axBMPH+FXs=
 cosmossdk.io/x/tx v0.13.0 h1:8lzyOh3zONPpZv2uTcUmsv0WTXy6T1/aCVDCqShmpzU=
 cosmossdk.io/x/tx v0.13.0/go.mod h1:CpNQtmoqbXa33/DVxWQNx5Dcnbkv2xGUhL7tYQ5wUsY=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
-filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek=
-filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns=
+filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
+filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
 github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMbk2FiG/kXiLl8BRyzTWDw7gX/Hz7Dd5eDMs=
 github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4=
 github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0=
@@ -385,8 +385,8 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p
 github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc=
 github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE=
 github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ=
-github.com/hdevalence/ed25519consensus v0.1.0 h1:jtBwzzcHuTmFrQN6xQZn6CQEO/V9f7HsjsjeEZ6auqU=
-github.com/hdevalence/ed25519consensus v0.1.0/go.mod h1:w3BHWjwJbFU29IRHL1Iqkw3sus+7FctEyM4RqDxYNzo=
+github.com/hdevalence/ed25519consensus v0.2.0 h1:37ICyZqdyj0lAZ8P4D1d1id3HqbbG1N3iBb1Tb4rdcU=
+github.com/hdevalence/ed25519consensus v0.2.0/go.mod h1:w3BHWjwJbFU29IRHL1Iqkw3sus+7FctEyM4RqDxYNzo=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
 github.com/huandu/go-assert v1.1.5 h1:fjemmA7sSfYHJD7CUqs9qTwwfdNAx7/j2/ZlHXzNB3c=
 github.com/huandu/go-assert v1.1.5/go.mod h1:yOLvuqZwmcHIC5rIzrBhT7D3Q9c3GFnd0JrPVhn/06U=