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.

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

[R4R] Change gas & related fields to unsigned integer type #2839

Merged
merged 11 commits into from
Nov 19, 2018
Merged
3 changes: 2 additions & 1 deletion PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,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