Merge PR #1746: Remove ctx.IsCheckTx

* Remove ctx.IsCheckTx
* Changelog to Pending
* Typo
* Simulate instead of Check in SignCheckDeliver
* Add descriptive comment
* Change to 'CheckTx implements ABCI'
* Alphabetize changelog

CHANGELOG.md

@@ -1,22 +1,5 @@
 # Changelog
 
-## PENDING
-
-BREAKING CHANGES
-* [x/stake] Fixed the period check for the inflation calculation
-
-FEATURES
-* [lcd] Can now query governance proposals by ProposalStatus
-
-IMPROVEMENTS
-* [baseapp] Allow any alphanumeric character in route
-* [x/stake] Add revoked to human-readable validator 
-* [cli] Improve error messages for all txs when the account doesn't exist
-* [tools] Remove `rm -rf vendor/` from `make get_vendor_deps`
-
-BUG FIXES
-*  \#1666 Add intra-tx counter to the genesis validators
-
 ## 0.22.0
 
 *July 16th, 2018*

PENDING.md

@@ -0,0 +1,17 @@
+## PENDING
+
+BREAKING CHANGES
+* [baseapp] Msgs are no longer run on CheckTx, removed `ctx.IsCheckTx()`
+* [x/stake] Fixed the period check for the inflation calculation
+
+FEATURES
+* [lcd] Can now query governance proposals by ProposalStatus
+
+IMPROVEMENTS
+* [baseapp] Allow any alphanumeric character in route
+* [cli] Improve error messages for all txs when the account doesn't exist
+* [tools] Remove `rm -rf vendor/` from `make get_vendor_deps`
+* [x/stake] Add revoked to human-readable validator 
+
+BUG FIXES
+*  \#1666 Add intra-tx counter to the genesis validators

baseapp/baseapp.go

@@ -436,7 +436,11 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 	return
 }
 
-// Implements ABCI
+// CheckTx implements ABCI
+// CheckTx runs the "basic checks" to see whether or not a transaction can possibly be executed,
+// first decoding, then the ante handler (which checks signatures/fees/ValidateBasic),
+// then finally the route match to see whether a handler exists. CheckTx does not run the actual
+// Msg handler function(s).
 func (app *BaseApp) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) {
 	// Decode the Tx.
 	var result sdk.Result
@@ -514,16 +518,11 @@ func (app *BaseApp) getContextForAnte(mode runTxMode, txBytes []byte) (ctx sdk.C
 		ctx = ctx.WithSigningValidators(app.signedValidators)
 	}
 
-	// Simulate a DeliverTx for gas calculation
-	if mode == runTxModeSimulate {
-		ctx = ctx.WithIsCheckTx(false)
-	}
-
 	return
 }
 
 // Iterates through msgs and executes them
-func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg) (result sdk.Result) {
+func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (result sdk.Result) {
 	// accumulate results
 	logs := make([]string, 0, len(msgs))
 	var data []byte   // NOTE: we just append them all (?!)
@@ -537,7 +536,11 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg) (result sdk.Result)
 			return sdk.ErrUnknownRequest("Unrecognized Msg type: " + msgType).Result()
 		}
 
-		msgResult := handler(ctx, msg)
+		var msgResult sdk.Result
+		// Skip actual execution for CheckTx
+		if mode != runTxModeCheck {
+			msgResult = handler(ctx, msg)
+		}
 
 		// NOTE: GasWanted is determined by ante handler and
 		// GasUsed by the GasMeter
