From 5508d901b5b57763cbb3c469232add540160d669 Mon Sep 17 00:00:00 2001 From: dusan-maksimovic Date: Wed, 31 Jan 2024 08:34:16 +0100 Subject: [PATCH] support fo governance keeper hooks returning errors --- x/gov/abci.go | 16 ++++++++++++++-- x/gov/keeper/deposit.go | 5 ++++- x/gov/keeper/hooks_test.go | 15 ++++++++++----- x/gov/keeper/proposal.go | 5 ++++- x/gov/keeper/vote.go | 5 ++++- x/gov/types/expected_keepers.go | 10 +++++----- x/gov/types/hooks.go | 32 ++++++++++++++++++++++---------- 7 files changed, 63 insertions(+), 25 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 8fc06069f8862..bc8764aec0dc7 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -31,7 +31,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) { } // 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( @@ -118,7 +124,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) { keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) // 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", diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 62b90bd46d5f8..f99be8b8bf3d1 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -147,7 +147,10 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd } // 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 + } ctx.EventManager().EmitEvent( sdk.NewEvent( diff --git a/x/gov/keeper/hooks_test.go b/x/gov/keeper/hooks_test.go index cf07cd1213297..f24c37f2b29a2 100644 --- a/x/gov/keeper/hooks_test.go +++ b/x/gov/keeper/hooks_test.go @@ -25,24 +25,29 @@ type MockGovHooksReceiver struct { AfterProposalVotingPeriodEndedValid bool } -func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) { +func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) error { h.AfterProposalSubmissionValid = true + return nil } -func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) { +func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) error { h.AfterProposalDepositValid = true + return nil } -func (h *MockGovHooksReceiver) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) { +func (h *MockGovHooksReceiver) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) error { h.AfterProposalVoteValid = true + return nil } -func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) { +func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) error { h.AfterProposalFailedMinDepositValid = true + return nil } -func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) { +func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) error { h.AfterProposalVotingPeriodEndedValid = true + return nil } func TestHooks(t *testing.T) { diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index b050b51fcf10e..ee1ac5e0c431b 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -94,7 +94,10 @@ func (keeper Keeper) SubmitProposal(ctx sdk.Context, messages []sdk.Msg, metadat keeper.SetProposalID(ctx, proposalID+1) // 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 + } ctx.EventManager().EmitEvent( sdk.NewEvent( diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index 2f24c37d2e93f..ec20dd4155a3e 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -32,7 +32,10 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A keeper.SetVote(ctx, vote) // 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 + } ctx.EventManager().EmitEvent( sdk.NewEvent( diff --git a/x/gov/types/expected_keepers.go b/x/gov/types/expected_keepers.go index 70f0e5fbe994c..0a0da4e9e1c09 100644 --- a/x/gov/types/expected_keepers.go +++ b/x/gov/types/expected_keepers.go @@ -56,11 +56,11 @@ type BankKeeper interface { // GovHooks event hooks for governance proposal object (noalias) type GovHooks interface { - AfterProposalSubmission(ctx sdk.Context, proposalID uint64) // Must be called after proposal is submitted - AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) // Must be called after a deposit is made - AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) // Must be called after a vote on a proposal is cast - AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) // Must be called when proposal fails to reach min deposit - AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) // Must be called when proposal's finishes it's voting period + AfterProposalSubmission(ctx sdk.Context, proposalID uint64) error // Must be called after proposal is submitted + AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) error // Must be called after a deposit is made + AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) error // Must be called after a vote on a proposal is cast + AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) error // Must be called when proposal fails to reach min deposit + AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) error // Must be called when proposal's finishes it's voting period } type GovHooksWrapper struct{ GovHooks } diff --git a/x/gov/types/hooks.go b/x/gov/types/hooks.go index 0a361687d9d2f..3ab4f883d3261 100644 --- a/x/gov/types/hooks.go +++ b/x/gov/types/hooks.go @@ -1,6 +1,8 @@ package types import ( + "errors" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -13,32 +15,42 @@ func NewMultiGovHooks(hooks ...GovHooks) MultiGovHooks { return hooks } -func (h MultiGovHooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) { +func (h MultiGovHooks) AfterProposalSubmission(ctx sdk.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 sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) { +func (h MultiGovHooks) AfterProposalDeposit(ctx sdk.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 sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) { +func (h MultiGovHooks) AfterProposalVote(ctx sdk.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 sdk.Context, proposalID uint64) { +func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx sdk.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 sdk.Context, proposalID uint64) { +func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx sdk.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 }