Skip to content

Commit

Permalink
Change gas & related fields to unsigned integer type (cosmos#2839)
Browse files Browse the repository at this point in the history
* Change gas & related fields to unsigned integer type
* Implement AddUint64Overflow
  • Loading branch information
alexanderbez authored and jackzampolin committed Nov 19, 2018
1 parent f525717 commit 6e813ab
Show file tree
Hide file tree
Showing 25 changed files with 127 additions and 68 deletions.
3 changes: 2 additions & 1 deletion PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ IMPROVEMENTS

* SDK
- [x/mock/simulation] [\#2720] major cleanup, introduction of helper objects, reorganization
- \#2821 Codespaces are now strings
- #2815 Gas unit fields changed from `int64` to `uint64`.
- #2821 Codespaces are now strings

* Tendermint
- #2796 Update to go-amino 0.14.1
Expand Down
10 changes: 5 additions & 5 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,8 @@ func (app *BaseApp) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) {
Code: uint32(result.Code),
Data: result.Data,
Log: result.Log,
GasWanted: result.GasWanted,
GasUsed: result.GasUsed,
GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(result.GasUsed), // TODO: Should type accept unsigned ints?
Tags: result.Tags,
}
}
Expand All @@ -477,8 +477,8 @@ func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) {
Codespace: string(result.Codespace),
Data: result.Data,
Log: result.Log,
GasWanted: result.GasWanted,
GasUsed: result.GasUsed,
GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(result.GasUsed), // TODO: Should type accept unsigned ints?
Tags: result.Tags,
}
}
Expand Down Expand Up @@ -614,7 +614,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
// NOTE: GasWanted should be returned by the AnteHandler. GasUsed is
// determined by the GasMeter. We need access to the context to get the gas
// meter so we initialize upfront.
var gasWanted int64
var gasWanted uint64