@@ -615,9 +618,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 
 	// run the ante handler
 	if app.anteHandler != nil {
-		newCtx, anteResult, abort := app.anteHandler(ctx, tx)
+		newCtx, result, abort := app.anteHandler(ctx, tx)
 		if abort {
-			return anteResult
+			return result
 		}
 		if !newCtx.IsZero() {
 			ctx = newCtx
@@ -636,7 +639,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 	}
 
 	ctx = ctx.WithMultiStore(msCache)
-	result = app.runMsgs(ctx, msgs)
+	result = app.runMsgs(ctx, msgs, mode)
 	result.GasWanted = gasWanted
 
 	// only update state if all messages pass and we're not in a simulation

examples/democoin/x/cool/handler.go

@@ -47,10 +47,6 @@ func handleMsgQuiz(ctx sdk.Context, k Keeper, msg MsgQuiz) sdk.Result {
 		return ErrIncorrectCoolAnswer(k.codespace, msg.CoolAnswer).Result()
 	}
 
-	if ctx.IsCheckTx() {
-		return sdk.Result{} // TODO
-	}
-
 	bonusCoins := sdk.Coins{sdk.NewCoin(msg.CoolAnswer, 69)}
 
 	_, _, err := k.ck.AddCoins(ctx, msg.Sender, bonusCoins)

examples/democoin/x/pow/handler.go

@@ -26,14 +26,6 @@ func handleMsgMine(ctx sdk.Context, pk Keeper, msg MsgMine) sdk.Result {
 		return err.Result()
 	}
 
-	// commented for now, makes testing difficult
-	// TODO figure out a better test method that allows early CheckTx return
-	/*
-		if ctx.IsCheckTx() {
-			return sdk.Result{} // TODO
-		}
-	*/
-
 	err = pk.ApplyValid(ctx, msg.Sender, newDiff, newCount)
 	if err != nil {
 		return err.Result()

types/context.go

@@ -41,7 +41,6 @@ func NewContext(ms MultiStore, header abci.Header, isCheckTx bool, logger log.Lo
 	c = c.WithBlockHeader(header)
 	c = c.WithBlockHeight(header.Height)
 	c = c.WithChainID(header.ChainID)
-	c = c.WithIsCheckTx(isCheckTx)
 	c = c.WithTxBytes(nil)
 	c = c.WithLogger(logger)
 	c = c.WithSigningValidators(nil)
@@ -127,7 +126,6 @@ const (
 	contextKeyBlockHeader
 	contextKeyBlockHeight
 	contextKeyChainID
-	contextKeyIsCheckTx
 	contextKeyTxBytes
 	contextKeyLogger
 	contextKeySigningValidators
@@ -151,9 +149,6 @@ func (c Context) BlockHeight() int64 {
 func (c Context) ChainID() string {
 	return c.Value(contextKeyChainID).(string)
 }
-func (c Context) IsCheckTx() bool {
-	return c.Value(contextKeyIsCheckTx).(bool)
-}
 func (c Context) TxBytes() []byte {
 	return c.Value(contextKeyTxBytes).([]byte)
 }
@@ -179,9 +174,6 @@ func (c Context) WithBlockHeight(height int64) Context {
 func (c Context) WithChainID(chainID string) Context {
 	return c.withValue(contextKeyChainID, chainID)
 }
-func (c Context) WithIsCheckTx(isCheckTx bool) Context {
-	return c.withValue(contextKeyIsCheckTx, isCheckTx)
-}
 func (c Context) WithTxBytes(txBytes []byte) Context {
 	return c.withValue(contextKeyTxBytes, txBytes)
 }

x/mock/test_utils.go

@@ -46,7 +46,8 @@ func SignCheckDeliver(
 	seq []int64, expPass bool, priv ...crypto.PrivKey,
 ) sdk.Result {
 	tx := GenTx(msgs, accNums, seq, priv...)
-	res := app.Check(tx)
+	// Must simulate now as CheckTx doesn't run Msgs anymore
+	res := app.Simulate(tx)
 
 	if expPass {
 		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)

x/slashing/app_test.go

@@ -113,6 +113,6 @@ func TestSlashingMsgs(t *testing.T) {
 	checkValidatorSigningInfo(t, mapp, keeper, sdk.ValAddress(addr1), false)
 
 	// unrevoke should fail with unknown validator
-	res := mock.CheckGenTx(t, mapp.BaseApp, []sdk.Msg{unrevokeMsg}, []int64{0}, []int64{1}, false, priv1)
+	res := mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{unrevokeMsg}, []int64{0}, []int64{1}, false, priv1)
 	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotRevoked), res.Code)
 }

x/slashing/handler.go

@@ -43,10 +43,6 @@ func handleMsgUnrevoke(ctx sdk.Context, msg MsgUnrevoke, k Keeper) sdk.Result {
 		return ErrValidatorJailed(k.codespace).Result()
 	}
 
-	if ctx.IsCheckTx() {
-		return sdk.Result{}
-	}
-
 	// Update the starting height (so the validator can't be immediately revoked again)
 	info.StartHeight = ctx.BlockHeight()
 	k.setValidatorSigningInfo(ctx, addr, info)