fix: skip same-sender non-sequential sequence and then add others txs new solution (backport #19177) (#19249)
Co-authored-by: Brann Bronzebeard <90186866+ZiHengLee@users.noreply.github.com> Co-authored-by: Facundo <facundomedica@gmail.com>
This commit is contained in:
mergify[bot]
GitHub
parent
0abf94a334
commit
c4a2fe2b89
@ -42,6 +42,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||
|
||||
* (abci): [#19200](https://github.com/cosmos/cosmos-sdk/pull/19200) Ensure that sdk side ve math matches cometbft
|
||||
* [#19106](https://github.com/cosmos/cosmos-sdk/pull/19106) Allow empty public keys when setting signatures. Public keys aren't needed for every transaction.
|
||||
* (baseapp) [#19198](https://github.com/cosmos/cosmos-sdk/pull/19198) Remove usage of pointers in logs in all OE goroutines.
|
||||
* (baseapp) [#19177](https://github.com/cosmos/cosmos-sdk/pull/19177) Fix baseapp DefaultProposalHandler same-sender non-sequential sequence
|
||||
|
||||
## [v0.50.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.3) - 2023-01-15
|
||||
|
||||
@ -63,7 +65,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* (baseapp) [#19198](https://github.com/cosmos/cosmos-sdk/pull/19198) Remove usage of pointers in logs in all OE goroutines.
|
||||
* (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 module's beginBlock and before DeliverTx, ensuring transaction processing always starts with the expected zeroed out block gas meter.
|
||||
* (baseapp) [#18895](https://github.com/cosmos/cosmos-sdk/pull/18895) Fix de-duplicating vote extensions during validation in ValidateVoteExtensions.
|
||||
|
||||
@ -154,17 +154,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(),
|
||||
}
|
||||
}
|
||||
|
||||
@ -224,8 +226,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
|
||||
@ -242,6 +276,16 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
|
||||
if stop {
|
||||
break
|
||||
}
|
||||
|
||||
txsLen := len(h.txSelector.SelectedTxs(ctx))
|
||||
for sender, seq := range txSignersSeqs {
|
||||
if txsLen != selectedTxsNums {
|
||||
selectedTxsSignersSeqs[sender] = seq
|
||||
} else if _, ok := selectedTxsSignersSeqs[sender]; !ok {
|
||||
selectedTxsSignersSeqs[sender] = seq - 1
|
||||
}
|
||||
}
|
||||
selectedTxsNums = txsLen
|
||||
}
|
||||
|
||||
iterator = iterator.Next()
|
||||
|
||||
@ -2,16 +2,19 @@ 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"
|
||||
dbm "github.com/cosmos/cosmos-db"
|
||||
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"
|
||||
@ -19,10 +22,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"
|
||||
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
|
||||
)
|
||||
|
||||
@ -33,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{
|
||||
@ -403,6 +409,162 @@ 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")
|
||||
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",
|
||||
}
|
||||
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: 195 + 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 {
|
||||
@ -411,3 +573,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)
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user