Skip to content

Commit a86a83f

Browse files
facundomedicaalexanderbez
facundomedica
and
alexanderbez
authored
test(baseapp): Refactor tx selector tests + better comments (#19284)
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
1 parent 639735d commit a86a83f

2 files changed

Lines changed: 114 additions & 133 deletions

File tree

‎baseapp/abci_utils.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
236236
return nil, err
237237
}
238238

239-
// if the signers aren't in selectedTxsSignersSeqs then we haven't seen them before
240-
// so we add them and return true so this tx gets selected.
239+
// If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before
240+
// so we add them and continue given that we don't need to check the sequence.
241241
shouldAdd := true
242242
txSignersSeqs := make(map[string]uint64)
243243
for _, signer := range signerData {
@@ -247,10 +247,9 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
247247
continue
248248
}
249249

250-
// if we have seen this signer before we check if the sequence we just got is
251-
// seq+1 and if it is we update the sequence and return true so this tx gets
252-
// selected. If it isn't seq+1 we return false so this tx doesn't get
253-
// selected (it could be the same sequence or seq+2 which are invalid).
250+
// If we have seen this signer before in this block, we must make
251+
// sure that the current sequence is seq+1; otherwise is invalid
252+
// and we skip it.
254253
if seq+1 != signer.Sequence {
255254
shouldAdd = false
256255
break
@@ -280,9 +279,16 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
280279

281280
txsLen := len(h.txSelector.SelectedTxs(ctx))
282281
for sender, seq := range txSignersSeqs {
282+
// If txsLen != selectedTxsNums is true, it means that we've
283+
// added a new tx to the selected txs, so we need to update
284+
// the sequence of the sender.
283285
if txsLen != selectedTxsNums {
284286
selectedTxsSignersSeqs[sender] = seq
285287
} else if _, ok := selectedTxsSignersSeqs[sender]; !ok {
288+
// The transaction hasn't been added but it passed the
289+
// verification, so we know that the sequence is correct.
290+
// So we set this sender's sequence to seq-1, in order
291+
// to avoid unnecessary calls to PrepareProposalVerifyTx.
286292
selectedTxsSignersSeqs[sender] = seq - 1
287293
}
288294
}

‎baseapp/abci_utils_test.go‎

Lines changed: 102 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package baseapp_test
22

33
import (
44
"bytes"
5-
"strings"
65
"testing"
76

87
abci "github.com/cometbft/cometbft/abci/types"
@@ -424,154 +423,138 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
424423
cdc := codectestutil.CodecOptions{}.NewCodec()
425424
baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry())
426425
txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)
427-
ctrl := gomock.NewController(s.T())
428-
app := mock.NewMockProposalTxVerifier(ctrl)
429-
mp1 := mempool.NewPriorityMempool(
430-
mempool.PriorityNonceMempoolConfig[int64]{
431-
TxPriority: mempool.NewDefaultTxPriority(),
432-
MaxTx: 0,
433-
SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(),
434-
},
435-
)
436-
mp2 := mempool.NewPriorityMempool(
437-
mempool.PriorityNonceMempoolConfig[int64]{
438-
TxPriority: mempool.NewDefaultTxPriority(),
439-
MaxTx: 0,
440-
SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(),
441-
},
442-
)
443-
ph1 := baseapp.NewDefaultProposalHandler(mp1, app)
444-
handler1 := ph1.PrepareProposalHandler()
445-
ph2 := baseapp.NewDefaultProposalHandler(mp2, app)
446-
handler2 := ph2.PrepareProposalHandler()
426+
447427
var (
448428
secret1 = []byte("secret1")
449429
secret2 = []byte("secret2")
450430
secret3 = []byte("secret3")
451431
secret4 = []byte("secret4")
452432
secret5 = []byte("secret5")
453433
secret6 = []byte("secret6")
454-
ctx1 = s.ctx.WithPriority(10)
455-
ctx2 = s.ctx.WithPriority(8)
456434
)
457435

