Apply review feedback

CosmWasm · Aug 25, 2022 · 46d9e25 · 46d9e25
1 parent 805c849
commit 46d9e25
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 9 deletions.
diff --git a/x/wasm/ioutils/utils.go b/x/wasm/ioutils/utils.go
@@ -17,7 +17,7 @@ var (
 
 // IsGzip returns checks if the file contents are gzip compressed
 func IsGzip(input []byte) bool {
-	return len(input) > 3 && bytes.Equal(gzipIdent, input[0:3])
+	return len(input) >= 3 && bytes.Equal(gzipIdent, input[0:3])
 }
 
 // IsWasm checks if the file contents are of wasm binary

diff --git a/x/wasm/ioutils/utils_test.go b/x/wasm/ioutils/utils_test.go
@@ -42,7 +42,7 @@ func TestIsGzip(t *testing.T) {
 	require.False(t, IsGzip(wasmCode))
 	require.False(t, IsGzip(someRandomStr))
 	require.False(t, IsGzip(nil))
-	require.False(t, IsGzip(gzipData[0:3]))
+	require.True(t, IsGzip(gzipData[0:3]))
 	require.True(t, IsGzip(gzipData))
 }
 

diff --git a/x/wasm/keeper/gas_register.go b/x/wasm/keeper/gas_register.go
@@ -52,10 +52,20 @@ const (
 	DefaultPerCustomEventCost uint64 = 20
 	// DefaultEventAttributeDataFreeTier number of bytes of total attribute data we do not charge.
 	DefaultEventAttributeDataFreeTier = 100
-	// DefaultPerByteUncompressCost is how much SDK gas we charge per source byte to unpack
-	DefaultPerByteUncompressCost uint64 = 2
 )
 
+// default: 0.15 gas.
+// see https://github.com/CosmWasm/wasmd/pull/898#discussion_r937727200
+var defaultPerByteUncompressCost = wasmvmtypes.UFraction{
+	Numerator:   15,
+	Denominator: 100,
+}
+
+// DefaultPerByteUncompressCost is how much SDK gas we charge per source byte to unpack
+func DefaultPerByteUncompressCost() wasmvmtypes.UFraction {
+	return defaultPerByteUncompressCost
+}
+
 // GasRegister abstract source for gas costs
 type GasRegister interface {
 	// NewContractInstanceCosts costs to crate a new contract instance from code
@@ -83,7 +93,7 @@ type WasmGasRegisterConfig struct {
 	// CompileCosts costs to persist and "compile" a new wasm contract
 	CompileCost sdk.Gas
 	// UncompressCost costs per byte to unpack a contract
-	UncompressCost sdk.Gas
+	UncompressCost wasmvmtypes.UFraction
 	// GasMultiplier is how many cosmwasm gas points = 1 sdk gas point
 	// SDK reference costs can be found here: https://github.com/cosmos/cosmos-sdk/blob/02c6c9fafd58da88550ab4d7d494724a477c8a68/store/types/gas.go#L153-L164
 	GasMultiplier sdk.Gas
@@ -113,7 +123,7 @@ func DefaultGasRegisterConfig() WasmGasRegisterConfig {
 		EventAttributeDataCost:     DefaultEventAttributeDataCost,
 		EventAttributeDataFreeTier: DefaultEventAttributeDataFreeTier,
 		ContractMessageDataCost:    DefaultContractMessageDataCost,
-		UncompressCost:             DefaultPerByteUncompressCost,
+		UncompressCost:             DefaultPerByteUncompressCost(),
 	}
 }
 
@@ -155,7 +165,7 @@ func (g WasmGasRegister) UncompressCosts(byteLength int) sdk.Gas {
 	if byteLength < 0 {
 		panic(sdkerrors.Wrap(types.ErrInvalid, "negative length"))
 	}
-	return g.c.UncompressCost * uint64(byteLength)
+	return g.c.UncompressCost.Mul(uint64(byteLength)).Floor()
 }
 
 // InstantiateContractCosts costs when interacting with a wasm contract

diff --git a/x/wasm/keeper/gas_register_test.go b/x/wasm/keeper/gas_register_test.go
@@ -5,6 +5,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/CosmWasm/wasmd/x/wasm/types"
+
 	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -430,3 +432,32 @@ func TestFromWasmVMGasConversion(t *testing.T) {
 		})
 	}
 }
+
+func TestUncompressCosts(t *testing.T) {
+	specs := map[string]struct {
+		lenIn int
+		exp   sdk.Gas
+	}{
+		"0": {
+			exp: 0,
+		},
+		"even": {
+			lenIn: 100,
+			exp:   15,
+		},
+		"round down when uneven": {
+			lenIn: 19,
+			exp:   2,
+		},
+		"max len": {
+			lenIn: types.MaxWasmSize,
+			exp:   122880,
+		},
+	}
+	for name, spec := range specs {
+		t.Run(name, func(t *testing.T) {
+			got := NewDefaultWasmGasRegister().UncompressCosts(spec.lenIn)
+			assert.Equal(t, spec.exp, got)
+		})
+	}
+}
diff --git a/x/wasm/keeper/keeper.go b/x/wasm/keeper/keeper.go
@@ -185,8 +185,8 @@ func (k Keeper) create(ctx sdk.Context, creator sdk.AccAddress, wasmCode []byte,
 			return 0, sdkerrors.Wrap(types.ErrCreateFailed, err.Error())
 		}
 	}
-	ctx.GasMeter().ConsumeGas(k.gasRegister.CompileCosts(len(wasmCode)), "Compiling wasm bytecode")
 
+	ctx.GasMeter().ConsumeGas(k.gasRegister.CompileCosts(len(wasmCode)), "Compiling wasm bytecode")
 	checksum, err := k.wasmVM.Create(wasmCode)
 	if err != nil {
 		return 0, sdkerrors.Wrap(types.ErrCreateFailed, err.Error())

diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go
@@ -374,7 +374,7 @@ func TestCreateWithBrokenGzippedPayload(t *testing.T) {
 	contractID, err := keeper.Create(ctx.WithGasMeter(gm), creator, wasmCode, nil)
 	require.Error(t, err)
 	assert.Empty(t, contractID)
-	assert.GreaterOrEqual(t, gm.GasConsumed(), sdk.Gas(1618464)) // 809232 * 2 (default uncompress costs) = 1618464
+	assert.GreaterOrEqual(t, gm.GasConsumed(), sdk.Gas(121384)) // 809232 * 0.15 (default uncompress costs) = 121384
 }
 
 func TestInstantiate(t *testing.T) {