Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(baseapp): Refactor tx selector tests + better comments #19284

Merged
merged 3 commits into from
Jan 30, 2024
Merged

test(baseapp): Refactor tx selector tests + better comments #19284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0f1a8e
add more tests
facundomedica Jan 29, 2024
1120219
add comments
facundomedica Jan 29, 2024
8121e91
Merge branch 'main' into facu/nonseq-tests
alexanderbez Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions baseapp/abci_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
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.
// If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before
// so we add them and continue given that we don't need to check the sequence.
shouldAdd := true
txSignersSeqs := make(map[string]uint64)
for _, signer := range signerData {
Expand All @@ -247,10 +247,9 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
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 we have seen this signer before in this block, we must make
// sure that the current sequence is seq+1; otherwise is invalid
// and we skip it.
if seq+1 != signer.Sequence {
shouldAdd = false
break
Expand Down Expand Up @@ -280,9 +279,16 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan

txsLen := len(h.txSelector.SelectedTxs(ctx))
for sender, seq := range txSignersSeqs {
// If txsLen != selectedTxsNums is true, it means that we've
// added a new tx to the selected txs, so we need to update
// the sequence of the sender.
if txsLen != selectedTxsNums {
selectedTxsSignersSeqs[sender] = seq
} else if _, ok := selectedTxsSignersSeqs[sender]; !ok {
// The transaction hasn't been added but it passed the
// verification, so we know that the sequence is correct.
// So we set this sender's sequence to seq-1, in order
// to avoid unnecessary calls to PrepareProposalVerifyTx.
selectedTxsSignersSeqs[sender] = seq - 1
}
}
Expand Down
229 changes: 102 additions & 127 deletions baseapp/abci_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package baseapp_test

import (
"bytes"
"strings"
"testing"

abci "github.com/cometbft/cometbft/abci/types"
Expand Down Expand Up @@ -424,154 +423,138 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
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")
secret5 = []byte("secret5")
secret6 = []byte("secret6")
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}, []uint64{1, 1})
tx6 := buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1, secret3}, []uint64{2, 1})
tx7 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret1, secret4}, []uint64{3, 1})
tx8 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret3, secret5}, []uint64{2, 1})
tx9 := buildMsg(s.T(), txConfig, []byte(`12`), [][]byte{secret2, secret6}, []uint64{2, 1})

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, 195)
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",
type testTx struct {
tx sdk.Tx
priority int64
bz []byte
size int
}

testTxs := []testTx{
// test 1
{tx: buildMsg(s.T(), txConfig, []byte(`0`), [][]byte{secret1}, []uint64{1}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`12345678910`), [][]byte{secret1}, []uint64{2}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`22`), [][]byte{secret1}, []uint64{3}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`32`), [][]byte{secret2}, []uint64{1}), priority: 8},
// test 2
{tx: buildMsg(s.T(), txConfig, []byte(`4`), [][]byte{secret1, secret2}, []uint64{3, 3}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`52345678910`), [][]byte{secret1, secret3}, []uint64{4, 3}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`62`), [][]byte{secret1, secret4}, []uint64{5, 3}), priority: 8},
{tx: buildMsg(s.T(), txConfig, []byte(`72`), [][]byte{secret3, secret5}, []uint64{4, 3}), priority: 8},
{tx: buildMsg(s.T(), txConfig, []byte(`82`), [][]byte{secret2, secret6}, []uint64{4, 3}), priority: 8},
// test 3
{tx: buildMsg(s.T(), txConfig, []byte(`9`), [][]byte{secret3, secret4}, []uint64{3, 3}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`1052345678910`), [][]byte{secret1, secret2}, []uint64{4, 4}), priority: 8},
{tx: buildMsg(s.T(), txConfig, []byte(`11`), [][]byte{secret1, secret2}, []uint64{5, 5}), priority: 8},
// test 4
{tx: buildMsg(s.T(), txConfig, []byte(`1252345678910`), [][]byte{secret1}, []uint64{3}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`13`), [][]byte{secret1}, []uint64{5}), priority: 10},
{tx: buildMsg(s.T(), txConfig, []byte(`14`), [][]byte{secret1}, []uint64{6}), priority: 8},
}

