Skip to content

Commit faca3c1

Browse files
refactor(x/gov)!: let hooks return an error (backport #18173) (#18174)
Co-authored-by: Julien Robert <julien@rbrt.fr>
1 parent 3abc61f commit faca3c1

8 files changed

Lines changed: 68 additions & 28 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,18 @@ Ref: https://keepachangelog.com/en/1.0.0/
4040

4141
### Features
4242

43-
* (server) [#18162](https://github.com/cosmos/cosmos-sdk/pull/18162) Start gRPC & API server in standalone mode
43+
* (server) [#18162](https://github.com/cosmos/cosmos-sdk/pull/18162) Start gRPC & API server in standalone mode.
44+
4445
### Improvements
4546

46-
* (x/staking/keeper) [#]18049(https://github.com/cosmos/cosmos-sdk/pull/18049) return early if Slash encounters zero tokens to burn.
47-
* (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.
47+
* (x/staking) [#18049](https://github.com/cosmos/cosmos-sdk/pull/18049) Return early if Slash encounters zero tokens to burn.
48+
* (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.
4849
* (keyring) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) Add `NewAutoCLIKeyring` for creating an AutoCLI keyring from a SDK keyring.
4950
* (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) Let `ToProtoConsensusParams()` return an error.
5051

5152
### Bug Fixes
5253

54+
* (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.
5355
* (x/gov) [#17873](https://github.com/cosmos/cosmos-sdk/pull/17873) Fail any inactive and active proposals that cannot be decoded.
5456
* (x/slashing) [#18016](https://github.com/cosmos/cosmos-sdk/pull/18016) Fixed builder function for missed blocks key (`validatorMissedBlockBitArrayPrefixKey`) in slashing/migration/v4
5557
* (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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
6565
}
6666

6767
// called when proposal become inactive
68-
keeper.Hooks().AfterProposalFailedMinDeposit(ctx, proposal.Id)
68+
cacheCtx, writeCache := ctx.CacheContext()
69+
err = keeper.Hooks().AfterProposalFailedMinDeposit(cacheCtx, proposal.Id)
70+
if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails
71+
writeCache()
72+
} else {
73+
keeper.Logger(ctx).Error("failed to execute AfterProposalFailedMinDeposit hook", "error", err)
74+
}
6975

7076
ctx.EventManager().EmitEvent(
7177
sdk.NewEvent(
@@ -228,7 +234,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
228234
}
229235

230236
// when proposal become active
231-
keeper.Hooks().AfterProposalVotingPeriodEnded(ctx, proposal.Id)
237+
cacheCtx, writeCache := ctx.CacheContext()
238+
err = keeper.Hooks().AfterProposalVotingPeriodEnded(cacheCtx, proposal.Id)
239+
if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails
240+
writeCache()
241+
} else {
242+
keeper.Logger(ctx).Error("failed to execute AfterProposalVotingPeriodEnded hook", "error", err)
243+
}
232244

233245
logger.Info(
234246
"proposal tallied",

x/gov/keeper/deposit.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito
115115
}
116116

117117
// called when deposit has been added to a proposal, however the proposal may not be active
118-
keeper.Hooks().AfterProposalDeposit(ctx, proposalID, depositorAddr)
118+
err = keeper.Hooks().AfterProposalDeposit(ctx, proposalID, depositorAddr)
119+
if err != nil {
120+
return false, err
121+
}
119122

120123
sdkCtx := sdk.UnwrapSDKContext(ctx)
121124
sdkCtx.EventManager().EmitEvent(

x/gov/keeper/hooks_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,29 @@ type MockGovHooksReceiver struct {
2727
AfterProposalVotingPeriodEndedValid bool
2828
}
2929

30-
func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx context.Context, proposalID uint64) {
30+
func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx context.Context, proposalID uint64) error {
3131
h.AfterProposalSubmissionValid = true
32+
return nil
3233
}
3334

34-
func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
35+
func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error {
3536
h.AfterProposalDepositValid = true
37+
return nil
3638
}
3739

38-
func (h *MockGovHooksReceiver) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) {
40+
func (h *MockGovHooksReceiver) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error {
3941
h.AfterProposalVoteValid = true
42+
return nil
4043
}
4144

42-
func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) {
45+
func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error {
4346
h.AfterProposalFailedMinDepositValid = true
47+
return nil
4448
}
4549

46-
func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) {
50+
func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error {
4751
h.AfterProposalVotingPeriodEndedValid = true
52+
return nil
4853
}
4954

5055
func TestHooks(t *testing.T) {

x/gov/keeper/proposal.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met
114114
}
115115

116116
// called right after a proposal is submitted
117-
keeper.Hooks().AfterProposalSubmission(ctx, proposalID)
117+
err = keeper.Hooks().AfterProposalSubmission(ctx, proposalID)
118+
if err != nil {
119+
return v1.Proposal{}, err
120+
}
118121

119122
sdkCtx.EventManager().EmitEvent(
120123
sdk.NewEvent(

x/gov/keeper/vote.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ func (keeper Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr s
4242
}
4343

4444
// called after a vote on a proposal is cast
45-
keeper.Hooks().AfterProposalVote(ctx, proposalID, voterAddr)
45+
err = keeper.Hooks().AfterProposalVote(ctx, proposalID, voterAddr)
46+
if err != nil {
47+
return err
48+
}
4649

4750
sdkCtx := sdk.UnwrapSDKContext(ctx)
4851
sdkCtx.EventManager().EmitEvent(

x/gov/types/expected_keepers.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ type BankKeeper interface {
6767

6868
// GovHooks event hooks for governance proposal object (noalias)
6969
type GovHooks interface {
70-
AfterProposalSubmission(ctx context.Context, proposalID uint64) // Must be called after proposal is submitted
71-
AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) // Must be called after a deposit is made
72-
AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) // Must be called after a vote on a proposal is cast
73-
AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) // Must be called when proposal fails to reach min deposit
74-
AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) // Must be called when proposal's finishes it's voting period
70+
AfterProposalSubmission(ctx context.Context, proposalID uint64) error // Must be called after proposal is submitted
71+
AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error // Must be called after a deposit is made
72+
AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error // Must be called after a vote on a proposal is cast
73+
AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error // Must be called when proposal fails to reach min deposit
74+
AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error // Must be called when proposal's finishes it's voting period
7575
}
7676

7777
type GovHooksWrapper struct{ GovHooks }

x/gov/types/hooks.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package types
22

33
import (
44
"context"
5+
"errors"
56

67
sdk "github.com/cosmos/cosmos-sdk/types"
78
)
@@ -15,32 +16,43 @@ func NewMultiGovHooks(hooks ...GovHooks) MultiGovHooks {
1516
return hooks
1617
}
1718

18-
func (h MultiGovHooks) AfterProposalSubmission(ctx context.Context, proposalID uint64) {
19+
func (h MultiGovHooks) AfterProposalSubmission(ctx context.Context, proposalID uint64) error {
20+
var errs error
1921
for i := range h {
20-
h[i].AfterProposalSubmission(ctx, proposalID)
22+
errs = errors.Join(errs, h[i].AfterProposalSubmission(ctx, proposalID))
2123
}
24+
25+
return errs
2226
}
2327

24-
func (h MultiGovHooks) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
28+
func (h MultiGovHooks) AfterProposalDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) error {
29+
var errs error
2530
for i := range h {
26-
h[i].AfterProposalDeposit(ctx, proposalID, depositorAddr)
31+
errs = errors.Join(errs, h[i].AfterProposalDeposit(ctx, proposalID, depositorAddr))
2732
}
33+
return errs
2834
}
2935

30-
func (h MultiGovHooks) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) {
36+
func (h MultiGovHooks) AfterProposalVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress) error {
37+
var errs error
3138
for i := range h {
32-
h[i].AfterProposalVote(ctx, proposalID, voterAddr)
39+
errs = errors.Join(errs, h[i].AfterProposalVote(ctx, proposalID, voterAddr))
3340
}
41+
return errs
3442
}
3543

36-
func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) {
44+
func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx context.Context, proposalID uint64) error {
45+
var errs error
3746
for i := range h {
38-
h[i].AfterProposalFailedMinDeposit(ctx, proposalID)
47+
errs = errors.Join(errs, h[i].AfterProposalFailedMinDeposit(ctx, proposalID))
3948
}
49+
return errs
4050
}
4151

42-
func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) {
52+
func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx context.Context, proposalID uint64) error {
53+
var errs error
4354
for i := range h {
44-
h[i].AfterProposalVotingPeriodEnded(ctx, proposalID)
55+
errs = errors.Join(errs, h[i].AfterProposalVotingPeriodEnded(ctx, proposalID))
4556
}
57+
return errs
4658
}

0 commit comments

Comments
 (0)