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

Jump to bottom

Update gas costs #636

Merged
merged 3 commits into from
Oct 8, 2021
Merged

Update gas costs #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3841b5c
Update gas costs
alpe Oct 8, 2021
91e8928
Review feedback
alpe Oct 8, 2021
bcb95cf
Review comments
alpe Oct 8, 2021
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
9 changes: 6 additions & 3 deletions x/wasm/keeper/gas_register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ const (
// such as hardcoding them in contracts.
//
// Please note that all gas prices returned to wasmvm should have this multiplied.
DefaultGasMultiplier uint64 = 15_000_000
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938055852
DefaultGasMultiplier uint64 = 140_000_000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good changes.

I would add some text where this came from.
Minimally linking to the Benchmarking PR and the discussion.
(For these 3 items)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were links to the related issues above already. I added the link to the PR discussion.

// DefaultInstanceCost is how much SDK gas we charge each time we load a WASM instance.
// Creating a new instance is costly, and this helps put a recursion limit to contracts calling contracts.
DefaultInstanceCost uint64 = 40_000
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938056803
DefaultInstanceCost uint64 = 60_000
// DefaultCompileCost is how much SDK gas is charged *per byte* for compiling WASM code.
DefaultCompileCost uint64 = 2
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938056803
DefaultCompileCost uint64 = 3
// DefaultEventAttributeDataCost is how much SDK gas is charged *per byte* for attribute data in events.
// This is used with len(key) + len(value)
DefaultEventAttributeDataCost uint64 = 1
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/keeper/gas_register_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestCompileCosts(t *testing.T) {
"one byte": {
srcLen: 1,
srcConfig: DefaultGasRegisterConfig(),
exp: sdk.Gas(2), // DefaultCompileCost
exp: sdk.Gas(3), // DefaultCompileCost
},
"zero byte": {
srcLen: 0,
Expand Down
4 changes: 2 additions & 2 deletions x/wasm/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func TestInstantiate(t *testing.T) {

gasAfter := ctx.GasMeter().GasConsumed()
if types.EnableGasVerification {
require.Equal(t, uint64(0x1221d), gasAfter-gasBefore)
require.Equal(t, uint64(0x16e8a), gasAfter-gasBefore)
}

// ensure it is stored properly
Expand Down Expand Up @@ -541,7 +541,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(0x12b02), gasAfter-gasBefore)
require.Equal(t, uint64(0x17621), gasAfter-gasBefore)
}
// ensure bob now exists and got both payments released
bobAcct = accKeeper.GetAccount(ctx, bob)
Expand Down
23 changes: 11 additions & 12 deletions x/wasm/keeper/recurse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.Acc

func TestGasCostOnQuery(t *testing.T) {
const (
GasNoWork uint64 = 44_171
GasNoWork uint64 = 63_787
// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
GasWork50 uint64 = 48_855 // this is a little shy of 50k gas - to keep an eye on the limit
GasWork50 uint64 = 64_268 // this is a little shy of 50k gas - to keep an eye on the limit

GasReturnUnhashed uint64 = 252
GasReturnHashed uint64 = 228
GasReturnUnhashed uint64 = 28
GasReturnHashed uint64 = 24
)

cases := map[string]struct {
Expand Down Expand Up @@ -98,8 +98,7 @@ func TestGasCostOnQuery(t *testing.T) {
Depth: 4,
Work: 50,
},
// FIXME: why -6... confused a bit by calculations, seems like rounding issues
expectedGas: 5*GasWork50 + 4*GasReturnHashed - 6,
expectedGas: 5*GasWork50 + 4*GasReturnHashed,
},
}

Expand Down Expand Up @@ -216,9 +215,9 @@ func TestLimitRecursiveQueryGas(t *testing.T) {

const (
// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
GasWork2k uint64 = 233_979 // = NewContractInstanceCosts + x // we have 6x gas used in cpu than in the instance
GasWork2k uint64 = 84103 // = NewContractInstanceCosts + x // we have 6x gas used in cpu than in the instance
// This is overhead for calling into a sub-contract
GasReturnHashed uint64 = 232
GasReturnHashed uint64 = 24
)

cases := map[string]struct {
Expand All @@ -235,7 +234,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
Work: 2000,
},
expectQueriesFromContract: 0,
expectedGas: GasWork2k + 1,
expectedGas: GasWork2k,
},
"recursion 5, lots of work": {
gasLimit: 4_000_000,
Expand All @@ -244,14 +243,14 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
Work: 2000,
},
expectQueriesFromContract: 5,
// FIXME: why +3 ... confused a bit by calculations, seems like rounding issues
expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed) + 3,
// FIXME: why +1 ... confused a bit by calculations, seems like rounding issues
expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed) + 1,
},
// this is where we expect an error...
// it has enough gas to run 4 times and die on the 5th (4th time dispatching to sub-contract)
// however, if we don't charge the cpu gas before sub-dispatching, we can recurse over 20 times
"deep recursion, should die on 5th level": {
gasLimit: 1_000_000,
gasLimit: 400_000,
msg: Recurse{
Depth: 50,
Work: 2000,
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 @@ -246,14 +246,14 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
"send tokens": {
submsgID: 5,
msg: validBankSend,
resultAssertions: []assertion{assertReturnedEvents(1), assertGasUsed(76000, 81000)},
resultAssertions: []assertion{assertReturnedEvents(1), assertGasUsed(96000, 101000)},
},
"not enough tokens": {
submsgID: 6,
msg: invalidBankSend,
subMsgError: true,
// uses less gas than the send tokens (cost of bank transfer)
resultAssertions: []assertion{assertGasUsed(57000, 59000), assertErrorString("insufficient funds")},
resultAssertions: []assertion{assertGasUsed(76000, 79000), assertErrorString("insufficient funds")},
},
"out of gas panic with no gas limit": {
submsgID: 7,
Expand All @@ -266,23 +266,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(1), assertGasUsed(76000, 81000)},
resultAssertions: []assertion{assertReturnedEvents(1), assertGasUsed(96000, 101000)},
},
"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(57000, 59000), assertErrorString("insufficient funds")},
resultAssertions: []assertion{assertGasUsed(76000, 79000), assertErrorString("insufficient funds")},
},
"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+52000, subGasLimit+54000), assertErrorString("out of gas")},
resultAssertions: []assertion{assertGasUsed(subGasLimit+71000, subGasLimit+74000), assertErrorString("out of gas")},
},
"instantiate contract gets address in data and events": {
submsgID: 21,
Expand Down