fix!: balance bonding/unbonding module pools (#122)

* wip

* updateBondedPoolPower

* wip `updateBondedPoolPower` (96% of 100%)

* wip: fix accounting with mint/burning absed on balance

* EndBlocker to burn NotBonded tokens

* why

* on remove: slash PoA validator 100%

* remove endblock

* Slash on new shares of 0

* lint

* update integration md

* expected: SlashingKeeper & BankKeeper

* use expected keeper within `NewKeeper`

* comment

* reduce to TokensToConsensusPower

* comment on bft consensus power

INTEGRATION.md

@@ -56,6 +56,7 @@ app.POAKeeper = poakeeper.NewKeeper(
     runtime.NewKVStoreService(keys[poatypes.StoreKey]),
     app.StakingKeeper,
     app.SlashingKeeper,
+    app.BankKeeper,
     authcodec.NewBech32Codec(sdk.Bech32PrefixValAddr),
     logger,
 )
@@ -75,23 +76,20 @@ app.ModuleManager = module.NewManager(
 app.ModuleManager.SetOrderBeginBlockers(
     ...
     poa.ModuleName,
-    stakingtypes.ModuleName,
     ...
 )
 
 // Add PoA to end blocker logic
 app.ModuleManager.SetOrderEndBlockers(
     ...
     poa.ModuleName,
-    stakingtypes.ModuleName,
     ...
 )
 
 // Add PoA to init genesis logic
 // NOTE: This must be after the staking module init genesis
 app.ModuleManager.SetOrderInitGenesis(
     ...
-    stakingtypes.ModuleName,
     poa.ModuleName,
     ...
 )
@@ -164,7 +162,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 
 ### Slashing - Genesis Params
 
-When setting up your network genesis, it is important to consider setting slash infractions to 0%.
+When setting up your network genesis, it is important to consider setting slash infractions to 0%. Setting downtime is more reasonable to diminish their weight in the network.
 
 ```json
 app_state.slashing.params.slash_fraction_double_sign

keeper/expected_keepers.go

@@ -0,0 +1,54 @@
+package keeper
+
+import (
+	"context"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
+)
+
+type BankKeeper interface {
+	GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin
+	MintCoins(ctx context.Context, moduleName string, amt sdk.Coins) error
+	SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
+	SendCoinsFromModuleToModule(ctx context.Context, senderModule, recipientModule string, amt sdk.Coins) error
+}
+
+type SlashingKeeper interface {
+	DeleteMissedBlockBitmap(ctx context.Context, addr sdk.ConsAddress) error
+	SetValidatorSigningInfo(ctx context.Context, address sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) error
+}
+
+// We use almost all the methods from StakingKeeper. Just using it directly
+/*
+type StakingKeeper interface {
+	Hooks() stakingtypes.StakingHooks
+
+	SetParams(ctx context.Context, params stakingtypes.Params) error
+	SetLastValidatorPower(ctx context.Context, operator sdk.ValAddress, power int64) error
+	SetDelegation(ctx context.Context, delegation stakingtypes.Delegation) error
+	SetValidator(ctx context.Context, validator stakingtypes.Validator) error
+	SetValidatorByConsAddr(ctx context.Context, validator stakingtypes.Validator) error
+	SetNewValidatorByPowerIndex(ctx context.Context, validator stakingtypes.Validator) error
+	SetLastTotalPower(ctx context.Context, power math.Int) error
+	DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAddress) error
+	GetLastTotalPower(ctx context.Context) (math.Int, error)
+
+	Slash(ctx context.Context, consAddr sdk.ConsAddress, infractionHeight, power int64, slashFactor math.LegacyDec) (math.Int, error)
+
+	MinCommissionRate(ctx context.Context) (math.LegacyDec, error)
+	BondDenom(ctx context.Context) (string, error)
+
+	GetValidator(ctx context.Context, addr sdk.ValAddress) (validator stakingtypes.Validator, err error)
+	GetAllValidators(ctx context.Context) (validators []stakingtypes.Validator, err error)
+	GetValidatorByConsAddr(ctx context.Context, consAddr sdk.ConsAddress) (validator stakingtypes.Validator, err error)
+	GetLastValidatorPower(ctx context.Context, operator sdk.ValAddress) (power int64, err error)
+
+	GetAllDelegations(ctx context.Context) (delegations []stakingtypes.Delegation, err error)
+
+	TokensFromConsensusPower(ctx context.Context, power int64) math.Int
+	TokensToConsensusPower(ctx context.Context, tokens math.Int) int64
+
+	PowerReduction(ctx context.Context) math.Int
+}
+*/

keeper/keeper.go

@@ -6,13 +6,16 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
 	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	"cosmossdk.io/collections"
 	addresscodec "cosmossdk.io/core/address"
 	storetypes "cosmossdk.io/core/store"
 	"cosmossdk.io/log"
+	sdkmath "cosmossdk.io/math"
 
 	"github.com/strangelove-ventures/poa"
 )
@@ -22,7 +25,8 @@ type Keeper struct {
 	validatorAddressCodec addresscodec.Codec
 
 	stakingKeeper *stakingkeeper.Keeper
-	slashKeeper   slashingkeeper.Keeper
+	slashKeeper   SlashingKeeper
+	bankKeeper    BankKeeper
 
 	logger log.Logger
 
@@ -40,7 +44,8 @@ func NewKeeper(
 	cdc codec.BinaryCodec,
 	storeService storetypes.KVStoreService,
 	sk *stakingkeeper.Keeper,
-	slk slashingkeeper.Keeper,
+	slk SlashingKeeper,
+	bk BankKeeper,
 	validatorAddressCodec addresscodec.Codec,
 	logger log.Logger,
 ) Keeper {
@@ -53,6 +58,7 @@ func NewKeeper(
 		stakingKeeper:         sk,
 		validatorAddressCodec: validatorAddressCodec,
 		slashKeeper:           slk,
+		bankKeeper:            bk,
 		logger:                logger,
 
 		// Stores
@@ -79,10 +85,14 @@ func (k Keeper) GetStakingKeeper() *stakingkeeper.Keeper {
 }
 
 // GetSlashingKeeper returns the slashing keeper.
-func (k Keeper) GetSlashingKeeper() slashingkeeper.Keeper {
+func (k Keeper) GetSlashingKeeper() SlashingKeeper {
 	return k.slashKeeper
 }
 
+func (k Keeper) GetBankKeeper() BankKeeper {
+	return k.bankKeeper
+}
+
 // GetAdmins returns the module's administrators with delegation of power control.
 func (k Keeper) GetAdmins(ctx context.Context) []string {
 	p, err := k.GetParams(ctx)
@@ -122,3 +132,47 @@ func (k Keeper) IsSenderValidator(ctx context.Context, sender string, expectedVa
 func (k Keeper) Logger() log.Logger {
 	return k.logger
 }
+
+// updateBondedPoolPower updates the bonded pool to the correct power for the network.
+func (k Keeper) UpdateBondedPoolPower(ctx context.Context) error {
+	newTotal := sdkmath.ZeroInt()
+
+	del, err := k.stakingKeeper.GetAllDelegations(ctx)
+	if err != nil {
+		return err
+	}
+
+	for _, d := range del {
+		newTotal = newTotal.Add(d.Shares.RoundInt())
+	}
+
+	bondDenom, err := k.stakingKeeper.BondDenom(ctx)
+	if err != nil {
+		return err
+	}
+
+	prevBal := k.bankKeeper.GetBalance(ctx, authtypes.NewModuleAddress(stakingtypes.BondedPoolName), bondDenom).Amount
+
+	if newTotal.Equal(prevBal) {
+		return nil
+	}
+
+	if newTotal.GT(prevBal) {
+		diff := newTotal.Sub(prevBal)
+		coins := sdk.NewCoins(sdk.NewCoin(bondDenom, diff))
+
+		if err := k.bankKeeper.MintCoins(ctx, minttypes.ModuleName, coins); err != nil {
+			return err
+		}
+
+		if err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, stakingtypes.BondedPoolName, coins); err != nil {
+			return err
+		}
+	}
+
+	// no need to check if it goes down. When it does, it's automatic from the staking module as tokens are moved from
+	// bonded -> ToNotBonded pool. As PoA, we do not want any tokens in the ToNotBonded pool, so when a validator is removed
+	// they are slashed 100% (since it is PoA this is fine) which decreases the BondedPool balance, and leave NotBonded at 0.
+
+	return nil
+}

keeper/keeper_test.go

@@ -91,7 +91,7 @@ func SetupTest(t *testing.T, baseValShares int64) *testFixture {
 	registerBaseSDKModules(f, encCfg, storeService, logger, require)
 
 	// Setup POA Keeper.
-	f.k = keeper.NewKeeper(encCfg.Codec, storeService, f.stakingKeeper, f.slashingKeeper, addresscodec.NewBech32Codec(sdk.Bech32PrefixValAddr), logger)
+	f.k = keeper.NewKeeper(encCfg.Codec, storeService, f.stakingKeeper, f.slashingKeeper, f.bankkeeper, addresscodec.NewBech32Codec(sdk.Bech32PrefixValAddr), logger)
 	f.msgServer = keeper.NewMsgServerImpl(f.k)
 	f.queryServer = keeper.NewQueryServerImpl(f.k)
 	f.appModule = poamodule.NewAppModule(encCfg.Codec, f.k)

keeper/msg_server.go

@@ -80,7 +80,7 @@ func (ms msgServer) SetPower(ctx context.Context, msg *poa.MsgSetPower) (*poa.Ms
 		}
 	}
 
-	return &poa.MsgSetPowerResponse{}, nil
+	return &poa.MsgSetPowerResponse{}, ms.k.UpdateBondedPoolPower(ctx)
 }
 
 func (ms msgServer) RemoveValidator(ctx context.Context, msg *poa.MsgRemoveValidator) (*poa.MsgRemoveValidatorResponse, error) {
@@ -143,7 +143,7 @@ func (ms msgServer) RemoveValidator(ctx context.Context, msg *poa.MsgRemoveValid
 		return nil, err
 	}
 
-	return &poa.MsgRemoveValidatorResponse{}, nil
+	return &poa.MsgRemoveValidatorResponse{}, ms.k.UpdateBondedPoolPower(ctx)
 }
 
 func (ms msgServer) RemovePending(ctx context.Context, msg *poa.MsgRemovePending) (*poa.MsgRemovePendingResponse, error) {
@@ -256,7 +256,7 @@ func (ms msgServer) CreateValidator(ctx context.Context, msg *poa.MsgCreateValid
 		return nil, err
 	}
 
-	return &poa.MsgCreateValidatorResponse{}, nil
+	return &poa.MsgCreateValidatorResponse{}, ms.k.UpdateBondedPoolPower(ctx)
 }
 
 // UpdateStakingParams wraps the x/staking module's UpdateStakingParams method so that only POA admins can invoke it.

keeper/msg_server_test.go

@@ -7,10 +7,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
-	"github.com/cosmos/cosmos-sdk/x/staking/types"
 
-	"cosmossdk.io/math"
+	sdkmath "cosmossdk.io/math"
 
 	"github.com/strangelove-ventures/poa"
 )
@@ -126,7 +124,7 @@ func TestSetPowerAndCreateValidator(t *testing.T) {
 	vals, err := f.stakingKeeper.GetValidators(f.ctx, 100)
 	require.NoError(err)
 
-	totalBonded := math.ZeroInt()
+	totalBonded := sdkmath.ZeroInt()
 	for _, val := range vals {
 		totalBonded = totalBonded.Add(val.GetBondedTokens())
 	}
@@ -336,7 +334,7 @@ func TestRemoveValidator(t *testing.T) {
 			isSelfRemovalAllowed: false,
 		},
 		{
-			name: "remove validator as itself",
+			name: "success; remove validator as itself",
 			request: &poa.MsgRemoveValidator{
 				Sender:           sdk.AccAddress(MustValAddressFromBech32(vals[1].OperatorAddress)).String(),
 				ValidatorAddress: vals[1].OperatorAddress,
@@ -361,7 +359,7 @@ func TestRemoveValidator(t *testing.T) {
 			currParams, _ := f.k.GetParams(f.ctx)
 			currParams.AllowValidatorSelfExit = tc.isSelfRemovalAllowed
 
-			err := f.k.SetParams(f.ctx, currParams)
+			err = f.k.SetParams(f.ctx, currParams)
 			require.NoError(err)
 
 			_, err = f.msgServer.RemoveValidator(f.ctx, tc.request)
@@ -372,15 +370,24 @@ func TestRemoveValidator(t *testing.T) {
 			} else {
 				require.NoError(err)
 
-				// This is only required in testing as we do not have a 'real' validator set
-				// signing blocks.
-				if err := f.MintTokensToBondedPool(t); err != nil {
-					panic(err)
-				}
-
-				_, err := f.IncreaseBlock(5, true)
+				_, err := f.IncreaseBlock(3, true)
 				require.NoError(err)
 			}
+
+			amt, err := f.stakingKeeper.TotalBondedTokens(f.ctx)
+			require.NoError(err)
+			require.True(amt.IsPositive())
+
+			notBondedPool := f.stakingKeeper.GetNotBondedPool(f.ctx)
+			bondDenom, err := f.stakingKeeper.BondDenom(f.ctx)
+			require.NoError(err)
+			bal := f.bankkeeper.GetBalance(f.ctx, notBondedPool.GetAddress(), bondDenom)
+			require.EqualValues(0, bal.Amount.Uint64())
+
+			// BondedRatio
+			bondRatio, err := f.stakingKeeper.BondedRatio(f.ctx)
+			require.NoError(err)
+			require.EqualValues(sdkmath.LegacyOneDec(), bondRatio)
 		})
 	}
 }
@@ -454,34 +461,3 @@ func TestMultipleUpdatesInASingleBlock(t *testing.T) {
 		})
 	}
 }
-
-// mintTokensToBondedPool mints tokens to the bonded pool so the validator set
-// in testing can be removed.
-// In the future, this same logic would be run during the migration from POA->POS.
-func (f *testFixture) MintTokensToBondedPool(t *testing.T) error {
-	t.Helper()
-	require := require.New(t)
-
-	bondDenom, err := f.stakingKeeper.BondDenom(f.ctx)
-	require.NoError(err)
-
-	validators, err := f.stakingKeeper.GetAllValidators(f.ctx)
-	require.NoError(err)
-
-	amt := int64(0)
-	for _, v := range validators {
-		amt += v.GetBondedTokens().Int64()
-	}
-
-	coins := sdk.NewCoins(sdk.NewCoin(bondDenom, math.NewInt(amt)))
-
-	if err := f.bankkeeper.MintCoins(f.ctx, minttypes.ModuleName, coins); err != nil {
-		return err
-	}
-
-	if err := f.bankkeeper.SendCoinsFromModuleToModule(f.ctx, minttypes.ModuleName, types.BondedPoolName, coins); err != nil {
-		return err
-	}
-
-	return nil
-}