chore: Reduce complexity (#70)

* test: appModule; increaseBlock blockers

* new: store.go

* rm: updateLastPower (duplicate)

* test: simplify removeVal

* simplify updatePOAPower

* title format rename

* stale todo

.github/workflows/lint-pr.yml → .github/workflows/title-format.yml

@@ -1,4 +1,4 @@
-name: Lint PR
+name: PR Title Lint
 on:
   pull_request_target:
     types:

keeper/keeper.go

@@ -50,31 +50,6 @@ func (k Keeper) GetSlashingKeeper() slashingkeeper.Keeper {
 	return k.slashKeeper
 }
 
-// SetParams sets the module parameters.
-func (k Keeper) SetParams(ctx context.Context, p poa.Params) error {
-	if err := p.Validate(); err != nil {
-		return err
-	}
-
-	store := k.storeService.OpenKVStore(ctx)
-	bz := k.cdc.MustMarshal(&p)
-	return store.Set(poa.ParamsKey, bz)
-}
-
-// GetParams returns the current module parameters.
-func (k Keeper) GetParams(ctx context.Context) (poa.Params, error) {
-	store := k.storeService.OpenKVStore(ctx)
-
-	bz, err := store.Get(poa.ParamsKey)
-	if err != nil || bz == nil {
-		return poa.DefaultParams(), err
-	}
-
-	var p poa.Params
-	k.cdc.MustUnmarshal(bz, &p)
-	return p, nil
-}
-
 // GetAdmins returns the module's administrators with delegation of power control.
 func (k Keeper) GetAdmins(ctx context.Context) []string {
 	p, err := k.GetParams(ctx)

keeper/keeper_test.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/strangelove-ventures/poa"
 	"github.com/strangelove-ventures/poa/keeper"
+	poamodule "github.com/strangelove-ventures/poa/module"
 
 	"cosmossdk.io/log"
 	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
@@ -54,6 +55,7 @@ type testFixture struct {
 	k           keeper.Keeper
 	msgServer   poa.MsgServer
 	queryServer poa.QueryServer
+	appModule   poamodule.AppModule
 
 	accountkeeper  authkeeper.AccountKeeper
 	stakingKeeper  *stakingkeeper.Keeper
@@ -94,6 +96,7 @@ func SetupTest(t *testing.T) *testFixture {
 	s.k = keeper.NewKeeper(encCfg.Codec, storeService, s.stakingKeeper, s.slashingKeeper, addresscodec.NewBech32Codec("cosmosvaloper"))
 	s.msgServer = keeper.NewMsgServerImpl(s.k)
 	s.queryServer = keeper.NewQueryServerImpl(s.k)
+	s.appModule = poamodule.NewAppModule(encCfg.Codec, s.k)
 
 	// register interfaces
 	authtypes.RegisterInterfaces(encCfg.InterfaceRegistry)
@@ -177,16 +180,8 @@ func (f *testFixture) createBaseStakingValidators(t *testing.T) {
 		}
 	}
 
-	// update validator set power
-	allVals, err := f.stakingKeeper.GetValidators(f.ctx, 100)
-	require.NoError(t, err)
-
-	totalPower := math.ZeroInt()
-	for _, val := range allVals {
-		totalPower = totalPower.Add(val.Tokens)
-	}
-
-	if err := f.stakingKeeper.SetLastTotalPower(f.ctx, totalPower); err != nil {
+	total := bondCoin.Amount.MulRaw(int64(len(vals)))
+	if err := f.stakingKeeper.SetLastTotalPower(f.ctx, total); err != nil {
 		panic(err)
 	}
 }
@@ -241,8 +236,21 @@ func (f *testFixture) CreatePendingValidator(name string, power uint64) sdk.ValA
 	return valAddr
 }
 
-func (f *testFixture) IncreaseBlock(amt int64) ([]abci.ValidatorUpdate, error) {
+func (f *testFixture) IncreaseBlock(amt int64, debug ...bool) ([]abci.ValidatorUpdate, error) {
 	f.ctx = f.ctx.WithBlockHeight(f.ctx.BlockHeight() + amt)
-	updates, err := f.stakingKeeper.ApplyAndReturnValidatorSetUpdates(f.ctx)
+
+	if err := f.k.GetStakingKeeper().BeginBlocker(f.ctx); err != nil {
+		return nil, err
+	}
+
+	updates, err := f.k.GetStakingKeeper().EndBlocker(f.ctx)
+	if len(debug) > 0 && debug[0] {
+		fmt.Printf("\nIncreaseBlock(...) updates: %+v\n", updates)
+	}
+
+	if err := f.appModule.BeginBlock(f.ctx); err != nil {
+		return nil, err
+	}
+
 	return updates, err
 }

keeper/msg_server.go

@@ -45,16 +45,12 @@ func (ms msgServer) SetPower(ctx context.Context, msg *poa.MsgSetPower) (*poa.Ms
 	}
 
 	if !msg.Unsafe {
-		totalPOAPower := math.ZeroInt()
-		allDelegations, err := ms.k.stakingKeeper.GetAllDelegations(ctx)
+		// Gets the cached last total power of the validator set
+		totalPOAPower, err := ms.k.stakingKeeper.GetLastTotalPower(ctx)
 		if err != nil {
 			return nil, err
 		}
 
-		for _, del := range allDelegations {
-			totalPOAPower = totalPOAPower.Add(del.Shares.TruncateInt())
-		}
-
 		if msg.Power > totalPOAPower.Mul(math.NewInt(30)).Quo(math.NewInt(100)).Uint64() {
 			return nil, poa.ErrUnsafePower
 		}
@@ -99,10 +95,6 @@ func (ms msgServer) RemoveValidator(ctx context.Context, msg *poa.MsgRemoveValid
 		return nil, err
 	}
 
-	if _, err := ms.updatePOAPower(ctx, msg.ValidatorAddress, 0); err != nil {
-		return nil, err
-	}
-
 	return &poa.MsgRemoveValidatorResponse{}, nil
 }
 
@@ -301,68 +293,79 @@ func (ms msgServer) updatePOAPower(ctx context.Context, valOpBech32 string, powe
 		return stakingtypes.Validator{}, err
 	}
 
-	// remove all delegations (for safety)
+	if err := ms.k.stakingKeeper.SetLastValidatorPower(ctx, valAddr, power); err != nil {
+		return stakingtypes.Validator{}, err
+	}
+
+	if err := ms.updateValidatorSet(ctx, power, val, valAddr); err != nil {
+		return stakingtypes.Validator{}, err
+	}
+
+	return val, nil
+}
+
+func (ms msgServer) updateValidatorSet(ctx context.Context, power int64, val stakingtypes.Validator, valAddr sdk.ValAddress) error {
+	sdkContext := sdk.UnwrapSDKContext(ctx)
+
 	delegations, err := ms.k.stakingKeeper.GetValidatorDelegations(ctx, valAddr)
 	if err != nil {
-		return stakingtypes.Validator{}, err
+		return err
 	}
 
 	for _, del := range delegations {
 		if err := ms.k.stakingKeeper.RemoveDelegation(ctx, del); err != nil {
-			return stakingtypes.Validator{}, err
+			return err
 		}
 	}
 
-	// set a single updated delegation of power
 	delegation := stakingtypes.Delegation{
 		DelegatorAddress: sdk.AccAddress(valAddr.Bytes()).String(),
 		ValidatorAddress: val.OperatorAddress,
 		Shares:           math.LegacyNewDec(power),
 	}
+	if err := ms.k.stakingKeeper.SetDelegation(ctx, delegation); err != nil {
+		return err
+	}
+
+	if power == 0 && sdkContext.BlockHeight() > 1 {
+		currPower, err := ms.k.stakingKeeper.GetLastValidatorPower(ctx, valAddr)
+		if err != nil {
+			return err
+		}
+
+		val.MinSelfDelegation = math.NewIntFromUint64(uint64(currPower) + 1)
+	}
 
 	val.Tokens = math.NewIntFromUint64(uint64(power))
 	val.DelegatorShares = delegation.Shares
 	val.Status = stakingtypes.Bonded
 	if err := ms.k.stakingKeeper.SetValidator(ctx, val); err != nil {
-		return stakingtypes.Validator{}, err
-	}
-
-	if err := ms.k.stakingKeeper.SetDelegation(ctx, delegation); err != nil {
-		return stakingtypes.Validator{}, err
+		return err
 	}
 
 	if err := ms.k.stakingKeeper.SetLastValidatorPower(ctx, valAddr, power); err != nil {
-		return stakingtypes.Validator{}, err
-	}
-
-	// Update the total power cache
-	totalSetPower, err := ms.getValidatorSetTotalPower(ctx)
-	if err != nil {
-		return stakingtypes.Validator{}, err
-	}
-
-	if err := ms.k.stakingKeeper.SetLastTotalPower(ctx, totalSetPower); err != nil {
-		return stakingtypes.Validator{}, err
+		return err
 	}
 
-	return val, nil
+	return ms.updateTotalPower(ctx)
 }
 
-func (ms msgServer) getValidatorSetTotalPower(ctx context.Context) (math.Int, error) {
-	vals, err := ms.k.stakingKeeper.GetAllValidators(ctx)
+func (ms msgServer) updateTotalPower(ctx context.Context) error {
+	allVals, err := ms.k.stakingKeeper.GetAllValidators(ctx)
 	if err != nil {
-		return math.ZeroInt(), err
+		return err
 	}
 
-	totalPower := math.ZeroInt()
-	for _, val := range vals {
-		if val.Status != stakingtypes.Bonded {
-			continue
-		}
-		totalPower = totalPower.Add(val.Tokens)
+	allTokens := math.ZeroInt()
+	for _, val := range allVals {
+		allTokens = allTokens.Add(val.Tokens)
+	}
+
+	if err := ms.k.stakingKeeper.SetLastTotalPower(ctx, allTokens); err != nil {
+		return err
 	}
 
-	return totalPower, nil
+	return nil
 }
 
 func (ms msgServer) isAdmin(ctx context.Context, fromAddr string) bool {

keeper/msg_server_test.go

@@ -202,11 +202,8 @@ func TestRemoveValidator(t *testing.T) {
 	vals, err := f.stakingKeeper.GetValidators(f.ctx, 100)
 	require.NoError(err)
 
-	for idx, v := range vals {
-		power := 10_000_000
-		if idx == 0 {
-			power = 1_000_000
-		}
+	for _, v := range vals {
+		power := 1_000_000
 
 		_, err = f.msgServer.SetPower(f.ctx, &poa.MsgSetPower{
 			Sender:           f.addrs[0].String(),
@@ -217,16 +214,16 @@ func TestRemoveValidator(t *testing.T) {
 		require.NoError(err)
 	}
 
-	f.ctx = f.ctx.WithBlockHeight(f.ctx.BlockHeight() + 5)
-	updates, err := f.stakingKeeper.ApplyAndReturnValidatorSetUpdates(f.ctx)
+	updates, err := f.IncreaseBlock(1, true)
 	require.NoError(err)
 	fmt.Printf("%+v", updates)
 	require.EqualValues(3, len(updates))
 
 	testCases := []struct {
-		name         string
-		request      *poa.MsgRemoveValidator
-		expectErrMsg string
+		name             string
+		request          *poa.MsgRemoveValidator
+		expectErrMsg     string
+		expectValUpdates int
 	}{
 		{
 			name: "set invalid authority (not an address)",
@@ -237,10 +234,9 @@ func TestRemoveValidator(t *testing.T) {
 			expectErrMsg: "not an authority",
 		},
 		{
-			name: "removal",
+			name: "remove one validator",
 			request: &poa.MsgRemoveValidator{
-				Sender: f.addrs[0].String(),
-				// The validator with 1/10th the power
+				Sender:           f.addrs[0].String(),
 				ValidatorAddress: vals[0].OperatorAddress,
 			},
 			expectErrMsg: "",
@@ -251,6 +247,7 @@ func TestRemoveValidator(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			_, err := f.msgServer.RemoveValidator(f.ctx, tc.request)
+
 			if tc.expectErrMsg != "" {
 				require.Error(err)
 				require.ErrorContains(err, tc.expectErrMsg)
@@ -263,15 +260,13 @@ func TestRemoveValidator(t *testing.T) {
 					panic(err)
 				}
 
-				u, err := f.IncreaseBlock(1)
+				_, err := f.IncreaseBlock(5, true)
 				require.NoError(err)
-				require.EqualValues(3, len(u))
-				fmt.Println(u, err)
 
-				u, err = f.IncreaseBlock(1)
+				// no updates on the next increase
+				u, err := f.IncreaseBlock(1, true)
 				require.NoError(err)
-				fmt.Println(u)
-				require.EqualValues(2, len(u))
+				require.EqualValues(0, len(u))
 			}
 		})
 

keeper/store.go

@@ -0,0 +1,32 @@
+package keeper
+
+import (
+	"context"
+
+	"github.com/strangelove-ventures/poa"
+)
+
+// SetParams sets the module parameters.
+func (k Keeper) SetParams(ctx context.Context, p poa.Params) error {
+	if err := p.Validate(); err != nil {
+		return err
+	}
+
+	store := k.storeService.OpenKVStore(ctx)
+	bz := k.cdc.MustMarshal(&p)
+	return store.Set(poa.ParamsKey, bz)
+}
+
+// GetParams returns the current module parameters.
+func (k Keeper) GetParams(ctx context.Context) (poa.Params, error) {
+	store := k.storeService.OpenKVStore(ctx)
+
+	bz, err := store.Get(poa.ParamsKey)
+	if err != nil || bz == nil {
+		return poa.DefaultParams(), err
+	}
+
+	var p poa.Params
+	k.cdc.MustUnmarshal(bz, &p)
+	return p, nil
+}