fix(cosmos): ensure simulated transactions can't trigger SwingSet

The former pattern of `vm.IsSimulation` was too brittle to handle our migration to the GRPC-based transaction handlers. Instead, intercept the actual invocation of CallToController and prevent it from running anything in SwingSet if in simulation mode. Also, make sure our InfiniteGasMeter is only assigned once, so that if we ever do Cosmos-level metering of SwingSet downcalls, there is only one place to adjust.
Agoric · Aug 21, 2021 · 997329a · 997329a
1 parent cf569cc
commit 997329a
Show file tree

Hide file tree

Showing 16 changed files with 53 additions and 147 deletions.
diff --git a/golang/cosmos/app/app.go b/golang/cosmos/app/app.go
@@ -358,7 +358,12 @@ func NewAgoricApp(
 	)
 
 	// This function is tricky to get right, so we build it ourselves.
-	callToController := func(ctx sdk.Context, str string) (string, error) {
+	callToController := func(ctx sdk.Context, str, simReturn string) (string, error) {
+		if vm.IsSimulation(ctx) {
+			// Just return the simReturn, since the message is being simulated.
+			return simReturn, nil
+		}
+		// We use SwingSet-level metering to charge the user for the call.
 		app.MustInitController(ctx)
 		defer vm.SetControllerContext(ctx)()
 		return sendToController(true, str)
@@ -565,7 +570,7 @@ func (app *GaiaApp) MustInitController(ctx sdk.Context) {
 	}
 	bz, err := json.Marshal(action)
 	if err == nil {
-		_, err = app.SwingSetKeeper.CallToController(ctx, string(bz))
+		_, err = app.SwingSetKeeper.CallToController(ctx, string(bz), "")
 	}
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "Cannot initialize Controller", err)

diff --git a/golang/cosmos/vm/controller.go b/golang/cosmos/vm/controller.go
@@ -25,6 +25,8 @@ var nameToPort = make(map[string]int)
 var lastPort = 0
 
 func SetControllerContext(ctx sdk.Context) func() {
+	// We are only called by the controller, so we assume that it is billing its
+	// own meter usage.
 	controllerContext.Context = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	return func() {
 		controllerContext.Context = sdk.Context{}

diff --git a/golang/cosmos/x/swingset/abci.go b/golang/cosmos/x/swingset/abci.go
@@ -49,7 +49,7 @@ func BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock, keeper Keeper) erro
 		return sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	return err
@@ -71,7 +71,7 @@ func EndBlock(ctx sdk.Context, req abci.RequestEndBlock, keeper Keeper) ([]abci.
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	if err != nil {
@@ -102,7 +102,7 @@ func CommitBlock(keeper Keeper) error {
 		return sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 	}
 
-	_, err = keeper.CallToController(sdk.Context{}, string(b))
+	_, err = keeper.CallToController(sdk.Context{}, string(b), "")
 
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	if err != nil {

diff --git a/golang/cosmos/x/swingset/genesis.go b/golang/cosmos/x/swingset/genesis.go
@@ -53,7 +53,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data *types.GenesisState) []abc
 		panic(err)
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 
 	if err != nil {
 		// NOTE: A failed BOOTSTRAP_BLOCK means that the SwingSet state is inconsistent.

diff --git a/golang/cosmos/x/swingset/handler.go b/golang/cosmos/x/swingset/handler.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
 	"github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/keeper"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -17,14 +16,6 @@ func NewHandler(k Keeper) sdk.Handler {
 	msgServer := keeper.NewMsgServerImpl(k)
 
 	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
-		if vm.IsSimulation(ctx) {
-			// We don't support simulation.
-			return &sdk.Result{}, nil
-		} else {
-			// The simulation was done, so now allow infinite gas.
-			ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
-		}
-
 		ctx = ctx.WithEventManager(sdk.NewEventManager())
 
 		switch msg := msg.(type) {

diff --git a/golang/cosmos/x/swingset/keeper/keeper.go b/golang/cosmos/x/swingset/keeper/keeper.go
@@ -26,7 +26,7 @@ type Keeper struct {
 	bankKeeper    bankkeeper.Keeper
 
 	// CallToController dispatches a message to the controlling process
-	CallToController func(ctx sdk.Context, str string) (string, error)
+	CallToController func(ctx sdk.Context, str, simReturn string) (string, error)
 }
 
 // A prefix of bytes, since KVStores can't handle empty slices as keys.
@@ -48,7 +48,7 @@ func stringToKey(keyStr string) []byte {
 func NewKeeper(
 	cdc codec.Codec, key sdk.StoreKey,
 	accountKeeper authkeeper.AccountKeeper, bankKeeper bankkeeper.Keeper,
-	callToController func(ctx sdk.Context, str string) (string, error),
+	callToController func(ctx sdk.Context, str, simReturn string) (string, error),
 ) Keeper {
 
 	return Keeper{

diff --git a/golang/cosmos/x/swingset/keeper/msg_server.go b/golang/cosmos/x/swingset/keeper/msg_server.go
@@ -58,7 +58,7 @@ func (keeper msgServer) DeliverInbound(goCtx context.Context, msg *types.MsgDeli
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	if err != nil {
 		return nil, err
@@ -104,7 +104,7 @@ func (keeper msgServer) Provision(goCtx context.Context, msg *types.MsgProvision
 		return nil, err
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	if err != nil {
 		return nil, err

diff --git a/golang/cosmos/x/swingset/storage.go b/golang/cosmos/x/swingset/storage.go
@@ -23,7 +23,7 @@ func NewStorageHandler(keeper Keeper) storageHandler {
 	return storageHandler{keeper: keeper}
 }
 
-func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret string, err error) {
+func (sh storageHandler) Receive(cctx *vm.ControllerContext, str string) (ret string, err error) {
 	keeper := sh.keeper
 	msg := new(storageMessage)
 	err = json.Unmarshal([]byte(str), &msg)
@@ -38,7 +38,7 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 			case sdk.ErrorOutOfGas:
 				err = fmt.Errorf(
 					"out of gas in location: %v; gasUsed: %d",
-					rType.Descriptor, ctx.Context.GasMeter().GasConsumed(),
+					rType.Descriptor, cctx.Context.GasMeter().GasConsumed(),
 				)
 			default:
 				// Not ErrorOutOfGas, so panic again.
@@ -53,11 +53,11 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 		storage := NewStorage()
 		storage.Value = msg.Value
 		//fmt.Printf("giving Keeper.SetStorage(%s) %s\n", msg.Key, storage.Value)
-		keeper.SetStorage(ctx.Context, msg.Key, storage)
+		keeper.SetStorage(cctx.Context, msg.Key, storage)
 		return "true", nil
 
 	case "get":
-		storage := keeper.GetStorage(ctx.Context, msg.Key)
+		storage := keeper.GetStorage(cctx.Context, msg.Key)
 		if storage.Value == "" {
 			return "null", nil
 		}
@@ -69,14 +69,14 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 		return string(s), nil
 
 	case "has":
-		storage := keeper.GetStorage(ctx.Context, msg.Key)
+		storage := keeper.GetStorage(cctx.Context, msg.Key)
 		if storage.Value == "" {
 			return "false", nil
 		}
 		return "true", nil
 
 	case "keys":
-		keys := keeper.GetKeys(ctx.Context, msg.Key)
+		keys := keeper.GetKeys(cctx.Context, msg.Key)
 		if keys.Keys == nil {
 			return "[]", nil
 		}
@@ -87,12 +87,12 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 		return string(bytes), nil
 
 	case "entries":
-		keys := keeper.GetKeys(ctx.Context, msg.Key)
+		keys := keeper.GetKeys(cctx.Context, msg.Key)
 		ents := make([][]string, len(keys.Keys))
 		for i, key := range keys.Keys {
 			ents[i] = make([]string, 2)
 			ents[i][0] = key
-			storage := keeper.GetStorage(ctx.Context, fmt.Sprintf("%s.%s", msg.Key, key))
+			storage := keeper.GetStorage(cctx.Context, fmt.Sprintf("%s.%s", msg.Key, key))
 			ents[i][1] = storage.Value
 		}
 		bytes, err := json.Marshal(ents)
@@ -102,10 +102,10 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 		return string(bytes), nil
 
 	case "values":
-		keys := keeper.GetKeys(ctx.Context, msg.Key)
+		keys := keeper.GetKeys(cctx.Context, msg.Key)
 		vals := make([]string, len(keys.Keys))
 		for i, key := range keys.Keys {
-			storage := keeper.GetStorage(ctx.Context, fmt.Sprintf("%s.%s", msg.Key, key))
+			storage := keeper.GetStorage(cctx.Context, fmt.Sprintf("%s.%s", msg.Key, key))
 			vals[i] = storage.Value
 		}
 		bytes, err := json.Marshal(vals)
@@ -115,7 +115,7 @@ func (sh storageHandler) Receive(ctx *vm.ControllerContext, str string) (ret str
 		return string(bytes), nil
 
 	case "size":
-		keys := keeper.GetKeys(ctx.Context, msg.Key)
+		keys := keeper.GetKeys(cctx.Context, msg.Key)
 		if keys.Keys == nil {
 			return "0", nil
 		}

diff --git a/golang/cosmos/x/vbank/handler.go b/golang/cosmos/x/vbank/handler.go
@@ -3,22 +3,13 @@ package vbank
 import (
 	"fmt"
 
-	"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // NewHandler returns a handler for "vbank" type messages.
 func NewHandler(keeper Keeper) sdk.Handler {
 	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
-		if vm.IsSimulation(ctx) {
-			// We don't support simulation.
-			return &sdk.Result{}, nil
-		} else {
-			// The simulation was done, so now allow infinite gas.
-			ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
-		}
-
 		switch msg := msg.(type) {
 		default:
 			errMsg := fmt.Sprintf("Unrecognized vbank Msg type: %T", msg)

diff --git a/golang/cosmos/x/vbank/keeper/keeper.go b/golang/cosmos/x/vbank/keeper/keeper.go
@@ -20,15 +20,15 @@ type Keeper struct {
 	bankKeeper       types.BankKeeper
 	feeCollectorName string
 	// CallToController dispatches a message to the controlling process
-	CallToController func(ctx sdk.Context, str string) (string, error)
+	CallToController func(ctx sdk.Context, str, simReturn string) (string, error)
 }
 
 // NewKeeper creates a new vbank Keeper instance
 func NewKeeper(
 	cdc codec.Codec, key sdk.StoreKey, paramSpace paramtypes.Subspace,
 	bankKeeper types.BankKeeper,
 	feeCollectorName string,
-	callToController func(ctx sdk.Context, str string) (string, error),
+	callToController func(ctx sdk.Context, str, simReturn string) (string, error),
 ) Keeper {
 
 	// set KeyTable if it has not already been set

diff --git a/golang/cosmos/x/vbank/module.go b/golang/cosmos/x/vbank/module.go
@@ -192,7 +192,7 @@ func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.V
 		panic(err)
 	}
 	if bz != nil {
-		_, err := am.CallToController(ctx, string(bz))
+		_, err := am.CallToController(ctx, string(bz), "")
 		if err != nil {
 			panic(err)
 		}

diff --git a/golang/cosmos/x/vbank/vbank.go b/golang/cosmos/x/vbank/vbank.go
@@ -227,9 +227,9 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
 	return
 }
 
-func (am AppModule) CallToController(ctx sdk.Context, send string) (string, error) {
+func (am AppModule) CallToController(ctx sdk.Context, send, simReturn string) (string, error) {
 	// fmt.Println("vbank.go upcall", send)
-	reply, err := am.keeper.CallToController(ctx, send)
+	reply, err := am.keeper.CallToController(ctx, send, simReturn)
 	// fmt.Println("vbank.go upcall reply", reply, err)
 	return reply, err
 }
diff --git a/golang/cosmos/x/vbank/vbank_test.go b/golang/cosmos/x/vbank/vbank_test.go
@@ -229,8 +229,8 @@ func (b *mockBank) SendCoinsFromModuleToModule(ctx sdk.Context, senderModule, re
 func makeTestKit(bank types.BankKeeper) (Keeper, sdk.Context) {
 	encodingConfig := params.MakeEncodingConfig()
 	cdc := encodingConfig.Marshaller
-	callToController := func(ctx sdk.Context, str string) (string, error) {
-		return "", nil
+	callToController := func(ctx sdk.Context, str, simReturn string) (string, error) {
+		return simReturn, nil
 	}
 
 	paramsTStoreKey := sdk.NewTransientStoreKey(paramstypes.TStoreKey)
@@ -472,9 +472,9 @@ func Test_EndBlock_Events(t *testing.T) {
 	}}
 	keeper, ctx := makeTestKit(bank)
 	msgsSent := []string{}
-	keeper.CallToController = func(ctx sdk.Context, str string) (string, error) {
+	keeper.CallToController = func(ctx sdk.Context, str, simReturn string) (string, error) {
 		msgsSent = append(msgsSent, str)
-		return "", nil
+		return simReturn, nil
 	}
 	am := NewAppModule(keeper)
 
@@ -546,9 +546,9 @@ func Test_EndBlock_Rewards(t *testing.T) {
 	}
 	keeper, ctx := makeTestKit(bank)
 	msgsSent := []string{}
-	keeper.CallToController = func(ctx sdk.Context, str string) (string, error) {
+	keeper.CallToController = func(ctx sdk.Context, str, simReturn string) (string, error) {
 		msgsSent = append(msgsSent, str)
-		return "", nil
+		return simReturn, nil
 	}
 	am := NewAppModule(keeper)
 

diff --git a/golang/cosmos/x/vibc/handler.go b/golang/cosmos/x/vibc/handler.go
@@ -4,22 +4,13 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // NewHandler returns a handler for "vibc" type messages.
 func NewHandler(keeper Keeper) sdk.Handler {
 	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
-		if vm.IsSimulation(ctx) {
-			// We don't support simulation.
-			return &sdk.Result{}, nil
-		} else {
-			// The simulation was done, so now allow infinite gas.
-			ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
-		}
-
 		switch msg := msg.(type) {
 		case *MsgSendPacket:
 			return handleMsgSendPacket(ctx, keeper, msg)
@@ -62,7 +53,7 @@ func handleMsgSendPacket(ctx sdk.Context, keeper Keeper, msg *MsgSendPacket) (*s
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 	}
 
-	_, err = keeper.CallToController(ctx, string(b))
+	_, err = keeper.CallToController(ctx, string(b), "")
 	// fmt.Fprintln(os.Stderr, "Returned from SwingSet", out, err)
 	if err != nil {
 		return nil, err