Merge PR cosmos#4389: stake invariance bug

* add trouble seed

* currentStakeRoundUp is now always atleast currentStake + smallest-decimal-precision

* remove unused code

* remove debugs

* @alexanderbez comment

* compile fix

* better comment, increase tolerance to 3 smallest decimal points

.pending/bugfixes/sdk/4383---currentStakeR

@@ -0,0 +1 @@
+#4383 - currentStakeRoundUp is now always atleast currentStake + smallest-decimal-precision

contrib/runsim/main.go

@@ -25,7 +25,7 @@ var (
 		3012, 4728, 37827, 981928, 87821, 891823782,
 		989182, 89182391, 11, 22, 44, 77, 99, 2020,
 		3232, 123123, 124124, 582582, 18931893,
-		29892989, 30123012, 47284728, 7601778,
+		29892989, 30123012, 47284728, 7601778, 8090485,
 	}
 
 	// goroutine-safe process map

simapp/sim_test.go

@@ -49,6 +49,7 @@ var (
 	lean        bool
 	commit      bool
 	period      int
+	onOperation bool // TODO Remove in favor of binary search for invariant violation
 )
 
 func init() {
@@ -61,15 +62,16 @@ func init() {
 	flag.BoolVar(&lean, "SimulationLean", false, "lean simulation log output")
 	flag.BoolVar(&commit, "SimulationCommit", false, "have the simulation commit")
 	flag.IntVar(&period, "SimulationPeriod", 1, "run slow invariants only once every period assertions")
+	flag.BoolVar(&onOperation, "SimulateEveryOperation", false, "run slow invariants every operation")
 }
 
 // helper function for populating input for SimulateFromSeed
 func getSimulateFromSeedInput(tb testing.TB, w io.Writer, app *SimApp) (
 	testing.TB, io.Writer, *baseapp.BaseApp, simulation.AppStateFn, int64,
-	simulation.WeightedOperations, sdk.Invariants, int, int, bool, bool) {
+	simulation.WeightedOperations, sdk.Invariants, int, int, bool, bool, bool) {
 
 	return tb, w, app.BaseApp, appStateFn, seed,
-		testAndRunTxs(app), invariants(app), numBlocks, blockSize, commit, lean
+		testAndRunTxs(app), invariants(app), numBlocks, blockSize, commit, lean, onOperation
 }
 
 func appStateFromGenesisFileFn(r *rand.Rand, accs []simulation.Account, genesisTimestamp time.Time,
@@ -584,6 +586,7 @@ func TestAppStateDeterminism(t *testing.T) {
 				100,
 				true,
 				false,
+				false,
 			)
 			appHash := app.LastCommitID().Hash
 			appHashList[j] = appHash
@@ -609,7 +612,7 @@ func BenchmarkInvariants(b *testing.B) {
 	// 2. Run parameterized simulation (w/o invariants)
 	_, err := simulation.SimulateFromSeed(
 		b, ioutil.Discard, app.BaseApp, appStateFn, seed, testAndRunTxs(app),
-		[]sdk.Invariant{}, numBlocks, blockSize, commit, lean,
+		[]sdk.Invariant{}, numBlocks, blockSize, commit, lean, onOperation,
 	)
 	if err != nil {
 		fmt.Println(err)

types/decimal.go

@@ -46,8 +46,9 @@ func precisionInt() *big.Int {
 }
 
 // nolint - common values
-func ZeroDec() Dec { return Dec{new(big.Int).Set(zeroInt)} }
-func OneDec() Dec  { return Dec{precisionInt()} }
+func ZeroDec() Dec     { return Dec{new(big.Int).Set(zeroInt)} }
+func OneDec() Dec      { return Dec{precisionInt()} }
+func SmallestDec() Dec { return Dec{new(big.Int).Set(oneInt)} }
 
 // calculate the precision multiplier
 func calcPrecisionMultiplier(prec int64) *big.Int {

x/distribution/keeper/delegation.go

@@ -98,9 +98,26 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d
 	// we had arbitrary-precision rationals.
 	currentStake := val.TokensFromShares(del.GetShares())
 	if stake.GT(currentStake) {
-		// account for rounding errors due to stake being multiplied by slash fractions
-		currentStakeRoundUp := val.TokensFromSharesRoundUp(del.GetShares())
-		if stake.Equal(currentStakeRoundUp) {
+
+		// Account for rounding inconsistencies between:
+		//     currentStake: calculated as in staking with a single computation
+		//     stake:        calculated as an accumulation of stake
+		//                   calculations across validator's distribution periods
+		// These inconsistencies are due to differing order of operations which
+		// will inevitably have  different accumulated rounding and may lead to
+		// the smallest decimal place being one greater in stake than
+		// currentStake. When we calculated slashing by period, even if we
+		// round down for each slash fraction, it's possible due to how much is
+		// being rounded that we slash less when slashing by period instead of
+		// for when we slash without periods. In other words, the single slash,
+		// and the slashing by period could both be rounding down but the
+		// slashing by period is simply rounding down less, thus making stake >
+		// currentStake
+		//
+		// A small amount of this error is tolerated and corrected for,
+		// however any greater amount should be considered a breach in expected
+		// behaviour.
+		if stake.Equal(currentStake.Add(sdk.SmallestDec().MulInt64(3))) {
 			stake = currentStake
 		} else {
 			panic(fmt.Sprintf("calculated final stake for delegator %s greater than current stake"+

x/simulation/params.go

@@ -13,9 +13,6 @@ const (
 
 	// Maximum time per block
 	maxTimePerBlock int64 = 10000
-
-	// TODO Remove in favor of binary search for invariant violation
-	onOperation bool = false
 )
 
 // TODO explain transitional matrix usage

x/simulation/simulate.go

@@ -21,16 +21,6 @@ import (
 // AppStateFn returns the app state json bytes, the genesis accounts, and the chain identifier
 type AppStateFn func(r *rand.Rand, accs []Account, genesisTimestamp time.Time) (appState json.RawMessage, accounts []Account, chainId string)
 
-// Simulate tests application by sending random messages.
-func Simulate(t *testing.T, app *baseapp.BaseApp,
-	appStateFn AppStateFn, ops WeightedOperations,
-	invariants sdk.Invariants, numBlocks, blockSize int, commit, lean bool) (bool, error) {
-
-	time := time.Now().UnixNano()
-	return SimulateFromSeed(t, os.Stdout, app, appStateFn, time, ops,
-		invariants, numBlocks, blockSize, commit, lean)
-}
-
 // initialize the chain for the simulation
 func initChain(
 	r *rand.Rand, params Params, accounts []Account,
@@ -56,7 +46,7 @@ func SimulateFromSeed(
 	tb testing.TB, w io.Writer, app *baseapp.BaseApp,
 	appStateFn AppStateFn, seed int64, ops WeightedOperations,
 	invariants sdk.Invariants,
-	numBlocks, blockSize int, commit, lean bool,
+	numBlocks, blockSize int, commit, lean, onOperation bool,
 ) (stopEarly bool, simError error) {
 
 	// in case we have to end early, don't os.Exit so that we can run cleanup code.
@@ -116,7 +106,7 @@ func SimulateFromSeed(
 	blockSimulator := createBlockSimulator(
 		testingMode, tb, t, w, params, eventStats.tally, invariants,
 		ops, operationQueue, timeOperationQueue,
-		numBlocks, blockSize, logWriter, lean)
+		numBlocks, blockSize, logWriter, lean, onOperation)
 
 	if !testingMode {
 		b.ResetTimer()
@@ -229,7 +219,7 @@ type blockSimFn func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context,
 func createBlockSimulator(testingMode bool, tb testing.TB, t *testing.T, w io.Writer, params Params,
 	event func(string), invariants sdk.Invariants, ops WeightedOperations,
 	operationQueue OperationQueue, timeOperationQueue []FutureOperation,
-	totalNumBlocks, avgBlockSize int, logWriter LogWriter, lean bool) blockSimFn {
+	totalNumBlocks, avgBlockSize int, logWriter LogWriter, lean, onOperation bool) blockSimFn {
 
 	lastBlocksizeState := 0 // state for [4 * uniform distribution]
 	blocksize := 0
@@ -274,10 +264,11 @@ func createBlockSimulator(testingMode bool, tb testing.TB, t *testing.T, w io.Wr
 			queueOperations(operationQueue, timeOperationQueue, futureOps)
 			if testingMode {
 				if onOperation {
+					fmt.Fprintf(w, "\rSimulating... block %d/%d, operation %d/%d. ",
+						header.Height, totalNumBlocks, opCount, blocksize)
 					eventStr := fmt.Sprintf("operation: %v", opMsg.String())
 					assertAllInvariants(t, app, invariants, eventStr, logWriter)
-				}
-				if opCount%50 == 0 {
+				} else if opCount%50 == 0 {
 					fmt.Fprintf(w, "\rSimulating... block %d/%d, operation %d/%d. ",
 						header.Height, totalNumBlocks, opCount, blocksize)
 				}