ctx := app.getContextForAnte(mode, txBytes)
ctx = app.initializeContext(ctx, mode)
Expand Down
10 changes: 5 additions & 5 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ func TestConcurrentCheckDeliver(t *testing.T) {
// Simulate() and Query("/app/simulate", txBytes) should give
// the same results.
func TestSimulateTx(t *testing.T) {
gasConsumed := int64(5)
gasConsumed := uint64(5)

anteOpt := func(bapp *BaseApp) {
bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
Expand Down Expand Up @@ -765,7 +765,7 @@ func TestRunInvalidTransaction(t *testing.T) {

// Test that transactions exceeding gas limits fail
func TestTxGasLimits(t *testing.T) {
gasGranted := int64(10)
gasGranted := uint64(10)
anteOpt := func(bapp *BaseApp) {
bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted))
Expand All @@ -790,7 +790,7 @@ func TestTxGasLimits(t *testing.T) {
}()

count := tx.(*txTest).Counter
newCtx.GasMeter().ConsumeGas(count, "counter-ante")
newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante")
res = sdk.Result{
GasWanted: gasGranted,
}
Expand All @@ -802,7 +802,7 @@ func TestTxGasLimits(t *testing.T) {
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) sdk.Result {
count := msg.(msgCounter).Counter
ctx.GasMeter().ConsumeGas(count, "counter-handler")
ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler")
return sdk.Result{}
})
}
Expand All @@ -813,7 +813,7 @@ func TestTxGasLimits(t *testing.T) {

testCases := []struct {
tx *txTest
gasUsed int64
gasUsed uint64
fail bool
}{
{newTxCounter(0, 0), 0, false},
Expand Down
8 changes: 4 additions & 4 deletions client/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func RegisterRestServerFlags(cmd *cobra.Command) *cobra.Command {
// GasSetting encapsulates the possible values passed through the --gas flag.
type GasSetting struct {
Simulate bool
Gas int64
Gas uint64
}

// Type returns the flag's value type.
Expand All @@ -140,18 +140,18 @@ func (v *GasSetting) String() string {
if v.Simulate {
return GasFlagSimulate
}
return strconv.FormatInt(v.Gas, 10)
return strconv.FormatUint(v.Gas, 10)
}

// ParseGasFlag parses the value of the --gas flag.
func ReadGasFlag(s string) (simulate bool, gas int64, err error) {
func ReadGasFlag(s string) (simulate bool, gas uint64, err error) {
switch s {
case "":
gas = DefaultGasLimit
case GasFlagSimulate:
simulate = true
default:
gas, err = strconv.ParseInt(s, 10, 64)
gas, err = strconv.ParseUint(s, 10, 64)
if err != nil {
err = fmt.Errorf("gas must be either integer or %q", GasFlagSimulate)
return
Expand Down
6 changes: 3 additions & 3 deletions client/lcd/lcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func TestCoinSend(t *testing.T) {

// test failure with negative gas
res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "-200", 0, "")
require.Equal(t, http.StatusInternalServerError, res.StatusCode, body)
require.Equal(t, http.StatusBadRequest, res.StatusCode, body)

// test failure with 0 gas
res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "0", 0, "")
Expand Down Expand Up @@ -389,8 +389,8 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) {
require.Nil(t, cdc.UnmarshalJSON([]byte(body), &resultTx))
require.Equal(t, uint32(0), resultTx.CheckTx.Code)
require.Equal(t, uint32(0), resultTx.DeliverTx.Code)
require.Equal(t, gasEstimate, resultTx.DeliverTx.GasWanted)
require.Equal(t, gasEstimate, resultTx.DeliverTx.GasUsed)
require.Equal(t, gasEstimate, uint64(resultTx.DeliverTx.GasWanted))
require.Equal(t, gasEstimate, uint64(resultTx.DeliverTx.GasUsed))
}

func TestTxs(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion client/utils/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func WriteErrorResponse(w http.ResponseWriter, status int, err string) {

// WriteSimulationResponse prepares and writes an HTTP
// response for transactions simulations.
func WriteSimulationResponse(w http.ResponseWriter, gas int64) {
func WriteSimulationResponse(w http.ResponseWriter, gas uint64) {
w.WriteHeader(http.StatusOK)
w.Write([]byte(fmt.Sprintf(`{"gas_estimate":%v}`, gas)))
}
Expand Down
10 changes: 5 additions & 5 deletions client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func EnrichCtxWithGas(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name

// CalculateGas simulates the execution of a transaction and returns
// both the estimate obtained by the query and the adjusted amount.
func CalculateGas(queryFunc func(string, common.HexBytes) ([]byte, error), cdc *amino.Codec, txBytes []byte, adjustment float64) (estimate, adjusted int64, err error) {
func CalculateGas(queryFunc func(string, common.HexBytes) ([]byte, error), cdc *amino.Codec, txBytes []byte, adjustment float64) (estimate, adjusted uint64, err error) {
// run a simulation (via /app/simulate query) to
// estimate gas and update TxBuilder accordingly
rawRes, err := queryFunc("/app/simulate", txBytes)
Expand Down Expand Up @@ -152,7 +152,7 @@ func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string,

// nolint
// SimulateMsgs simulates the transaction and returns the gas estimate and the adjusted value.
func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (estimated, adjusted int64, err error) {
func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (estimated, adjusted uint64, err error) {
txBytes, err := txBldr.BuildWithPubKey(name, msgs)
if err != nil {
return
Expand All @@ -161,11 +161,11 @@ func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name stri
return
}

func adjustGasEstimate(estimate int64, adjustment float64) int64 {
return int64(adjustment * float64(estimate))
func adjustGasEstimate(estimate uint64, adjustment float64) uint64 {
return uint64(adjustment * float64(estimate))
}

func parseQueryResponse(cdc *amino.Codec, rawRes []byte) (int64, error) {
func parseQueryResponse(cdc *amino.Codec, rawRes []byte) (uint64, error) {
var simulationResult sdk.Result
if err := cdc.UnmarshalBinaryLengthPrefixed(rawRes, &simulationResult); err != nil {
return 0, err
Expand Down
12 changes: 6 additions & 6 deletions client/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ func TestParseQueryResponse(t *testing.T) {
cdc := app.MakeCodec()
sdkResBytes := cdc.MustMarshalBinaryLengthPrefixed(sdk.Result{GasUsed: 10})
gas, err := parseQueryResponse(cdc, sdkResBytes)
assert.Equal(t, gas, int64(10))
assert.Equal(t, gas, uint64(10))
assert.Nil(t, err)
gas, err = parseQueryResponse(cdc, []byte("fuzzy"))
assert.Equal(t, gas, int64(0))
assert.Equal(t, gas, uint64(0))
assert.NotNil(t, err)
}

func TestCalculateGas(t *testing.T) {
cdc := app.MakeCodec()
makeQueryFunc := func(gasUsed int64, wantErr bool) func(string, common.HexBytes) ([]byte, error) {
makeQueryFunc := func(gasUsed uint64, wantErr bool) func(string, common.HexBytes) ([]byte, error) {
return func(string, common.HexBytes) ([]byte, error) {
if wantErr {
return nil, errors.New("")
Expand All @@ -32,15 +32,15 @@ func TestCalculateGas(t *testing.T) {
}
}
type args struct {
queryFuncGasUsed int64
queryFuncGasUsed uint64
queryFuncWantErr bool
adjustment float64
}
tests := []struct {
name string
args args
wantEstimate int64
wantAdjusted int64
wantEstimate uint64
wantAdjusted uint64
wantErr bool
}{
{"error", args{0, true, 1.2}, 0, 0, true},
Expand Down
3 changes: 1 addition & 2 deletions cmd/gaia/app/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ func (ga *GenesisAccount) ToAccount() (acc *auth.BaseAccount) {
}
}


// Create the core parameters for genesis initialization for gaia
// note that the pubkey input is this machines pubkey
func GaiaAppGenState(cdc *codec.Codec, genDoc tmtypes.GenesisDoc, appGenTxs []json.RawMessage) (
Expand Down Expand Up @@ -279,7 +278,7 @@ func CollectStdTxs(cdc *codec.Codec, moniker string, genTxsDir string, genDoc tm

func NewDefaultGenesisAccount(addr sdk.AccAddress) GenesisAccount {
accAuth := auth.NewBaseAccountWithAddress(addr)
coins :=sdk.Coins{
coins := sdk.Coins{
{"fooToken", sdk.NewInt(1000)},
{bondDenom, freeFermionsAcc},
}
Expand Down
9 changes: 4 additions & 5 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) {
require.True(t, success)
require.Empty(t, stderr)
msg := unmarshalStdTx(t, stdout)
require.Equal(t, msg.Fee.Gas, int64(client.DefaultGasLimit))
require.Equal(t, msg.Fee.Gas, uint64(client.DefaultGasLimit))
require.Equal(t, len(msg.Msgs), 1)
require.Equal(t, 0, len(msg.GetSignatures()))

Expand All @@ -486,7 +486,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) {
require.True(t, success)
require.Empty(t, stderr)
msg = unmarshalStdTx(t, stdout)
require.Equal(t, msg.Fee.Gas, int64(100))
require.Equal(t, msg.Fee.Gas, uint64(100))
require.Equal(t, len(msg.Msgs), 1)
require.Equal(t, 0, len(msg.GetSignatures()))

Expand Down Expand Up @@ -543,9 +543,8 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) {
}

require.Nil(t, app.MakeCodec().UnmarshalJSON([]byte(stdout), &result))
require.Equal(t, msg.Fee.Gas, result.Response.GasUsed)
require.Equal(t, msg.Fee.Gas, result.Response.GasWanted)

require.Equal(t, msg.Fee.Gas, uint64(result.Response.GasUsed))
require.Equal(t, msg.Fee.Gas, uint64(result.Response.GasWanted))
tests.WaitForNextNBlocksTM(2, port)

barAcc := executeGetAccount(t, fmt.Sprintf("gaiacli query account %s %v", barAddr, flags))
Expand Down
14 changes: 7 additions & 7 deletions cmd/gaia/init/genesis_accts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ func TestAddGenesisAccount(t *testing.T) {
}{
{
"valid account",
args{
app.GenesisState{},
addr1,
sdk.Coins{},
},
false},
args{
app.GenesisState{},
addr1,
sdk.Coins{},
},
false},
{"dup account", args{
app.GenesisState{Accounts: []app.GenesisAccount{app.GenesisAccount{Address:addr1}}},
app.GenesisState{Accounts: []app.GenesisAccount{{Address: addr1}}},
addr1,
sdk.Coins{}}, true},
}
Expand Down
1 change: 0 additions & 1 deletion docs/examples/democoin/cmd/democoind/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const (
flagClientHome = "home-client"
)


// coolGenAppParams sets up the app_state and appends the cool app state
func CoolAppGenState(cdc *codec.Codec, genDoc tmtypes.GenesisDoc, appGenTxs []json.RawMessage) (
appState json.RawMessage, err error) {
Expand Down
2 changes: 0 additions & 2 deletions server/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ import (
tmtypes "github.com/tendermint/tendermint/types"
)


// SimpleGenTx is a simple genesis tx
type SimpleGenTx struct {
Addr sdk.AccAddress `json:"addr"`
}

//_____________________________________________________________________


// Generate a genesis transaction
func SimpleAppGenTx(cdc *codec.Codec, pk crypto.PubKey) (
appGenTx, cliPrint json.RawMessage, validator types.GenesisValidator, err error) {
Expand Down
6 changes: 3 additions & 3 deletions store/gaskvstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ func testGasKVStoreWrap(t *testing.T, store KVStore) {
meter := sdk.NewGasMeter(10000)

store = store.Gas(meter, sdk.GasConfig{HasCost: 10})
require.Equal(t, int64(0), meter.GasConsumed())
require.Equal(t, uint64(0), meter.GasConsumed())

store.Has([]byte("key"))
require.Equal(t, int64(10), meter.GasConsumed())
require.Equal(t, uint64(10), meter.GasConsumed())

store = store.Gas(meter, sdk.GasConfig{HasCost: 20})

store.Has([]byte("key"))
require.Equal(t, int64(40), meter.GasConsumed())
require.Equal(t, uint64(40), meter.GasConsumed())
}

func TestGasKVStoreWrap(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ func (c Context) WithConsensusParams(params *abci.ConsensusParams) Context {
if params == nil {
return c
}

// TODO: Do we need to handle invalid MaxGas values?
return c.withValue(contextKeyConsensusParams, params).
WithGasMeter(NewGasMeter(params.BlockSize.MaxGas))
WithGasMeter(NewGasMeter(uint64(params.BlockSize.MaxGas)))
}

func (c Context) WithChainID(chainID string) Context { return c.withValue(contextKeyChainID, chainID) }
Expand Down
25 changes: 22 additions & 3 deletions types/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@ var (
)

// Gas measured by the SDK
type Gas = int64
type Gas = uint64

// ErrorOutOfGas defines an error thrown when an action results in out of gas.
type ErrorOutOfGas struct {
Descriptor string
}

// ErrorGasOverflow defines an error thrown when an action results gas consumption
// unsigned integer overflow.
type ErrorGasOverflow struct {
Descriptor string
}

// GasMeter interface to track gas consumption
type GasMeter interface {
GasConsumed() Gas
Expand All @@ -49,7 +55,14 @@ func (g *basicGasMeter) GasConsumed() Gas {
}

func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) {
g.consumed += amount
var overflow bool

// TODO: Should we set the consumed field after overflow checking?
g.consumed, overflow = AddUint64Overflow(g.consumed, amount)
if overflow {
panic(ErrorGasOverflow{descriptor})
}

if g.consumed > g.limit {
panic(ErrorOutOfGas{descriptor})
}
Expand All @@ -71,7 +84,13 @@ func (g *infiniteGasMeter) GasConsumed() Gas {
}

func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) {
g.consumed += amount
var overflow bool

// TODO: Should we set the consumed field after overflow checking?
g.consumed, overflow = AddUint64Overflow(g.consumed, amount)
if overflow {
panic(ErrorGasOverflow{descriptor})
}
}

// GasConfig defines gas cost for each operation on KVStores
Expand Down
Loading

0 comments on commit 6e813ab

Please sign in to comment.