From 147f1b9c970aabf6e4bb8dc9a8ab7d4941b05380 Mon Sep 17 00:00:00 2001 From: SuperLee <9109321+superlee_8@user.noreply.gitee.com> Date: Tue, 23 Jan 2024 20:40:46 +0800 Subject: [PATCH] skip same-sender non-sequential sequence and then add others txs --- CHANGELOG.md | 1 + baseapp/abci_utils.go | 54 ++++++++-- baseapp/abci_utils_test.go | 203 ++++++++++++++++++++++++++++++++++++- 3 files changed, 249 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 661d890e2091c..ad074bf03d3bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ Every Module contains its own CHANGELOG.md. Please refer to the module you are i ### Bug Fixes +* (baseapp) [#19177](https://github.com/cosmos/cosmos-sdk/pull/19177) 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. diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index b09a22b33ecfd..989148ff2cb54 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -157,17 +157,19 @@ type ( // DefaultProposalHandler defines the default ABCI PrepareProposal and // ProcessProposal handlers. DefaultProposalHandler struct { - mempool mempool.Mempool - txVerifier ProposalTxVerifier - txSelector TxSelector + mempool mempool.Mempool + txVerifier ProposalTxVerifier + txSelector TxSelector + signerExtAdapter mempool.SignerExtractionAdapter } ) func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) *DefaultProposalHandler { return &DefaultProposalHandler{ - mempool: mp, - txVerifier: txVerifier, - txSelector: NewDefaultTxSelector(), + mempool: mp, + txVerifier: txVerifier, + txSelector: NewDefaultTxSelector(), + signerExtAdapter: mempool.NewDefaultSignerExtractionAdapter(), } } @@ -227,8 +229,40 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan } iterator := h.mempool.Select(ctx, req.Txs) + selectedTxsSignersSeqs := make(map[string]uint64) + var selectedTxsNums int for iterator != nil { memTx := iterator.Tx() + signerData, err := h.signerExtAdapter.GetSigners(memTx) + if err != nil { + return nil, err + } + + // if the signers aren't in selectedTxsSignersSeqs then we haven't seen them before + // so we add them and return true so this tx gets selected. + shouldAdd := true + txSignersSeqs := make(map[string]uint64) + for _, signer := range signerData { + seq, ok := selectedTxsSignersSeqs[signer.Signer.String()] + if !ok { + txSignersSeqs[signer.Signer.String()] = signer.Sequence + continue + } + + // if we have seen this signer before we check if the sequence we just got is + // seq+1 and if it is we update the sequence and return true so this tx gets + // selected. If it isn't seq+1 we return false so this tx doesn't get + // selected (it could be the same sequence or seq+2 which are invalid). + if seq+1 != signer.Sequence { + shouldAdd = false + break + } + txSignersSeqs[signer.Signer.String()] = signer.Sequence + } + if !shouldAdd { + 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 @@ -245,6 +279,14 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan if stop { break } + + txsLen := len(h.txSelector.SelectedTxs(ctx)) + if txsLen != selectedTxsNums { + for sender, seq := range txSignersSeqs { + selectedTxsSignersSeqs[sender] = seq + } + selectedTxsNums = txsLen + } } iterator = iterator.Next() diff --git a/baseapp/abci_utils_test.go b/baseapp/abci_utils_test.go index ac72b90e8e267..6255bbfcd4956 100644 --- a/baseapp/abci_utils_test.go +++ b/baseapp/abci_utils_test.go @@ -2,10 +2,11 @@ package baseapp_test import ( "bytes" + "strings" "testing" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cometbft/cometbft/crypto/secp256k1" + cmtsecp256k1 "github.com/cometbft/cometbft/crypto/secp256k1" cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" cmttypes "github.com/cometbft/cometbft/types" @@ -13,6 +14,7 @@ import ( protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "cosmossdk.io/log" @@ -21,10 +23,13 @@ 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/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/mempool" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" ) const ( @@ -34,11 +39,11 @@ const ( type testValidator struct { consAddr sdk.ConsAddress tmPk cmtprotocrypto.PublicKey - privKey secp256k1.PrivKey + privKey cmtsecp256k1.PrivKey } func newTestValidator() testValidator { - privkey := secp256k1.GenPrivKey() + privkey := cmtsecp256k1.GenPrivKey() pubkey := privkey.PubKey() tmPk := cmtprotocrypto.PublicKey{ Sum: &cmtprotocrypto.PublicKey_Secp256K1{ @@ -415,6 +420,160 @@ 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) + mp1 := mempool.NewPriorityMempool( + mempool.PriorityNonceMempoolConfig[int64]{ + TxPriority: mempool.NewDefaultTxPriority(), + MaxTx: 0, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), + }, + ) + mp2 := mempool.NewPriorityMempool( + mempool.PriorityNonceMempoolConfig[int64]{ + TxPriority: mempool.NewDefaultTxPriority(), + MaxTx: 0, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), + }, + ) + ph1 := baseapp.NewDefaultProposalHandler(mp1, app) + handler1 := ph1.PrepareProposalHandler() + ph2 := baseapp.NewDefaultProposalHandler(mp2, app) + handler2 := ph2.PrepareProposalHandler() + var ( + secret1 = []byte("secret1") + secret2 = []byte("secret2") + secret3 = []byte("secret3") + secret4 = []byte("secret4") + ctx1 = s.ctx.WithPriority(10) + ctx2 = s.ctx.WithPriority(8) + ) + + tx1 := buildMsg(s.T(), txConfig, []byte(`1`), [][]byte{secret1}, []uint64{1}) + tx2 := buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1}, []uint64{2}) + tx3 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret1}, []uint64{3}) + tx4 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret2}, []uint64{1}) + err := mp1.Insert(ctx1, tx1) + s.Require().NoError(err) + err = mp1.Insert(ctx1, tx2) + s.Require().NoError(err) + err = mp1.Insert(ctx1, tx3) + s.Require().NoError(err) + err = mp1.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) + + tx5 := buildMsg(s.T(), txConfig, []byte(`1`), [][]byte{secret1, secret2, secret3}, []uint64{1, 1, 1}) + tx6 := buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1, secret3}, []uint64{2, 2}) + tx7 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret1, secret4}, []uint64{3, 1}) + tx8 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret3, secret4}, []uint64{3, 2}) + tx9 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret2, secret4}, []uint64{2, 3}) + + err = mp2.Insert(ctx1, tx5) + s.Require().NoError(err) + err = mp2.Insert(ctx1, tx6) + s.Require().NoError(err) + err = mp2.Insert(ctx2, tx7) + s.Require().NoError(err) + err = mp2.Insert(ctx2, tx8) + s.Require().NoError(err) + err = mp2.Insert(ctx2, tx9) + s.Require().NoError(err) + txBz5, err := txConfig.TxEncoder()(tx5) + s.Require().NoError(err) + txBz6, err := txConfig.TxEncoder()(tx6) + s.Require().NoError(err) + txBz7, err := txConfig.TxEncoder()(tx7) + s.Require().NoError(err) + txBz8, err := txConfig.TxEncoder()(tx8) + s.Require().NoError(err) + txBz9, err := txConfig.TxEncoder()(tx9) + s.Require().NoError(err) + app.EXPECT().PrepareProposalVerifyTx(tx5).Return(txBz5, nil).AnyTimes() + app.EXPECT().PrepareProposalVerifyTx(tx6).Return(txBz6, nil).AnyTimes() + app.EXPECT().PrepareProposalVerifyTx(tx7).Return(txBz7, nil).AnyTimes() + app.EXPECT().PrepareProposalVerifyTx(tx8).Return(txBz8, nil).AnyTimes() + app.EXPECT().PrepareProposalVerifyTx(tx9).Return(txBz9, nil).AnyTimes() + txDataSize5 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz5})) + txDataSize6 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz6})) + txDataSize7 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz7})) + txDataSize8 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz8})) + txDataSize9 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz9})) + s.Require().Equal(txDataSize5, 277) + s.Require().Equal(txDataSize6, 205) + s.Require().Equal(txDataSize7, 196) + s.Require().Equal(txDataSize8, 196) + s.Require().Equal(txDataSize9, 196) + + mapTxs := map[string]string{ + string(txBz1): "1", + string(txBz2): "2", + string(txBz3): "3", + string(txBz4): "4", + string(txBz5): "5", + string(txBz6): "6", + string(txBz7): "7", + string(txBz8): "8", + string(txBz9): "9", + } + testCases := map[string]struct { + ctx sdk.Context + req *abci.RequestPrepareProposal + handler sdk.PrepareProposalHandler + 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, + }, + handler: handler1, + expectedTxs: [][]byte{txBz1, txBz4}, + }, + "skip multi-signers msg non-sequential sequence": { + ctx: s.ctx, + req: &abci.RequestPrepareProposal{ + Txs: [][]byte{txBz5, txBz6, txBz7, txBz8, txBz9}, + MaxTxBytes: 277 + 196, + }, + handler: handler2, + expectedTxs: [][]byte{txBz5, txBz9}, + }, + } + + for name, tc := range testCases { + s.Run(name, func() { + resp, err := tc.handler(tc.ctx, tc.req) + s.Require().NoError(err) + s.Require().EqualValues(toHumanReadable(mapTxs, resp.Txs), toHumanReadable(mapTxs, tc.expectedTxs)) + }) + } +} + func marshalDelimitedFn(msg proto.Message) ([]byte, error) { var buf bytes.Buffer if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { @@ -423,3 +582,41 @@ func marshalDelimitedFn(msg proto.Message) ([]byte, error) { return buf.Bytes(), nil } + +func buildMsg(t *testing.T, txConfig client.TxConfig, value []byte, secrets [][]byte, nonces []uint64) sdk.Tx { + t.Helper() + builder := txConfig.NewTxBuilder() + _ = builder.SetMsgs( + &baseapptestutil.MsgKeyValue{Value: value}, + ) + require.Equal(t, len(secrets), len(nonces)) + signatures := make([]signingtypes.SignatureV2, 0) + for index, secret := range secrets { + nonce := nonces[index] + privKey := secp256k1.GenPrivKeyFromSecret(secret) + pubKey := privKey.PubKey() + signatures = append(signatures, signingtypes.SignatureV2{ + PubKey: pubKey, + Sequence: nonce, + Data: &signingtypes.SingleSignatureData{}, + }) + } + setTxSignatureWithSecret(t, builder, signatures...) + return builder.GetTx() +} + +func toHumanReadable(mapTxs map[string]string, txs [][]byte) string { + strs := []string{} + for _, v := range txs { + strs = append(strs, mapTxs[string(v)]) + } + return strings.Join(strs, ",") +} + +func setTxSignatureWithSecret(t *testing.T, builder client.TxBuilder, signatures ...signingtypes.SignatureV2) { + t.Helper() + err := builder.SetSignatures( + signatures..., + ) + require.NoError(t, err) +}