cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.17k stars 3.57k forks source link

[Feature]: sdk should be able to provide PrivValidator for cometbft #21064

Open islishude opened 1 month ago

islishude commented 1 month ago

sdk can only provide fixed filepv to cometbft, it can not be updated by apps.

https://github.com/cosmos/cosmos-sdk/blob/826d4d3f2860a49b43c0dfb47a44132527c0059a/server/start.go#L375-L378

cometbft checks if the socket provider configuration is not empty, and use the socket provider then

https://github.com/cometbft/cometbft/blob/24b39c5ae75f6cec77fedd9c3a27a305aa711fcc/node/node.go#L335-L341

I think we can give application access to provide their own PrivValidator.

I propose a change to add a method ValidatorKeyProvider() cmttypes.PrivValidator to the Application interface.

sdk can have a built-in filepv provider in the baseapp

if apps want to have their own provider, they can overwrite the NodeKeyProvider func.

so sdk doesn't have visible breaking changes.

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index a89b48bcba..7aefdc086f 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -24,6 +24,7 @@ import (
        "cosmossdk.io/store/snapshots"
        storetypes "cosmossdk.io/store/types"

+       cmttypes "github.com/cometbft/cometbft/types"
        "github.com/cosmos/cosmos-sdk/baseapp/oe"
        "github.com/cosmos/cosmos-sdk/codec"
        codectypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -60,6 +61,8 @@ var _ servertypes.ABCI = (*BaseApp)(nil)

 // BaseApp reflects the ABCI application implementation.
 type BaseApp struct {
+       validatorKeyProvider cmttypes.PrivValidator
+
        // initialized on creation
        logger            log.Logger
        name              string                      // application name from abci.BlockInfo
diff --git a/baseapp/options.go b/baseapp/options.go
index 7218a283e7..b63aefa380 100644
--- a/baseapp/options.go
+++ b/baseapp/options.go
@@ -5,6 +5,7 @@ import (
        "io"
        "math"

+       cmttypes "github.com/cometbft/cometbft/types"
        dbm "github.com/cosmos/cosmos-db"

        "cosmossdk.io/store/metrics"
@@ -351,6 +352,17 @@ func (app *BaseApp) SetExtendVoteHandler(handler sdk.ExtendVoteHandler) {
        app.extendVote = handler
 }

+func (app *BaseApp) SetPrivValidatorProvider(pv cmttypes.PrivValidator) {
+       if app.sealed {
+               panic("SetPrivValidatorProvider() on sealed BaseApp")
+       }
+       app.validatorKeyProvider = pv
+}
+
+func (app *BaseApp) ValidatorKeyProvider() cmttypes.PrivValidator {
+       return app.validatorKeyProvider
+}
+
 func (app *BaseApp) SetVerifyVoteExtensionHandler(handler sdk.VerifyVoteExtensionHandler) {
        if app.sealed {
                panic("SetVerifyVoteExtensionHandler() on sealed BaseApp")
diff --git a/server/start.go b/server/start.go
index c2a812a7b5..a962efcecb 100644
--- a/server/start.go
+++ b/server/start.go
@@ -374,10 +374,15 @@ func startCmtNode(
        }

        cmtApp := NewCometABCIWrapper(app)
+
+       pvp := app.ValidatorKeyProvider()
+       if pvp == nil {
+               pvp = pvm.LoadOrGenFilePV(cfg.PrivValidatorKeyFile(), cfg.PrivValidatorStateFile())
+       }
        tmNode, err = node.NewNodeWithContext(
                ctx,
                cfg,
-               pvm.LoadOrGenFilePV(cfg.PrivValidatorKeyFile(), cfg.PrivValidatorStateFile()),
+               pvp,
                nodeKey,
                proxy.NewLocalClientCreator(cmtApp),
                getGenDocProvider(cfg),
diff --git a/server/types/app.go b/server/types/app.go
index 3b5feab3c0..7a4e4b4be0 100644
--- a/server/types/app.go
+++ b/server/types/app.go
@@ -62,6 +62,8 @@ type (
                // Close is called in start cmd to gracefully cleanup resources.
                // Must be safe to be called multiple times.
                Close() error
+
+               ValidatorKeyProvider() cmttypes.PrivValidator
        }

        // AppCreator is a function that allows us to lazily initialize an
islishude commented 1 month ago

a use case to me:

I want to use aws ksm for my validator node, and I don't want to use a socket provider due to I have to have a independent process to use it.