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

refactor: simplify hooks implementation #13396

Merged
merged 9 commits into from
Sep 27, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ cosmovisor:


mocks: $(MOCKS_DIR)
@go install github.com/golang/mock/mockgen@v1.6.0
sh ./scripts/mockgen.sh
.PHONY: mocks

Expand Down
3 changes: 1 addition & 2 deletions scripts/mockgen.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
#!/usr/bin/env bash

mockgen_cmd="go run github.com/golang/mock/mockgen"
mockgen_cmd="mockgen"
$mockgen_cmd -source=client/account_retriever.go -package mock -destination testutil/mock/account_retriever.go
$mockgen_cmd -package mock -destination testutil/mock/tendermint_tm_db_DB.go github.com/tendermint/tm-db DB
$mockgen_cmd -source db/types.go -package mock -destination testutil/mock/db/types.go
$mockgen_cmd -source=types/module/module.go -package mock -destination testutil/mock/types_module_module.go
$mockgen_cmd -source=types/invariant.go -package mock -destination testutil/mock/types_invariant.go
$mockgen_cmd -source=types/router.go -package mock -destination testutil/mock/types_router.go
Expand Down
3 changes: 3 additions & 0 deletions store/tools/ics23/iavl/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func GetNonKey(allkeys [][]byte, loc tmproofs.Where) []byte {
// returns a list of all keys in sorted order
func BuildTree(size int) (tree *iavl.MutableTree, keys [][]byte, err error) {
tree, err = iavl.NewMutableTree(db.NewMemDB(), 0)
if err != nil {
return nil, nil, err
}

// insert lots of info and store the bytes
keys = make([][]byte, size)
Expand Down
9 changes: 7 additions & 2 deletions x/distribution/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ type Hooks struct {
var _ stakingtypes.StakingHooks = Hooks{}

// Create new distribution hooks
func (k Keeper) Hooks() Hooks { return Hooks{k} }
func (k Keeper) Hooks() Hooks {
return Hooks{k}
}

// initialize validator distribution record
func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
Expand Down Expand Up @@ -109,7 +111,10 @@ func (h Hooks) BeforeValidatorSlashed(ctx sdk.Context, valAddr sdk.ValAddress, f
return nil
}

func (h Hooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) error { return nil }
func (h Hooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) error {
return nil
}

func (h Hooks) AfterValidatorBonded(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions x/gov/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal

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

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

ctx.EventManager().EmitEvent(
sdk.NewEvent(
Expand Down Expand Up @@ -104,7 +104,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)

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

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

logger.Info(
"proposal tallied",
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ 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.AfterProposalDeposit(ctx, proposalID, depositorAddr)
keeper.Hooks().AfterProposalDeposit(ctx, proposalID, depositorAddr)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
Expand Down
44 changes: 0 additions & 44 deletions x/gov/keeper/hooks.go

This file was deleted.

10 changes: 10 additions & 0 deletions x/gov/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ func NewKeeper(
}
}

// Hooks gets the hooks for governance *Keeper {
func (keeper *Keeper) Hooks() types.GovHooks {
if keeper.hooks == nil {
// return a no-op implementation if no hooks are set
return types.MultiGovHooks{}
}

return keeper.hooks
}

// SetHooks sets the hooks for governance
func (keeper *Keeper) SetHooks(gh types.GovHooks) *Keeper {
if keeper.hooks != nil {
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (keeper Keeper) SubmitProposal(ctx sdk.Context, messages []sdk.Msg, metadat
keeper.SetProposalID(ctx, proposalID+1)

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

ctx.EventManager().EmitEvent(
sdk.NewEvent(
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ 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.AfterProposalVote(ctx, proposalID, voterAddr)
keeper.Hooks().AfterProposalVote(ctx, proposalID, voterAddr)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
Expand Down
67 changes: 26 additions & 41 deletions x/slashing/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,26 @@ import (
"github.com/cosmos/cosmos-sdk/x/slashing/types"
)

func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _ sdk.ValAddress) error {
// Update the signing info start height or create a new signing info
signingInfo, found := k.GetValidatorSigningInfo(ctx, address)
var _ types.StakingHooks = Hooks{}

// Hooks wrapper struct for slashing keeper
type Hooks struct {
k Keeper
}

// Return the slashing hooks
func (k Keeper) Hooks() Hooks {
return Hooks{k}
}

// AfterValidatorBonded updates the signing info start height or create a new signing info
func (h Hooks) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error {
signingInfo, found := h.k.GetValidatorSigningInfo(ctx, consAddr)
if found {
signingInfo.StartHeight = ctx.BlockHeight()
} else {
signingInfo = types.NewValidatorSigningInfo(
address,
consAddr,
ctx.BlockHeight(),
0,
time.Unix(0, 0),
Expand All @@ -25,53 +37,26 @@ func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _
)
}

k.SetValidatorSigningInfo(ctx, address, signingInfo)
h.k.SetValidatorSigningInfo(ctx, consAddr, signingInfo)

return nil
}

// AfterValidatorRemoved deletes the address-pubkey relation when a validator is removed,
func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, consAddr sdk.ConsAddress, _ sdk.ValAddress) error {
h.k.deleteAddrPubkeyRelation(ctx, crypto.Address(consAddr))
return nil
}

// AfterValidatorCreated adds the address-pubkey relation when a validator is created.
func (k Keeper) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
validator := k.sk.Validator(ctx, valAddr)
func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
validator := h.k.sk.Validator(ctx, valAddr)
consPk, err := validator.ConsPubKey()
if err != nil {
return err
}

return k.AddPubkey(ctx, consPk)
}

// AfterValidatorRemoved deletes the address-pubkey relation when a validator is removed,
func (k Keeper) AfterValidatorRemoved(ctx sdk.Context, address sdk.ConsAddress) error {
k.deleteAddrPubkeyRelation(ctx, crypto.Address(address))
return nil
}

// Hooks wrapper struct for slashing keeper
type Hooks struct {
k Keeper
}

var _ types.StakingHooks = Hooks{}

// Return the wrapper struct
func (k Keeper) Hooks() Hooks {
return Hooks{k}
}

// Implements sdk.ValidatorHooks
func (h Hooks) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error {
return h.k.AfterValidatorBonded(ctx, consAddr, valAddr)
}

// Implements sdk.ValidatorHooks
func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, consAddr sdk.ConsAddress, _ sdk.ValAddress) error {
return h.k.AfterValidatorRemoved(ctx, consAddr)
}

// Implements sdk.ValidatorHooks
func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
return h.k.AfterValidatorCreated(ctx, valAddr)
return h.k.AddPubkey(ctx, consPk)
}

func (h Hooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
Expand Down
6 changes: 3 additions & 3 deletions x/slashing/keeper/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func (s *KeeperTestSuite) TestAfterValidatorBonded() {
require := s.Require()

valAddr := sdk.ValAddress(consAddr.Bytes())
keeper.AfterValidatorBonded(ctx, consAddr, valAddr)
keeper.Hooks().AfterValidatorBonded(ctx, consAddr, valAddr)

_, ok := keeper.GetValidatorSigningInfo(ctx, consAddr)
require.True(ok)
Expand All @@ -28,14 +28,14 @@ func (s *KeeperTestSuite) TestAfterValidatorCreatedOrRemoved() {
require.NoError(err)

s.stakingKeeper.EXPECT().Validator(ctx, valAddr).Return(validator)
err = keeper.AfterValidatorCreated(ctx, valAddr)
err = keeper.Hooks().AfterValidatorCreated(ctx, valAddr)
require.NoError(err)

ePubKey, err := keeper.GetPubkey(ctx, addr.Bytes())
require.NoError(err)
require.Equal(ePubKey, pubKey)

err = keeper.AfterValidatorRemoved(ctx, sdk.ConsAddress(addr))
err = keeper.Hooks().AfterValidatorRemoved(ctx, sdk.ConsAddress(addr), nil)
require.NoError(err)

_, err = keeper.GetPubkey(ctx, addr.Bytes())
Expand Down
98 changes: 98 additions & 0 deletions x/slashing/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading