Merge PR #2255: Offset validator set power calculations by one block

.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

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

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

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)
 	}
-}
+}

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")
-}
+}

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

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

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

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),
 	}
 }
 

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

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)

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 {