Merge pull request cosmos#567 from CosmWasm/553-adjust-default-msg-gas

Adjust default msg gas
notional-labs · Jul 28, 2021 · 0dbcef0 · 0dbcef0
2 parents 267643f + 1032853
commit 0dbcef0
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 32 deletions.
diff --git a/x/wasm/keeper/gas_register.go b/x/wasm/keeper/gas_register.go
@@ -25,8 +25,11 @@ const (
 	// This is used with len(key) + len(value)
 	DefaultEventAttributeDataCost uint64 = 1
 	// DefaultContractMessageDataCost is how much SDK gas is charged *per byte* of the message that goes to the contract
-	// This is used with len(msg)
-	DefaultContractMessageDataCost uint64 = 1
+	// This is used with len(msg). Note that the message is deserialized in the receiving contract and this is charged
+	// with wasm gas already. The derserialization of results is also charged in wasmvm. I am unsure if we need to add
+	// additional costs here.
+	// Note: also used for error fields on reply, and data on reply. Maybe these should be pulled out to a different (non-zero) field
+	DefaultContractMessageDataCost uint64 = 0
 	// DefaultPerAttributeCost is how much SDK gas we charge per attribute count.
 	DefaultPerAttributeCost uint64 = 10
 	// DefaultPerCustomEventCost is how much SDK gas we charge per event count.
@@ -144,7 +147,7 @@ func (g WasmGasRegister) ReplyCosts(pinned bool, reply wasmvmtypes.Reply) sdk.Ga
 		msgLen += len(reply.Result.Ok.Data)
 		var attrs []wasmvmtypes.EventAttribute
 		for _, e := range reply.Result.Ok.Events {
-			msgLen += len(e.Type)
+			eventGas += sdk.Gas(len(e.Type)) * g.c.EventAttributeDataCost
 			attrs = append(e.Attributes)
 		}
 		// apply free tier on the whole set not per event

diff --git a/x/wasm/keeper/gas_register_test.go b/x/wasm/keeper/gas_register_test.go
@@ -59,13 +59,13 @@ func TestNewContractInstanceCosts(t *testing.T) {
 			srcLen:    1,
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(1),
+			exp:       DefaultContractMessageDataCost,
 		},
 		"big msg - pinned": {
 			srcLen:    math.MaxUint32,
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(math.MaxUint32),
+			exp:       DefaultContractMessageDataCost * sdk.Gas(math.MaxUint32),
 		},
 		"empty msg - pinned": {
 			srcLen:    0,
@@ -76,18 +76,17 @@ func TestNewContractInstanceCosts(t *testing.T) {
 		"small msg - unpinned": {
 			srcLen:    1,
 			srcConfig: DefaultGasRegisterConfig(),
-			pinned:    true,
-			exp:       sdk.Gas(1),
+			exp:       DefaultContractMessageDataCost + DefaultInstanceCost,
 		},
 		"big msg - unpinned": {
 			srcLen:    math.MaxUint32,
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(math.MaxUint32 + 40_000),
+			exp:       sdk.Gas(DefaultContractMessageDataCost*math.MaxUint32 + DefaultInstanceCost),
 		},
 		"empty msg - unpinned": {
 			srcLen:    0,
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000),
+			exp:       sdk.Gas(DefaultInstanceCost),
 		},
 
 		"negative len": {
@@ -123,13 +122,13 @@ func TestContractInstanceCosts(t *testing.T) {
 			srcLen:    1,
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(1),
+			exp:       DefaultContractMessageDataCost,
 		},
 		"big msg - pinned": {
 			srcLen:    math.MaxUint32,
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(math.MaxUint32),
+			exp:       sdk.Gas(DefaultContractMessageDataCost * math.MaxUint32),
 		},
 		"empty msg - pinned": {
 			srcLen:    0,
@@ -140,18 +139,17 @@ func TestContractInstanceCosts(t *testing.T) {
 		"small msg - unpinned": {
 			srcLen:    1,
 			srcConfig: DefaultGasRegisterConfig(),
-			pinned:    true,
-			exp:       sdk.Gas(1),
+			exp:       DefaultContractMessageDataCost + DefaultInstanceCost,
 		},
 		"big msg - unpinned": {
 			srcLen:    math.MaxUint32,
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(math.MaxUint32 + 40_000),
+			exp:       sdk.Gas(DefaultContractMessageDataCost*math.MaxUint32 + DefaultInstanceCost),
 		},
 		"empty msg - unpinned": {
 			srcLen:    0,
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000),
+			exp:       sdk.Gas(DefaultInstanceCost),
 		},
 
 		"negative len": {
@@ -195,7 +193,7 @@ func TestReplyCost(t *testing.T) {
 			},
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(3 + 10 + 1), // len("foo") + 1 * DefaultPerAttributeCost + len(data)
+			exp:       sdk.Gas(3*DefaultEventAttributeDataCost + DefaultPerAttributeCost + DefaultContractMessageDataCost), // 3 == len("foo")
 		},
 		"subcall response with events - pinned": {
 			src: wasmvmtypes.Reply{
@@ -209,7 +207,7 @@ func TestReplyCost(t *testing.T) {
 			},
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(3 + 10), // len("foo") + 1 * DefaultPerAttributeCost
+			exp:       sdk.Gas(3*DefaultEventAttributeDataCost + DefaultPerAttributeCost), // 3 == len("foo")
 		},
 		"subcall response with events exceeds free tier- pinned": {
 			src: wasmvmtypes.Reply{
@@ -223,7 +221,7 @@ func TestReplyCost(t *testing.T) {
 			},
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(3 + 10 + 6), // len("foo") + 1 * DefaultPerAttributeCost + len("myData")
+			exp:       sdk.Gas((3+6)*DefaultEventAttributeDataCost + DefaultPerAttributeCost), // 3 == len("foo"), 6 == len("myData")
 		},
 		"subcall response error - pinned": {
 			src: wasmvmtypes.Reply{
@@ -233,7 +231,7 @@ func TestReplyCost(t *testing.T) {
 			},
 			srcConfig: DefaultGasRegisterConfig(),
 			pinned:    true,
-			exp:       sdk.Gas(3), // len("foo")
+			exp:       3 * DefaultContractMessageDataCost,
 		},
 		"subcall response with events and data - unpinned": {
 			src: wasmvmtypes.Reply{
@@ -247,7 +245,7 @@ func TestReplyCost(t *testing.T) {
 				},
 			},
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000 + 3 + 10 + 1), // DefaultInstanceCost len("foo") + 1 * DefaultPerAttributeCost + len(data)
+			exp:       sdk.Gas(DefaultInstanceCost + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost + DefaultContractMessageDataCost),
 		},
 		"subcall response with events - unpinned": {
 			src: wasmvmtypes.Reply{
@@ -260,7 +258,7 @@ func TestReplyCost(t *testing.T) {
 				},
 			},
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000 + 3 + 10), // DefaultInstanceCost + len("foo") + 1 * DefaultPerAttributeCost
+			exp:       sdk.Gas(DefaultInstanceCost + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost),
 		},
 		"subcall response with events exceeds free tier- unpinned": {
 			src: wasmvmtypes.Reply{
@@ -273,7 +271,7 @@ func TestReplyCost(t *testing.T) {
 				},
 			},
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000 + 3 + 10 + 6), // DefaultInstanceCost + len("foo") + 1 * DefaultPerAttributeCost + len("myData")
+			exp:       sdk.Gas(DefaultInstanceCost + (3+6)*DefaultEventAttributeDataCost + DefaultPerAttributeCost), // 3 == len("foo"), 6 == len("myData")
 		},
 		"subcall response error - unpinned": {
 			src: wasmvmtypes.Reply{
@@ -282,7 +280,7 @@ func TestReplyCost(t *testing.T) {
 				},
 			},
 			srcConfig: DefaultGasRegisterConfig(),
-			exp:       sdk.Gas(40_000 + 3), // DefaultInstanceCost + len("foo")
+			exp:       sdk.Gas(DefaultInstanceCost + 3*DefaultContractMessageDataCost),
 		},
 	}
 	for name, spec := range specs {

diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go
@@ -284,7 +284,7 @@ func TestInstantiate(t *testing.T) {
 
 	gasAfter := ctx.GasMeter().GasConsumed()
 	if types.EnableGasVerification {
-		require.Equal(t, uint64(0x12280), gasAfter-gasBefore)
+		require.Equal(t, uint64(0x12206), gasAfter-gasBefore)
 	}
 
 	// ensure it is stored properly
@@ -517,7 +517,7 @@ func TestExecute(t *testing.T) {
 	// make sure gas is properly deducted from ctx
 	gasAfter := ctx.GasMeter().GasConsumed()
 	if types.EnableGasVerification {
-		require.Equal(t, uint64(0x12afd), gasAfter-gasBefore)
+		require.Equal(t, uint64(0x12aef), gasAfter-gasBefore)
 	}
 	// ensure bob now exists and got both payments released
 	bobAcct = accKeeper.GetAccount(ctx, bob)

diff --git a/x/wasm/keeper/recurse_test.go b/x/wasm/keeper/recurse_test.go
@@ -57,12 +57,12 @@ func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.Acc
 
 func TestGasCostOnQuery(t *testing.T) {
 	const (
-		GasNoWork uint64 = 44_240
+		GasNoWork uint64 = 44_149
 		// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
-		GasWork50 uint64 = 49_901 // this is a little shy of 50k gas - to keep an eye on the limit
+		GasWork50 uint64 = 49_809 // this is a little shy of 50k gas - to keep an eye on the limit
 
-		GasReturnUnhashed uint64 = 197
-		GasReturnHashed   uint64 = 173
+		GasReturnUnhashed uint64 = 256
+		GasReturnHashed   uint64 = 232
 	)
 
 	cases := map[string]struct {
@@ -221,9 +221,9 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
 
 	const (
 		// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
-		GasWork2k uint64 = 273_170 // = NewContractInstanceCosts + x // we have 6x gas used in cpu than in the instance
+		GasWork2k uint64 = 273_076 // = NewContractInstanceCosts + x // we have 6x gas used in cpu than in the instance
 		// This is overhead for calling into a sub-contract
-		GasReturnHashed uint64 = 177
+		GasReturnHashed uint64 = 236
 	)
 
 	cases := map[string]struct {

diff --git a/x/wasm/keeper/relay_test.go b/x/wasm/keeper/relay_test.go
@@ -411,7 +411,7 @@ func TestOnRecvPacket(t *testing.T) {
 		},
 		"submessage reply can overwrite ack data": {
 			contractAddr:   example.Contract,
-			expContractGas: myContractGas + 10 + DefaultInstanceCost + 3708,
+			expContractGas: myContractGas + 10 + DefaultInstanceCost + 3707,
 			contractResp: &wasmvmtypes.IBCReceiveResponse{
 				Acknowledgement: []byte("myAck"),
 				Messages:        []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyAlways, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}},