diff --git a/.circleci/config.yml b/.circleci/config.yml index ad4ca5a16..69ba3c4cc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -136,6 +136,24 @@ jobs: export PATH="$GOBIN:$PATH" make test_sim_gaia_fast + test_sim_gaia_multi_seed: + <<: *defaults + parallelism: 1 + steps: + - attach_workspace: + at: /tmp/workspace + - checkout + - run: + name: dependencies + command: | + export PATH="$GOBIN:$PATH" + make get_vendor_deps + - run: + name: Test multi-seed Gaia simulation + command: | + export PATH="$GOBIN:$PATH" + make test_sim_gaia_multi_seed + test_cover: <<: *defaults parallelism: 4 @@ -240,6 +258,9 @@ workflows: - test_sim_gaia_fast: requires: - setup_dependencies + - test_sim_gaia_multi_seed: + requires: + - setup_dependencies - test_cover: requires: - setup_dependencies diff --git a/Makefile b/Makefile index acf81fc97..5225076fa 100644 --- a/Makefile +++ b/Makefile @@ -162,9 +162,9 @@ test_sim_gaia_fast: @echo "Running quick Gaia simulation. This may take several minutes..." @go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=400 -SimulationBlockSize=200 -SimulationCommit=true -v -timeout 24h -test_sim_gaia_full: - @echo "Running full multi-seed Gaia simulation. This may take awhile!" - @sh scripts/multisim.sh +test_sim_gaia_multi_seed: + @echo "Running multi-seed Gaia simulation. This may take awhile!" + @bash scripts/multisim.sh 10 SIM_NUM_BLOCKS ?= 210 SIM_BLOCK_SIZE ?= 200 @@ -241,4 +241,4 @@ localnet-stop: check_tools check_dev_tools get_tools get_dev_tools get_vendor_deps draw_deps test test_cli test_unit \ test_cover test_lint benchmark devdoc_init devdoc devdoc_save devdoc_update \ build-linux build-docker-gaiadnode localnet-start localnet-stop \ -format check-ledger test_sim_gaia_nondeterminism test_sim_modules test_sim_gaia_fast test_sim_gaia_slow update_tools update_dev_tools +format check-ledger test_sim_gaia_nondeterminism test_sim_modules test_sim_gaia_fast test_sim_gaia_multi_seed update_tools update_dev_tools diff --git a/PENDING.md b/PENDING.md index f0dfe58aa..dbae0cc2c 100644 --- a/PENDING.md +++ b/PENDING.md @@ -28,6 +28,7 @@ BREAKING CHANGES * [x/stake, x/slashing] [#1305](https://github.com/cosmos/cosmos-sdk/issues/1305) - Rename "revoked" to "jailed" * [x/stake] [#1676] Revoked and jailed validators put into the unbonding state * [x/stake] [#1877] Redelegations/unbonding-delegation from unbonding validator have reduced time + * [x/slashing] \#1789 Slashing changes for Tendermint validator set offset (NextValSet) * [x/stake] [\#2040](https://github.com/cosmos/cosmos-sdk/issues/2040) Validator operator type has now changed to `sdk.ValAddress` * [x/stake] [\#2221](https://github.com/cosmos/cosmos-sdk/issues/2221) New @@ -41,6 +42,7 @@ BREAKING CHANGES * [x/gov] [#2195] Governance uses BFT Time * [x/gov] \#2256 Removed slashing for governance non-voting validators * [simulation] \#2162 Added back correct supply invariants + * [x/slashing] \#2430 Simulate more slashes, check if validator is jailed before jailing * [x/stake] \#2393 Removed `CompleteUnbonding` and `CompleteRedelegation` Msg types, and instead added unbonding/redelegation queues to endblocker * SDK diff --git a/client/keys/add.go b/client/keys/add.go index 29ed3e456..21a372e2f 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -241,6 +241,7 @@ func AddNewKeyRequestHandler(indent bool) http.HandlerFunc { PostProcessResponse(w, cdc, keyOutput, indent) } } + // function to just a new seed to display in the UI before actually persisting it in the keybase func getSeed(algo keys.SigningAlgo) string { kb := client.MockKeyBase() @@ -341,4 +342,4 @@ func RecoverRequestHandler(indent bool) http.HandlerFunc { PostProcessResponse(w, cdc, keyOutput, indent) } -} \ No newline at end of file +} diff --git a/client/keys/errors.go b/client/keys/errors.go index 2689a918f..9c6139d7a 100644 --- a/client/keys/errors.go +++ b/client/keys/errors.go @@ -16,4 +16,4 @@ func errMissingPassword() error { func errMissingSeed() error { return fmt.Errorf("you have to specify seed for key recover") -} \ No newline at end of file +} diff --git a/client/keys/utils.go b/client/keys/utils.go index a4fa867eb..4ca8fc8f4 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -12,9 +12,9 @@ import ( "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "net/http" - "github.com/cosmos/cosmos-sdk/codec" ) // KeyDBName is the directory under root where we store the keys diff --git a/client/rpc/validators.go b/client/rpc/validators.go index f937ac4c8..fc37881cb 100644 --- a/client/rpc/validators.go +++ b/client/rpc/validators.go @@ -11,10 +11,10 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/spf13/viper" tmtypes "github.com/tendermint/tendermint/types" - "github.com/cosmos/cosmos-sdk/client/utils" ) // TODO these next two functions feel kinda hacky based on their placement diff --git a/scripts/multisim.sh b/scripts/multisim.sh index be59b0f1a..8ffa338b8 100755 --- a/scripts/multisim.sh +++ b/scripts/multisim.sh @@ -1,12 +1,14 @@ #!/bin/bash -seeds=(1 2 4 7 9 20 32 123 4728 37827 981928 87821 891823782 989182 89182391) +seeds=(1 2 4 7 9 20 32 123 124 582 1893 2989 3012 4728 37827 981928 87821 891823782 989182 89182391) +blocks=$1 -echo "Running multi-seed simulation with seeds: ${seeds[@]}" +echo "Running multi-seed simulation with seeds ${seeds[@]}" +echo "Running $blocks blocks per seed" echo "Edit scripts/multisim.sh to add new seeds. Keeping parameters in the file makes failures easy to reproduce." -echo "This script will kill all sub-simulations on SIGINT/SIGTERM/EXIT (i.e. Ctrl-C)." +echo "This script will kill all sub-simulations on SIGINT/SIGTERM (i.e. Ctrl-C)." -trap 'kill $(jobs -pr)' SIGINT SIGTERM EXIT +trap 'kill $(jobs -pr)' SIGINT SIGTERM tmpdir=$(mktemp -d) echo "Using temporary log directory: $tmpdir" @@ -16,7 +18,7 @@ sim() { echo "Running full Gaia simulation with seed $seed. This may take awhile!" file="$tmpdir/gaia-simulation-seed-$seed-date-$(date -Iseconds -u).stdout" echo "Writing stdout to $file..." - go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=1000 \ + go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=$blocks \ -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -v -timeout 24h > $file } @@ -26,7 +28,7 @@ for seed in ${seeds[@]}; do sim $seed & pids[${i}]=$! i=$(($i+1)) - sleep 0.1 # start in order, nicer logs + sleep 10 # start in order, nicer logs done echo "Simulation processes spawned, waiting for completion..." @@ -37,10 +39,13 @@ i=0 for pid in ${pids[*]}; do wait $pid last=$? - if [ $last -ne 0 ]; then - seed=${seeds[${i}]} + seed=${seeds[${i}]} + if [ $last -ne 0 ] + then echo "Simulation with seed $seed failed!" code=1 + else + echo "Simulation with seed $seed OK" fi i=$(($i+1)) done diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 18aef8bde..c8a02569c 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -19,9 +19,9 @@ func NewValidatorDistInfo(operatorAddr sdk.ValAddress, currentHeight int64) Vali return ValidatorDistInfo{ OperatorAddr: operatorAddr, FeePoolWithdrawalHeight: currentHeight, - Pool: DecCoins{}, - PoolCommission: DecCoins{}, - DelAccum: NewTotalAccum(currentHeight), + Pool: DecCoins{}, + PoolCommission: DecCoins{}, + DelAccum: NewTotalAccum(currentHeight), } } diff --git a/x/mock/simulation/constants.go b/x/mock/simulation/constants.go index 544da50e3..a96d4541f 100644 --- a/x/mock/simulation/constants.go +++ b/x/mock/simulation/constants.go @@ -14,7 +14,7 @@ const ( numKeys int = 250 // Chance that double-signing evidence is found on a given block - evidenceFraction float64 = 0.01 + evidenceFraction float64 = 0.5 // TODO Remove in favor of binary search for invariant violation onOperation bool = false diff --git a/x/mock/simulation/random_simulate_blocks.go b/x/mock/simulation/random_simulate_blocks.go index 51705cbef..0fc9c21a9 100644 --- a/x/mock/simulation/random_simulate_blocks.go +++ b/x/mock/simulation/random_simulate_blocks.go @@ -77,6 +77,9 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, } validators := initChain(r, accs, setups, app, appStateFn) + // Second variable to keep pending validator set (delayed one block since TM 0.24) + // Initially this is the same as the initial validator set + nextValidators := validators header := abci.Header{Height: 0, Time: timestamp} opCount := 0 @@ -160,8 +163,9 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, // Generate a random RequestBeginBlock with the current validator set for the next block request = RandomRequestBeginBlock(r, validators, livenessTransitionMatrix, evidenceFraction, pastTimes, pastVoteInfos, event, header) - // Update the validator set - validators = updateValidators(tb, r, validators, res.ValidatorUpdates, event) + // Update the validator set, which will be reflected in the application on the next block + validators = nextValidators + nextValidators = updateValidators(tb, r, validators, res.ValidatorUpdates, event) } if stopEarly { DisplayEvents(events) diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index 0c1c70b36..94dd9f0d0 100644 --- a/x/slashing/keeper.go +++ b/x/slashing/keeper.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/params" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" ) @@ -56,19 +57,28 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio // Double sign confirmed logger.Info(fmt.Sprintf("Confirmed double sign from %s at height %d, age of %d less than max age of %d", pubkey.Address(), infractionHeight, age, maxEvidenceAge)) + // We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height. + // Note that this *can* result in a negative "distributionHeight", up to -ValidatorUpdateDelay, + // i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block. + // That's fine since this is just used to filter unbonding delegations & redelegations. + distributionHeight := infractionHeight - stake.ValidatorUpdateDelay + // Cap the amount slashed to the penalty for the worst infraction // within the slashing period when this infraction was committed fraction := k.SlashFractionDoubleSign(ctx) - revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, infractionHeight) + revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, distributionHeight) logger.Info(fmt.Sprintf("Fraction slashed capped by slashing period from %v to %v", fraction, revisedFraction)) // Slash validator - k.validatorSet.Slash(ctx, consAddr, infractionHeight, power, revisedFraction) + k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, revisedFraction) - // Jail validator - k.validatorSet.Jail(ctx, consAddr) + // Jail validator if not already jailed + validator := k.validatorSet.ValidatorByConsAddr(ctx, consAddr) + if !validator.GetJailed() { + k.validatorSet.Jail(ctx, consAddr) + } - // Set validator jail duration + // Set or updated validator jail duration signInfo, found := k.getValidatorSigningInfo(ctx, consAddr) if !found { panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) @@ -123,7 +133,13 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p // Downtime confirmed: slash and jail the validator logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d", pubkey.Address(), minHeight, k.MinSignedPerWindow(ctx))) - k.validatorSet.Slash(ctx, consAddr, height, power, k.SlashFractionDowntime(ctx)) + // We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height, + // and subtract an additional 1 since this is the LastCommit. + // Note that this *can* result in a negative "distributionHeight" up to -ValidatorUpdateDelay-1, + // i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block. + // That's fine since this is just used to filter unbonding delegations & redelegations. + distributionHeight := height - stake.ValidatorUpdateDelay - 1 + k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx)) k.validatorSet.Jail(ctx, consAddr) signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeUnbondDuration(ctx)) } else { diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index e32e7af5b..cf67af192 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -22,20 +22,21 @@ func init() { // Test that a validator is slashed correctly // when we discover evidence of infraction -// TODO fix this test to not be using the same pubkey/address for signing and operating, it's confusing func TestHandleDoubleSign(t *testing.T) { // initial setup ctx, ck, sk, _, keeper := createTestInput(t) + // validator added pre-genesis + ctx = ctx.WithBlockHeight(-1) sk = sk.WithHooks(keeper.Hooks()) amtInt := int64(100) - addr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt) - got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(addr, val, amt)) + operatorAddr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt) + got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(operatorAddr, val, amt)) require.True(t, got.IsOK()) validatorUpdates := stake.EndBlocker(ctx, sk) keeper.AddValidators(ctx, validatorUpdates) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) - require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower())) + require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower())) // handle a signature to set signing info keeper.handleValidatorSignature(ctx, val.Address(), amtInt, true) @@ -44,13 +45,13 @@ func TestHandleDoubleSign(t *testing.T) { keeper.handleDoubleSign(ctx, val.Address(), 0, time.Unix(0, 0), amtInt) // should be jailed - require.True(t, sk.Validator(ctx, addr).GetJailed()) + require.True(t, sk.Validator(ctx, operatorAddr).GetJailed()) // unjail to measure power - sk.Unjail(ctx, sdk.ConsAddress(addr)) // TODO distinguish cons address + sk.Unjail(ctx, sdk.ConsAddress(val.Address())) // power should be reduced require.Equal( t, sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20))), - sk.Validator(ctx, addr).GetPower(), + sk.Validator(ctx, operatorAddr).GetPower(), ) ctx = ctx.WithBlockHeader(abci.Header{Time: time.Unix(1, 0).Add(keeper.MaxEvidenceAge(ctx))}) @@ -58,74 +59,74 @@ func TestHandleDoubleSign(t *testing.T) { keeper.handleDoubleSign(ctx, val.Address(), 0, time.Unix(0, 0), amtInt) require.Equal( t, sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20))), - sk.Validator(ctx, addr).GetPower(), + sk.Validator(ctx, operatorAddr).GetPower(), ) } // Test that the amount a validator is slashed for multiple double signs // is correctly capped by the slashing period in which they were committed -// TODO properly distinguish between consensus and operator address is variable names func TestSlashingPeriodCap(t *testing.T) { // initial setup ctx, ck, sk, _, keeper := createTestInput(t) sk = sk.WithHooks(keeper.Hooks()) amtInt := int64(100) - addr, amt := addrs[0], sdk.NewInt(amtInt) - valConsPubKey, valConsAddr := pks[0], sdk.ConsAddress(pks[0].Address()) - got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(addr, valConsPubKey, amt)) + operatorAddr, amt := addrs[0], sdk.NewInt(amtInt) + valConsPubKey, valConsAddr := pks[0], pks[0].Address() + got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(operatorAddr, valConsPubKey, amt)) require.True(t, got.IsOK()) validatorUpdates := stake.EndBlocker(ctx, sk) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) keeper.AddValidators(ctx, validatorUpdates) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) - require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower())) + require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower())) // handle a signature to set signing info - keeper.handleValidatorSignature(ctx, valConsPubKey.Address(), amtInt, true) + keeper.handleValidatorSignature(ctx, valConsAddr, amtInt, true) // double sign less than max age - keeper.handleDoubleSign(ctx, valConsPubKey.Address(), 0, time.Unix(0, 0), amtInt) + keeper.handleDoubleSign(ctx, valConsAddr, 1, time.Unix(0, 0), amtInt) // should be jailed - require.True(t, sk.Validator(ctx, addr).GetJailed()) - // end block - stake.EndBlocker(ctx, sk) - // update block height - ctx = ctx.WithBlockHeight(int64(1)) - // unjail to measure power - sk.Unjail(ctx, valConsAddr) - // end block - stake.EndBlocker(ctx, sk) - // power should be reduced - expectedPower := sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20))) - require.Equal(t, expectedPower, sk.Validator(ctx, addr).GetPower()) - - // double sign again, same slashing period - keeper.handleDoubleSign(ctx, valConsPubKey.Address(), 0, time.Unix(0, 0), amtInt) - // should be jailed - require.True(t, sk.Validator(ctx, addr).GetJailed()) + require.True(t, sk.Validator(ctx, operatorAddr).GetJailed()) // end block stake.EndBlocker(ctx, sk) // update block height ctx = ctx.WithBlockHeight(int64(2)) // unjail to measure power - sk.Unjail(ctx, valConsAddr) + sk.Unjail(ctx, sdk.ConsAddress(valConsAddr)) + // end block + stake.EndBlocker(ctx, sk) + // power should be reduced + expectedPower := sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20))) + require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower()) + + // double sign again, same slashing period + keeper.handleDoubleSign(ctx, valConsAddr, 1, time.Unix(0, 0), amtInt) + // should be jailed + require.True(t, sk.Validator(ctx, operatorAddr).GetJailed()) + // end block + stake.EndBlocker(ctx, sk) + // update block height + ctx = ctx.WithBlockHeight(int64(3)) + // unjail to measure power + sk.Unjail(ctx, sdk.ConsAddress(valConsAddr)) // end block stake.EndBlocker(ctx, sk) // power should be equal, no more should have been slashed expectedPower = sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20))) - require.Equal(t, expectedPower, sk.Validator(ctx, addr).GetPower()) + require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower()) // double sign again, new slashing period - keeper.handleDoubleSign(ctx, valConsPubKey.Address(), 2, time.Unix(0, 0), amtInt) + keeper.handleDoubleSign(ctx, valConsAddr, 3, time.Unix(0, 0), amtInt) // should be jailed - require.True(t, sk.Validator(ctx, addr).GetJailed()) + require.True(t, sk.Validator(ctx, operatorAddr).GetJailed()) // unjail to measure power - sk.Unjail(ctx, valConsAddr) + sk.Unjail(ctx, sdk.ConsAddress(valConsAddr)) // end block stake.EndBlocker(ctx, sk) // power should be reduced expectedPower = sdk.NewDecFromInt(amt).Mul(sdk.NewDec(18).Quo(sdk.NewDec(20))) - require.Equal(t, expectedPower, sk.Validator(ctx, addr).GetPower()) + require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower()) } // Test a validator through uptime, downtime, revocation, @@ -196,6 +197,35 @@ func TestHandleAbsentValidator(t *testing.T) { validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Unbonding, validator.GetStatus()) + slashAmt := sdk.NewDec(amtInt).Mul(keeper.SlashFractionDowntime(ctx)).RoundInt64() + + // validator should have been slashed + require.Equal(t, amtInt-slashAmt, validator.GetTokens().RoundInt64()) + + // 502nd block *also* missed (since the LastCommit would have still included the just-unbonded validator) + height++ + ctx = ctx.WithBlockHeight(height) + keeper.handleValidatorSignature(ctx, val.Address(), amtInt, false) + info, found = keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address())) + require.True(t, found) + require.Equal(t, int64(0), info.StartHeight) + require.Equal(t, keeper.SignedBlocksWindow(ctx)-keeper.MinSignedPerWindow(ctx)-2, info.SignedBlocksCounter) + + // end block + stake.EndBlocker(ctx, sk) + + // validator should not have been slashed any more, since it was already jailed + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) + require.Equal(t, amtInt-slashAmt, validator.GetTokens().RoundInt64()) + + // 502nd block *double signed* (oh no!) + keeper.handleDoubleSign(ctx, val.Address(), height, ctx.BlockHeader().Time, amtInt) + + // validator should have been slashed + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) + secondSlashAmt := sdk.NewDec(amtInt).Mul(keeper.SlashFractionDoubleSign(ctx)).RoundInt64() + require.Equal(t, amtInt-slashAmt-secondSlashAmt, validator.GetTokens().RoundInt64()) + // unrevocation should fail prior to jail expiration got = slh(ctx, NewMsgUnjail(addr)) require.False(t, got.IsOK()) @@ -214,14 +244,14 @@ func TestHandleAbsentValidator(t *testing.T) { // validator should have been slashed pool = sk.GetPool(ctx) - slashAmt := sdk.NewDec(amtInt).Mul(keeper.SlashFractionDowntime(ctx)).RoundInt64() - require.Equal(t, amtInt-slashAmt, pool.BondedTokens.RoundInt64()) + require.Equal(t, amtInt-slashAmt-secondSlashAmt, pool.BondedTokens.RoundInt64()) // validator start height should have been changed info, found = keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address())) require.True(t, found) require.Equal(t, height, info.StartHeight) - require.Equal(t, keeper.SignedBlocksWindow(ctx)-keeper.MinSignedPerWindow(ctx)-1, info.SignedBlocksCounter) + // we've missed 2 blocks more than the maximum + require.Equal(t, keeper.SignedBlocksWindow(ctx)-keeper.MinSignedPerWindow(ctx)-2, info.SignedBlocksCounter) // validator should not be immediately jailed again height++ diff --git a/x/slashing/keys.go b/x/slashing/keys.go index 1f84a285d..ec453cedc 100644 --- a/x/slashing/keys.go +++ b/x/slashing/keys.go @@ -4,6 +4,7 @@ import ( "encoding/binary" sdk "github.com/cosmos/cosmos-sdk/types" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" ) // key prefix bytes @@ -34,7 +35,8 @@ func GetValidatorSlashingPeriodPrefix(v sdk.ConsAddress) []byte { // stored by *Tendermint* address (not operator address) followed by start height func GetValidatorSlashingPeriodKey(v sdk.ConsAddress, startHeight int64) []byte { b := make([]byte, 8) - binary.LittleEndian.PutUint64(b, uint64(startHeight)) + // this needs to be height + ValidatorUpdateDelay because the slashing period for genesis validators starts at height -ValidatorUpdateDelay + binary.BigEndian.PutUint64(b, uint64(startHeight+stake.ValidatorUpdateDelay)) return append(GetValidatorSlashingPeriodPrefix(v), b...) } diff --git a/x/slashing/slashing_period.go b/x/slashing/slashing_period.go index fc57e663a..0595d5eeb 100644 --- a/x/slashing/slashing_period.go +++ b/x/slashing/slashing_period.go @@ -5,6 +5,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" ) // Cap an infraction's slash amount by the slashing period in which it was committed @@ -15,7 +16,7 @@ func (k Keeper) capBySlashingPeriod(ctx sdk.Context, address sdk.ConsAddress, fr // Sanity check if slashingPeriod.EndHeight > 0 && slashingPeriod.EndHeight < infractionHeight { - panic(fmt.Sprintf("slashing period ended before infraction: infraction height %d, slashing period ended at %d", infractionHeight, slashingPeriod.EndHeight)) + panic(fmt.Sprintf("slashing period ended before infraction: validator %s, infraction height %d, slashing period ended at %d", address, infractionHeight, slashingPeriod.EndHeight)) } // Calculate the updated total slash amount @@ -43,7 +44,7 @@ func (k Keeper) getValidatorSlashingPeriodForHeight(ctx sdk.Context, address sdk end := sdk.PrefixEndBytes(GetValidatorSlashingPeriodKey(address, height)) iterator := store.ReverseIterator(start, end) if !iterator.Valid() { - panic("expected to find slashing period, but none was found") + panic(fmt.Sprintf("expected to find slashing period for validator %s before height %d, but none was found", address, height)) } slashingPeriod = k.unmarshalSlashingPeriodKeyValue(iterator.Key(), iterator.Value()) return @@ -68,7 +69,7 @@ func (k Keeper) unmarshalSlashingPeriodKeyValue(key []byte, value []byte) Valida var slashingPeriodValue ValidatorSlashingPeriodValue k.cdc.MustUnmarshalBinary(value, &slashingPeriodValue) address := sdk.ConsAddress(key[1 : 1+sdk.AddrLen]) - startHeight := int64(binary.LittleEndian.Uint64(key[1+sdk.AddrLen : 1+sdk.AddrLen+8])) + startHeight := int64(binary.BigEndian.Uint64(key[1+sdk.AddrLen:1+sdk.AddrLen+8]) - uint64(stake.ValidatorUpdateDelay)) return ValidatorSlashingPeriod{ ValidatorAddr: address, StartHeight: startHeight, diff --git a/x/stake/genesis.go b/x/stake/genesis.go index 7c3724136..3cd104e66 100644 --- a/x/stake/genesis.go +++ b/x/stake/genesis.go @@ -18,6 +18,12 @@ import ( // the bonded validators. // Returns final validator set after applying all declaration and delegations func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res []abci.ValidatorUpdate, err error) { + + // We need to pretend to be "n blocks before genesis", where "n" is the validator update delay, + // so that e.g. slashing periods are correctly initialized for the validator set + // e.g. with a one-block offset - the first TM block is at height 0, so state updates applied from genesis.json are in block -1. + ctx = ctx.WithBlockHeight(-types.ValidatorUpdateDelay) + keeper.SetPool(ctx, data.Pool) keeper.SetParams(ctx, data.Params) keeper.InitIntraTxCounter(ctx) diff --git a/x/stake/keeper/slash_test.go b/x/stake/keeper/slash_test.go index 7959679c6..edc7ff16b 100644 --- a/x/stake/keeper/slash_test.go +++ b/x/stake/keeper/slash_test.go @@ -191,6 +191,34 @@ func TestSlashAtFutureHeight(t *testing.T) { require.Panics(t, func() { keeper.Slash(ctx, consAddr, 1, 10, fraction) }) } +// test slash at a negative height +// this just represents pre-genesis and should have the same effect as slashing at height 0 +func TestSlashAtNegativeHeight(t *testing.T) { + ctx, keeper, _ := setupHelper(t, 10) + consAddr := sdk.ConsAddress(PKs[0].Address()) + fraction := sdk.NewDecWithPrec(5, 1) + + oldPool := keeper.GetPool(ctx) + validator, found := keeper.GetValidatorByConsAddr(ctx, consAddr) + require.True(t, found) + keeper.Slash(ctx, consAddr, -2, 10, fraction) + + // read updated state + validator, found = keeper.GetValidatorByConsAddr(ctx, consAddr) + require.True(t, found) + newPool := keeper.GetPool(ctx) + + // end block + updates := keeper.ApplyAndReturnValidatorSetUpdates(ctx) + require.Equal(t, 1, len(updates), "cons addr: %v, updates: %v", []byte(consAddr), updates) + + validator = keeper.mustGetValidator(ctx, validator.OperatorAddr) + // power decreased + require.Equal(t, sdk.NewDec(5), validator.GetPower()) + // pool bonded shares decreased + require.Equal(t, sdk.NewDec(5).RoundInt64(), oldPool.BondedTokens.Sub(newPool.BondedTokens).RoundInt64()) +} + // tests Slash at the current height func TestSlashValidatorAtCurrentHeight(t *testing.T) { ctx, keeper, _ := setupHelper(t, 10) diff --git a/x/stake/types/params.go b/x/stake/types/params.go index 4dcc3782a..c3685f7c3 100644 --- a/x/stake/types/params.go +++ b/x/stake/types/params.go @@ -9,9 +9,17 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// defaultUnbondingTime reflects three weeks in seconds as the default -// unbonding time. -const defaultUnbondingTime time.Duration = 60 * 60 * 24 * 3 * time.Second +const ( + // defaultUnbondingTime reflects three weeks in seconds as the default + // unbonding time. + defaultUnbondingTime time.Duration = 60 * 60 * 24 * 3 * time.Second + + // Delay, in blocks, between when validator updates are returned to Tendermint and when they are applied + // For example, if this is 0, the validator set at the end of a block will sign the next block, or + // if this is 1, the validator set at the end of a block will sign the block after the next. + // Constant as this should not change without a hard fork. + ValidatorUpdateDelay int64 = 1 +) // Params defines the high level settings for staking type Params struct {