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

Add hooks to governance actions #9133

Merged
merged 10 commits into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rosetta) [\#8729](https://github.com/cosmos/cosmos-sdk/pull/8729) Data API fully supports balance tracking. Construction API can now construct any message supported by the application.
* [\#8754](https://github.com/cosmos/cosmos-sdk/pull/8875) Added support for reverse iteration to pagination.
* [#9088](https://github.com/cosmos/cosmos-sdk/pull/9088) Added implementation to ADR-28 Derived Addresses.
* [\#9133](https://github.com/cosmos/cosmos-sdk/pull/9133) Added hooks for governance actions.

### Client Breaking Changes
* [\#8363](https://github.com/cosmos/cosmos-sdk/pull/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address.
Expand Down
8 changes: 7 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,17 @@ func NewSimApp(
AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper))
app.GovKeeper = govkeeper.NewKeeper(
govKeeper := govkeeper.NewKeeper(
appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper,
&stakingKeeper, govRouter,
)

app.GovKeeper = *govKeeper.SetHooks(
govtypes.NewMultiGovHooks(
// register the governance hooks
),
)

// create evidence keeper with router
evidenceKeeper := evidencekeeper.NewKeeper(
appCodec, keys[evidencetypes.StoreKey], &app.StakingKeeper, app.SlashingKeeper,
Expand Down
6 changes: 6 additions & 0 deletions x/gov/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
keeper.DeleteProposal(ctx, proposal.ProposalId)
keeper.DeleteDeposits(ctx, proposal.ProposalId)

// called when proposal become inactive
keeper.AfterProposalFailedMinDeposit(ctx, proposal.ProposalId)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeInactiveProposal,
Expand Down Expand Up @@ -89,6 +92,9 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
keeper.SetProposal(ctx, proposal)
keeper.RemoveFromActiveProposalQueue(ctx, proposal.ProposalId, proposal.VotingEndTime)

// when proposal become active
keeper.AfterProposalVotingPeriodEnded(ctx, proposal.ProposalId)

logger.Info(
"proposal tallied",
"proposal", proposal.ProposalId,
Expand Down
3 changes: 3 additions & 0 deletions x/gov/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd
deposit = types.NewDeposit(proposalID, depositorAddr, depositAmount)
}

// called when deposit has been added to a proposal, however the proposal may not be active
keeper.AfterProposalDeposit(ctx, proposalID, depositorAddr)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeProposalDeposit,
Expand Down
44 changes: 44 additions & 0 deletions x/gov/keeper/hooks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/types"
)

// Implements GovHooks interface
var _ types.GovHooks = Keeper{}

// AfterProposalSubmission - call hook if registered
func (keeper Keeper) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {
if keeper.hooks != nil {
keeper.hooks.AfterProposalSubmission(ctx, proposalID)
}
}

// AfterProposalDeposit - call hook if registered
func (keeper Keeper) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
if keeper.hooks != nil {
keeper.hooks.AfterProposalDeposit(ctx, proposalID, depositorAddr)
}
}

// AfterProposalVote - call hook if registered
func (keeper Keeper) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) {
if keeper.hooks != nil {
keeper.hooks.AfterProposalVote(ctx, proposalID, voterAddr)
}
}

// AfterProposalFailedMinDeposit - call hook if registered
func (keeper Keeper) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {
if keeper.hooks != nil {
keeper.hooks.AfterProposalFailedMinDeposit(ctx, proposalID)
}
}

// AfterProposalVotingPeriodEnded - call hook if registered
func (keeper Keeper) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) {
if keeper.hooks != nil {
keeper.hooks.AfterProposalVotingPeriodEnded(ctx, proposalID)
}
}
96 changes: 96 additions & 0 deletions x/gov/keeper/hooks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package keeper_test

import (
"testing"
"time"

"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov"
"github.com/cosmos/cosmos-sdk/x/gov/keeper"
"github.com/cosmos/cosmos-sdk/x/gov/types"
)

var _ types.GovHooks = &MockGovHooksReceiver{}

// GovHooks event hooks for governance proposal object (noalias)
type MockGovHooksReceiver struct {
AfterProposalSubmissionValid bool
AfterProposalDepositValid bool
AfterProposalVoteValid bool
AfterProposalFailedMinDepositValid bool
AfterProposalVotingPeriodEndedValid bool
}

func (h *MockGovHooksReceiver) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {
h.AfterProposalSubmissionValid = true
}

func (h *MockGovHooksReceiver) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
h.AfterProposalDepositValid = true
}

func (h *MockGovHooksReceiver) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) {
h.AfterProposalVoteValid = true
}
func (h *MockGovHooksReceiver) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {
h.AfterProposalFailedMinDepositValid = true
}
func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) {
h.AfterProposalVotingPeriodEndedValid = true
}

func TestHooks(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we have a testing suite for this instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wdym?

app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})

minDeposit := app.GovKeeper.GetDepositParams(ctx).MinDeposit
addrs := simapp.AddTestAddrs(app, ctx, 1, minDeposit[0].Amount)

govHooksReceiver := MockGovHooksReceiver{}

app.GovKeeper = *keeper.UpdateHooks(&app.GovKeeper,
types.NewMultiGovHooks(
&govHooksReceiver,
),
)

require.False(t, govHooksReceiver.AfterProposalSubmissionValid)
require.False(t, govHooksReceiver.AfterProposalDepositValid)
require.False(t, govHooksReceiver.AfterProposalVoteValid)
require.False(t, govHooksReceiver.AfterProposalFailedMinDepositValid)
require.False(t, govHooksReceiver.AfterProposalVotingPeriodEndedValid)

