Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize "pinned" to "discount" for cases where contract is in memory #1799

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Generalize "pinned" to "discount" for cases where contract is in memory #1799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
315bf5c
Generalize "pinned" to "discount" for cases where contract is in memory
webmaster128 Jan 26, 2024
3206540
Adapt tests to gas value changes
webmaster128 Jan 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x/wasm/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1861,7 +1861,7 @@ func TestPinnedContractLoops(t *testing.T) {
},
}, 0, nil
}
ctx = ctx.WithGasMeter(storetypes.NewGasMeter(20000))
ctx = ctx.WithGasMeter(storetypes.NewGasMeter(30_000))
require.PanicsWithValue(t, storetypes.ErrorOutOfGas{Descriptor: "ReadFlat"}, func() {
_, err := k.execute(ctx, example.Contract, RandomAccountAddress(t), anyMsg, nil)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/keeper/relay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func TestOnRecvPacket(t *testing.T) {
},
"submessage reply can overwrite ack data": {
contractAddr: example.Contract,
expContractGas: myContractGas + storageCosts,
expContractGas: types.DefaultInstanceCostDiscount + myContractGas + storageCosts,
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Expand Down
10 changes: 5 additions & 5 deletions x/wasm/keeper/submsg_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,14 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
"send tokens": {
submsgID: 5,
msg: validBankSend,
resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(105000, 106000)},
resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(107_000, 108_000)},
},
"not enough tokens": {
submsgID: 6,
msg: invalidBankSend,
subMsgError: true,
// uses less gas than the send tokens (cost of bank transfer)
resultAssertions: []assertion{assertGasUsed(76000, 79000), assertErrorString("codespace: sdk, code: 5")},
resultAssertions: []assertion{assertGasUsed(78_000, 81_000), assertErrorString("codespace: sdk, code: 5")},
},
"out of gas panic with no gas limit": {
submsgID: 7,
Expand All @@ -263,23 +263,23 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
msg: validBankSend,
gasLimit: &subGasLimit,
// uses same gas as call without limit (note we do not charge the 40k on reply)
resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(105000, 106000)},
resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(107_000, 108_000)},
},
"not enough tokens with limit": {
submsgID: 16,
msg: invalidBankSend,
subMsgError: true,
gasLimit: &subGasLimit,
// uses same gas as call without limit (note we do not charge the 40k on reply)
resultAssertions: []assertion{assertGasUsed(77700, 77800), assertErrorString("codespace: sdk, code: 5")},
resultAssertions: []assertion{assertGasUsed(79_700, 79_800), assertErrorString("codespace: sdk, code: 5")},
},
"out of gas caught with gas limit": {
submsgID: 17,
msg: infiniteLoop,
subMsgError: true,
gasLimit: &subGasLimit,
// uses all the subGasLimit, plus the 52k or so for the main contract
resultAssertions: []assertion{assertGasUsed(subGasLimit+73000, subGasLimit+74000), assertErrorString("codespace: sdk, code: 11")},
resultAssertions: []assertion{assertGasUsed(subGasLimit+75_000, subGasLimit+76_000), assertErrorString("codespace: sdk, code: 11")},
},
"instantiate contract gets address in data and events": {
submsgID: 21,
Expand Down
12 changes: 6 additions & 6 deletions x/wasm/keeper/wasmtesting/gas_register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
// MockGasRegister mock that implements keeper.GasRegister
type MockGasRegister struct {
CompileCostFn func(byteLength int) storetypes.Gas
SetupContractCostFn func(pinned bool, msgLen int) storetypes.Gas
ReplyCostFn func(pinned bool, reply wasmvmtypes.Reply) storetypes.Gas
SetupContractCostFn func(discount bool, msgLen int) storetypes.Gas
ReplyCostFn func(discount bool, reply wasmvmtypes.Reply) storetypes.Gas
EventCostsFn func(evts []wasmvmtypes.EventAttribute) storetypes.Gas
ToWasmVMGasFn func(source storetypes.Gas) uint64
FromWasmVMGasFn func(source uint64) storetypes.Gas
Expand All @@ -31,18 +31,18 @@ func (m MockGasRegister) UncompressCosts(byteLength int) storetypes.Gas {
return m.UncompressCostsFn(byteLength)
}

func (m MockGasRegister) SetupContractCost(pinned bool, msgLen int) storetypes.Gas {
func (m MockGasRegister) SetupContractCost(discount bool, msgLen int) storetypes.Gas {
if m.SetupContractCostFn == nil {
panic("not expected to be called")
}
return m.SetupContractCostFn(pinned, msgLen)
return m.SetupContractCostFn(discount, msgLen)
}

func (m MockGasRegister) ReplyCosts(pinned bool, reply wasmvmtypes.Reply) storetypes.Gas {
func (m MockGasRegister) ReplyCosts(discount bool, reply wasmvmtypes.Reply) storetypes.Gas {
if m.ReplyCostFn == nil {
panic("not expected to be called")
}
return m.ReplyCostFn(pinned, reply)
return m.ReplyCostFn(discount, reply)
}

func (m MockGasRegister) EventCosts(evts []wasmvmtypes.EventAttribute, _ wasmvmtypes.Events) storetypes.Gas {
Expand Down
43 changes: 31 additions & 12 deletions x/wasm/types/gas_register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ const (
// Creating a new instance is costly, and this helps put a recursion limit to contracts calling contracts.
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938056803
DefaultInstanceCost uint64 = 60_000
// DefaultInstanceCostDiscount is charged instead of DefaultInstanceCost for cases where
// we assume the contract is loaded from an in-memory cache.
// For a long time it was implicitly just 0 in those cases.
// Now we use something small that roughly reflects the 45µs startup time (30x cheaper than DefaultInstanceCost).
DefaultInstanceCostDiscount uint64 = 2_000
// DefaultCompileCost is how much SDK gas is charged *per byte* for compiling WASM code.
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938056803
DefaultCompileCost uint64 = 3
Expand Down Expand Up @@ -75,9 +80,9 @@ type GasRegister interface {
UncompressCosts(byteLength int) storetypes.Gas
// SetupContractCost are charged when interacting with a Wasm contract, i.e. every time
// the contract is prepared for execution through any entry point (execute/instantiate/sudo/query/ibc_*/...).
SetupContractCost(pinned bool, msgLen int) storetypes.Gas
SetupContractCost(discount bool, msgLen int) storetypes.Gas
// ReplyCosts costs to to handle a message reply
ReplyCosts(pinned bool, reply wasmvmtypes.Reply) storetypes.Gas
ReplyCosts(discount bool, reply wasmvmtypes.Reply) storetypes.Gas
// EventCosts costs to persist an event
EventCosts(attrs []wasmvmtypes.EventAttribute, events wasmvmtypes.Events) storetypes.Gas
// ToWasmVMGas converts from Cosmos SDK gas units to [CosmWasm gas] (aka. wasmvm gas)
Expand All @@ -92,8 +97,16 @@ type GasRegister interface {

// WasmGasRegisterConfig config type
type WasmGasRegisterConfig struct {
// InstanceCost costs when interacting with a wasm contract
// InstanceCost are charged when interacting with a Wasm contract.
// "Instance" refers to the in-memory Instance of the Wasm runtime, not the contract address on chain.
// InstanceCost are part of a contract's setup cost.
InstanceCost storetypes.Gas
// InstanceCostDiscount is a discounted version of InstanceCost. It is charged whenever
// we can reasonably assume that a contract is in one of the in-memory caches. E.g.
// when the contract is pinned or we send a reply to a contract that was executed before.
// See also https://github.com/CosmWasm/wasmd/issues/1798 for more thinking around
// discount cases.
InstanceCostDiscount storetypes.Gas
// CompileCosts costs to persist and "compile" a new wasm contract
CompileCost storetypes.Gas
// UncompressCost costs per byte to unpack a contract
Expand All @@ -120,6 +133,7 @@ type WasmGasRegisterConfig struct {
func DefaultGasRegisterConfig() WasmGasRegisterConfig {
return WasmGasRegisterConfig{
InstanceCost: DefaultInstanceCost,
InstanceCostDiscount: DefaultInstanceCostDiscount,
CompileCost: DefaultCompileCost,
GasMultiplier: DefaultGasMultiplier,
EventPerAttributeCost: DefaultPerAttributeCost,
Expand Down Expand Up @@ -167,20 +181,25 @@ func (g WasmGasRegister) UncompressCosts(byteLength int) storetypes.Gas {
return g.c.UncompressCost.Mul(uint64(byteLength)).Floor()
}

// SetupContractCost costs when interacting with a wasm contract
func (g WasmGasRegister) SetupContractCost(pinned bool, msgLen int) storetypes.Gas {
// SetupContractCost costs when interacting with a wasm contract.
// Set discount to true in cases where you can reasonably assume the contract
// is loaded from an in-memory cache (e.g. pinned contracts or replys).
func (g WasmGasRegister) SetupContractCost(discount bool, msgLen int) storetypes.Gas {
if msgLen < 0 {
panic(errorsmod.Wrap(ErrInvalid, "negative length"))
}
dataCosts := storetypes.Gas(msgLen) * g.c.ContractMessageDataCost
if pinned {
return dataCosts
dataCost := storetypes.Gas(msgLen) * g.c.ContractMessageDataCost
if discount {
return g.c.InstanceCostDiscount + dataCost
} else {
return g.c.InstanceCost + dataCost
}
return g.c.InstanceCost + dataCosts
}

// ReplyCosts costs to to handle a message reply
func (g WasmGasRegister) ReplyCosts(pinned bool, reply wasmvmtypes.Reply) storetypes.Gas {
// ReplyCosts costs to to handle a message reply.
// Set discount to true in cases where you can reasonably assume the contract
// is loaded from an in-memory cache (e.g. pinned contracts or replys).
func (g WasmGasRegister) ReplyCosts(discount bool, reply wasmvmtypes.Reply) storetypes.Gas {
var eventGas storetypes.Gas
msgLen := len(reply.Result.Err)
if reply.Result.Ok != nil {
Expand All @@ -193,7 +212,7 @@ func (g WasmGasRegister) ReplyCosts(pinned bool, reply wasmvmtypes.Reply) storet
// apply free tier on the whole set not per event
eventGas += g.EventCosts(attrs, nil)
}
return eventGas + g.SetupContractCost(pinned, msgLen)
return eventGas + g.SetupContractCost(discount, msgLen)
}

// EventCosts costs to persist an event
Expand Down
35 changes: 17 additions & 18 deletions x/wasm/types/gas_register_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,19 @@ func TestSetupContractCost(t *testing.T) {
srcLen: 1,
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: DefaultContractMessageDataCost,
exp: DefaultInstanceCostDiscount + DefaultContractMessageDataCost,
},
"big msg - pinned": {
srcLen: math.MaxUint32,
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: DefaultContractMessageDataCost * math.MaxUint32,
exp: DefaultInstanceCostDiscount + DefaultContractMessageDataCost*math.MaxUint32,
},
"empty msg - pinned": {
srcLen: 0,
pinned: true,
srcConfig: DefaultGasRegisterConfig(),
exp: storetypes.Gas(0),
exp: DefaultInstanceCostDiscount,
},
"small msg - unpinned": {
srcLen: 1,
Expand All @@ -89,7 +89,6 @@ func TestSetupContractCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost,
},

"negative len": {
srcLen: -1,
srcConfig: DefaultGasRegisterConfig(),
Expand Down Expand Up @@ -118,7 +117,7 @@ func TestReplyCost(t *testing.T) {
exp storetypes.Gas
expPanic bool
}{
"subcall response with events and data - pinned": {
"submessage reply with events and data - pinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -131,9 +130,9 @@ func TestReplyCost(t *testing.T) {
},
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost + DefaultContractMessageDataCost, // 3 == len("foo")
exp: DefaultInstanceCostDiscount + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost + DefaultContractMessageDataCost, // 3 == len("foo")
},
"subcall response with events - pinned": {
"submessage reply with events - pinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -145,9 +144,9 @@ func TestReplyCost(t *testing.T) {
},
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost, // 3 == len("foo")
exp: DefaultInstanceCostDiscount + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost, // 3 == len("foo")
},
"subcall response with events exceeds free tier- pinned": {
"submessage reply with events exceeds free tier - pinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -159,19 +158,19 @@ func TestReplyCost(t *testing.T) {
},
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: (3+6)*DefaultEventAttributeDataCost + DefaultPerAttributeCost, // 3 == len("foo"), 6 == len("myData")
exp: DefaultInstanceCostDiscount + (3+6)*DefaultEventAttributeDataCost + DefaultPerAttributeCost, // 3 == len("foo"), 6 == len("myData")
},
"subcall response error - pinned": {
"submessage reply error - pinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Err: "foo",
},
},
srcConfig: DefaultGasRegisterConfig(),
pinned: true,
exp: 3 * DefaultContractMessageDataCost,
exp: DefaultInstanceCostDiscount + 3*DefaultContractMessageDataCost,
},
"subcall response with events and data - unpinned": {
"submessage reply with events and data - unpinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -185,7 +184,7 @@ func TestReplyCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost + DefaultContractMessageDataCost,
},
"subcall response with events - unpinned": {
"submessage reply with events - unpinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -198,7 +197,7 @@ func TestReplyCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost + 3*DefaultEventAttributeDataCost + DefaultPerAttributeCost,
},
"subcall response with events exceeds free tier- unpinned": {
"submessage reply with events exceeds free tier - unpinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -211,7 +210,7 @@ func TestReplyCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost + (3+6)*DefaultEventAttributeDataCost + DefaultPerAttributeCost, // 3 == len("foo"), 6 == len("myData")
},
"subcall response error - unpinned": {
"submessage reply error - unpinned": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Err: "foo",
Expand All @@ -220,7 +219,7 @@ func TestReplyCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost + 3*DefaultContractMessageDataCost,
},
"subcall response with empty events": {
"submessage reply with empty events": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{
Expand All @@ -231,7 +230,7 @@ func TestReplyCost(t *testing.T) {
srcConfig: DefaultGasRegisterConfig(),
exp: DefaultInstanceCost,
},
"subcall response with events unset": {
"submessage reply with events unset": {
src: wasmvmtypes.Reply{
Result: wasmvmtypes.SubMsgResult{
Ok: &wasmvmtypes.SubMsgResponse{},
Expand Down