Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there a way to override the staking keeper ? #22515

Closed
dillu24 opened this issue Nov 12, 2024 · 5 comments
Closed

Is there a way to override the staking keeper ? #22515

dillu24 opened this issue Nov 12, 2024 · 5 comments

Comments

@dillu24
Copy link

dillu24 commented Nov 12, 2024

Description

I want to add some custom logic to the staking keeper's Slash function. Is it possible to do this without having to fork the Cosmos SDK? I was thinking of having something of this sort and being imported in app.go.

package staking

import (
	modulev1 "cosmossdk.io/api/cosmos/staking/module/v1"
	"cosmossdk.io/core/appmodule"
	"github.com/cosmos/cosmos-sdk/codec"
	"github.com/cosmos/cosmos-sdk/x/staking"
	"github.com/cosmos/cosmos-sdk/x/staking/exported"
	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

type WrappedStakingKeeper struct {
	*stakingkeeper.Keeper
}

func NewWrappedStakingKeeper(stakingKeeper *stakingkeeper.Keeper) *WrappedStakingKeeper {
	return &WrappedStakingKeeper{
		Keeper: stakingKeeper,
	}
}

// TODO: Overwrite staking module keeper functions

type AppModule struct {
	staking.AppModule
}

func NewAppModule(
	cdc codec.Codec,
	wrappedStakingKeeper *WrappedStakingKeeper,
	ak stakingtypes.AccountKeeper,
	bk stakingtypes.BankKeeper,
	ls exported.Subspace,
) AppModule {
	return AppModule{
		AppModule: staking.NewAppModule(cdc, wrappedStakingKeeper, ak, bk, ls), // type error
	}
}

// App Wiring Setup
func init() {
	appmodule.Register(
		&modulev1.Module{},
		appmodule.Provide(ProvideModule),
		appmodule.Invoke(staking.InvokeSetStakingHooks),
	)
}

func ProvideModule(in staking.ModuleInputs) staking.ModuleOutputs {
	stakingModuleOutputs := staking.ProvideModule(in)

	wrappedStakingKeeper := NewWrappedStakingKeeper(stakingModuleOutputs.StakingKeeper)
	stakingModuleOutputs.StakingKeeper = wrappedStakingKeeper // Type error

	appModule := NewAppModule(
		in.Cdc,
		wrappedStakingKeeper,
		in.AccountKeeper,
		in.BankKeeper,
		in.LegacySubspace,
	)
	stakingModuleOutputs.Module = appModule

	return stakingModuleOutputs
}

The problem with the code above is that the evidence and slashing modules expect a stakingkeeper.Keeper , therefore, I can't really make use of it in the app config. Any suggestions please?

I am on Cosmos SDK version v0.50.10

@github-project-automation github-project-automation bot moved this to 📋 Backlog in Cosmos-SDK Nov 12, 2024
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Nov 12, 2024
@julienrbrt
Copy link
Member

If modules depends on the concrete staking keeper type, then wrapping it won't really work yeah.
Best thing is to fork the staking module by forking the SDK (from 0.52 you'll just need to fork the staking module).

@julienrbrt julienrbrt added T:question and removed needs-triage Issue that needs to be triaged labels Nov 12, 2024
@dillu24
Copy link
Author

dillu24 commented Nov 12, 2024

If modules depends on the concrete staking keeper type, then wrapping it won't really work yeah. Best thing is to fork the staking module by forking the SDK (from 0.52 you'll just need to fork the staking module).

Hey @julienrbrt thanks for your reply. I indeed was considering this approach too.

What do you think about forking the staking module only instead of the entire SDK? i.e. copying over the staking module code-base into my chain and wiring up the without using the SDKs staking module.

Of course this would mean that some default cosmos sdk modules would still make use of the types coming from the default Cosmos SDK staking module. In my case I just want to modify some staking module keeper functions, so I guess this should work?

@julienrbrt
Copy link
Member

You will need to fork the entire SDK in v0.50, because the staking module is part of the cosmos-sdk go module.
In v0.52 you'll be able to fork only the module you want to modify.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to 🥳 Done in Cosmos-SDK Nov 13, 2024
@migueldingli1997
Copy link
Contributor

migueldingli1997 commented Nov 13, 2024

Hey @julienrbrt in our case we've copied over x/staking into our repository (without x/staking/types) and managed to get the chain running with this custom x/staking that we copied over. I assume it's fine because the other modules only require a StakingKeeper interface generally, so no errors come up. Are there any negative repercussions that we're not seeing? Would you still recommend forking the SDK even if we managed to get the chain running? 🙏

@julienrbrt
Copy link
Member

Hey @julienrbrt in our case we've copied over x/staking into our repository (without x/staking/types) and managed to get the chain running with this custom x/staking that we copied over. I assume it's fine because the other modules only require a StakingKeeper interface generally, so no errors come up. Are there any negative repercussions that we're not seeing? Would you still recommend forking the SDK even if we managed to get the chain running? 🙏

This will work fine until a module requires the staking keeper concrete type. Then your fork won't be working. Normally modules should use expected interfaces, but I believe some modules do not and that's when problem can arise. If every module are using expected interfaces and you indeed keep the types folder, then this solution will work fine yes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🥳 Done
Development

No branches or pull requests

3 participants