refactor(x/gov)!: let hooks return an error (backport #18173)

CHANGELOG.md

@@ -40,16 +40,18 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
-* (server) [#18162](https://github.com/cosmos/cosmos-sdk/pull/18162) Start gRPC & API server in standalone mode
+* (server) [#18162](https://github.com/cosmos/cosmos-sdk/pull/18162) Start gRPC & API server in standalone mode.
+
 ### Improvements
 
-* (x/staking/keeper) [#]18049(https://github.com/cosmos/cosmos-sdk/pull/18049) return early if Slash encounters zero tokens to burn.
-* (x/staking/keeper) [#18035](https://github.com/cosmos/cosmos-sdk/pull/18035) Hoisted out of the redelegation loop, the non-changing validator and delegator addresses parsing.
+* (x/staking) [#18049](https://github.com/cosmos/cosmos-sdk/pull/18049) Return early if Slash encounters zero tokens to burn.
+* (x/staking) [#18035](https://github.com/cosmos/cosmos-sdk/pull/18035) Hoisted out of the redelegation loop, the non-changing validator and delegator addresses parsing.
 * (keyring) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) Add `NewAutoCLIKeyring` for creating an AutoCLI keyring from a SDK keyring.
 * (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) Let `ToProtoConsensusParams()` return an error.
 
 ### Bug Fixes
 
+* (x/gov) [#18173](https://github.com/cosmos/cosmos-sdk/pull/18173) Gov hooks now return an error and are *blocking* when they fail. Expect for `AfterProposalFailedMinDeposit` and `AfterProposalVotingPeriodEnded` which log the error and continue.
 * (x/gov) [#17873](https://github.com/cosmos/cosmos-sdk/pull/17873) Fail any inactive and active proposals that cannot be decoded.
 * (x/slashing) [#18016](https://github.com/cosmos/cosmos-sdk/pull/18016) Fixed builder function for missed blocks key (`validatorMissedBlockBitArrayPrefixKey`) in slashing/migration/v4
 * (x/bank) [#18107](https://github.com/cosmos/cosmos-sdk/pull/18107) Add missing keypair of SendEnabled to restore legacy param set before migration.

x/gov/abci.go

@@ -65,7 +65,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 		}
 
 		// called when proposal become inactive
-		keeper.Hooks().AfterProposalFailedMinDeposit(ctx, proposal.Id)
+		cacheCtx, writeCache := ctx.CacheContext()
+		err = keeper.Hooks().AfterProposalFailedMinDeposit(cacheCtx, proposal.Id)
+		if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails
+			writeCache()
+		} else {
+			keeper.Logger(ctx).Error("failed to execute AfterProposalFailedMinDeposit hook", "error", err)
+		}
 
 		ctx.EventManager().EmitEvent(
 			sdk.NewEvent(
@@ -228,7 +234,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 		}
 
 		// when proposal become active
-		keeper.Hooks().AfterProposalVotingPeriodEnded(ctx, proposal.Id)
+		cacheCtx, writeCache := ctx.CacheContext()
+		err = keeper.Hooks().AfterProposalVotingPeriodEnded(cacheCtx, proposal.Id)
+		if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails
+			writeCache()
+		} else {
+			keeper.Logger(ctx).Error("failed to execute AfterProposalVotingPeriodEnded hook", "error", err)
+		}
 
 		logger.Info(
 			"proposal tallied",

x/gov/keeper/deposit.go

@@ -115,7 +115,10 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito
 	}
 
 	// called when deposit has been added to a proposal, however the proposal may not be active
-	keeper.Hooks().AfterProposalDeposit(ctx, proposalID, depositorAddr)
+	err = keeper.Hooks().AfterProposalDeposit(ctx, proposalID, depositorAddr)
+	if err != nil {
+		return false, err
+	}
 
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 	sdkCtx.EventManager().EmitEvent(

x/gov/keeper/hooks_test.go

@@ -27,24 +27,29 @@ type MockGovHooksReceiver struct {
 	AfterProposalVotingPeriodEndedValid bool
 }
 
-func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx context.Context, proposalID uint64) {
+func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx context.Context, proposalID uint64) error {
 	h.AfterProposalSubmissionValid = true
+	return nil
 }
 
-func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
+func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error {
 	h.AfterProposalDepositValid = true
+	return nil
 }
 
-func (h *MockGovHooksReceiver) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) {
+func (h *MockGovHooksReceiver) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error {
 	h.AfterProposalVoteValid = true
+	return nil
 }
 
-func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) {
+func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error {
 	h.AfterProposalFailedMinDepositValid = true
+	return nil
 }
 
-func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) {
+func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error {
 	h.AfterProposalVotingPeriodEndedValid = true
+	return nil
 }
 
 func TestHooks(t *testing.T) {

x/gov/keeper/proposal.go

@@ -114,7 +114,10 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met
 	}
 
 	// called right after a proposal is submitted
-	keeper.Hooks().AfterProposalSubmission(ctx, proposalID)
+	err = keeper.Hooks().AfterProposalSubmission(ctx, proposalID)
+	if err != nil {
+		return v1.Proposal{}, err
+	}
 
 	sdkCtx.EventManager().EmitEvent(
 		sdk.NewEvent(

x/gov/keeper/vote.go

@@ -42,7 +42,10 @@ func (keeper Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr s
 	}
 
 	// called after a vote on a proposal is cast
-	keeper.Hooks().AfterProposalVote(ctx, proposalID, voterAddr)
+	err = keeper.Hooks().AfterProposalVote(ctx, proposalID, voterAddr)
+	if err != nil {
+		return err
+	}
 
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 	sdkCtx.EventManager().EmitEvent(

x/gov/types/expected_keepers.go

@@ -67,11 +67,11 @@ type BankKeeper interface {
 
 // GovHooks event hooks for governance proposal object (noalias)
 type GovHooks interface {
-	AfterProposalSubmission(ctx context.Context, proposalID uint64)                            // Must be called after proposal is submitted
-	AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) // Must be called after a deposit is made
-	AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress)        // Must be called after a vote on a proposal is cast
-	AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64)                      // Must be called when proposal fails to reach min deposit
-	AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64)                     // Must be called when proposal's finishes it's voting period
+	AfterProposalSubmission(ctx context.Context, proposalID uint64) error                            // Must be called after proposal is submitted
+	AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error // Must be called after a deposit is made
+	AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error        // Must be called after a vote on a proposal is cast
+	AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error                      // Must be called when proposal fails to reach min deposit
+	AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error                     // Must be called when proposal's finishes it's voting period
 }
 
 type GovHooksWrapper struct{ GovHooks }

x/gov/types/hooks.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"context"
+	"errors"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
@@ -15,32 +16,43 @@ func NewMultiGovHooks(hooks ...GovHooks) MultiGovHooks {
 	return hooks
 }
 
-func (h MultiGovHooks) AfterProposalSubmission(ctx context.Context, proposalID uint64) {
+func (h MultiGovHooks) AfterProposalSubmission(ctx context.Context, proposalID uint64) error {
+	var errs error
 	for i := range h {
-		h[i].AfterProposalSubmission(ctx, proposalID)
+		errs = errors.Join(errs, h[i].AfterProposalSubmission(ctx, proposalID))
 	}
+
+	return errs
 }
 
-func (h MultiGovHooks) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
+func (h MultiGovHooks) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error {
+	var errs error
 	for i := range h {
-		h[i].AfterProposalDeposit(ctx, proposalID, depositorAddr)
+		errs = errors.Join(errs, h[i].AfterProposalDeposit(ctx, proposalID, depositorAddr))
 	}
+	return errs
 }
 
-func (h MultiGovHooks) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) {
+func (h MultiGovHooks) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error {
+	var errs error
 	for i := range h {
-		h[i].AfterProposalVote(ctx, proposalID, voterAddr)
+		errs = errors.Join(errs, h[i].AfterProposalVote(ctx, proposalID, voterAddr))
 	}
+	return errs
 }
 
-func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) {
+func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error {
+	var errs error
 	for i := range h {
-		h[i].AfterProposalFailedMinDeposit(ctx, proposalID)
+		errs = errors.Join(errs, h[i].AfterProposalFailedMinDeposit(ctx, proposalID))
 	}
+	return errs
 }
 
-func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) {
+func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error {
+	var errs error
 	for i := range h {
-		h[i].AfterProposalVotingPeriodEnded(ctx, proposalID)
+		errs = errors.Join(errs, h[i].AfterProposalVotingPeriodEnded(ctx, proposalID))
 	}
+	return errs
 }