From 684ee627d1752c760a3e492a307619e4b586afd5 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 27 Sep 2022 19:39:04 +0200 Subject: [PATCH 1/3] fix: attempt at fixing flaky e2e tests (#13401) * fix: attempt at fixing flaky e2e tests * `make format` * add links --- .github/workflows/build-skip.yml | 1 + .github/workflows/test-e2e-skip.yml | 1 + .github/workflows/test-integration-skip.yml | 1 + baseapp/baseapp.go | 1 - tests/e2e/group/tx.go | 2 ++ x/auth/client/testutil/suite.go | 1 + 6 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-skip.yml b/.github/workflows/build-skip.yml index bbcd02f30371..c96e8a1022db 100644 --- a/.github/workflows/build-skip.yml +++ b/.github/workflows/build-skip.yml @@ -1,5 +1,6 @@ name: Build SimApp # This workflow allows to skip the build step if the PR does not contain any changes to the code +# See https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks on: pull_request: paths-ignore: diff --git a/.github/workflows/test-e2e-skip.yml b/.github/workflows/test-e2e-skip.yml index c4622f3dc502..41ab5415225b 100644 --- a/.github/workflows/test-e2e-skip.yml +++ b/.github/workflows/test-e2e-skip.yml @@ -1,5 +1,6 @@ name: Tests E2E # This workflow allows to skip the e2e step if the PR does not contain any changes to the code +# See https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks on: pull_request: paths-ignore: diff --git a/.github/workflows/test-integration-skip.yml b/.github/workflows/test-integration-skip.yml index 3c0fef380327..e8621fdc5e73 100644 --- a/.github/workflows/test-integration-skip.yml +++ b/.github/workflows/test-integration-skip.yml @@ -1,5 +1,6 @@ name: Tests Integration # This workflow allows to skip the integration step if the PR does not contain any changes to the code +# See https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks on: pull_request: paths-ignore: diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 98d4b3bedd42..247099390985 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -270,7 +270,6 @@ func (app *BaseApp) MountStore(key storetypes.StoreKey, typ storetypes.StoreType // LoadLatestVersion loads the latest application version. It will panic if // called more than once on a running BaseApp. func (app *BaseApp) LoadLatestVersion() error { - err := app.storeLoader(app.cms) if err != nil { return fmt.Errorf("failed to load latest version: %w", err) diff --git a/tests/e2e/group/tx.go b/tests/e2e/group/tx.go index 7488806c798e..c50ef6f0fc3d 100644 --- a/tests/e2e/group/tx.go +++ b/tests/e2e/group/tx.go @@ -2151,6 +2151,8 @@ func (s *IntegrationTestSuite) TestTxLeaveGroup() { ), ) s.Require().NoError(err, out.String()) + s.Require().NoError(s.network.WaitForNextBlock()) + var txResp sdk.TxResponse s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txResp), out.String()) txResp, err = clitestutil.GetTxResponse(s.network, val.ClientCtx, txResp.TxHash) diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index 9ac15b7aa2de..b4620fe4f7cc 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -1587,6 +1587,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() { ) require.NoError(err) require.NoError(s.network.WaitForNextBlock()) + require.NoError(s.network.WaitForNextBlock()) var txRes sdk.TxResponse require.NoError(val0.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) From dcb0c9c04c5d8406bede521cde94335015185b2b Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 28 Sep 2022 02:16:13 +0800 Subject: [PATCH 2/3] fix: double close (#13400) --- CHANGELOG.md | 1 + baseapp/deliver_tx_test.go | 20 ++++++++++---------- snapshots/helpers_test.go | 1 - snapshots/manager_test.go | 2 +- snapshots/stream.go | 4 ---- snapshots/types/format.go | 2 +- store/rootmulti/snapshot_test.go | 2 +- 7 files changed, 14 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d08dcb773d94..f8d2180a3d08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -176,6 +176,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#13051](https://github.com/cosmos/cosmos-sdk/pull/13051) In SubmitPropsal, when a legacy msg fails it's handler call, wrap the error as ErrInvalidProposalContent (instead of ErrNoProposalHandlerExists). * (x/gov) [#13045](https://github.com/cosmos/cosmos-sdk/pull/13045) Fix gov migrations for v3(0.46). * (Store) [#13334](https://github.com/cosmos/cosmos-sdk/pull/13334) Call streaming listeners for deliver tx event, it was removed accidentally. +* (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19. ### Deprecated diff --git a/baseapp/deliver_tx_test.go b/baseapp/deliver_tx_test.go index 7eacbee32c65..453ddab35c48 100644 --- a/baseapp/deliver_tx_test.go +++ b/baseapp/deliver_tx_test.go @@ -666,7 +666,7 @@ func TestSnapshotWithPruning(t *testing.T) { pruningOpts: pruningtypes.NewPruningOptions(pruningtypes.PruningNothing), }, expectedSnapshots: []*abci.Snapshot{ - {Height: 20, Format: 2, Chunks: 5}, + {Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5}, }, }, "prune everything with snapshot": { @@ -678,7 +678,7 @@ func TestSnapshotWithPruning(t *testing.T) { pruningOpts: pruningtypes.NewPruningOptions(pruningtypes.PruningEverything), }, expectedSnapshots: []*abci.Snapshot{ - {Height: 20, Format: 2, Chunks: 5}, + {Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5}, }, }, "default pruning with snapshot": { @@ -690,7 +690,7 @@ func TestSnapshotWithPruning(t *testing.T) { pruningOpts: pruningtypes.NewPruningOptions(pruningtypes.PruningDefault), }, expectedSnapshots: []*abci.Snapshot{ - {Height: 20, Format: 2, Chunks: 5}, + {Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5}, }, }, "custom": { @@ -702,8 +702,8 @@ func TestSnapshotWithPruning(t *testing.T) { pruningOpts: pruningtypes.NewCustomPruningOptions(12, 12), }, expectedSnapshots: []*abci.Snapshot{ - {Height: 25, Format: 2, Chunks: 6}, - {Height: 20, Format: 2, Chunks: 5}, + {Height: 25, Format: snapshottypes.CurrentFormat, Chunks: 6}, + {Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5}, }, }, "no snapshots": { @@ -724,9 +724,9 @@ func TestSnapshotWithPruning(t *testing.T) { pruningOpts: pruningtypes.NewPruningOptions(pruningtypes.PruningNothing), }, expectedSnapshots: []*abci.Snapshot{ - {Height: 9, Format: 2, Chunks: 2}, - {Height: 6, Format: 2, Chunks: 2}, - {Height: 3, Format: 2, Chunks: 1}, + {Height: 9, Format: snapshottypes.CurrentFormat, Chunks: 2}, + {Height: 6, Format: snapshottypes.CurrentFormat, Chunks: 2}, + {Height: 3, Format: snapshottypes.CurrentFormat, Chunks: 1}, }, }, } @@ -788,7 +788,7 @@ func TestLoadSnapshotChunk(t *testing.T) { blocks: 2, blockTxs: 5, snapshotInterval: 2, - snapshotKeepRecent: 2, + snapshotKeepRecent: snapshottypes.CurrentFormat, pruningOpts: pruningtypes.NewPruningOptions(pruningtypes.PruningNothing), } app, err := setupBaseAppWithSnapshots(t, setupConfig) @@ -802,7 +802,7 @@ func TestLoadSnapshotChunk(t *testing.T) { }{ "Existing snapshot": {2, snapshottypes.CurrentFormat, 1, false}, "Missing height": {100, snapshottypes.CurrentFormat, 1, true}, - "Missing format": {2, 3, 1, true}, + "Missing format": {2, snapshottypes.CurrentFormat + 1, 1, true}, "Missing chunk": {2, snapshottypes.CurrentFormat, 9, true}, "Zero height": {0, snapshottypes.CurrentFormat, 1, true}, "Zero format": {2, 0, 1, true}, diff --git a/snapshots/helpers_test.go b/snapshots/helpers_test.go index 5a9e44daf660..449b9573909c 100644 --- a/snapshots/helpers_test.go +++ b/snapshots/helpers_test.go @@ -90,7 +90,6 @@ func snapshotItems(items [][]byte, ext snapshottypes.ExtensionSnapshotter) [][]b return snapshottypes.WriteExtensionPayload(protoWriter, payload) }) _ = protoWriter.Close() - _ = zWriter.Close() _ = bufWriter.Flush() _ = chunkWriter.Close() }() diff --git a/snapshots/manager_test.go b/snapshots/manager_test.go index 58a302e87ca8..ee4c3b471189 100644 --- a/snapshots/manager_test.go +++ b/snapshots/manager_test.go @@ -95,7 +95,7 @@ func TestManager_Take(t *testing.T) { Height: 5, Format: snapshotter.SnapshotFormat(), Chunks: 1, - Hash: []uint8{0x89, 0xfa, 0x18, 0xbc, 0x5a, 0xe3, 0xdc, 0x36, 0xa6, 0x95, 0x5, 0x17, 0xf9, 0x2, 0x1a, 0x55, 0x36, 0x16, 0x5d, 0x4b, 0x8b, 0x2b, 0x3d, 0xfd, 0xe, 0x2f, 0xb6, 0x40, 0x6b, 0xc3, 0xbc, 0x23}, + Hash: []uint8{0xc5, 0xf7, 0xfe, 0xea, 0xd3, 0x4d, 0x3e, 0x87, 0xff, 0x41, 0xa2, 0x27, 0xfa, 0xcb, 0x38, 0x17, 0xa, 0x5, 0xeb, 0x27, 0x4e, 0x16, 0x5e, 0xf3, 0xb2, 0x8b, 0x47, 0xd1, 0xe6, 0x94, 0x7e, 0x8b}, Metadata: types.Metadata{ ChunkHashes: checksums(expectChunks), }, diff --git a/snapshots/stream.go b/snapshots/stream.go index 88bef076812f..fa61e1457f44 100644 --- a/snapshots/stream.go +++ b/snapshots/stream.go @@ -57,10 +57,6 @@ func (sw *StreamWriter) Close() error { sw.chunkWriter.CloseWithError(err) return err } - if err := sw.zWriter.Close(); err != nil { - sw.chunkWriter.CloseWithError(err) - return err - } if err := sw.bufWriter.Flush(); err != nil { sw.chunkWriter.CloseWithError(err) return err diff --git a/snapshots/types/format.go b/snapshots/types/format.go index d5e960660ac9..317b6a6e329e 100644 --- a/snapshots/types/format.go +++ b/snapshots/types/format.go @@ -3,4 +3,4 @@ package types // CurrentFormat is the currently used format for snapshots. Snapshots using the same format // must be identical across all nodes for a given height, so this must be bumped when the binary // snapshot output changes. -const CurrentFormat uint32 = 2 +const CurrentFormat uint32 = 3 diff --git a/store/rootmulti/snapshot_test.go b/store/rootmulti/snapshot_test.go index bad1603da7c9..de7880788eee 100644 --- a/store/rootmulti/snapshot_test.go +++ b/store/rootmulti/snapshot_test.go @@ -128,7 +128,7 @@ func TestMultistoreSnapshot_Checksum(t *testing.T) { "aa048b4ee0f484965d7b3b06822cf0772cdcaad02f3b1b9055e69f2cb365ef3c", "7921eaa3ed4921341e504d9308a9877986a879fe216a099c86e8db66fcba4c63", "a4a864e6c02c9fca5837ec80dc84f650b25276ed7e4820cf7516ced9f9901b86", - "ca2879ac6e7205d257440131ba7e72bef784cd61642e32b847729e543c1928b9", + "980925390cc50f14998ecb1e87de719ca9dd7e72f5fefbe445397bf670f36c31", }}, } for _, tc := range testcases { From 53519ea5b3f4462c83fd5267911244f7b27db572 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 27 Sep 2022 20:37:26 +0200 Subject: [PATCH 3/3] refactor: simplify hooks implementation (#13396) --- Makefile | 1 + scripts/mockgen.sh | 3 +- store/tools/ics23/iavl/helpers/helpers.go | 3 + x/distribution/keeper/hooks.go | 9 +- x/gov/abci.go | 4 +- x/gov/keeper/deposit.go | 2 +- x/gov/keeper/hooks.go | 44 --------- x/gov/keeper/keeper.go | 10 ++ x/gov/keeper/proposal.go | 2 +- x/gov/keeper/vote.go | 2 +- x/slashing/keeper/hooks.go | 67 +++++-------- x/slashing/keeper/hooks_test.go | 6 +- x/slashing/testutil/expected_keepers_mocks.go | 98 +++++++++++++++++++ x/slashing/types/expected_keepers.go | 10 +- x/staking/keeper/delegation.go | 12 +-- x/staking/keeper/delegation_test.go | 4 +- x/staking/keeper/genesis.go | 6 +- x/staking/keeper/hooks.go | 89 ----------------- x/staking/keeper/keeper.go | 10 ++ x/staking/keeper/msg_server.go | 4 +- x/staking/keeper/slash.go | 8 +- x/staking/keeper/val_state_change.go | 10 +- x/staking/keeper/validator.go | 4 +- x/staking/keeper/validator_test.go | 13 ++- 24 files changed, 209 insertions(+), 212 deletions(-) delete mode 100644 x/gov/keeper/hooks.go delete mode 100644 x/staking/keeper/hooks.go diff --git a/Makefile b/Makefile index 249e00d3bc75..40dce3fc3fb7 100644 --- a/Makefile +++ b/Makefile @@ -146,6 +146,7 @@ cosmovisor: mocks: $(MOCKS_DIR) + @go install github.com/golang/mock/mockgen@v1.6.0 sh ./scripts/mockgen.sh .PHONY: mocks diff --git a/scripts/mockgen.sh b/scripts/mockgen.sh index bc6cfd468f7b..dc55cd1fb24e 100755 --- a/scripts/mockgen.sh +++ b/scripts/mockgen.sh @@ -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 diff --git a/store/tools/ics23/iavl/helpers/helpers.go b/store/tools/ics23/iavl/helpers/helpers.go index d552166a25b8..4f41600b65ea 100644 --- a/store/tools/ics23/iavl/helpers/helpers.go +++ b/store/tools/ics23/iavl/helpers/helpers.go @@ -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) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index bfae8482da5e..aa7a68422ea2 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -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 { @@ -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 } diff --git a/x/gov/abci.go b/x/gov/abci.go index f45cc68c3ff0..cc49b808f81f 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -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) ctx.EventManager().EmitEvent( sdk.NewEvent( @@ -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) logger.Info( "proposal tallied", diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 51de86003aaa..0a25fc9975b8 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -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( diff --git a/x/gov/keeper/hooks.go b/x/gov/keeper/hooks.go deleted file mode 100644 index a63efdcc74f4..000000000000 --- a/x/gov/keeper/hooks.go +++ /dev/null @@ -1,44 +0,0 @@ -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) - } -} diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 307152f9633a..34e8e975d37e 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -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 { diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 2eb75f4cd143..cb1807410927 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -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( diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index 4e2436ff84b5..038310a916a4 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -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( diff --git a/x/slashing/keeper/hooks.go b/x/slashing/keeper/hooks.go index a306f76c210d..01f2c2690a60 100644 --- a/x/slashing/keeper/hooks.go +++ b/x/slashing/keeper/hooks.go @@ -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), @@ -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 { diff --git a/x/slashing/keeper/hooks_test.go b/x/slashing/keeper/hooks_test.go index d07b050632f4..9fe301699953 100644 --- a/x/slashing/keeper/hooks_test.go +++ b/x/slashing/keeper/hooks_test.go @@ -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) @@ -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()) diff --git a/x/slashing/testutil/expected_keepers_mocks.go b/x/slashing/testutil/expected_keepers_mocks.go index d8d729920c69..321ca0561f44 100644 --- a/x/slashing/testutil/expected_keepers_mocks.go +++ b/x/slashing/testutil/expected_keepers_mocks.go @@ -396,6 +396,34 @@ func (m *MockStakingHooks) EXPECT() *MockStakingHooksMockRecorder { return m.recorder } +// AfterDelegationModified mocks base method. +func (m *MockStakingHooks) AfterDelegationModified(ctx types.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AfterDelegationModified", ctx, delAddr, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// AfterDelegationModified indicates an expected call of AfterDelegationModified. +func (mr *MockStakingHooksMockRecorder) AfterDelegationModified(ctx, delAddr, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AfterDelegationModified", reflect.TypeOf((*MockStakingHooks)(nil).AfterDelegationModified), ctx, delAddr, valAddr) +} + +// AfterValidatorBeginUnbonding mocks base method. +func (m *MockStakingHooks) AfterValidatorBeginUnbonding(ctx types.Context, consAddr types.ConsAddress, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AfterValidatorBeginUnbonding", ctx, consAddr, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// AfterValidatorBeginUnbonding indicates an expected call of AfterValidatorBeginUnbonding. +func (mr *MockStakingHooksMockRecorder) AfterValidatorBeginUnbonding(ctx, consAddr, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AfterValidatorBeginUnbonding", reflect.TypeOf((*MockStakingHooks)(nil).AfterValidatorBeginUnbonding), ctx, consAddr, valAddr) +} + // AfterValidatorBonded mocks base method. func (m *MockStakingHooks) AfterValidatorBonded(ctx types.Context, consAddr types.ConsAddress, valAddr types.ValAddress) error { m.ctrl.T.Helper() @@ -437,3 +465,73 @@ func (mr *MockStakingHooksMockRecorder) AfterValidatorRemoved(ctx, consAddr, val mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AfterValidatorRemoved", reflect.TypeOf((*MockStakingHooks)(nil).AfterValidatorRemoved), ctx, consAddr, valAddr) } + +// BeforeDelegationCreated mocks base method. +func (m *MockStakingHooks) BeforeDelegationCreated(ctx types.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeforeDelegationCreated", ctx, delAddr, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeforeDelegationCreated indicates an expected call of BeforeDelegationCreated. +func (mr *MockStakingHooksMockRecorder) BeforeDelegationCreated(ctx, delAddr, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeforeDelegationCreated", reflect.TypeOf((*MockStakingHooks)(nil).BeforeDelegationCreated), ctx, delAddr, valAddr) +} + +// BeforeDelegationRemoved mocks base method. +func (m *MockStakingHooks) BeforeDelegationRemoved(ctx types.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeforeDelegationRemoved", ctx, delAddr, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeforeDelegationRemoved indicates an expected call of BeforeDelegationRemoved. +func (mr *MockStakingHooksMockRecorder) BeforeDelegationRemoved(ctx, delAddr, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeforeDelegationRemoved", reflect.TypeOf((*MockStakingHooks)(nil).BeforeDelegationRemoved), ctx, delAddr, valAddr) +} + +// BeforeDelegationSharesModified mocks base method. +func (m *MockStakingHooks) BeforeDelegationSharesModified(ctx types.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeforeDelegationSharesModified", ctx, delAddr, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeforeDelegationSharesModified indicates an expected call of BeforeDelegationSharesModified. +func (mr *MockStakingHooksMockRecorder) BeforeDelegationSharesModified(ctx, delAddr, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeforeDelegationSharesModified", reflect.TypeOf((*MockStakingHooks)(nil).BeforeDelegationSharesModified), ctx, delAddr, valAddr) +} + +// BeforeValidatorModified mocks base method. +func (m *MockStakingHooks) BeforeValidatorModified(ctx types.Context, valAddr types.ValAddress) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeforeValidatorModified", ctx, valAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeforeValidatorModified indicates an expected call of BeforeValidatorModified. +func (mr *MockStakingHooksMockRecorder) BeforeValidatorModified(ctx, valAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeforeValidatorModified", reflect.TypeOf((*MockStakingHooks)(nil).BeforeValidatorModified), ctx, valAddr) +} + +// BeforeValidatorSlashed mocks base method. +func (m *MockStakingHooks) BeforeValidatorSlashed(ctx types.Context, valAddr types.ValAddress, fraction types.Dec) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeforeValidatorSlashed", ctx, valAddr, fraction) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeforeValidatorSlashed indicates an expected call of BeforeValidatorSlashed. +func (mr *MockStakingHooksMockRecorder) BeforeValidatorSlashed(ctx, valAddr, fraction interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeforeValidatorSlashed", reflect.TypeOf((*MockStakingHooks)(nil).BeforeValidatorSlashed), ctx, valAddr, fraction) +} diff --git a/x/slashing/types/expected_keepers.go b/x/slashing/types/expected_keepers.go index c4cdfec1dc53..0b1a8afe8c9c 100644 --- a/x/slashing/types/expected_keepers.go +++ b/x/slashing/types/expected_keepers.go @@ -59,7 +59,15 @@ type StakingKeeper interface { // StakingHooks event hooks for staking validator object (noalias) type StakingHooks interface { AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error // Must be called when a validator is created + BeforeValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) error // Must be called when a validator's state changes AfterValidatorRemoved(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error // Must be called when a validator is deleted - AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error // Must be called when a validator is bonded + AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error // Must be called when a validator is bonded + AfterValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error // Must be called when a validator begins unbonding + + BeforeDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error // Must be called when a delegation is created + BeforeDelegationSharesModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error // Must be called when a delegation's shares are modified + BeforeDelegationRemoved(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error // Must be called when a delegation is removed + AfterDelegationModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error + BeforeValidatorSlashed(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) error } diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index c63286e2769e..94ab596d337d 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -103,7 +103,7 @@ func (k Keeper) RemoveDelegation(ctx sdk.Context, delegation types.Delegation) e delegatorAddress := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress) // TODO: Consider calling hooks outside of the store wrapper functions, it's unobvious. - if err := k.BeforeDelegationRemoved(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { + if err := k.Hooks().BeforeDelegationRemoved(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { return err } @@ -630,9 +630,9 @@ func (k Keeper) Delegate( // call the appropriate hook if present if found { - err = k.BeforeDelegationSharesModified(ctx, delAddr, validator.GetOperator()) + err = k.Hooks().BeforeDelegationSharesModified(ctx, delAddr, validator.GetOperator()) } else { - err = k.BeforeDelegationCreated(ctx, delAddr, validator.GetOperator()) + err = k.Hooks().BeforeDelegationCreated(ctx, delAddr, validator.GetOperator()) } if err != nil { @@ -689,7 +689,7 @@ func (k Keeper) Delegate( k.SetDelegation(ctx, delegation) // Call the after-modification hook - if err := k.AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { + if err := k.Hooks().AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { return newShares, err } @@ -707,7 +707,7 @@ func (k Keeper) Unbond( } // call the before-delegation-modified hook - if err := k.BeforeDelegationSharesModified(ctx, delAddr, valAddr); err != nil { + if err := k.Hooks().BeforeDelegationSharesModified(ctx, delAddr, valAddr); err != nil { return amount, err } @@ -745,7 +745,7 @@ func (k Keeper) Unbond( } else { k.SetDelegation(ctx, delegation) // call the after delegation modification hook - err = k.AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()) + err = k.Hooks().AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()) } if err != nil { diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 0102898bb215..669818b3b34f 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -610,7 +610,7 @@ func (s *KeeperTestSuite) TestRedelegationMaxEntries() { require.Equal(valTokens, issuedShares.RoundInt()) s.bankKeeper.EXPECT().SendCoinsFromModuleToModule(gomock.Any(), stakingtypes.NotBondedPoolName, stakingtypes.BondedPoolName, gomock.Any()) - validator = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator, true) + _ = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator, true) val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) selfDelegation := stakingtypes.NewDelegation(val0AccAddr, addrVals[0], issuedShares) keeper.SetDelegation(ctx, selfDelegation) @@ -732,7 +732,7 @@ func (s *KeeperTestSuite) TestRedelegateFromUnbondingValidator() { validator2, issuedShares = validator2.AddTokensFromDel(valTokens) require.Equal(valTokens, issuedShares.RoundInt()) s.bankKeeper.EXPECT().SendCoinsFromModuleToModule(gomock.Any(), stakingtypes.NotBondedPoolName, stakingtypes.BondedPoolName, gomock.Any()) - validator2 = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator2, true) + _ = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator2, true) header := ctx.BlockHeader() blockHeight := int64(10) diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index b990b600c5d8..1aeea695e2e6 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -41,7 +41,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) (res []ab // Call the creation hook if not exported if !data.Exported { - if err := k.AfterValidatorCreated(ctx, validator.GetOperator()); err != nil { + if err := k.Hooks().AfterValidatorCreated(ctx, validator.GetOperator()); err != nil { panic(err) } } @@ -68,7 +68,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) (res []ab // Call the before-creation hook if not exported if !data.Exported { - if err := k.BeforeDelegationCreated(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { + if err := k.Hooks().BeforeDelegationCreated(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { panic(err) } } @@ -77,7 +77,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) (res []ab // Call the after-modification hook if not exported if !data.Exported { - if err := k.AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { + if err := k.Hooks().AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil { panic(err) } } diff --git a/x/staking/keeper/hooks.go b/x/staking/keeper/hooks.go deleted file mode 100644 index 91375c9e3881..000000000000 --- a/x/staking/keeper/hooks.go +++ /dev/null @@ -1,89 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/types" -) - -// Implements StakingHooks interface -var _ types.StakingHooks = Keeper{} - -// AfterValidatorCreated - call hook if registered -func (k Keeper) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.AfterValidatorCreated(ctx, valAddr) - } - return nil -} - -// BeforeValidatorModified - call hook if registered -func (k Keeper) BeforeValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.BeforeValidatorModified(ctx, valAddr) - } - return nil -} - -// AfterValidatorRemoved - call hook if registered -func (k Keeper) AfterValidatorRemoved(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.AfterValidatorRemoved(ctx, consAddr, valAddr) - } - return nil -} - -// AfterValidatorBonded - call hook if registered -func (k Keeper) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.AfterValidatorBonded(ctx, consAddr, valAddr) - } - return nil -} - -// AfterValidatorBeginUnbonding - call hook if registered -func (k Keeper) AfterValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.AfterValidatorBeginUnbonding(ctx, consAddr, valAddr) - } - return nil -} - -// BeforeDelegationCreated - call hook if registered -func (k Keeper) BeforeDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.BeforeDelegationCreated(ctx, delAddr, valAddr) - } - return nil -} - -// BeforeDelegationSharesModified - call hook if registered -func (k Keeper) BeforeDelegationSharesModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.BeforeDelegationSharesModified(ctx, delAddr, valAddr) - } - return nil -} - -// BeforeDelegationRemoved - call hook if registered -func (k Keeper) BeforeDelegationRemoved(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - k.hooks.BeforeDelegationRemoved(ctx, delAddr, valAddr) - } - return nil -} - -// AfterDelegationModified - call hook if registered -func (k Keeper) AfterDelegationModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error { - if k.hooks != nil { - return k.hooks.AfterDelegationModified(ctx, delAddr, valAddr) - } - return nil -} - -// BeforeValidatorSlashed - call hook if registered -func (k Keeper) BeforeValidatorSlashed(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) error { - if k.hooks != nil { - return k.hooks.BeforeValidatorSlashed(ctx, valAddr, fraction) - } - return nil -} diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 975ee64f9a1b..675b0d41b067 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -66,6 +66,16 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+types.ModuleName) } +// Hooks gets the hooks for staking *Keeper { +func (keeper *Keeper) Hooks() types.StakingHooks { + if keeper.hooks == nil { + // return a no-op implementation if no hooks are set + return types.MultiStakingHooks{} + } + + return keeper.hooks +} + // SetHooks Set the validator hooks func (k *Keeper) SetHooks(sh types.StakingHooks) { if k.hooks != nil { diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 37ee9c3e89d7..681f158a746b 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -114,7 +114,7 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa k.SetNewValidatorByPowerIndex(ctx, validator) // call the after-creation hook - if err := k.AfterValidatorCreated(ctx, validator.GetOperator()); err != nil { + if err := k.Hooks().AfterValidatorCreated(ctx, validator.GetOperator()); err != nil { return nil, err } @@ -170,7 +170,7 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida } // call the before-modification hook since we're about to update the commission - if err := k.BeforeValidatorModified(ctx, valAddr); err != nil { + if err := k.Hooks().BeforeValidatorModified(ctx, valAddr); err != nil { return nil, err } diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index c0a147510963..7b239e588f11 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -65,7 +65,9 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh operatorAddress := validator.GetOperator() // call the before-modification hook - k.BeforeValidatorModified(ctx, operatorAddress) + if err := k.Hooks().BeforeValidatorModified(ctx, operatorAddress); err != nil { + k.Logger(ctx).Error("failed to call before validator modified hook", "error", err) + } // Track remaining slash amount for the validator // This will decrease when we slash unbondings and @@ -123,7 +125,9 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh effectiveFraction = math.LegacyOneDec() } // call the before-slashed hook - k.BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction) + if err := k.Hooks().BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction); err != nil { + k.Logger(ctx).Error("failed to call before validator slashed hook", "error", err) + } } // Deduct from validator's bonded tokens and update the validator. diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index ea8a24ebb405..5d63c458dca5 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -298,7 +298,10 @@ func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) (types if err != nil { return validator, err } - k.AfterValidatorBonded(ctx, consAddr, validator.GetOperator()) + + if err := k.Hooks().AfterValidatorBonded(ctx, consAddr, validator.GetOperator()); err != nil { + return validator, err + } return validator, err } @@ -333,7 +336,10 @@ func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validat if err != nil { return validator, err } - k.AfterValidatorBeginUnbonding(ctx, consAddr, validator.GetOperator()) + + if err := k.Hooks().AfterValidatorBeginUnbonding(ctx, consAddr, validator.GetOperator()); err != nil { + return validator, err + } return validator, nil } diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index afd6fc1d72b4..2589de1af881 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -183,7 +183,9 @@ func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) { store.Delete(types.GetValidatorsByPowerIndexKey(validator, k.PowerReduction(ctx))) // call hooks - k.AfterValidatorRemoved(ctx, valConsAddr, validator.GetOperator()) + if err := k.Hooks().AfterValidatorRemoved(ctx, valConsAddr, validator.GetOperator()); err != nil { + k.Logger(ctx).Error("error in after validator removed hook", "error", err) + } } // get groups of validators diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index bf9c6fa2fcf1..b639360f450b 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -10,7 +10,6 @@ import ( stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/teststaking" - "github.com/cosmos/cosmos-sdk/x/staking/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/tendermint/tendermint/abci/types" ) @@ -88,11 +87,11 @@ func (s *KeeperTestSuite) TestValidatorBasics() { require := s.Require() // construct the validators - var validators [3]types.Validator + var validators [3]stakingtypes.Validator powers := []int64{9, 8, 7} for i, power := range powers { validators[i] = teststaking.NewValidator(s.T(), sdk.ValAddress(PKs[i].Address().Bytes()), PKs[i]) - validators[i].Status = types.Unbonded + validators[i].Status = stakingtypes.Unbonded validators[i].Tokens = math.ZeroInt() tokens := keeper.TokensFromConsensusPower(ctx, power) @@ -131,11 +130,11 @@ func (s *KeeperTestSuite) TestValidatorBasics() { resVals = keeper.GetLastValidators(ctx) require.Equal(1, len(resVals)) require.True(validators[0].MinEqual(&resVals[0])) - require.Equal(types.Bonded, validators[0].Status) + require.Equal(stakingtypes.Bonded, validators[0].Status) require.True(keeper.TokensFromConsensusPower(ctx, 9).Equal(validators[0].BondedTokens())) // modify a records, save, and retrieve - validators[0].Status = types.Bonded + validators[0].Status = stakingtypes.Bonded validators[0].Tokens = keeper.TokensFromConsensusPower(ctx, 10) validators[0].DelegatorShares = sdk.NewDecFromInt(validators[0].Tokens) validators[0] = stakingkeeper.TestingUpdateValidator(keeper, ctx, validators[0], true) @@ -169,7 +168,7 @@ func (s *KeeperTestSuite) TestValidatorBasics() { func() { keeper.RemoveValidator(ctx, validators[1].GetOperator()) }) // shouldn't be able to remove if there are still tokens left - validators[1].Status = types.Unbonded + validators[1].Status = stakingtypes.Unbonded keeper.SetValidator(ctx, validators[1]) require.PanicsWithValue("attempting to remove a validator which still contains tokens", func() { keeper.RemoveValidator(ctx, validators[1].GetOperator()) }) @@ -229,7 +228,7 @@ func (s *KeeperTestSuite) TestApplyAndReturnValidatorSetUpdatesPowerDecrease() { require := s.Require() powers := []int64{100, 100} - var validators [2]types.Validator + var validators [2]stakingtypes.Validator for i, power := range powers { validators[i] = teststaking.NewValidator(s.T(), sdk.ValAddress(PKs[i].Address().Bytes()), PKs[i])