for i := range testTxs {
bz, err := txConfig.TxEncoder()(testTxs[i].tx)
s.Require().NoError(err)
testTxs[i].bz = bz
testTxs[i].size = int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{bz}))
}

s.Require().Equal(testTxs[0].size, 111)
s.Require().Equal(testTxs[1].size, 121)
s.Require().Equal(testTxs[2].size, 112)
s.Require().Equal(testTxs[3].size, 112)
s.Require().Equal(testTxs[4].size, 195)
s.Require().Equal(testTxs[5].size, 205)
s.Require().Equal(testTxs[6].size, 196)
s.Require().Equal(testTxs[7].size, 196)
s.Require().Equal(testTxs[8].size, 196)

testCases := map[string]struct {
ctx sdk.Context
txInputs []testTx
req *abci.RequestPrepareProposal
handler sdk.PrepareProposalHandler
expectedTxs [][]byte
expectedTxs []int
}{
"skip same-sender non-sequential sequence and then add others txs": {
ctx: s.ctx,
ctx: s.ctx,
txInputs: []testTx{testTxs[0], testTxs[1], testTxs[2], testTxs[3]},
req: &abci.RequestPrepareProposal{
Txs: [][]byte{txBz1, txBz2, txBz3, txBz4},
MaxTxBytes: 111 + 112,
},
handler: handler1,
expectedTxs: [][]byte{txBz1, txBz4},
expectedTxs: []int{0, 3},
},
"skip multi-signers msg non-sequential sequence": {
ctx: s.ctx,
ctx: s.ctx,
txInputs: []testTx{testTxs[4], testTxs[5], testTxs[6], testTxs[7], testTxs[8]},
req: &abci.RequestPrepareProposal{
MaxTxBytes: 195 + 196,
},
expectedTxs: []int{4, 8},
},
"only the first tx is added": {
// Because tx 10 is valid, tx 11 can't be valid as they have higher sequence numbers.
ctx: s.ctx,
txInputs: []testTx{testTxs[9], testTxs[10], testTxs[11]},
req: &abci.RequestPrepareProposal{
Txs: [][]byte{txBz5, txBz6, txBz7, txBz8, txBz9},
MaxTxBytes: 195 + 196,
},
handler: handler2,
expectedTxs: [][]byte{txBz5, txBz9},
expectedTxs: []int{9},
},
"no txs added": {
// Becasuse the first tx was deemed valid but too big, the next expected valid sequence is tx[0].seq (3), so
// the rest of the txs fail because they have a seq of 4.
ctx: s.ctx,
txInputs: []testTx{testTxs[12], testTxs[13], testTxs[14]},
req: &abci.RequestPrepareProposal{
MaxTxBytes: 112,
},
expectedTxs: []int{},
},
}

for name, tc := range testCases {
s.Run(name, func() {
resp, err := tc.handler(tc.ctx, tc.req)
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)

for _, v := range tc.txInputs {
app.EXPECT().PrepareProposalVerifyTx(v.tx).Return(v.bz, nil).AnyTimes()
s.NoError(mp.Insert(s.ctx.WithPriority(v.priority), v.tx))
tc.req.Txs = append(tc.req.Txs, v.bz)
}

resp, err := ph.PrepareProposalHandler()(tc.ctx, tc.req)
s.Require().NoError(err)
s.Require().EqualValues(toHumanReadable(mapTxs, resp.Txs), toHumanReadable(mapTxs, tc.expectedTxs))
respTxIndexes := []int{}
for _, tx := range resp.Txs {
for i, v := range testTxs {
if bytes.Equal(tx, v.bz) {
respTxIndexes = append(respTxIndexes, i)
}
}
}

s.Require().EqualValues(tc.expectedTxs, respTxIndexes)
})
}
}
Expand Down Expand Up @@ -607,14 +590,6 @@ func buildMsg(t *testing.T, txConfig client.TxConfig, value []byte, secrets [][]
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(
Expand Down
Loading