tp := TestProposal
_, err := app.GovKeeper.SubmitProposal(ctx, tp)
require.NoError(t, err)
require.True(t, govHooksReceiver.AfterProposalSubmissionValid)

newHeader := ctx.BlockHeader()
newHeader.Time = ctx.BlockHeader().Time.Add(app.GovKeeper.GetDepositParams(ctx).MaxDepositPeriod).Add(time.Duration(1) * time.Second)
ctx = ctx.WithBlockHeader(newHeader)
gov.EndBlocker(ctx, app.GovKeeper)

require.True(t, govHooksReceiver.AfterProposalFailedMinDepositValid)

p2, err := app.GovKeeper.SubmitProposal(ctx, tp)
require.NoError(t, err)

activated, err := app.GovKeeper.AddDeposit(ctx, p2.ProposalId, addrs[0], minDeposit)
require.True(t, activated)
require.NoError(t, err)
require.True(t, govHooksReceiver.AfterProposalDepositValid)

err = app.GovKeeper.AddVote(ctx, p2.ProposalId, addrs[0], types.NewNonSplitVoteOption(types.OptionYes))
require.NoError(t, err)
require.True(t, govHooksReceiver.AfterProposalVoteValid)

newHeader = ctx.BlockHeader()
newHeader.Time = ctx.BlockHeader().Time.Add(app.GovKeeper.GetVotingParams(ctx).VotingPeriod).Add(time.Duration(1) * time.Second)
ctx = ctx.WithBlockHeader(newHeader)
gov.EndBlocker(ctx, app.GovKeeper)
require.True(t, govHooksReceiver.AfterProposalVotingPeriodEndedValid)
}
8 changes: 8 additions & 0 deletions x/gov/keeper/internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package keeper

import "github.com/cosmos/cosmos-sdk/x/gov/types"

func UpdateHooks(k *Keeper, h types.GovHooks) *Keeper {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you define this on the other test file instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, so the reason it's like this is that the hooks field in the keeper is unexposed, so it can only be accessed by the keeper package, not the keeper_test package.

And we don't want to add this function to the keeper.go file because that then it would be available to all other modules, which isn't what we want. The trick is to put it in a file with the _test suffix but still in the keeper package, so it's only built during tests and thus accessible to other tests, but not other modules.

See: https://stackoverflow.com/questions/24622388/how-to-test-a-unexported-private-function-in-go-golang

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! thanks for the clarification

k.hooks = h
return k
}
14 changes: 14 additions & 0 deletions x/gov/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ type Keeper struct {
// The reference to the DelegationSet and ValidatorSet to get information about validators and delegators
sk types.StakingKeeper

// GovHooks
hooks types.GovHooks

// The (unexposed) keys used to access the stores from the Context.
storeKey sdk.StoreKey

Expand Down Expand Up @@ -66,6 +69,17 @@ func NewKeeper(
}
}

// SetHooks sets the hooks for governance
func (keeper *Keeper) SetHooks(gh types.GovHooks) *Keeper {
sunnya97 marked this conversation as resolved.
Show resolved Hide resolved
if keeper.hooks != nil {
panic("cannot set governance hooks twice")
}

keeper.hooks = gh

return keeper
}

// Logger returns a module-specific logger.
func (keeper Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", "x/"+types.ModuleName)
Expand Down
3 changes: 3 additions & 0 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ func (keeper Keeper) SubmitProposal(ctx sdk.Context, content types.Content) (typ
keeper.InsertInactiveProposalQueue(ctx, proposalID, proposal.DepositEndTime)
keeper.SetProposalID(ctx, proposalID+1)

// called right after a proposal is submitted
keeper.AfterProposalSubmission(ctx, proposalID)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeSubmitProposal,
Expand Down
3 changes: 3 additions & 0 deletions x/gov/keeper/vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A
vote := types.NewVote(proposalID, voterAddr, options)
keeper.SetVote(ctx, vote)

// called after a vote on a proposal is cast
keeper.AfterProposalVote(ctx, proposalID, voterAddr)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeProposalVote,
Expand Down
13 changes: 13 additions & 0 deletions x/gov/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,16 @@ type BankKeeper interface {
SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, name string, amt sdk.Coins) error
}

// Event Hooks
// These can be utilized to communicate between a governance keeper and another
// keepers.

// 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
}
42 changes: 42 additions & 0 deletions x/gov/types/hooks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package types

import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

var _ GovHooks = MultiGovHooks{}

// combine multiple governance hooks, all hook functions are run in array sequence
type MultiGovHooks []GovHooks

func NewMultiGovHooks(hooks ...GovHooks) MultiGovHooks {
return hooks
}

func (h MultiGovHooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {
for i := range h {
h[i].AfterProposalSubmission(ctx, proposalID)
}
}

func (h MultiGovHooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
for i := range h {
h[i].AfterProposalDeposit(ctx, proposalID, depositorAddr)
}
}

func (h MultiGovHooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) {
for i := range h {
h[i].AfterProposalVote(ctx, proposalID, voterAddr)
}
}
func (h MultiGovHooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {
for i := range h {
h[i].AfterProposalFailedMinDeposit(ctx, proposalID)
}
}
func (h MultiGovHooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) {
for i := range h {
h[i].AfterProposalVotingPeriodEnded(ctx, proposalID)
}
}