From e75ae016235544df6c70f9d9d4dacdbfc2095f9c Mon Sep 17 00:00:00 2001 From: leven Date: Thu, 23 Nov 2023 21:08:11 +0800 Subject: [PATCH] fix: defaultTxSelector tx byte size calculations are consistent with cometbft --- CHANGELOG.md | 1 + baseapp/abci_utils.go | 3 ++- baseapp/abci_utils_test.go | 20 ++++++++++++++++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b37ba4013414..636eda0fae2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (client/server) [#18345](https://github.com/cosmos/cosmos-sdk/pull/18345) Consistently set viper prefix in client and server. It defaults for the binary name for both client and server. * (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test. * (simulation) [#18196](https://github.com/cosmos/cosmos-sdk/pull/18196) Fix the problem of `validator set is empty after InitGenesis` in simulation test. +* (baseapp) [#18551](https://github.com/cosmos/cosmos-sdk/pull/18551) Fix SelectTxForProposal the calculation method of tx bytes size is inconsistent with CometBFT ### API Breaking Changes diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 0b570ab52a69..4cf9fa8e2282 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -10,6 +10,7 @@ import ( cryptoenc "github.com/cometbft/cometbft/crypto/encoding" cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + cmttypes "github.com/cometbft/cometbft/types" protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" @@ -368,7 +369,7 @@ func (ts *defaultTxSelector) Clear() { } func (ts *defaultTxSelector) SelectTxForProposal(_ context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte) bool { - txSize := uint64(len(txBz)) + txSize := uint64(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz})) var txGasLimit uint64 if memTx != nil { diff --git a/baseapp/abci_utils_test.go b/baseapp/abci_utils_test.go index f204250382e0..bf3748101e17 100644 --- a/baseapp/abci_utils_test.go +++ b/baseapp/abci_utils_test.go @@ -8,6 +8,7 @@ import ( "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" dbm "github.com/cosmos/cosmos-db" protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" @@ -310,6 +311,9 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection() s.Require().NoError(err) s.Require().Len(txBz, 152) + txDataSize := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz})) + s.Require().Equal(txDataSize, 155) + testCases := map[string]struct { ctx sdk.Context req *abci.RequestPrepareProposal @@ -331,7 +335,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection() }), req: &abci.RequestPrepareProposal{ Txs: [][]byte{txBz, txBz, txBz, txBz, txBz}, - MaxTxBytes: 456, + MaxTxBytes: 465, }, expectedTxs: 0, }, @@ -339,10 +343,18 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection() ctx: s.ctx, req: &abci.RequestPrepareProposal{ Txs: [][]byte{txBz, txBz, txBz, txBz, txBz}, - MaxTxBytes: 456, + MaxTxBytes: 465, }, expectedTxs: 3, }, + "large max tx bytes len calculation": { + ctx: s.ctx, + req: &abci.RequestPrepareProposal{ + Txs: [][]byte{txBz, txBz, txBz, txBz, txBz}, + MaxTxBytes: 456, + }, + expectedTxs: 2, + }, "max gas and tx bytes": { ctx: s.ctx.WithConsensusParams(cmtproto.ConsensusParams{ Block: &cmtproto.BlockParams{ @@ -351,7 +363,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection() }), req: &abci.RequestPrepareProposal{ Txs: [][]byte{txBz, txBz, txBz, txBz, txBz}, - MaxTxBytes: 456, + MaxTxBytes: 465, }, expectedTxs: 2, }, @@ -360,7 +372,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection() for name, tc := range testCases { s.Run(name, func() { // iterate multiple times to ensure the tx selector is cleared each time - for i := 0; i < 5; i++ { + for i := 0; i < 6; i++ { resp, err := handler(tc.ctx, tc.req) s.Require().NoError(err) s.Require().Len(resp.Txs, tc.expectedTxs)