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

fix!: balance bonding/unbonding module pools #122

Merged
merged 16 commits into from
Feb 22, 2024
Merged
6 changes: 2 additions & 4 deletions INTEGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ app.POAKeeper = poakeeper.NewKeeper(
runtime.NewKVStoreService(keys[poatypes.StoreKey]),
app.StakingKeeper,
app.SlashingKeeper,
app.BankKeeper,
authcodec.NewBech32Codec(sdk.Bech32PrefixValAddr),
logger,
)
Expand All @@ -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,
...
)
Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions keeper/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -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
}
*/
62 changes: 58 additions & 4 deletions keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -22,7 +25,8 @@ type Keeper struct {
validatorAddressCodec addresscodec.Codec

stakingKeeper *stakingkeeper.Keeper
slashKeeper slashingkeeper.Keeper
slashKeeper SlashingKeeper
bankKeeper BankKeeper

logger log.Logger

Expand All @@ -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 {
Expand All @@ -53,6 +58,7 @@ func NewKeeper(
stakingKeeper: sk,
validatorAddressCodec: validatorAddressCodec,
slashKeeper: slk,
bankKeeper: bk,
logger: logger,

// Stores
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Copy link
Member Author

Choose a reason for hiding this comment

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

I probably call this function more than needed. Only is called on a change to the validator set, so see this as fine given sets will be mostly static

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
}
2 changes: 1 addition & 1 deletion keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
64 changes: 20 additions & 44 deletions keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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)
})
}
}
Expand Down Expand Up @@ -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
}
Loading
Loading