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

R4R: Update gas costs to more reasonable numbers for GoS #3052

Merged
merged 3 commits into from
Dec 10, 2018
Merged

R4R: Update gas costs to more reasonable numbers for GoS #3052

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
0a08171
WIP update gas figuresg
jaekwon Dec 9, 2018
2b5f36b
fix test
jaekwon Dec 10, 2018
a390d04
Bump sim period; PENDING
jaekwon Dec 10, 2018
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
1 change: 1 addition & 0 deletions PENDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ BREAKING CHANGES
- [#128](https://github.com/tendermint/devops/issues/128) Updated CircleCI job to trigger website build on every push to master/develop.
- [\#2994](https://github.com/cosmos/cosmos-sdk/pull/2994) Change wrong-password error message.
- \#3009 Added missing Gaia genesis verification
- [gas] \#3052 Updated gas costs to more reasonable numbers

* SDK
- [auth] \#2952 Signatures are no longer serialized on chain with the account number and sequence number
Expand Down
2 changes: 1 addition & 1 deletion scripts/multisim.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ sim() {
file="$tmpdir/gaia-simulation-seed-$seed-date-$(date -u +"%Y-%m-%dT%H:%M:%S+00:00").stdout"
echo "Writing stdout to $file..."
go test ./cmd/gaia/app -run $testname -SimulationEnabled=true -SimulationNumBlocks=$blocks \
-SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -v -timeout 24h > $file
-SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -SimulationPeriod=5 -v -timeout 24h > $file
}

i=0
Expand Down
2 changes: 1 addition & 1 deletion store/gaskvstore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,6 @@ func (gi *gasIterator) Close() {
func (gi *gasIterator) consumeSeekGas() {
value := gi.Value()

gi.gasMeter.ConsumeGas(gi.gasConfig.ValueCostPerByte*sdk.Gas(len(value)), sdk.GasValuePerByteDesc)
gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*sdk.Gas(len(value)), sdk.GasValuePerByteDesc)
gi.gasMeter.ConsumeGas(gi.gasConfig.IterNextCostFlat, sdk.GasIterNextCostFlatDesc)
}
14 changes: 7 additions & 7 deletions store/gaskvstore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,26 @@ import (
)

func newGasKVStore() KVStore {
meter := sdk.NewGasMeter(1000)
meter := sdk.NewGasMeter(10000)
mem := dbStoreAdapter{dbm.NewMemDB()}
return NewGasKVStore(meter, sdk.KVGasConfig(), mem)
}

func TestGasKVStoreBasic(t *testing.T) {
mem := dbStoreAdapter{dbm.NewMemDB()}
meter := sdk.NewGasMeter(1000)
meter := sdk.NewGasMeter(10000)
st := NewGasKVStore(meter, sdk.KVGasConfig(), mem)
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
st.Set(keyFmt(1), valFmt(1))
require.Equal(t, valFmt(1), st.Get(keyFmt(1)))
st.Delete(keyFmt(1))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Equal(t, meter.GasConsumed(), sdk.Gas(193))
require.Equal(t, meter.GasConsumed(), sdk.Gas(6429))
}

func TestGasKVStoreIterator(t *testing.T) {
mem := dbStoreAdapter{dbm.NewMemDB()}
meter := sdk.NewGasMeter(1000)
meter := sdk.NewGasMeter(10000)
st := NewGasKVStore(meter, sdk.KVGasConfig(), mem)
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Empty(t, st.Get(keyFmt(2)), "Expected `key2` to be empty")
Expand All @@ -50,7 +50,7 @@ func TestGasKVStoreIterator(t *testing.T) {
iterator.Next()
require.False(t, iterator.Valid())
require.Panics(t, iterator.Next)
require.Equal(t, meter.GasConsumed(), sdk.Gas(384))
require.Equal(t, meter.GasConsumed(), sdk.Gas(6987))
}

func TestGasKVStoreOutOfGasSet(t *testing.T) {
Expand All @@ -62,7 +62,7 @@ func TestGasKVStoreOutOfGasSet(t *testing.T) {

func TestGasKVStoreOutOfGasIterator(t *testing.T) {
mem := dbStoreAdapter{dbm.NewMemDB()}
meter := sdk.NewGasMeter(200)
meter := sdk.NewGasMeter(20000)
st := NewGasKVStore(meter, sdk.KVGasConfig(), mem)
st.Set(keyFmt(1), valFmt(1))
iterator := st.Iterator(nil, nil)
Expand All @@ -71,7 +71,7 @@ func TestGasKVStoreOutOfGasIterator(t *testing.T) {
}

func testGasKVStoreWrap(t *testing.T, store KVStore) {
meter := sdk.NewGasMeter(10000)
meter := sdk.NewGasMeter(100000)

store = store.Gas(meter, sdk.GasConfig{HasCost: 10})
require.Equal(t, uint64(0), meter.GasConsumed())
Expand Down
16 changes: 7 additions & 9 deletions types/gas.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,19 @@ type GasConfig struct {
ReadCostPerByte Gas
WriteCostFlat Gas
WriteCostPerByte Gas
ValueCostPerByte Gas
IterNextCostFlat Gas
}

// KVGasConfig returns a default gas config for KVStores.
func KVGasConfig() GasConfig {
return GasConfig{
HasCost: 10,
DeleteCost: 10,
ReadCostFlat: 10,
ReadCostPerByte: 1,
WriteCostFlat: 10,
WriteCostPerByte: 10,
ValueCostPerByte: 1,
IterNextCostFlat: 15,
HasCost: 1000,
DeleteCost: 1000,
ReadCostFlat: 1000,
ReadCostPerByte: 3,
WriteCostFlat: 2000,
WriteCostPerByte: 30,
IterNextCostFlat: 30,
}
}

Expand Down
10 changes: 5 additions & 5 deletions x/auth/ante.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import (
)

const (
memoCostPerByte sdk.Gas = 1
ed25519VerifyCost = 59
secp256k1VerifyCost = 100
maxMemoCharacters = 100
memoCostPerByte sdk.Gas = 3
ed25519VerifyCost = 590
secp256k1VerifyCost = 1000
maxMemoCharacters = 256

// how much gas = 1 atom
gasPerUnitCost = 1000
gasPerUnitCost = 10000
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only efficacious if ctx.MinimumFees() is nonzero, maybe we should set that for GoS

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we set this if it's set per validator? Simply state a recommendation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just set a non-zero default. I think @zmanian will mention it in the docs, which works too


// max total number of sigs per tx
txSigLimit = 7
Expand Down
15 changes: 8 additions & 7 deletions x/auth/ante_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package auth

import (
"fmt"
"strings"
"testing"

codec "github.com/cosmos/cosmos-sdk/codec"
Expand All @@ -20,7 +21,7 @@ func newTestMsg(addrs ...sdk.AccAddress) *sdk.TestMsg {
}

func newStdFee() StdFee {
return NewStdFee(5000,
return NewStdFee(50000,
sdk.NewInt64Coin("atom", 150),
)
}
Expand Down Expand Up @@ -437,13 +438,13 @@ func TestAnteHandlerMemoGas(t *testing.T) {
checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeOutOfGas)

// memo too large
fee = NewStdFee(2001, sdk.NewInt64Coin("atom", 0))
tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsd")
fee = NewStdFee(9000, sdk.NewInt64Coin("atom", 0))
tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("01234567890", 500))
checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeMemoTooLarge)

// tx with memo has enough gas
fee = NewStdFee(1100, sdk.NewInt64Coin("atom", 0))
tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd")
fee = NewStdFee(9000, sdk.NewInt64Coin("atom", 0))
tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("0123456789", 10))
checkValidTx(t, anteHandler, ctx, tx, false)
}

Expand Down Expand Up @@ -706,8 +707,8 @@ func TestAdjustFeesByGas(t *testing.T) {
args args
want sdk.Coins
}{
{"nil coins", args{sdk.Coins{}, 10000}, sdk.Coins{}},
{"nil coins", args{sdk.Coins{sdk.NewInt64Coin("A", 10), sdk.NewInt64Coin("B", 0)}, 10000}, sdk.Coins{sdk.NewInt64Coin("A", 20), sdk.NewInt64Coin("B", 10)}},
{"nil coins", args{sdk.Coins{}, 100000}, sdk.Coins{}},
{"nil coins", args{sdk.Coins{sdk.NewInt64Coin("A", 10), sdk.NewInt64Coin("B", 0)}, 100000}, sdk.Coins{sdk.NewInt64Coin("A", 20), sdk.NewInt64Coin("B", 10)}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion x/auth/stdtx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestStdSignBytes(t *testing.T) {
}{
{
args{"1234", 3, 6, defaultFee, []sdk.Msg{sdk.NewTestMsg(addr)}, "memo"},
fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr),
fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"50000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr),
},
}
for i, tc := range tests {
Expand Down
14 changes: 0 additions & 14 deletions x/bank/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth"
)

const (
costGetCoins sdk.Gas = 10
costHasCoins sdk.Gas = 10
costSetCoins sdk.Gas = 100
costSubtractCoins sdk.Gas = 10
costAddCoins sdk.Gas = 10
)

//-----------------------------------------------------------------------------
// Keeper

Expand Down Expand Up @@ -150,7 +142,6 @@ func (keeper BaseViewKeeper) HasCoins(ctx sdk.Context, addr sdk.AccAddress, amt
//-----------------------------------------------------------------------------

func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.Coins {
ctx.GasMeter().ConsumeGas(costGetCoins, "getCoins")
acc := am.GetAccount(ctx, addr)
if acc == nil {
return sdk.Coins{}
Expand All @@ -159,7 +150,6 @@ func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.C
}

func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) sdk.Error {
ctx.GasMeter().ConsumeGas(costSetCoins, "setCoins")
acc := am.GetAccount(ctx, addr)
if acc == nil {
acc = am.NewAccountWithAddress(ctx, addr)
Expand All @@ -175,14 +165,11 @@ func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s

// HasCoins returns whether or not an account has at least amt coins.
func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) bool {
ctx.GasMeter().ConsumeGas(costHasCoins, "hasCoins")
return getCoins(ctx, am, addr).IsAllGTE(amt)
}

// SubtractCoins subtracts amt from the coins at the addr.
func subtractCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) {
ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins")

oldCoins := getCoins(ctx, am, addr)
newCoins, hasNeg := oldCoins.SafeMinus(amt)
if hasNeg {
Expand All @@ -196,7 +183,6 @@ func subtractCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress,

// AddCoins adds amt to the coins at the addr.
func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) {
ctx.GasMeter().ConsumeGas(costAddCoins, "addCoins")
oldCoins := getCoins(ctx, am, addr)
newCoins := oldCoins.Plus(amt)
if !newCoins.IsNotNegative() {
Expand Down