458-
tx1 := buildMsg(s.T(), txConfig, []byte(`1`), [][]byte{secret1}, []uint64{1})
459-
tx2 := buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1}, []uint64{2})
460-
tx3 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret1}, []uint64{3})
461-
tx4 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret2}, []uint64{1})
462-
err := mp1.Insert(ctx1, tx1)
463-
s.Require().NoError(err)
464-
err = mp1.Insert(ctx1, tx2)
465-
s.Require().NoError(err)
466-
err = mp1.Insert(ctx1, tx3)
467-
s.Require().NoError(err)
468-
err = mp1.Insert(ctx2, tx4)
469-
s.Require().NoError(err)
470-
txBz1, err := txConfig.TxEncoder()(tx1)
471-
s.Require().NoError(err)
472-
txBz2, err := txConfig.TxEncoder()(tx2)
473-
s.Require().NoError(err)
474-
txBz3, err := txConfig.TxEncoder()(tx3)
475-
s.Require().NoError(err)
476-
txBz4, err := txConfig.TxEncoder()(tx4)
477-
s.Require().NoError(err)
478-
app.EXPECT().PrepareProposalVerifyTx(tx1).Return(txBz1, nil).AnyTimes()
479-
app.EXPECT().PrepareProposalVerifyTx(tx2).Return(txBz2, nil).AnyTimes()
480-
app.EXPECT().PrepareProposalVerifyTx(tx3).Return(txBz3, nil).AnyTimes()
481-
app.EXPECT().PrepareProposalVerifyTx(tx4).Return(txBz4, nil).AnyTimes()
482-
txDataSize1 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz1}))
483-
txDataSize2 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz2}))
484-
txDataSize3 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz3}))
485-
txDataSize4 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz4}))
486-
s.Require().Equal(txDataSize1, 111)
487-
s.Require().Equal(txDataSize2, 121)
488-
s.Require().Equal(txDataSize3, 112)
489-
s.Require().Equal(txDataSize4, 112)
490-
491-
tx5 := buildMsg(s.T(), txConfig, []byte(`1`), [][]byte{secret1, secret2}, []uint64{1, 1})
492-
tx6 := buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1, secret3}, []uint64{2, 1})
493-
tx7 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret1, secret4}, []uint64{3, 1})
494-
tx8 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret3, secret5}, []uint64{2, 1})
495-
tx9 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret2, secret6}, []uint64{2, 1})
496-
497-
err = mp2.Insert(ctx1, tx5)
498-
s.Require().NoError(err)
499-
err = mp2.Insert(ctx1, tx6)
500-
s.Require().NoError(err)
501-
err = mp2.Insert(ctx2, tx7)
502-
s.Require().NoError(err)
503-
err = mp2.Insert(ctx2, tx8)
504-
s.Require().NoError(err)
505-
err = mp2.Insert(ctx2, tx9)
506-
s.Require().NoError(err)
507-
txBz5, err := txConfig.TxEncoder()(tx5)
508-
s.Require().NoError(err)
509-
txBz6, err := txConfig.TxEncoder()(tx6)
510-
s.Require().NoError(err)
511-
txBz7, err := txConfig.TxEncoder()(tx7)
512-
s.Require().NoError(err)
513-
txBz8, err := txConfig.TxEncoder()(tx8)
514-
s.Require().NoError(err)
515-
txBz9, err := txConfig.TxEncoder()(tx9)
516-
s.Require().NoError(err)
517-
app.EXPECT().PrepareProposalVerifyTx(tx5).Return(txBz5, nil).AnyTimes()
518-
app.EXPECT().PrepareProposalVerifyTx(tx6).Return(txBz6, nil).AnyTimes()
519-
app.EXPECT().PrepareProposalVerifyTx(tx7).Return(txBz7, nil).AnyTimes()
520-
app.EXPECT().PrepareProposalVerifyTx(tx8).Return(txBz8, nil).AnyTimes()
521-
app.EXPECT().PrepareProposalVerifyTx(tx9).Return(txBz9, nil).AnyTimes()
522-
txDataSize5 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz5}))
523-
txDataSize6 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz6}))
524-
txDataSize7 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz7}))
525-
txDataSize8 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz8}))
526-
txDataSize9 := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz9}))
527-
s.Require().Equal(txDataSize5, 195)
528-
s.Require().Equal(txDataSize6, 205)
529-
s.Require().Equal(txDataSize7, 196)
530-
s.Require().Equal(txDataSize8, 196)
531-
s.Require().Equal(txDataSize9, 196)
532-
533-
mapTxs := map[string]string{
534-
string(txBz1): "1",
535-
string(txBz2): "2",
536-
string(txBz3): "3",
537-
string(txBz4): "4",
538-
string(txBz5): "5",
539-
string(txBz6): "6",
540-
string(txBz7): "7",
541-
string(txBz8): "8",
542-
string(txBz9): "9",
436+
type testTx struct {
437+
tx sdk.Tx
438+
priority int64
439+
bz []byte
440+
size int
441+
}
442+
443+
testTxs := []testTx{
444+
// test 1
445+
{tx: buildMsg(s.T(), txConfig, []byte(`0`), [][]byte{secret1}, []uint64{1}), priority: 10},
446+
{tx: buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1}, []uint64{2}), priority: 10},
447+
{tx: buildMsg(s.T(), txConfig, []byte(`22`), [][]byte{secret1}, []uint64{3}), priority: 10},
448+
{tx: buildMsg(s.T(), txConfig, []byte(`32`), [][]byte{secret2}, []uint64{1}), priority: 8},
449+
// test 2
450+
{tx: buildMsg(s.T(), txConfig, []byte(`4`), [][]byte{secret1, secret2}, []uint64{3, 3}), priority: 10},
451+
{tx: buildMsg(s.T(), txConfig, []byte(`52345678910`), [][]byte{secret1, secret3}, []uint64{4, 3}), priority: 10},
452+
{tx: buildMsg(s.T(), txConfig, []byte(`62`), [][]byte{secret1, secret4}, []uint64{5, 3}), priority: 8},
453+
{tx: buildMsg(s.T(), txConfig, []byte(`72`), [][]byte{secret3, secret5}, []uint64{4, 3}), priority: 8},
454+
{tx: buildMsg(s.T(), txConfig, []byte(`82`), [][]byte{secret2, secret6}, []uint64{4, 3}), priority: 8},
455+
// test 3
456+
{tx: buildMsg(s.T(), txConfig, []byte(`9`), [][]byte{secret3, secret4}, []uint64{3, 3}), priority: 10},
457+
{tx: buildMsg(s.T(), txConfig, []byte(`1052345678910`), [][]byte{secret1, secret2}, []uint64{4, 4}), priority: 8},
458+
{tx: buildMsg(s.T(), txConfig, []byte(`11`), [][]byte{secret1, secret2}, []uint64{5, 5}), priority: 8},
459+
// test 4
460+
{tx: buildMsg(s.T(), txConfig, []byte(`1252345678910`), [][]byte{secret1}, []uint64{3}), priority: 10},
461+
{tx: buildMsg(s.T(), txConfig, []byte(`13`), [][]byte{secret1}, []uint64{5}), priority: 10},
462+
{tx: buildMsg(s.T(), txConfig, []byte(`14`), [][]byte{secret1}, []uint64{6}), priority: 8},
463+
}
464+
465+
for i := range testTxs {
466+
bz, err := txConfig.TxEncoder()(testTxs[i].tx)
467+
s.Require().NoError(err)
468+
testTxs[i].bz = bz
469+
testTxs[i].size = int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{bz}))
543470
}
471+
472+
s.Require().Equal(testTxs[0].size, 111)
473+
s.Require().Equal(testTxs[1].size, 121)
474+
s.Require().Equal(testTxs[2].size, 112)
475+
s.Require().Equal(testTxs[3].size, 112)
476+
s.Require().Equal(testTxs[4].size, 195)
477+
s.Require().Equal(testTxs[5].size, 205)
478+
s.Require().Equal(testTxs[6].size, 196)
479+
s.Require().Equal(testTxs[7].size, 196)
480+
s.Require().Equal(testTxs[8].size, 196)
481+
544482
testCases := map[string]struct {
545483
ctx sdk.Context
484+
txInputs []testTx
546485
req *abci.RequestPrepareProposal
547486
handler sdk.PrepareProposalHandler
548-
expectedTxs [][]byte
487+
expectedTxs []int
549488
}{
550489
"skip same-sender non-sequential sequence and then add others txs": {
551-
ctx: s.ctx,
490+
ctx: s.ctx,
491+
txInputs: []testTx{testTxs[0], testTxs[1], testTxs[2], testTxs[3]},
552492
req: &abci.RequestPrepareProposal{
553-
Txs: [][]byte{txBz1, txBz2, txBz3, txBz4},
554493
MaxTxBytes: 111 + 112,
555494
},
556-
handler: handler1,
557-
expectedTxs: [][]byte{txBz1, txBz4},
495+
expectedTxs: []int{0, 3},
558496
},
559497
"skip multi-signers msg non-sequential sequence": {
560-
ctx: s.ctx,
498+
ctx: s.ctx,
499+
txInputs: []testTx{testTxs[4], testTxs[5], testTxs[6], testTxs[7], testTxs[8]},
500+
req: &abci.RequestPrepareProposal{
501+
MaxTxBytes: 195 + 196,
502+
},
503+
expectedTxs: []int{4, 8},
504+
},
505+
"only the first tx is added": {
506+
// Because tx 10 is valid, tx 11 can't be valid as they have higher sequence numbers.
507+
ctx: s.ctx,
508+
txInputs: []testTx{testTxs[9], testTxs[10], testTxs[11]},
561509
req: &abci.RequestPrepareProposal{
562-
Txs: [][]byte{txBz5, txBz6, txBz7, txBz8, txBz9},
563510
MaxTxBytes: 195 + 196,
564511
},
565-
handler: handler2,
566-
expectedTxs: [][]byte{txBz5, txBz9},
512+
expectedTxs: []int{9},
513+
},
514+
"no txs added": {
515+
// Becasuse the first tx was deemed valid but too big, the next expected valid sequence is tx[0].seq (3), so
516+
// the rest of the txs fail because they have a seq of 4.
517+
ctx: s.ctx,
518+
txInputs: []testTx{testTxs[12], testTxs[13], testTxs[14]},
519+
req: &abci.RequestPrepareProposal{
520+
MaxTxBytes: 112,
521+
},
522+
expectedTxs: []int{},
567523
},
568524
}
569525

