From 741842f6ad9f036a8e64e0d797139c1cd7b6b743 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 10:19:01 +0200 Subject: [PATCH 01/10] feat(server/v2/cometbft): wire mempool config --- server/v2/cometbft/config.go | 6 +++++ server/v2/cometbft/flags.go | 13 ++++++----- server/v2/cometbft/mempool/config.go | 19 ++++++++++------ server/v2/cometbft/mempool/doc.go | 6 +---- server/v2/cometbft/options.go | 9 ++++---- server/v2/cometbft/server.go | 6 +++-- server/v2/commands.go | 4 ++-- simapp/v2/simdv2/cmd/commands.go | 6 ++++- simapp/v2/simdv2/cmd/config.go | 34 ++++++++++++++++++++++++++++ 9 files changed, 76 insertions(+), 27 deletions(-) diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index 81f3aeb33354..9ec34f697f3d 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -1,6 +1,8 @@ package cometbft import ( + "cosmossdk.io/server/v2/cometbft/mempool" + cmtcfg "github.com/cometbft/cometbft/config" ) @@ -20,6 +22,7 @@ func DefaultAppTomlConfig() *AppTomlConfig { Transport: "socket", Trace: false, Standalone: false, + Mempool: mempool.DefaultConfig(), } } @@ -32,6 +35,9 @@ type AppTomlConfig struct { Transport string `mapstructure:"transport" toml:"transport" comment:"transport defines the CometBFT RPC server transport protocol: socket, grpc"` Trace bool `mapstructure:"trace" toml:"trace" comment:"trace enables the CometBFT RPC server to output trace information about its internal operations."` Standalone bool `mapstructure:"standalone" toml:"standalone" comment:"standalone starts the application without the CometBFT node. The node should be started separately."` + + // Sub configs + Mempool mempool.Config `mapstructure:"mempool" toml:"mempool" comment:"mempool defines the configuration for the SDK built-in app-side mempool implementations."` } // CfgOption is a function that allows to overwrite the default server configuration. diff --git a/server/v2/cometbft/flags.go b/server/v2/cometbft/flags.go index 55fa0a14e771..216755f9885b 100644 --- a/server/v2/cometbft/flags.go +++ b/server/v2/cometbft/flags.go @@ -51,10 +51,11 @@ func prefix(f string) string { // Server flags var ( - Standalone = prefix("standalone") - FlagAddress = prefix("address") - FlagTransport = prefix("transport") - FlagHaltHeight = prefix("halt-height") - FlagHaltTime = prefix("halt-time") - FlagTrace = prefix("trace") + Standalone = prefix("standalone") + FlagAddress = prefix("address") + FlagTransport = prefix("transport") + FlagHaltHeight = prefix("halt-height") + FlagHaltTime = prefix("halt-time") + FlagTrace = prefix("trace") + FlagMempoolMaxTxs = prefix("mempool.max-txs") ) diff --git a/server/v2/cometbft/mempool/config.go b/server/v2/cometbft/mempool/config.go index 38de18844107..e07d6a6e7167 100644 --- a/server/v2/cometbft/mempool/config.go +++ b/server/v2/cometbft/mempool/config.go @@ -1,11 +1,16 @@ package mempool -// Config defines the configurations for the SDK built-in app-side mempool -// implementations. +var DefaultMaxTx = -1 + +// Config defines the configurations for the SDK built-in app-side mempool implementations. type Config struct { - // MaxTxs defines the behavior of the mempool. A negative value indicates - // the mempool is disabled entirely, zero indicates that the mempool is - // unbounded in how many txs it may contain, and a positive value indicates - // the maximum amount of txs it may contain. - MaxTxs int `mapstructure:"max-txs"` + // MaxTxs defines the maximum number of transactions that can be in the mempool. + MaxTxs int `mapstructure:"max-txs" toml:"max-txs" comment:"max-txs defines the maximum number of transactions that can be in the mempool. A value of 0 indicates an unbounded mempool, a negative value disables the app-side mempool."` +} + +// DefaultConfig returns a default configuration for the SDK built-in app-side mempool implementations. +func DefaultConfig() Config { + return Config{ + MaxTxs: DefaultMaxTx, + } } diff --git a/server/v2/cometbft/mempool/doc.go b/server/v2/cometbft/mempool/doc.go index f9857f3a9fae..eb6c72aced18 100644 --- a/server/v2/cometbft/mempool/doc.go +++ b/server/v2/cometbft/mempool/doc.go @@ -1,6 +1,2 @@ -/* -The mempool package defines a few mempool services which can be used in conjunction with your consensus implementation - -*/ - +// Package mempool defines a few mempool services which can be used in conjunction with your consensus implementation. package mempool diff --git a/server/v2/cometbft/options.go b/server/v2/cometbft/options.go index 1e0a389882e0..78a4602bbbb8 100644 --- a/server/v2/cometbft/options.go +++ b/server/v2/cometbft/options.go @@ -9,14 +9,15 @@ import ( ) // ServerOptions defines the options for the CometBFT server. +// Options are func that are able to take CometBFT app.toml section config and config.toml config. type ServerOptions[T transaction.Tx] struct { - Mempool mempool.Mempool[T] + Mempool func(cfg map[string]any) mempool.Mempool[T] PrepareProposalHandler handlers.PrepareHandler[T] ProcessProposalHandler handlers.ProcessHandler[T] VerifyVoteExtensionHandler handlers.VerifyVoteExtensionhandler ExtendVoteHandler handlers.ExtendVoteHandler - SnapshotOptions snapshots.SnapshotOptions + SnapshotOptions func(cfg map[string]any) snapshots.SnapshotOptions AddrPeerFilter types.PeerFilter // filter peers by address and port IdPeerFilter types.PeerFilter // filter peers by node ID @@ -26,12 +27,12 @@ type ServerOptions[T transaction.Tx] struct { // It defaults to a NoOpMempool and NoOp handlers. func DefaultServerOptions[T transaction.Tx]() ServerOptions[T] { return ServerOptions[T]{ - Mempool: mempool.NoOpMempool[T]{}, + Mempool: func(cfg map[string]any) mempool.Mempool[T] { return mempool.NoOpMempool[T]{} }, PrepareProposalHandler: handlers.NoOpPrepareProposal[T](), ProcessProposalHandler: handlers.NoOpProcessProposal[T](), VerifyVoteExtensionHandler: handlers.NoOpVerifyVoteExtensionHandler(), ExtendVoteHandler: handlers.NoOpExtendVote(), - SnapshotOptions: snapshots.NewSnapshotOptions(0, 0), + SnapshotOptions: func(cfg map[string]any) snapshots.SnapshotOptions { return snapshots.NewSnapshotOptions(0, 0) }, AddrPeerFilter: nil, IdPeerFilter: nil, } diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index 26cd99ff97cd..717a3dfe7629 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -24,6 +24,7 @@ import ( "cosmossdk.io/log" serverv2 "cosmossdk.io/server/v2" cometlog "cosmossdk.io/server/v2/cometbft/log" + "cosmossdk.io/server/v2/cometbft/mempool" "cosmossdk.io/server/v2/cometbft/types" "cosmossdk.io/store/v2/snapshots" @@ -105,7 +106,7 @@ func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logg appI.Name(), appI.GetConsensusAuthority(), appI.GetAppManager(), - s.serverOptions.Mempool, + s.serverOptions.Mempool(cfg), indexEvents, appI.GetGPRCMethodsToMessageMap(), store, @@ -127,7 +128,7 @@ func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logg if err != nil { return err } - consensus.snapshotManager = snapshots.NewManager(snapshotStore, s.serverOptions.SnapshotOptions, sc, ss, nil, s.logger) + consensus.snapshotManager = snapshots.NewManager(snapshotStore, s.serverOptions.SnapshotOptions(cfg), sc, ss, nil, s.logger) s.Consensus = consensus @@ -240,6 +241,7 @@ func (s *CometBFTServer[T]) StartCmdFlags() *pflag.FlagSet { flags.Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node") flags.Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log") flags.Bool(Standalone, false, "Run app without CometBFT") + flags.Int(FlagMempoolMaxTxs, mempool.DefaultMaxTx, "Sets MaxTx value for the app-side mempool") // add comet flags, we use an empty command to avoid duplicating CometBFT's AddNodeFlags. // we can then merge the flag sets. diff --git a/server/v2/commands.go b/server/v2/commands.go index 8651074f406f..da8e0ec6e537 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -38,14 +38,14 @@ func AddCommands[T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[T], logger log.Logger, - serverCfg ServerConfig, + globalServerCfg ServerConfig, components ...ServerComponent[T], ) error { if len(components) == 0 { return errors.New("no components provided") } - server := NewServer(logger, serverCfg, components...) + server := NewServer(logger, globalServerCfg, components...) originalPersistentPreRunE := rootCmd.PersistentPreRunE rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // set the default command outputs diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index d58619170e0d..916f4b2be9da 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -74,7 +74,11 @@ func initRootCmd[T transaction.Tx]( newApp, logger, initServerConfig(), - cometbft.New(&genericTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), + cometbft.New( + &genericTxDecoder[T]{txConfig}, + initCometOptions[T](), + initCometConfig(), + ), grpc.New[T](), store.New[T](newApp), ); err != nil { diff --git a/simapp/v2/simdv2/cmd/config.go b/simapp/v2/simdv2/cmd/config.go index c7f6b707c89c..0ef848e69343 100644 --- a/simapp/v2/simdv2/cmd/config.go +++ b/simapp/v2/simdv2/cmd/config.go @@ -2,8 +2,13 @@ package cmd import ( "strings" + "time" + cmtcfg "github.com/cometbft/cometbft/config" + + "cosmossdk.io/core/transaction" serverv2 "cosmossdk.io/server/v2" + "cosmossdk.io/server/v2/cometbft" clientconfig "github.com/cosmos/cosmos-sdk/client/config" "github.com/cosmos/cosmos-sdk/crypto/keyring" @@ -68,3 +73,32 @@ func initServerConfig() serverv2.ServerConfig { return serverCfg } + +// initCometConfig helps to override default comet config template and configs. +func initCometConfig() cometbft.CfgOption { + cfg := cmtcfg.DefaultConfig() + + // display only warn logs by default except for p2p and state + cfg.LogLevel = "*:warn,p2p:info,state:info" + // increase block timeout + cfg.Consensus.TimeoutCommit = 5 * time.Second + // overwrite default pprof listen address + cfg.RPC.PprofListenAddress = "localhost:6060" + + return cometbft.OverwriteDefaultConfigTomlConfig(cfg) +} + +func initCometOptions[T transaction.Tx]() cometbft.ServerOptions[T] { + serverOptions := cometbft.DefaultServerOptions[T]() + + // TOOD mempool interface doesn't match! + + // overwrite app mempool, using max-txs option + // if maxTxs := cast.ToInt(cfg.Get(cometbft.FlagMempoolMaxTxs)); maxTxs >= 0 { + // serverOptions.Mempool = mempool.NewSenderNonceMempool( + // mempool.SenderNonceMaxTxOpt(maxTxs), + // ) + // } + + return serverOptions +} From 55edf6c9081127487f2a198af993ceaf7e06cae0 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 10:26:30 +0200 Subject: [PATCH 02/10] lint --- server/v2/cometbft/config.go | 4 ++-- server/v2/cometbft/options.go | 6 +++--- simapp/v2/simdv2/cmd/config.go | 14 +++++++++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index 9ec34f697f3d..4525e7563c27 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -1,9 +1,9 @@ package cometbft import ( - "cosmossdk.io/server/v2/cometbft/mempool" - cmtcfg "github.com/cometbft/cometbft/config" + + "cosmossdk.io/server/v2/cometbft/mempool" ) // Config is the configuration for the CometBFT application diff --git a/server/v2/cometbft/options.go b/server/v2/cometbft/options.go index 78a4602bbbb8..1cc335ba30a6 100644 --- a/server/v2/cometbft/options.go +++ b/server/v2/cometbft/options.go @@ -9,14 +9,14 @@ import ( ) // ServerOptions defines the options for the CometBFT server. -// Options are func that are able to take CometBFT app.toml section config and config.toml config. +// When an options takes a map[string]any, it is able to access the app.tom's cometbft section and the config.toml config. type ServerOptions[T transaction.Tx] struct { - Mempool func(cfg map[string]any) mempool.Mempool[T] PrepareProposalHandler handlers.PrepareHandler[T] ProcessProposalHandler handlers.ProcessHandler[T] VerifyVoteExtensionHandler handlers.VerifyVoteExtensionhandler ExtendVoteHandler handlers.ExtendVoteHandler + Mempool func(cfg map[string]any) mempool.Mempool[T] SnapshotOptions func(cfg map[string]any) snapshots.SnapshotOptions AddrPeerFilter types.PeerFilter // filter peers by address and port @@ -27,11 +27,11 @@ type ServerOptions[T transaction.Tx] struct { // It defaults to a NoOpMempool and NoOp handlers. func DefaultServerOptions[T transaction.Tx]() ServerOptions[T] { return ServerOptions[T]{ - Mempool: func(cfg map[string]any) mempool.Mempool[T] { return mempool.NoOpMempool[T]{} }, PrepareProposalHandler: handlers.NoOpPrepareProposal[T](), ProcessProposalHandler: handlers.NoOpProcessProposal[T](), VerifyVoteExtensionHandler: handlers.NoOpVerifyVoteExtensionHandler(), ExtendVoteHandler: handlers.NoOpExtendVote(), + Mempool: func(cfg map[string]any) mempool.Mempool[T] { return mempool.NoOpMempool[T]{} }, SnapshotOptions: func(cfg map[string]any) snapshots.SnapshotOptions { return snapshots.NewSnapshotOptions(0, 0) }, AddrPeerFilter: nil, IdPeerFilter: nil, diff --git a/simapp/v2/simdv2/cmd/config.go b/simapp/v2/simdv2/cmd/config.go index 0ef848e69343..ec222cf8846f 100644 --- a/simapp/v2/simdv2/cmd/config.go +++ b/simapp/v2/simdv2/cmd/config.go @@ -91,13 +91,17 @@ func initCometConfig() cometbft.CfgOption { func initCometOptions[T transaction.Tx]() cometbft.ServerOptions[T] { serverOptions := cometbft.DefaultServerOptions[T]() - // TOOD mempool interface doesn't match! + // TODO mempool interface doesn't match! // overwrite app mempool, using max-txs option - // if maxTxs := cast.ToInt(cfg.Get(cometbft.FlagMempoolMaxTxs)); maxTxs >= 0 { - // serverOptions.Mempool = mempool.NewSenderNonceMempool( - // mempool.SenderNonceMaxTxOpt(maxTxs), - // ) + // serverOptions.Mempool = func(cfg map[string]any) mempool.Mempool[T] { + // if maxTxs := cast.ToInt(cfg[cometbft.FlagMempoolMaxTxs]); maxTxs >= 0 { + // return mempool.NewSenderNonceMempool( + // mempool.SenderNonceMaxTxOpt(maxTxs), + // ) + // } + + // return mempool.NoOpMempool[T]{} // } return serverOptions From 6c1ca6db56388be54a21c38a4268a9b3eb52e0d5 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 10:51:14 +0200 Subject: [PATCH 03/10] wording --- server/v2/cometbft/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/cometbft/options.go b/server/v2/cometbft/options.go index 1cc335ba30a6..ba65f3084dc4 100644 --- a/server/v2/cometbft/options.go +++ b/server/v2/cometbft/options.go @@ -9,7 +9,7 @@ import ( ) // ServerOptions defines the options for the CometBFT server. -// When an options takes a map[string]any, it is able to access the app.tom's cometbft section and the config.toml config. +// When an option takes a map[string]any, it can access the app.tom's cometbft section and the config.toml config. type ServerOptions[T transaction.Tx] struct { PrepareProposalHandler handlers.PrepareHandler[T] ProcessProposalHandler handlers.ProcessHandler[T] From f2eb2950bb50e09890cc5b45d50847174eae47aa Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 10:54:05 +0200 Subject: [PATCH 04/10] refactor(mempool)!: match server/v2/cometbft and sdk mempool interface --- baseapp/abci_utils.go | 22 ++++++++++++------- server/v2/cometbft/abci.go | 7 +++--- server/v2/cometbft/handlers/defaults.go | 2 +- .../v2/cometbft/internal/mock/mock_mempool.go | 3 ++- server/v2/cometbft/mempool/mempool.go | 11 +++++++--- server/v2/cometbft/mempool/noop.go | 9 ++++---- simapp/v2/go.mod | 2 +- simapp/v2/simdv2/cmd/config.go | 6 ++--- types/mempool/mempool.go | 4 ++-- types/mempool/noop.go | 4 ++-- types/mempool/priority_nonce.go | 6 ++--- types/mempool/sender_nonce.go | 6 ++--- 12 files changed, 47 insertions(+), 35 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 63c13fa8620b..9b17548dab61 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -264,19 +264,25 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan defer h.txSelector.Clear() + // decode transactions + decodedTxs := make([]sdk.Tx, len(req.Txs)) + for i, txBz := range req.Txs { + tx, err := h.txVerifier.TxDecode(txBz) + if err != nil { + return nil, err + } + + decodedTxs[i] = tx + } + // If the mempool is nil or NoOp we simply return the transactions // requested from CometBFT, which, by default, should be in FIFO order. // // Note, we still need to ensure the transactions returned respect req.MaxTxBytes. _, isNoOp := h.mempool.(mempool.NoOpMempool) if h.mempool == nil || isNoOp { - for _, txBz := range req.Txs { - tx, err := h.txVerifier.TxDecode(txBz) - if err != nil { - return nil, err - } - - stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz) + for _, tx := range decodedTxs { + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, tx.Bytes()) if stop { break } @@ -291,7 +297,7 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan selectedTxsNums int invalidTxs []sdk.Tx // invalid txs to be removed out of the loop to avoid dead lock ) - h.mempool.SelectBy(ctx, req.Txs, func(memTx sdk.Tx) bool { + h.mempool.SelectBy(ctx, decodedTxs, func(memTx sdk.Tx) bool { unorderedTx, ok := memTx.(sdk.TxWithUnordered) isUnordered := ok && unorderedTx.GetUnordered() txSignersSeqs := make(map[string]uint64) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 06c1e43e5ecb..b66323da54b2 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -471,9 +471,10 @@ func (c *Consensus[T]) FinalizeBlock( } // remove txs from the mempool - err = c.mempool.Remove(decodedTxs) - if err != nil { - return nil, fmt.Errorf("unable to remove txs: %w", err) + for _, tx := range decodedTxs { + if err = c.mempool.Remove(tx); err != nil { + return nil, fmt.Errorf("unable to remove txs: %w", err) + } } c.lastCommittedHeight.Store(req.Height) diff --git a/server/v2/cometbft/handlers/defaults.go b/server/v2/cometbft/handlers/defaults.go index 18ad38669c9c..d8f43bb2fd25 100644 --- a/server/v2/cometbft/handlers/defaults.go +++ b/server/v2/cometbft/handlers/defaults.go @@ -79,7 +79,7 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { // check again. _, err := app.ValidateTx(ctx, memTx) if err != nil { - err := h.mempool.Remove([]T{memTx}) + err := h.mempool.Remove(memTx) if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { return nil, err } diff --git a/server/v2/cometbft/internal/mock/mock_mempool.go b/server/v2/cometbft/internal/mock/mock_mempool.go index 89d51e955fae..401b12d60139 100644 --- a/server/v2/cometbft/internal/mock/mock_mempool.go +++ b/server/v2/cometbft/internal/mock/mock_mempool.go @@ -15,5 +15,6 @@ type MockMempool[T transaction.Tx] struct{} func (MockMempool[T]) Insert(context.Context, T) error { return nil } func (MockMempool[T]) Select(context.Context, []T) mempool.Iterator[T] { return nil } +func (MockMempool[T]) SelectBy(context.Context, []T, func(T) bool) {} func (MockMempool[T]) CountTx() int { return 0 } -func (MockMempool[T]) Remove([]T) error { return nil } +func (MockMempool[T]) Remove(T) error { return nil } diff --git a/server/v2/cometbft/mempool/mempool.go b/server/v2/cometbft/mempool/mempool.go index 3cb81c871508..0b28c5a2b922 100644 --- a/server/v2/cometbft/mempool/mempool.go +++ b/server/v2/cometbft/mempool/mempool.go @@ -19,13 +19,18 @@ type Mempool[T transaction.Tx] interface { Insert(context.Context, T) error // Select returns an Iterator over the app-side mempool. If txs are specified, - // then they shall be incorporated into the Iterator. The Iterator must be - // closed by the caller. + // then they shall be incorporated into the Iterator. The Iterator is not thread-safe to use. Select(context.Context, []T) Iterator[T] + // SelectBy use callback to iterate over the mempool, it's thread-safe to use. + SelectBy(context.Context, []T, func(T) bool) + + // CountTx returns the number of transactions currently in the mempool. + CountTx() int + // Remove attempts to remove a transaction from the mempool, returning an error // upon failure. - Remove([]T) error + Remove(T) error } // Iterator defines an app-side mempool iterator interface that is as minimal as diff --git a/server/v2/cometbft/mempool/noop.go b/server/v2/cometbft/mempool/noop.go index 86cea55e2c53..2a4222f8be43 100644 --- a/server/v2/cometbft/mempool/noop.go +++ b/server/v2/cometbft/mempool/noop.go @@ -16,7 +16,8 @@ var _ Mempool[transaction.Tx] = (*NoOpMempool[transaction.Tx])(nil) // is FIFO-ordered by default. type NoOpMempool[T transaction.Tx] struct{} -func (NoOpMempool[T]) Insert(context.Context, T) error { return nil } -func (NoOpMempool[T]) Select(context.Context, []T) Iterator[T] { return nil } -func (NoOpMempool[T]) CountTx() int { return 0 } -func (NoOpMempool[T]) Remove([]T) error { return nil } +func (NoOpMempool[T]) Insert(context.Context, T) error { return nil } +func (NoOpMempool[T]) Select(context.Context, []T) Iterator[T] { return nil } +func (NoOpMempool[T]) SelectBy(context.Context, []T, func(T) bool) {} +func (NoOpMempool[T]) CountTx() int { return 0 } +func (NoOpMempool[T]) Remove(T) error { return nil } diff --git a/simapp/v2/go.mod b/simapp/v2/go.mod index e4003e4c9012..7039e8e9ff3f 100644 --- a/simapp/v2/go.mod +++ b/simapp/v2/go.mod @@ -35,6 +35,7 @@ require ( github.com/cosmos/cosmos-db v1.0.3-0.20240911104526-ddc3f09bfc22 // indirect // this version is not used as it is always replaced by the latest Cosmos SDK version github.com/cosmos/cosmos-sdk v0.53.0 + github.com/spf13/cast v1.7.0 // indirect github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.19.0 @@ -193,7 +194,6 @@ require ( github.com/sasha-s/go-deadlock v0.3.5 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.11.0 // indirect - github.com/spf13/cast v1.7.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/supranational/blst v0.3.13 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect diff --git a/simapp/v2/simdv2/cmd/config.go b/simapp/v2/simdv2/cmd/config.go index ec222cf8846f..51a7adb178e9 100644 --- a/simapp/v2/simdv2/cmd/config.go +++ b/simapp/v2/simdv2/cmd/config.go @@ -91,13 +91,11 @@ func initCometConfig() cometbft.CfgOption { func initCometOptions[T transaction.Tx]() cometbft.ServerOptions[T] { serverOptions := cometbft.DefaultServerOptions[T]() - // TODO mempool interface doesn't match! - // overwrite app mempool, using max-txs option // serverOptions.Mempool = func(cfg map[string]any) mempool.Mempool[T] { // if maxTxs := cast.ToInt(cfg[cometbft.FlagMempoolMaxTxs]); maxTxs >= 0 { - // return mempool.NewSenderNonceMempool( - // mempool.SenderNonceMaxTxOpt(maxTxs), + // return sdkmempool.NewSenderNonceMempool( + // sdkmempool.SenderNonceMaxTxOpt(maxTxs), // ) // } diff --git a/types/mempool/mempool.go b/types/mempool/mempool.go index 4f8f82f16fa7..6aa29ff3263c 100644 --- a/types/mempool/mempool.go +++ b/types/mempool/mempool.go @@ -14,10 +14,10 @@ type Mempool interface { // Select returns an Iterator over the app-side mempool. If txs are specified, // then they shall be incorporated into the Iterator. The Iterator is not thread-safe to use. - Select(context.Context, [][]byte) Iterator + Select(context.Context, []sdk.Tx) Iterator // SelectBy use callback to iterate over the mempool, it's thread-safe to use. - SelectBy(context.Context, [][]byte, func(sdk.Tx) bool) + SelectBy(context.Context, []sdk.Tx, func(sdk.Tx) bool) // CountTx returns the number of transactions currently in the mempool. CountTx() int diff --git a/types/mempool/noop.go b/types/mempool/noop.go index 33c002080f82..6f9bbf9ae83a 100644 --- a/types/mempool/noop.go +++ b/types/mempool/noop.go @@ -17,7 +17,7 @@ var _ Mempool = (*NoOpMempool)(nil) type NoOpMempool struct{} func (NoOpMempool) Insert(context.Context, sdk.Tx) error { return nil } -func (NoOpMempool) Select(context.Context, [][]byte) Iterator { return nil } -func (NoOpMempool) SelectBy(context.Context, [][]byte, func(sdk.Tx) bool) {} +func (NoOpMempool) Select(context.Context, []sdk.Tx) Iterator { return nil } +func (NoOpMempool) SelectBy(context.Context, []sdk.Tx, func(sdk.Tx) bool) {} func (NoOpMempool) CountTx() int { return 0 } func (NoOpMempool) Remove(sdk.Tx) error { return nil } diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index c0f16a16002c..b324801ff5ec 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -361,13 +361,13 @@ func (i *PriorityNonceIterator[C]) Tx() sdk.Tx { // // NOTE: It is not safe to use this iterator while removing transactions from // the underlying mempool. -func (mp *PriorityNonceMempool[C]) Select(ctx context.Context, txs [][]byte) Iterator { +func (mp *PriorityNonceMempool[C]) Select(ctx context.Context, txs []sdk.Tx) Iterator { mp.mtx.Lock() defer mp.mtx.Unlock() return mp.doSelect(ctx, txs) } -func (mp *PriorityNonceMempool[C]) doSelect(_ context.Context, _ [][]byte) Iterator { +func (mp *PriorityNonceMempool[C]) doSelect(_ context.Context, _ []sdk.Tx) Iterator { if mp.priorityIndex.Len() == 0 { return nil } @@ -383,7 +383,7 @@ func (mp *PriorityNonceMempool[C]) doSelect(_ context.Context, _ [][]byte) Itera } // SelectBy will hold the mutex during the iteration, callback returns if continue. -func (mp *PriorityNonceMempool[C]) SelectBy(ctx context.Context, txs [][]byte, callback func(sdk.Tx) bool) { +func (mp *PriorityNonceMempool[C]) SelectBy(ctx context.Context, txs []sdk.Tx, callback func(sdk.Tx) bool) { mp.mtx.Lock() defer mp.mtx.Unlock() diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index f9cb9f200a23..60d1e2991940 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -167,13 +167,13 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { // // NOTE: It is not safe to use this iterator while removing transactions from // the underlying mempool. -func (snm *SenderNonceMempool) Select(ctx context.Context, txs [][]byte) Iterator { +func (snm *SenderNonceMempool) Select(ctx context.Context, txs []sdk.Tx) Iterator { snm.mtx.Lock() defer snm.mtx.Unlock() return snm.doSelect(ctx, txs) } -func (snm *SenderNonceMempool) doSelect(_ context.Context, _ [][]byte) Iterator { +func (snm *SenderNonceMempool) doSelect(_ context.Context, _ []sdk.Tx) Iterator { var senders []string senderCursors := make(map[string]*skiplist.Element) @@ -202,7 +202,7 @@ func (snm *SenderNonceMempool) doSelect(_ context.Context, _ [][]byte) Iterator } // SelectBy will hold the mutex during the iteration, callback returns if continue. -func (snm *SenderNonceMempool) SelectBy(ctx context.Context, txs [][]byte, callback func(sdk.Tx) bool) { +func (snm *SenderNonceMempool) SelectBy(ctx context.Context, txs []sdk.Tx, callback func(sdk.Tx) bool) { snm.mtx.Lock() defer snm.mtx.Unlock() From c3556189830573dce12e17d1caf520a65442afcd Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 10:55:32 +0200 Subject: [PATCH 05/10] updates --- server/v2/cometbft/mempool/noop.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/v2/cometbft/mempool/noop.go b/server/v2/cometbft/mempool/noop.go index 2a4222f8be43..25cffcf69979 100644 --- a/server/v2/cometbft/mempool/noop.go +++ b/server/v2/cometbft/mempool/noop.go @@ -4,8 +4,11 @@ import ( "context" "cosmossdk.io/core/transaction" + + sdk "github.com/cosmos/cosmos-sdk/types" ) +var _ Mempool[sdk.Tx] = (*NoOpMempool[sdk.Tx])(nil) // verify interface at compile time var _ Mempool[transaction.Tx] = (*NoOpMempool[transaction.Tx])(nil) // NoOpMempool defines a no-op mempool. Transactions are completely discarded and From 94c68582122730459b9ba342a7da739c49c99c26 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 11:23:22 +0200 Subject: [PATCH 06/10] feedback Co-authored-by: Marko --- baseapp/abci_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 9b17548dab61..98f8a70fad23 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -282,7 +282,7 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan _, isNoOp := h.mempool.(mempool.NoOpMempool) if h.mempool == nil || isNoOp { for _, tx := range decodedTxs { - stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, tx.Bytes()) + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, req.Txs[i]) if stop { break } From 99ff2a442f5fe31dae2a3a788245831efb1044f2 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 11:28:53 +0200 Subject: [PATCH 07/10] typo Co-authored-by: Marko --- server/v2/cometbft/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index b66323da54b2..c8f789c3cebf 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -473,7 +473,7 @@ func (c *Consensus[T]) FinalizeBlock( // remove txs from the mempool for _, tx := range decodedTxs { if err = c.mempool.Remove(tx); err != nil { - return nil, fmt.Errorf("unable to remove txs: %w", err) + return nil, fmt.Errorf("unable to remove tx: %w", err) } } From d3ccd441f1bb14d0d1e383953c8e9941b7c6349f Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 13:57:23 +0200 Subject: [PATCH 08/10] build Co-authored-by: Marko --- baseapp/abci_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 98f8a70fad23..b5552dcabf75 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -281,7 +281,7 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan // Note, we still need to ensure the transactions returned respect req.MaxTxBytes. _, isNoOp := h.mempool.(mempool.NoOpMempool) if h.mempool == nil || isNoOp { - for _, tx := range decodedTxs { + for i, tx := range decodedTxs { stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, req.Txs[i]) if stop { break From 572ec03856eb25c9b46e1e9e2e0ec359ba814bcc Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 21:07:43 +0200 Subject: [PATCH 09/10] cl --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6641b37dfddc..ace79f6e9e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (baseapp) [#21256](https://github.com/cosmos/cosmos-sdk/pull/21256) Halt height will not commit the block indicated, meaning that if halt-height is set to 10, only blocks until 9 (included) will be committed. This is to go back to the original behavior before a change was introduced in v0.50.0. ### API Breaking Changes + +* (types/mempool) [#21744](https://github.com/cosmos/cosmos-sdk/pull/21744) Update types/mempool.Mempool interface to take decoded transactions. This avoid to decode the transaction twice. * (sims)[#21613](https://github.com/cosmos/cosmos-sdk/pull/21613) Add sims2 framework and factory methods for simpler message factories in modules ### Deprecated From ee627c9d7f9931e8ad1c6b3c88bf1212dc728890 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 17 Sep 2024 13:41:14 +0200 Subject: [PATCH 10/10] fix test --- baseapp/abci_utils_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/baseapp/abci_utils_test.go b/baseapp/abci_utils_test.go index 7dae39542c18..e2c88c2431b7 100644 --- a/baseapp/abci_utils_test.go +++ b/baseapp/abci_utils_test.go @@ -691,6 +691,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe ph := baseapp.NewDefaultProposalHandler(mp, app) for _, v := range tc.txInputs { + app.EXPECT().TxDecode(v.bz).Return(v.tx, nil).AnyTimes() 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)