diff --git a/CHANGELOG.md b/CHANGELOG.md index b487b6102166..5d7602c02fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i ### Improvements * (genutil) [#21701](https://github.com/cosmos/cosmos-sdk/pull/21701) Improved error messages for genesis validation. +* (sims)[#21613](https://github.com/cosmos/cosmos-sdk/pull/21613) Add sims2 framework and factory methods for simpler message factories in modules ### Bug Fixes @@ -58,7 +59,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i ### API Breaking Changes -* (sims)[#21613](https://github.com/cosmos/cosmos-sdk/pull/21613) Add sims2 framework and factory methods for simpler message factories in modules +* (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. ### Deprecated diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index e7bb5904a7f7..bc0cb9daccdd 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 i, tx := range decodedTxs { + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, req.Txs[i]) 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/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) diff --git a/runtime/app.go b/runtime/app.go index e2aa5798dac5..e63da54c0e12 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -6,6 +6,8 @@ import ( "slices" abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cmted25519 "github.com/cometbft/cometbft/crypto/ed25519" "google.golang.org/grpc" runtimev1alpha1 "cosmossdk.io/api/cosmos/app/runtime/v1alpha1" @@ -30,6 +32,9 @@ import ( authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" ) +// KeyGenF is a function that generates a private key for use by comet. +type KeyGenF = func() (cmtcrypto.PrivKey, error) + // App is a wrapper around BaseApp and ModuleManager that can be used in hybrid // app.go/app config scenarios or directly as a servertypes.Application instance. // To get an instance of *App, *AppBuilder must be requested as a dependency @@ -308,3 +313,10 @@ var _ servertypes.Application = &App{} type hasServicesV1 interface { RegisterServices(grpc.ServiceRegistrar) error } + +// ValidatorKeyProvider returns a function that generates a private key for use by comet. +func (a *App) ValidatorKeyProvider() KeyGenF { + return func() (cmtcrypto.PrivKey, error) { + return cmted25519.GenPrivKey(), nil + } +} diff --git a/schema/diff/diff_test.go b/schema/diff/diff_test.go index d4f4c56ca698..6b12c6c2e6cb 100644 --- a/schema/diff/diff_test.go +++ b/schema/diff/diff_test.go @@ -333,7 +333,6 @@ func TestCompareModuleSchemas(t *testing.T) { func requireModuleSchema(t *testing.T, types ...schema.Type) schema.ModuleSchema { t.Helper() - s, err := schema.CompileModuleSchema(types...) if err != nil { t.Fatal(err) diff --git a/server/start.go b/server/start.go index 65822e60338e..cff5807fbdb4 100644 --- a/server/start.go +++ b/server/start.go @@ -374,9 +374,7 @@ func startCmtNode( return nil, cleanupFn, err } - pv, err := pvm.LoadOrGenFilePV(cfg.PrivValidatorKeyFile(), cfg.PrivValidatorStateFile(), func() (cmtcrypto.PrivKey, error) { - return cmted25519.GenPrivKey(), nil - }) // TODO: make this modular + pv, err := pvm.LoadOrGenFilePV(cfg.PrivValidatorKeyFile(), cfg.PrivValidatorStateFile(), app.ValidatorKeyProvider()) if err != nil { return nil, cleanupFn, err } diff --git a/server/types/app.go b/server/types/app.go index d08cfff0ea9b..ca0a55ca1a0f 100644 --- a/server/types/app.go +++ b/server/types/app.go @@ -5,6 +5,7 @@ import ( "io" cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" + cmtcrypto "github.com/cometbft/cometbft/crypto" cmttypes "github.com/cometbft/cometbft/types" "github.com/cosmos/gogoproto/grpc" @@ -57,6 +58,9 @@ type ( // SnapshotManager return the snapshot manager SnapshotManager() *snapshots.Manager + // ValidatorKeyProvider returns a function that generates a validator key + ValidatorKeyProvider() func() (cmtcrypto.PrivKey, error) + // Close is called in start cmd to gracefully cleanup resources. // Must be safe to be called multiple times. Close() error diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index c38bf7fc18cd..dfd113434093 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -467,9 +467,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 tx: %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..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 @@ -16,7 +19,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/server/v2/cometbft/options.go b/server/v2/cometbft/options.go index ba65f3084dc4..0f5091da70a0 100644 --- a/server/v2/cometbft/options.go +++ b/server/v2/cometbft/options.go @@ -1,6 +1,9 @@ package cometbft import ( + cmtcrypto "github.com/cometbft/cometbft/crypto" + cmted22519 "github.com/cometbft/cometbft/crypto/ed25519" + "cosmossdk.io/core/transaction" "cosmossdk.io/server/v2/cometbft/handlers" "cosmossdk.io/server/v2/cometbft/mempool" @@ -8,6 +11,8 @@ import ( "cosmossdk.io/store/v2/snapshots" ) +type keyGenF = func() (cmtcrypto.PrivKey, error) + // ServerOptions defines the options for the CometBFT server. // 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 { @@ -15,6 +20,7 @@ type ServerOptions[T transaction.Tx] struct { ProcessProposalHandler handlers.ProcessHandler[T] VerifyVoteExtensionHandler handlers.VerifyVoteExtensionhandler ExtendVoteHandler handlers.ExtendVoteHandler + KeygenF keyGenF Mempool func(cfg map[string]any) mempool.Mempool[T] SnapshotOptions func(cfg map[string]any) snapshots.SnapshotOptions @@ -35,5 +41,6 @@ func DefaultServerOptions[T transaction.Tx]() ServerOptions[T] { SnapshotOptions: func(cfg map[string]any) snapshots.SnapshotOptions { return snapshots.NewSnapshotOptions(0, 0) }, AddrPeerFilter: nil, IdPeerFilter: nil, + KeygenF: func() (cmtcrypto.PrivKey, error) { return cmted22519.GenPrivKey(), nil }, } } diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index 09742b9b3003..2f402522c099 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -11,8 +11,6 @@ import ( abciserver "github.com/cometbft/cometbft/abci/server" cmtcmd "github.com/cometbft/cometbft/cmd/cometbft/commands" cmtcfg "github.com/cometbft/cometbft/config" - cmtcrypto "github.com/cometbft/cometbft/crypto" - cmted25519 "github.com/cometbft/cometbft/crypto/ed25519" "github.com/cometbft/cometbft/node" "github.com/cometbft/cometbft/p2p" pvm "github.com/cometbft/cometbft/privval" @@ -159,9 +157,7 @@ func (s *CometBFTServer[T]) Start(ctx context.Context) error { pv, err := pvm.LoadOrGenFilePV( s.config.ConfigTomlConfig.PrivValidatorKeyFile(), s.config.ConfigTomlConfig.PrivValidatorStateFile(), - func() (cmtcrypto.PrivKey, error) { - return cmted25519.GenPrivKey(), nil - }, + s.serverOptions.KeygenF, ) if err != nil { return err diff --git a/simapp/app.go b/simapp/app.go index a110e267c84a..1127b73b8acf 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -11,6 +11,8 @@ import ( "path/filepath" abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cmted25519 "github.com/cometbft/cometbft/crypto/ed25519" "github.com/cosmos/gogoproto/proto" "github.com/spf13/cast" @@ -831,6 +833,14 @@ func (app *SimApp) RegisterNodeService(clientCtx client.Context, cfg config.Conf nodeservice.RegisterNodeService(clientCtx, app.GRPCQueryRouter(), cfg) } +// ValidatorKeyProvider returns a function that generates a validator key +// Supported key types are those supported by Comet: ed25519, secp256k1, bls12-381 +func (app *SimApp) ValidatorKeyProvider() runtime.KeyGenF { + return func() (cmtcrypto.PrivKey, error) { + return cmted25519.GenPrivKey(), nil + } +} + // GetMaccPerms returns a copy of the module account permissions // // NOTE: This is solely to be used for testing purposes. diff --git a/simapp/v2/go.mod b/simapp/v2/go.mod index d25ff700c46b..7998f620d58b 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 @@ -194,7 +195,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()