570526
for name, tc := range testCases {
571527
s.Run(name, func() {
572-
resp, err := tc.handler(tc.ctx, tc.req)
528+
ctrl := gomock.NewController(s.T())
529+
app := mock.NewMockProposalTxVerifier(ctrl)
530+
mp := mempool.NewPriorityMempool(
531+
mempool.PriorityNonceMempoolConfig[int64]{
532+
TxPriority: mempool.NewDefaultTxPriority(),
533+
MaxTx: 0,
534+
SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(),
535+
},
536+
)
537+
538+
ph := baseapp.NewDefaultProposalHandler(mp, app)
539+
540+
for _, v := range tc.txInputs {
541+
app.EXPECT().PrepareProposalVerifyTx(v.tx).Return(v.bz, nil).AnyTimes()
542+
s.NoError(mp.Insert(s.ctx.WithPriority(v.priority), v.tx))
543+
tc.req.Txs = append(tc.req.Txs, v.bz)
544+
}
545+
546+
resp, err := ph.PrepareProposalHandler()(tc.ctx, tc.req)
573547
s.Require().NoError(err)
574-
s.Require().EqualValues(toHumanReadable(mapTxs, resp.Txs), toHumanReadable(mapTxs, tc.expectedTxs))
548+
respTxIndexes := []int{}
549+
for _, tx := range resp.Txs {
550+
for i, v := range testTxs {
551+
if bytes.Equal(tx, v.bz) {
552+
respTxIndexes = append(respTxIndexes, i)
553+
}
554+
}
555+
}
556+
557+
s.Require().EqualValues(tc.expectedTxs, respTxIndexes)
575558
})
576559
}
577560
}
@@ -607,14 +590,6 @@ func buildMsg(t *testing.T, txConfig client.TxConfig, value []byte, secrets [][]
607590
return builder.GetTx()
608591
}
609592

610-
func toHumanReadable(mapTxs map[string]string, txs [][]byte) string {
611-
strs := []string{}
612-
for _, v := range txs {
613-
strs = append(strs, mapTxs[string(v)])
614-
}
615-
return strings.Join(strs, ",")
616-
}
617-
618593
func setTxSignatureWithSecret(t *testing.T, builder client.TxBuilder, signatures ...signingtypes.SignatureV2) {
619594
t.Helper()
620595
err := builder.SetSignatures(

0 commit comments

Comments
 (0)
⚡